Attention is currently required from: laforge, pespin, dexter. pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33119 ) Change subject: Use always RFN when handling RACH indications ...................................................................... Use always RFN when handling RACH indications The previous code was really confusing, passing full FNs as RFNs under certain external conditions, and then assuming the RFN input of rfn_to_fn() function could actually be a FN. As a result, we had a lot of code behaving slightly different depending on whether the incomding FN from pcuif was filled in by a BSC or a BTS. Avoid this b ehavior differentiation and always assume the most restricted one, aka RFN. Change-Id: Ib3b5702168195b595711cd0ff32c211b9aba429d --- M src/bts.cpp M src/pcu_l1_if.cpp M src/pcu_utils.h M tests/tbf/TbfTest.cpp 4 files changed, 44 insertions(+), 22 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/19/33119/1 diff --git a/src/bts.cpp b/src/bts.cpp index d4bb35f..8d6f156 100644 --- a/src/bts.cpp +++ b/src/bts.cpp @@ -52,7 +52,6 @@ #include <errno.h> #include <string.h> -#define RFN_MODULUS 42432 #define RFN_THRESHOLD RFN_MODULUS / 2 extern void *tall_pcu_ctx; @@ -744,25 +743,13 @@ uint32_t m_cur_rfn; uint32_t fn_rounded; - /* make sure RFN does not exceed the maximum possible value of a valid - * GSM frame number. */ - OSMO_ASSERT(rfn < GSM_MAX_FN); - - /* Note: If a BTS is sending in a rach request it will be fully aware - * of the frame number. If the PCU is used in a BSC-co-located setup. - * The BSC will forward the incoming RACH request. The RACH request - * only contains the relative frame number (Fn % 42432) in its request - * reference. This PCU implementation has to fit both scenarios, so - * we need to assume that Fn is a relative frame number. */ - /* Ensure that all following calculations are performed with the * relative frame number */ - if (rfn >= RFN_MODULUS) - return rfn; + OSMO_ASSERT(rfn < RFN_MODULUS); /* Compute an internal relative frame number from the full internal frame number */ - m_cur_rfn = bts->cur_fn % RFN_MODULUS; + m_cur_rfn = fn2rfn(bts->cur_fn); /* Compute a "rounded" version of the internal frame number, which * exactly fits in the RFN_MODULUS raster */ diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp index aadc640..cda7af7 100644 --- a/src/pcu_l1_if.cpp +++ b/src/pcu_l1_if.cpp @@ -630,7 +630,7 @@ .ra = 0x00, .trx_nr = trx_nr, .ts_nr = ts_nr, - .rfn = fn, + .rfn = fn2rfn(fn), .qta = qta, }; @@ -640,11 +640,22 @@ static int pcu_rx_rach_ind(struct gprs_rlcmac_bts *bts, const struct gsm_pcu_if_rach_ind *rach_ind) { int rc = 0; - int current_fn = bts_current_frame_number(bts); + uint32_t current_fn = bts_current_frame_number(bts); + uint16_t rfn; - LOGP(DL1IF, LOGL_INFO, "RACH request received: sapi=%d " - "qta=%d, ra=0x%02x, fn=%u, cur_fn=%d, is_11bit=%d\n", rach_ind->sapi, rach_ind->qta, - rach_ind->ra, rach_ind->fn, current_fn, rach_ind->is_11bit); + /* Note: If a BSC is sending a RACH req to us, it is actually forwarding it to + * us from BTS as a result of receiving an RFN (Fn % 42432) over RSL + * (see 3GPP TS 48.058, section 9.3.19). + * If a BTS is sending a RACH req to us, it may contain a full FN + * (current osmo-bts does that) instead of an RFN. + * For consistency, and taking into account the BSC case limitations, + * work always with RFNs here: + */ + rfn = fn2rfn(rach_ind->fn); + + LOGP(DL1IF, LOGL_INFO, + "RACH request received: sapi=%d qta=%d, ra=0x%02x, fn=%u (rfn=%u), cur_fn=%d, is_11bit=%d\n", + rach_ind->sapi, rach_ind->qta, rach_ind->ra, rach_ind->fn, rfn, current_fn, rach_ind->is_11bit); if (OSMO_UNLIKELY(rach_ind->fn > GSM_TDMA_HYPERFRAME - 1)) { LOGP(DL1IF, LOGL_ERROR, "RACH request contains fn=%u that exceeds valid limits (0-%u) -- ignored!\n", @@ -658,7 +669,7 @@ .ra = rach_ind->ra, .trx_nr = rach_ind->trx_nr, .ts_nr = rach_ind->ts_nr, - .rfn = rach_ind->fn, + .rfn = rfn, .qta = rach_ind->qta, }; diff --git a/src/pcu_utils.h b/src/pcu_utils.h index 2dd86ac..0be37e9 100644 --- a/src/pcu_utils.h +++ b/src/pcu_utils.h @@ -77,6 +77,12 @@ return x & -x; } +#define RFN_MODULUS 42432 +static inline uint16_t fn2rfn(uint32_t fn) +{ + return fn % RFN_MODULUS; +} + /* Used to store a C++ class in a llist used by C code */ struct llist_item { struct llist_head list; /* item used by llist */ diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp index bbd6821..ae8b56f 100644 --- a/tests/tbf/TbfTest.cpp +++ b/tests/tbf/TbfTest.cpp @@ -80,7 +80,7 @@ .ra = ra, .trx_nr = 0, .ts_nr = 0, - .rfn = Fn, + .rfn = fn2rfn(Fn), .qta = qta, }; -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33119 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ib3b5702168195b595711cd0ff32c211b9aba429d Gerrit-Change-Number: 33119 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier@sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Attention: laforge <laforge@osmocom.org> Gerrit-Attention: pespin <pespin@sysmocom.de> Gerrit-Attention: dexter <pmaier@sysmocom.de> Gerrit-MessageType: newchange