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

[S] Change in libosmocore[master]: osmo_io: Remove missing functions from map file
by fixeria 30 Jun '23

30 Jun '23

Attention is currently required from: daniel. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33537 ) Change subject: osmo_io: Remove missing functions from map file ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33537 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4dbc1cf9b05521325287803f00781001b80945ef Gerrit-Change-Number: 33537 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Jun 2023 18:24:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: osmo_io: Make the test more deterministic between backends
by fixeria 30 Jun '23

30 Jun '23

Attention is currently required from: daniel. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33496 ) Change subject: osmo_io: Make the test more deterministic between backends ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33496 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibdd26a4aeadfbd6c5039c8a31cc120d3c98be727 Gerrit-Change-Number: 33496 Gerrit-PatchSet: 4 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Jun 2023 18:23:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmo-netif[master]: stream: Add IPA send function/IPA-mode read to srv
by arehbein 30 Jun '23

30 Jun '23

Attention is currently required from: laforge. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201 to look at the new patch set (#6). Change subject: stream: Add IPA send function/IPA-mode read to srv ...................................................................... stream: Add IPA send function/IPA-mode read to srv Also: Adapt the example to work with the new changes. Related OS#5753, OS#5751 Change-Id: I61e1fe59166c46595efe8c1f32b8f2607cb6c529 --- M examples/ipa-stream-server.c M include/osmocom/netif/stream.h M src/stream.c M tests/stream/stream_test.c M tests/stream/stream_test.ok 5 files changed, 136 insertions(+), 38 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/01/33201/6 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I61e1fe59166c46595efe8c1f32b8f2607cb6c529 Gerrit-Change-Number: 33201 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmo-netif[master]: ipa: Add helper osmo_ipa_ext_msg_alloc()
by arehbein 30 Jun '23

30 Jun '23

Attention is currently required from: arehbein. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33423 to look at the new patch set (#5). Change subject: ipa: Add helper osmo_ipa_ext_msg_alloc() ...................................................................... ipa: Add helper osmo_ipa_ext_msg_alloc() Add helper that allocates headroom for extended IPA headers automatically to go with IPA trx mode Related: OS#5753 Change-Id: I231026442503e6d7f720fd100f38c01916235d63 --- M include/osmocom/netif/ipa.h M src/ipa.c 2 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/23/33423/5 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33423 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I231026442503e6d7f720fd100f38c01916235d63 Gerrit-Change-Number: 33423 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmo-netif[master]: example: Remove call to osmo_ipa_process_msg()
by arehbein 30 Jun '23

30 Jun '23

Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33205 ) Change subject: example: Remove call to osmo_ipa_process_msg() ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > why do it that way? why is it needed? why is it better than having it properly separated in multiple […] I would say the answer is part of the discussion that has been led on the IRC Osmocom development channel -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33205 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I53283ec7bd7f07dfa612681ae84af93d5cd098b9 Gerrit-Change-Number: 33205 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 30 Jun 2023 17:46:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmo-netif[master]: stream: Move helper functions
by arehbein 30 Jun '23

30 Jun '23

arehbein has abandoned this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33203 ) Change subject: stream: Move helper functions ...................................................................... Abandoned squashed into previous commit -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33203 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I318965538e5329c44d0910694621b5e1f1db0626 Gerrit-Change-Number: 33203 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: abandon

1 0

[M] Change in libosmo-netif[master]: stream: Add IPA send function/IPA-mode read to srv
by arehbein 30 Jun '23

30 Jun '23

Attention is currently required from: laforge. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33201 ) Change subject: stream: Add IPA send function/IPA-mode read to srv ...................................................................... Patch Set 5: (4 comments) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/fddbaf17_de768f0c PS1, Line 632: SRV > why SRV? isn't it CLI_OR_INP or something like that? The idea was to name the macro after the use-case it was designed for, since in all (or at least a lot of) `*_srv_*` code, `LOGP(DLINP, ...)` is done for logging https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/c6119e9e_32c596be PS1, Line 1952: from the msgb structure's l1 and possibly l2 headers > why from the l1/l2 headers? Wouldn't it be more in-line with existign code to use something like the […] Iirc, the agreement had been to have the IPA read callback pull the headers from the message, but preserve them in l1h/l2h to enable access to them in case upper layers still need that information. And since that information hence could thus already be expected to be there (depending on the type of message received), I decided to give the caller of this function the option to access the protocol information this way for convenience. I can of course scratch it https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/134a8705_a5c083cd PS1, Line 1959: OSMO_ASSERT > below non-ipa code has OSMO_ASSERT(conn), too. Yours not. […] Yes, because `conn` isn't used by this function before it is passed to `osmo_stream_srv_send()`, which then does the check. File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/4918c925_ddf83f80 PS5, Line 1947: /* msgb_l1(msg) is expected to be set */ > I dont really get this part. […] The idea was providing the possibility of quickly reusing the headers of a message received in IPA mode (in which case `msg->l1h` will point to the IPA header, `msg->l2h` will point to the first octet after the IPA header and `msg->data` will point to the IPA payload). If we have just created a regular message buffer with an IPA message/a complete IPA message, then we would have `msg->l1h == data` and hence also the header sitting at `msg->l1h` (although `msg->l2h` would have to be set as well in case of using `IPAC_PROTO_OSMO`, but this wasn't the use-case I had in mind, what I had in mind was the above paragraph). It was something that I added because the usecase described in the first paragraph seemed convenient when I wrote the patch. I can remove the feature using these helpers along with the helper functions themselves any time. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I61e1fe59166c46595efe8c1f32b8f2607cb6c529 Gerrit-Change-Number: 33201 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Fri, 30 Jun 2023 17:31:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0

[M] Change in osmo-pcu[master]: nacc_fsm: Add support for NACC with UTRAN and E-UTRAN cells
by pespin 30 Jun '23

30 Jun '23

Attention is currently required from: dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33538 ) Change subject: nacc_fsm: Add support for NACC with UTRAN and E-UTRAN cells ...................................................................... Patch Set 2: (6 comments) File src/nacc_fsm.c: https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/df520929_4a4b94ad PS2, Line 273: /* Classify the RAT type of the cell that is proposed in the PacketCellChangeNotification. In case the RAT is GERAN, Can you maybe convert these defines to an enum and provide spec reference on where to find this exactly? https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/4399dc1a_b5e37f74 PS2, Line 278: static int fill_neigh_key_from_bts_pkt_cell_chg_not(struct nacc_fsm_ctx *ctx, I actually think you don't need to return 0/1/2 here, see below. https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/a3c6d948_0b4e7031 PS2, Line 344: return NACC_EUTRAN_CELL; maybe set ctx->neigh_key_valid = false explicitly for readers to understand better (and to make sure the field is initialized/overwritten). https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/c23ec402_025c8c87 PS2, Line 398: } else if (rc == NACC_UTRAN_CELL || rc == NACC_EUTRAN_CELL) { I think in here what you may actually want to do is: """ if (!ctx->neigh_key_valid) { /* When the proposed call is an UTRAN or EUTRAN cell, we won't provide any system information * to the UE. Instead we will send the PacketCellChangeContinue message immediately. This also * applies in the case of re-transmissions. See also: 3GPP TS 48.018, section 8c.6.1. */ LOGPFSML(ctx->fi, LOGL_NOTICE, "Target cell is an UTRAN or EUTRAN cell => no system information provided.\n"); nacc_fsm_state_chg(ctx->fi, NACC_ST_TX_CELL_CHG_CONTINUE); return; } """ So you don't really need to check whether it's GERAN/UTRAN/EUTRAN, but whether we actually got any neigh to ask information for... if no info to get, then continue straight to tx PKT CELL CHG CONTINUE. https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/04090646_fbc2c7fd PS2, Line 433: else if (rc == NACC_UTRAN_CELL || rc == NACC_EUTRAN_CELL) { see same as above. File src/neigh_cache.h: https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/a99d13c3_1966e7da PS2, Line 68: /* TODO: This function seems to be used only in neigh_cache.c, make it static? */ this is definetly another patch. Fine with making it static. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33538 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I96280f0ec5955ed3cb17641bf4118496c929bdac Gerrit-Change-Number: 33538 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: Fri, 30 Jun 2023 15:29:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: ctrl: Add rach max delay
by pespin 30 Jun '23

30 Jun '23

Attention is currently required from: matanp. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/33541 ) Change subject: ctrl: Add rach max delay ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/33541 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I113211cc7e7ef20fac7e9d30e156522406ad62f7 Gerrit-Change-Number: 33541 Gerrit-PatchSet: 1 Gerrit-Owner: matanp <matan1008(a)gmail.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: matanp <matan1008(a)gmail.com> Gerrit-Comment-Date: Fri, 30 Jun 2023 15:20:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes 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 30 Jun '23

30 Jun '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 2: (4 comments) File pcu/PCU_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/daf078d2_caad… PS2, Line 5336: private function f_outbound_nacc_success_utran_eutran(inout GprsMS ms, PCUIF_info_ind info_ind, I think you can really split the UTRAN and EUTRAN cases in different functions, it will be easier to extend or modify in the future. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/07fb9275_76f1… PS2, Line 5351: cell_chg_notif := ts_RlcMacUlCtrl_PKT_CELL_CHG_NOTIF_EUTRAN(ms.ul_tbf.tfi, req_earfcn, phys_layer_cell_id); If this is the only thing that changes, maybe pass template (value) RlcmacUlCtrlMsg cell_chg_notif as a param? https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/9ec037dd_b258… PS2, Line 5378: private function f_outbound_nacc_success(inout GprsMS ms, PCUIF_info_ind info_ind, This one should be renamed to _geran in another patch maybe? https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/84c6e3e3_1157… PS2, Line 5427: function f_TC_nacc_outbound_success(integer ran_type) runs on RAW_PCU_Test_CT { please enum or string, this is totally hack -- 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: 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: Fri, 30 Jun 2023 15:16:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0