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.