Attention is currently required from: pespin.
Hello Jenkins Builder, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540
to look at the new patch set (#3).
Change subject: PCU_Tests: test NACC procedure with UTRAN and E-UTRAN cell
......................................................................
PCU_Tests: test NACC procedure with UTRAN and E-UTRAN cell
When the PacketCellChangeNotification proposes an UTRAN or E-UTRAN cell,
then the PCU will not provide system information. Instead it will directly
conclude the NACC procedure with a PacketCellChangeContinue message.
Related: OS#6044
Change-Id: Idae86a458fd44ac81bab183ed1865b1c1bdbfd66
---
M library/RLCMAC_CSN1_Templates.ttcn
M pcu/PCU_Tests.ttcn
2 files changed, 203 insertions(+), 11 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/40/33540/3
--
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: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: pespin.
dexter 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:
(4 comments)
File pcu/PCU_Tests.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/1db3cf2d_9759…
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 […]
Done
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/8bc1fef0_7ab5…
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 a […]
Done
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/aefa301b_7e15…
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?
Done
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33540/comment/9d322a86_edf0…
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
Done
--
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: 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
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 4: Code-Review+1
--
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: 4
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 12:46:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
dexter 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 4:
(1 comment)
File src/nacc_fsm.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/76cd4991_3d1364ea
PS3, Line 282: ctx->neigh_key_present = false;
> I didn't notice you are setting this already to false in here, so then there's no need to set it to […]
Ah, ok, thought that it would make sense for a second, but yes, lets remove the ctx->neigh_key_present = false; lines again.
--
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: 4
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 12:44:13 +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: pespin.
Hello Jenkins Builder, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-pcu/+/33538
to look at the new patch set (#4).
Change subject: nacc_fsm: Add support for NACC with UTRAN and E-UTRAN cells
......................................................................
nacc_fsm: Add support for NACC with UTRAN and E-UTRAN cells
The NACC procedure in OsmoPCU currently only supports NACC with GERAN
cells. When an UTRAN or E-UTRAN cell is proposed in the
PacketCellChangeNotification, then the FSM is immediately terminated,
meaning that the NACC procedure can not commence.
When the NACC procedure is carried out for UTRAN or E-UTRAN cells, the
PCU will not send any system information to the UE. Instead it
immediately sends a PacketCellChangeContinue message. This also means
that we do not carry out any RAN Information Requests in the background.
This patch adds logic to detect if the proprosed cell is an UTRAN or
E-UTRAN cell and the adds a new transition to the FSM so that a short
route to NACC_ST_TX_CELL_CHG_CONTINUE can be taken.
Related: OS#6044
Change-Id: I96280f0ec5955ed3cb17641bf4118496c929bdac
---
M src/nacc_fsm.c
M src/nacc_fsm.h
2 files changed, 127 insertions(+), 18 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/38/33538/4
--
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: 4
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-MessageType: newpatchset
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 3:
(1 comment)
File src/nacc_fsm.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/b25ca21b_cad27b9f
PS3, Line 282: ctx->neigh_key_present = false;
I didn't notice you are setting this already to false in here, so then there's no need to set it to false in all other code paths.
--
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: 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 11:41:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
dexter 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 3:
(6 comments)
File src/nacc_fsm.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/859ae223_965633d8
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 exac […]
(I came up with this, those values are just return codes. The spec encodes the RAT type in this weird tree structure.)
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/e56f75ff_3b3d2ee5
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.
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/867fef4a_24da7415
PS2, Line 344: return NACC_EUTRAN_CELL;
> maybe set ctx->neigh_key_valid = false explicitly for readers to understand better (and to make sure […]
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/242020b6_9df15c10
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: […]
I think this makes sense. This also simplifies fill_neigh_key_from_bts_pkt_cell_chg_not() and we can check ctx->neigh_key_valid (now renamed to ctx->neigh_key_present). No neigh_key_present + return code 0 => no system information provided.
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/dcc28292_249da948
PS2, Line 433: else if (rc == NACC_UTRAN_CELL || rc == NACC_EUTRAN_CELL) {
> see same as above.
Done
File src/neigh_cache.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/e5b0e89f_012028c9
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.
Done
--
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: 3
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 10:55:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment