July 2023 - gerrit-log - lists.osmocom.org

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by fixeria 03 Jul '23

03 Jul '23

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... modem: properly handle Dedicated mode or TBF IE We need to distinguish between Uplink and Downlink TBF assignment in grr_rx_imm_ass(), because matching the Request Reference IE makes sense only for the Uplink TBF assignment. Uplink TBFs are requested by the UEs by sending RACH, while Downlink TBFs are assigned by the network itself. The Request Reference IE is only valid for Uplink assignments and shall be ignored in messages assigning Downlink TBFs. Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Related: OS#5500 --- M src/host/layer23/src/modem/grr.c 1 file changed, 54 insertions(+), 13 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/src/host/layer23/src/modem/grr.c b/src/host/layer23/src/modem/grr.c index b290c8b..bd55f30 100644 --- a/src/host/layer23/src/modem/grr.c +++ b/src/host/layer23/src/modem/grr.c @@ -253,22 +253,44 @@ uint8_t ch_type, ch_subch, ch_ts; int rc; - /* Discard CS channel assignment */ - if ((ia->page_mode >> 4) == 0) { - LOGP(DRR, LOGL_INFO, "%s(): Discard CS channel assignment\n", __func__); + /* 3GPP TS 44.018, section 10.5.2.25b "Dedicated mode or TBF". + * As per table 9.1.18.1, only the value part (4 bits) is present in the + * IMMEDIATE ASSIGNMENT message. In struct gsm48_imm_ass it's combined + * with the Page Mode IE, perhaps due to historical reasons. */ + const uint8_t dm_or_tbf = ia->page_mode >> 4; + + /* T/D flag: discard dedicated channel assignment */ + if ((dm_or_tbf & (1 << 0)) == 0) { + LOGP(DRR, LOGL_INFO, + "%s(): Discarding IMM ASS: dedicated channel assignment\n", + __func__); + return 0; + } + /* NRA flag: discard No Resource Allocated */ + if ((dm_or_tbf & (1 << 3)) != 0) { + LOGP(DRR, LOGL_INFO, + "%s(): Discarding IMM ASS: NRA flag is set\n", + __func__); return 0; } - if (rr->state != GSM48_RR_ST_CONN_PEND) { - LOGP(DRR, LOGL_INFO, "%s(): rr_state != GSM48_RR_ST_CONN_PEND\n", __func__); - return 0; - } - if (!grr_match_req_ref(ms, &ia->req_ref)) { - LOGP(DRR, LOGL_INFO, "%s(): req_ref mismatch (RA=0x%02x, T1=%u, T3=%u, T2=%u, FN=%u)\n", - __func__, ia->req_ref.ra, ia->req_ref.t1, - ia->req_ref.t3_high << 3 | ia->req_ref.t3_low, ia->req_ref.t2, - _gsm48_req_ref2fn(&ia->req_ref)); - return 0; + /* If this is an Uplink TBF assignment, check the Request Reference IE. + * Checking this IE in Downlink TBF assignment makes no sense because + * no CHANNEL REQUEST was sent by the MS prior to it. */ + if ((dm_or_tbf & (1 << 1)) == 0) { + if (rr->state != GSM48_RR_ST_CONN_PEND) { + LOGP(DRR, LOGL_INFO, + "%s(): rr_state != GSM48_RR_ST_CONN_PEND\n", __func__); + return 0; + } + if (!grr_match_req_ref(ms, &ia->req_ref)) { + LOGP(DRR, LOGL_INFO, + "%s(): req_ref mismatch (RA=0x%02x, T1=%u, T3=%u, T2=%u, FN=%u)\n", + __func__, ia->req_ref.ra, ia->req_ref.t1, + ia->req_ref.t3_high << 3 | ia->req_ref.t3_low, ia->req_ref.t2, + _gsm48_req_ref2fn(&ia->req_ref)); + return 0; + } } if (rsl_dec_chan_nr(ia->chan_desc.chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) { -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by fixeria 03 Jul '23

03 Jul '23

Attention is currently required from: osmith, laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 19:04:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by fixeria 03 Jul '23

03 Jul '23

Attention is currently required from: osmith, laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33414 ) Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... Patch Set 5: (2 comments) File src/host/layer23/src/modem/grr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33414/comment/8c655dd6_e5ae06e5 PS3, Line 278: if ((dm_or_tbf & (1 << 1)) == 0) { > Ok, what exactly am I supposed to clarify? How do I make it cleaner? […] Done File src/host/layer23/src/modem/grr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33414/comment/cbed97bb_15c97015 PS4, Line 279: E > A Done -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 18:44:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmocom-bb[master]: modem: properly handle Dedicated mode or TBF IE
by fixeria 03 Jul '23

03 Jul '23

Attention is currently required from: laforge, fixeria. Hello osmith, Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 to look at the new patch set (#5). Change subject: modem: properly handle Dedicated mode or TBF IE ...................................................................... modem: properly handle Dedicated mode or TBF IE We need to distinguish between Uplink and Downlink TBF assignment in grr_rx_imm_ass(), because matching the Request Reference IE makes sense only for the Uplink TBF assignment. Uplink TBFs are requested by the UEs by sending RACH, while Downlink TBFs are assigned by the network itself. The Request Reference IE is only valid for Uplink assignments and shall be ignored in messages assigning Downlink TBFs. Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Related: OS#5500 --- M src/host/layer23/src/modem/grr.c 1 file changed, 54 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/14/33414/5 -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33414 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Idb9b3203147be3b42256c0bcab3ecdabcf2d2fa9 Gerrit-Change-Number: 33414 Gerrit-PatchSet: 5 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-mgw[master]: mgcp_client: tweak extract_codec_name() implementation
by neels 03 Jul '23

03 Jul '23

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/33535 ) Change subject: mgcp_client: tweak extract_codec_name() implementation ...................................................................... mgcp_client: tweak extract_codec_name() implementation Instead of calling strlen() for every loop iteration, just use strchr() to find the position of '/'. Change-Id: Ifc7302b6c5f9288a622e33c3e8b5fe0e7037dbdc --- M src/libosmo-mgcp-client/mgcp_client.c 1 file changed, 16 insertions(+), 5 deletions(-) Approvals: Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c index 75c7b14..78652a1 100644 --- a/src/libosmo-mgcp-client/mgcp_client.c +++ b/src/libosmo-mgcp-client/mgcp_client.c @@ -69,17 +69,16 @@ static char *extract_codec_name(const char *str) { static char buf[64]; - unsigned int i; + char *pos; if (!str) return NULL; osmo_strlcpy(buf, str, sizeof(buf)); - for (i = 0; i < strlen(buf); i++) { - if (buf[i] == '/') - buf[i] = '\0'; - } + pos = strchr(buf, '/'); + if (pos) + *pos = '\0'; return buf; } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/33535 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ifc7302b6c5f9288a622e33c3e8b5fe0e7037dbdc Gerrit-Change-Number: 33535 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-mgw[master]: mgcp: fix "L: a:" header parsing: heed ";" separator
by neels 03 Jul '23

03 Jul '23

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/33534 ) Change subject: mgcp: fix "L: a:" header parsing: heed ";" separator ...................................................................... mgcp: fix "L: a:" header parsing: heed ";" separator In the "L: a:" header, read the first codec name only up to the ";" separator, and ignore the rest. According to RFC-2705, the "L: a:" header may include multiple codecs like "GSM-EFR;GSM" in: L: p:20, a:GSM-EFR;GSM, nt:IN osmo-mgw can handle only a single codec here. Since recently, osmo-msc is our first client that may actually send more than one codec. This uncovered a bug that leads to failing voice calls: * osmo-mgw parses the entire list "GSM-EFR;GSM" as a single codec name, * puts that into the ptmap without scrutiny, * and even sends it back in the OK response, in the *SDP* part, as a single "GSM-EFR;GSM" codec entry. We do not care very much about the "a:" codec list, because we always establish codecs via SDP later. So all we need to fix this is: parse the first codec done correctly, and ignore the rest. Related: OS#6081 Change-Id: I0342e85b32ed89f3a1fdb6131c3c8ded8f47a455 --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 33 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified falconia: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve neels: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 80e0f8a..978af42 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -672,8 +672,8 @@ case 'a': /* FIXME: LCO also supports the negotiation of more than one codec. * (e.g. a:PCMU;G726-32) But this implementation only supports a single - * codec only. */ - if (sscanf(lco_id + 1, ":%16[^,]", codec) == 1) { + * codec only. Ignoring all but the first codec. */ + if (sscanf(lco_id + 1, ":%16[^,;]", codec) == 1) { talloc_free(lco->codec); /* MGCP header is case insensive, and we'll need codec in uppercase when using it later: */ -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/33534 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I0342e85b32ed89f3a1fdb6131c3c8ded8f47a455 Gerrit-Change-Number: 33534 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-mgw[master]: mgcp: fix "L: a:" header parsing: heed ";" separator
by neels 03 Jul '23

03 Jul '23

neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/33534 ) Change subject: mgcp: fix "L: a:" header parsing: heed ";" separator ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/33534 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I0342e85b32ed89f3a1fdb6131c3c8ded8f47a455 Gerrit-Change-Number: 33534 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 15:25:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-ttcn3-hacks[master]: PCU_Tests: verify that PacketCellChangeContine contains an ARFCN and ...
by pespin 03 Jul '23

03 Jul '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539 ) Change subject: PCU_Tests: verify that PacketCellChangeContine contains an ARFCN and BSIC ...................................................................... Patch Set 2: Code-Review+1 (2 comments) File pcu/PCU_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539/comment/40bec134_b8d8… PS1, Line 5368: f_rx_rlcmac_dl_block(dl_block, sched_fn); > I need to modify the arfcn/bsic parameters before I pass the template. Ack https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539/comment/763dacba_cdc2… PS1, Line 5370: cell_chg_cont.u.cell_chg_continue.arfcn_bsic_presence := '1'B > I thought about doing this but then I noticed that it might be not so good to add three more paramet […] I somehow thought that it was already being passed before, but I somehow mislooked. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I4b8f3312088e3d2bc4b90702485e7c6a8d39f954 Gerrit-Change-Number: 33539 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 13:54:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-ttcn3-hacks[master]: PCU_Tests: test NACC procedure with UTRAN and E-UTRAN cell
by pespin 03 Jul '23

03 Jul '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540 ) Change subject: PCU_Tests: test NACC procedure with UTRAN and E-UTRAN cell ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Idae86a458fd44ac81bab183ed1865b1c1bdbfd66 Gerrit-Change-Number: 33540 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 13:52:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-ttcn3-hacks[master]: PCU_Tests: verify that PacketCellChangeContine contains an ARFCN and ...
by dexter 03 Jul '23

03 Jul '23

Attention is currently required from: pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539 ) Change subject: PCU_Tests: verify that PacketCellChangeContine contains an ARFCN and BSIC ...................................................................... Patch Set 2: (2 comments) File pcu/PCU_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539/comment/4b9674b9_f591… PS1, Line 5368: f_rx_rlcmac_dl_block(dl_block, sched_fn); > why not passing this constructed cell_cfg_cont here in f_rx_rlcmac_dl_block() as it was before? I need to modify the arfcn/bsic parameters before I pass the template. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539/comment/5b7ddd1b_5d82… PS1, Line 5370: cell_chg_cont.u.cell_chg_continue.arfcn_bsic_presence := '1'B > you can probably extend the f_rx_rlcmac_dl_block to have those passed params maybe. I thought about doing this but then I noticed that it might be not so good to add three more parameters just for a single test. I think it is better to just modify the required bits here instead of adding more and more parameters. Especially those CSN.1 RLCMAC templates have a ton of variations. (This does not mean that it is technically impossible to add arfcn_bsic_present,arfcn,bsic parameters to the template, but I Think it won't look good.) -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33539 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I4b8f3312088e3d2bc4b90702485e7c6a8d39f954 Gerrit-Change-Number: 33539 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 13:25:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0