neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35095?usp=email )

Change subject: early-IA: use the correct TRX ......................................................................

early-IA: use the correct TRX

In early-Immediate-Assignment, the BSC sends the IMM ASS message directly after it sent the Channel Activation message, and osmo-bts should cache it until the Channel Activation is complete.

So far the code had a bug: it assumed that the lchan was on the same TRX where the IMM ASS is transmitted -- but actually, 'trx' refers to the BCCH channel's TRX, i.e. always c0.

Instead, look up the correct TRX by the ARFCN in the IMM ASS message.

Now, when frequency hopping is enabled, there will be no ARFCN in the IMM ASS message, hence this fix does not work with frequency hopping. Related osmo-bsc patch disallows this combination.

(To also support frequency hopping, osmo-bsc would need to modify the RSL protocol: send the IMM ASS message as a custom IE directly as part of the Channel Activation. Then it is always possible to correllate the IMM ASS with a specific trx and lchan, no matter what information it contains. However, early-IA is a "bad" feature in itself as it "promotes" having high latency on Abis. It seems unnecessary to do extra work to also support this odd use case for frequency hopping.)

Related: osmo-bsc I8d375e5155be7b53034d5c0be5566d2f33af5db0 Related: SYS#6655 Change-Id: Id9a930e5c67122812b229dc27ea2bfe246b67611 --- M src/common/rsl.c 1 file changed, 67 insertions(+), 9 deletions(-)

Approvals: neels: Looks good to me, approved Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve dexter: Looks good to me, but someone else must approve daniel: Looks good to me, but someone else must approve

diff --git a/src/common/rsl.c b/src/common/rsl.c index b5ec17f..bc2a5ea 100644 --- a/src/common/rsl.c +++ b/src/common/rsl.c @@ -371,6 +371,16 @@ return true; }

+static struct gsm_bts_trx *trx_lookup_by_arfcn(struct llist_head *trx_list, uint16_t arfcn) +{ + struct gsm_bts_trx *trx; + llist_for_each_entry(trx, trx_list, list) { + if (trx->arfcn == arfcn) + return trx; + } + return NULL; +} + static struct gsm_lchan *lchan_lookup(struct gsm_bts_trx *trx, uint8_t chan_nr, const char *log_name) { @@ -1383,18 +1393,33 @@ * the channel is active. Hence we still wait for the activation, but don't need the Abis roundtrip of Activ ACK * -> Immediate Assignment via the BSC. * If anything is wrong with the sizes or the lchan lookup, behave normally, i.e. do not do the RR IA caching, - * but just send the RR message to the MS as-is. */ + * but just send the RR message to the MS as-is. + * 'trx' here is the TRX of the BCCH channel. To find the correct TRX for the IMM ASS target, we need to look up + * the ARFCN that is contained in the IMM ASS message. When frequency hopping is enabled, there will not be an + * ARFCN, so we cannot support early-IA with frequency hopping enabled. */ if (msg->len >= sizeof(struct gsm48_imm_ass)) { struct gsm48_imm_ass *rr_ia = (void*)msg->data; - struct gsm_lchan *ia_target_lchan = lchan_lookup(trx, rr_ia->chan_desc.chan_nr, "Early IA check: "); - if (ia_target_lchan && ia_target_lchan->state != LCHAN_S_ACTIVE) { - /* Target lchan is not yet active. Cache the IA. - * If a previous IA is still lingering, free it. */ - msgb_free(ia_target_lchan->early_rr_ia); - ia_target_lchan->early_rr_ia = msg; + if (rr_ia->chan_desc.h0.h == 0) { + /* hopping is disabled. */ + struct gsm_bts_trx *ia_target_trx; + uint16_t arfcn; + arfcn = (rr_ia->chan_desc.h0.arfcn_high << 8) + rr_ia->chan_desc.h0.arfcn_low;

- /* return 1 means: don't msgb_free() the msg */ - return 1; + ia_target_trx = trx_lookup_by_arfcn(&trx->bts->trx_list, arfcn); + if (ia_target_trx) { + /* found the ARFCN's trx */ + struct gsm_lchan *ia_target_lchan; + ia_target_lchan = lchan_lookup(ia_target_trx, rr_ia->chan_desc.chan_nr, "Early IA check: "); + if (ia_target_lchan && ia_target_lchan->state != LCHAN_S_ACTIVE) { + /* Target lchan is not yet active. Cache the IA. + * If a previous IA is still lingering, free it. */ + msgb_free(ia_target_lchan->early_rr_ia); + ia_target_lchan->early_rr_ia = msg; + + /* return 1 means: don't msgb_free() the msg */ + return 1; + } + } } }