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