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
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
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
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
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
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
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
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