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

[S] Change in libosmo-netif[master]: stream: Drop dot at the end of log line
by laforge 08 Jun '23

08 Jun '23

Attention is currently required from: pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33224 ) Change subject: stream: Drop dot at the end of log line ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: thanks. I always found those quite useless ;) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33224 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I59b0e56088c60e3de5f3a2f38f158e16074a65c9 Gerrit-Change-Number: 33224 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 10:45:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: gsm/ipa: Add segmentation callback
by laforge 08 Jun '23

08 Jun '23

Attention is currently required from: arehbein, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33083 ) Change subject: gsm/ipa: Add segmentation callback ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33083 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3a639e6896cc3b3fc8e9b2e1a58254710efa0d3f Gerrit-Change-Number: 33083 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 10:44:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

08 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 4: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/f6e213c7_64b0b6bb PS3, Line 12: Use OTC_SELECT > > I personally suggest using stack-allocated buffers for those cases where we don't have/want static […] Agree with Harald. -- 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: 4 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: Thu, 08 Jun 2023 09:30:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: l1ctl_proto: add 'start_fn' field to UL/DL TBF CFG.req messages
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33222 ) Change subject: l1ctl_proto: add 'start_fn' field to UL/DL TBF CFG.req messages ...................................................................... Patch Set 1: (1 comment) File src/host/layer23/src/modem/rlcmac.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33222/comment/c7fec2be_0803ebb8 PS1, Line 166: 0xffffffff /* TODO: start Fn */); iiuc this 0xffffff is a specific meaning which means "use next available FN"? Can we add a define for it? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33222 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ibb6a05165fe1c81268fb0e3674adae4065e78171 Gerrit-Change-Number: 33222 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 09:28:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

08 Jun '23

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 14: (1 comment) Patchset: PS14: This patch caused a regression: https://jenkins.osmocom.org/jenkins/view/TTCN3/job/ttcn3-bts-test/2050/ The following two testcases started to fail: * BTS_Tests.TC_pcu_data_req_imm_ass_pch * BTS_Tests:hopping.TC_pcu_data_req_imm_ass_pch ``` Timeout waiting for PCU DATA.cnf BTS_Tests.ttcn:9033 BTS_Tests control part BTS_Tests.ttcn:5838 TC_pcu_data_req_imm_ass_pch testcase ``` Please modify `TC_pcu_data_req_imm_ass_pch` to not expect the confirmation, function `f_PCUIF_tx_imm_ass_pch` has an optional parameter `wait_for_cnf` for that. -- 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: 14 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: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 09:26:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmo-hnbgw[master]: cfg: add 'hnbgw' / 'plmn MCC MNC'
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178 ) Change subject: cfg: add 'hnbgw' / 'plmn MCC MNC' ...................................................................... Patch Set 5: (1 comment) File src/osmo-hnbgw/hnbgw_l3.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178/comment/d3fe4054_a35f4456 PS3, Line 281: osmo_plmn_from_bcd(ies->lai.pLMNidentity.buf, &local_plmn); > a log line here would say something like […] I didn't write the details because I expected you to know them better 😄 So, IIUC the idea here is that we prefer from now on that the user configures the PLMN in the config file, and taking the PLMN on-the-fly from sent/received messages is more like a hack for backward compatibility (may I have understood it wrong?). So, the point here is to notify through logging to the user that we are setting the PLMN here dynamically, but that the preferred way would be for them to configure it in the cfg file. This also allows the user to see if what we set dynamically matches their expectations. Regarding your examples, I think only the first one is useful to log for the reasons mentioned by me above. Maybe add text to hint the user to update the config file. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: If404c3859acdfba274c719c7cb7d16f97d831a2a Gerrit-Change-Number: 33178 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 09:24:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-hnbgw[master]: cfg: add 'hnbgw' / 'plmn MCC MNC'
by pespin 08 Jun '23

08 Jun '23

Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178 ) Change subject: cfg: add 'hnbgw' / 'plmn MCC MNC' ...................................................................... Patch Set 5: (1 comment) Patchset: PS4: > (btw, in case it is interesting to know why this occurs / to explain that this is not meant to ignor […] No worries, I was not thinking you were explicitly ignoring the comments. I write "not yet addressed" because it has several benefits: - Remark it in case you didn't see it / forgot about it - Remind me of the last review state after I re-review it, and for others to see that there's still stuff not done. In any case, I'd welcome if you could as much as possible fix all the comments before pushing a new version, since that means I don't need to do several rounds of review. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: If404c3859acdfba274c719c7cb7d16f97d831a2a Gerrit-Change-Number: 33178 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 09:19:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

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

08 Jun '23

Attention is currently required from: neels, laforge. pespin 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 5: (1 comment) File include/osmocom/hnbgw/context_map.h: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173/comment/1aa5ef49_b58d4380 PS3, Line 70: MAP_SCCP_EV_CN_LINK_LOST, > Scenario: […] IMHO LINK_LOST seems to indicate that lower layer connection (eg. SCTP) or even lower layer link (eg. ethernet) was lost. If you want to have a single event for all those, which goes from SCTP lost to RANAP RESET because they are to be acted the same way, then fine, but don't only mention "The CN link has a RESET" case. Otherwise, if you plan to have those as separate event, I'd rather call this EV_RX_RANAP_RESET or alike. -- 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: 5 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: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 08 Jun 2023 09:16:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

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

08 Jun '23

Attention is currently required from: neels, pespin. laforge 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 4: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/3b94c0a4_2fa9aa83 PS3, Line 12: Use OTC_SELECT > I personally suggest using stack-allocated buffers for those cases where we don't have/want static buffers. Agreed. > But I'm really surprised to hear any praise for static buffers at all. I thought that the consensus and aim is to get away from static buffers I'm not aware of such a notion/consensus. The heap allocated variants to me only make sense in cases where they don't work for some specific reason. Also, it is a big difference whether some function is called once (like to generate a fsm name, or some other name of an object that has a certain lifetime) in a while, or a function is called potentially thousands of times per second. > IIRC there is a log line that prints two distinct cell IDs If you know that, then I would suggest to special-case that one statement. > If you mean to imply that this is inefficient and expensive related to the rest of osmo-hnbgw I don't want to make such decisions specific to each and every program. The fact that there may be some other horribly inefficient parts in osmo-hnbgw shouldn't be seen as an excuse to introduce more *unless* there's a clear advantage. Also, given that the user triggering this development is very keen on efficiency, and we are currently spending an awful lot of time on osmo-bsc optimizations. Sooner or later I expect the same will happen with osmo-hnbgw, as it gets loaded. -- 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: 4 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: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 08:36:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: cnpool: extract Mobile Identity from RANAP payload
by laforge 08 Jun '23

08 Jun '23

Attention is currently required from: neels, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 ) Change subject: cnpool: extract Mobile Identity from RANAP payload ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I373d665c9684b607207f68094188eab63209db51 Gerrit-Change-Number: 33133 Gerrit-PatchSet: 5 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-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 08 Jun 2023 08:28:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0