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

[S] Change in osmo-hnbgw[master]: fix umts_cell_id_name(): show CID
by pespin 07 Jun '23

07 Jun '23

Attention is currently required from: neels, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169 ) Change subject: fix umts_cell_id_name(): show CID ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/11674beb_58da39d6 PS3, Line 12: Use OTC_SELECT > I'm also a bit worried about this change, depending on how often we end up logging the cell-ID. […] That's usually my opinion too, but since I have the feeling I'm usually the only one against this I refrain from making the same comments again and again. But yeah, I second that and I would avoid heap allocation on code paths potentially used tons of times, like some function used to log some object. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Id903b8579ded8b908e644808e440d373bcca8da4 Gerrit-Change-Number: 33169 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:56:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0

[S] Change in osmo-hnbgw[master]: include Global RNC-ID in RESET
by laforge 07 Jun '23

07 Jun '23

Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33179 ) Change subject: include Global RNC-ID in RESET ...................................................................... Patch Set 4: Code-Review+1 (1 comment) File src/osmo-hnbgw/cnlink.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33179/comment/7586cbc1_441ddb35 PS4, Line 147: / why are we handling that case if it is in violation of the 3GPP specs? For backwards compatibility with old config files? In that case it is ok but we should probably warn the user (via the log) that his config is missing vital information and hence we're sending non-compliant messages. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33179 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I2cd5d7ea86a6ed0916befe219dbf21373afbd95b Gerrit-Change-Number: 33179 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:56:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: cnpool: return Paging Resp to the exact CN link that Paged
by laforge 07 Jun '23

07 Jun '23

Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33177 ) Change subject: cnpool: return Paging Resp to the exact CN link that Paged ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33177 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I907dbcaeb442ca5630146f8cad40601c448fc40e Gerrit-Change-Number: 33177 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:53:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-hnbgw[master]: use cnlink state in cnpool decisions
by laforge 07 Jun '23

07 Jun '23

Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33176 ) Change subject: use cnlink state in cnpool decisions ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33176 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I28490a4a27bcda8fd689db8b8652e451103e3a9d Gerrit-Change-Number: 33176 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:53:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: detect in/active CN links by RANAP RESET
by laforge 07 Jun '23

07 Jun '23

Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33175 ) Change subject: detect in/active CN links by RANAP RESET ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33175 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Id3eefdea889a736fd5957b80280fa45b9547b792 Gerrit-Change-Number: 33175 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:53:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: paging: parse PCUIF data indication outside of paging.c
by dexter 07 Jun '23

07 Jun '23

dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32733 ) Change subject: paging: parse PCUIF data indication outside of paging.c ...................................................................... paging: parse PCUIF data indication outside of paging.c The function paging_add_macblock takes a data and length parameter. The first three byte of that string are the last three digits of the IMSI from which the paging group is calculated. As the layout of this data buffer is a property of the PCUIF interface API, we should do this separation outside of paging.c. Also we should supply the IMSI as a valid null terminated string since PCUIF v.11 also uses this format. Change-Id: I9f3799916e8102bf1ce0f21891f2d24f43091f01 Related: OS#5927 --- M include/osmo-bts/paging.h M src/common/paging.c M src/common/pcu_sock.c 3 files changed, 42 insertions(+), 16 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved Jenkins Builder: Verified diff --git a/include/osmo-bts/paging.h b/include/osmo-bts/paging.h index f1ae8e3..c009831 100644 --- a/include/osmo-bts/paging.h +++ b/include/osmo-bts/paging.h @@ -37,8 +37,7 @@ /* Add a ready formatted MAC block message to the paging queue, this can be an IMMEDIATE ASSIGNMENT, or a * PAGING COMMAND (from the PCU) */ -int paging_add_macblock(struct paging_state *ps, - const uint8_t *data, uint8_t len); +int paging_add_macblock(struct paging_state *ps, const char *imsi, const uint8_t *macblock); /* generate paging message for given gsm time */ int paging_gen_msg(struct paging_state *ps, uint8_t *out_buf, struct gsm_time *gt, diff --git a/src/common/paging.c b/src/common/paging.c index 284b71c..f49a36b 100644 --- a/src/common/paging.c +++ b/src/common/paging.c @@ -269,12 +269,13 @@ /* Add a ready formatted MAC block message to the paging queue, this can be an IMMEDIATE ASSIGNMENT, or a * PAGING COMMAND (from the PCU) */ -int paging_add_macblock(struct paging_state *ps, - const uint8_t *data, uint8_t len) +int paging_add_macblock(struct paging_state *ps, const char *imsi, const uint8_t *macblock) { struct llist_head *group_q; struct paging_record *pr; - uint16_t imsi, paging_group; + uint16_t paging_group; + uint16_t _imsi; + size_t imsi_len = strlen(imsi); check_congestion(ps); @@ -285,17 +286,16 @@ return -ENOSPC; } - if (len != GSM_MACBLOCK_LEN + 3) { - LOGP(DPAG, LOGL_ERROR, "MAC block with invalid length %d (GSM_MACBLOCK_LEN + 3)\n", len); + /* Tha paging group is calculated from the last three digits of the IMSI */ + if (imsi_len < 3) { + LOGP(DPAG, LOGL_ERROR, "IMSI with invalid length %zu (expecting at least the last 3 digits)\n", imsi_len); return -EINVAL; } - len -= 3; - - imsi = 100 * ((*(data++)) - '0'); - imsi += 10 * ((*(data++)) - '0'); - imsi += (*(data++)) - '0'; - paging_group = gsm0502_calc_paging_group(&ps->chan_desc, imsi); - + imsi = imsi + imsi_len - 3; + _imsi = 100 * ((*(imsi++)) - '0'); + _imsi += 10 * ((*(imsi++)) - '0'); + _imsi += (*(imsi++)) - '0'; + paging_group = gsm0502_calc_paging_group(&ps->chan_desc, _imsi); group_q = &ps->paging_queue[paging_group]; pr = talloc_zero(ps, struct paging_record); @@ -305,7 +305,7 @@ LOGP(DPAG, LOGL_INFO, "Add MAC block to paging queue (group=%u)\n", paging_group); - memcpy(pr->u.macblock.msg, data, GSM_MACBLOCK_LEN); + memcpy(pr->u.macblock.msg, macblock, GSM_MACBLOCK_LEN); /* enqueue the new message to the HEAD of the queue */ llist_add(&pr->list, group_q); diff --git a/src/common/pcu_sock.c b/src/common/pcu_sock.c index 00debad..ea244b8 100644 --- a/src/common/pcu_sock.c +++ b/src/common/pcu_sock.c @@ -667,6 +667,7 @@ struct gsm_bts_trx_ts *ts; struct msgb *msg; int rc = 0; + char imsi[4]; LOGP(DPCU, LOGL_DEBUG, "Data request received: sapi=%s arfcn=%d " "block=%d data=%s\n", sapi_string[data_req->sapi], @@ -675,7 +676,14 @@ switch (data_req->sapi) { case PCU_IF_SAPI_PCH: - paging_add_macblock(bts->paging_state, data_req->data, data_req->len); + OSMO_STRLCPY_ARRAY(imsi, (char *)data_req->data); + if (data_req->len-3 != GSM_MACBLOCK_LEN) { + LOGP(DPCU, LOGL_ERROR, "MAC block with invalid length %d (expecting GSM_MACBLOCK_LEN = %d)\n", + data_req->len-3, GSM_MACBLOCK_LEN); + rc = -ENOMEM; + break; + } + paging_add_macblock(bts->paging_state, imsi, data_req->data + 3); break; case PCU_IF_SAPI_AGCH: msg = msgb_alloc(data_req->len, "pcu_agch"); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32733 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I9f3799916e8102bf1ce0f21891f2d24f43091f01 Gerrit-Change-Number: 32733 Gerrit-PatchSet: 9 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-hnbgw[master]: add ranap_domain_name()
by laforge 07 Jun '23

07 Jun '23

Attention is currently required from: neels. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33174 ) Change subject: add ranap_domain_name() ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33174 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ib3c5d772ccea8c854eec007de5c996d1b6640bc0 Gerrit-Change-Number: 33174 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:52:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-hnbgw[master]: cnpool: add context_map_cnlink_lost() handling
by laforge 07 Jun '23

07 Jun '23

Attention is currently required from: neels, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173 ) Change subject: cnpool: add context_map_cnlink_lost() handling ...................................................................... Patch Set 4: (1 comment) File include/osmocom/hnbgw/context_map.h: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173/comment/01676d82_831c3e13 PS3, Line 70: MAP_SCCP_EV_CN_LINK_LOST, > if it has RESET, why calling it LOST? :D I'm also a bit confused. "RESET" to me means some IuReset procedure is happening using RANAP messages over an established link. link-lost means something different. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Ic0a6fcfb747dc093528ca2bd12a269ad390d465c Gerrit-Change-Number: 33173 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:52:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: paging: do not confirm PAGING COMMAND messages
by fixeria 07 Jun '23

07 Jun '23

Attention is currently required from: dexter. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32734 ) Change subject: paging: do not confirm PAGING COMMAND messages ...................................................................... Patch Set 12: (1 comment) File src/common/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/1716b5dc_25d99523 PS12, Line 684: gsm48_imm_ass->msg_type Another problem here is that you're accessing `data_req->data` (via `struct gsm48_imm_ass *`) before checking the `data_req->len`. This is not super critical, because AFAIR the `data_req->data` is a static buffer of some large size. But still, it's more logical to check the data length first. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32734 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d Gerrit-Change-Number: 32734 Gerrit-PatchSet: 12 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:51:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in pysim[master]: ts_102221: Add "resume_uicc" command
by dexter 07 Jun '23

07 Jun '23

Attention is currently required from: laforge. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/33210 ) Change subject: ts_102221: Add "resume_uicc" command ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS2: > A power cycle is definitely required as per the specs. […] Ah nice, this works. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/33210 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I3322fde74f680e77954e1d3e18a32ef5662759f2 Gerrit-Change-Number: 33210 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 07 Jun 2023 09:51:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0