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

5 files changed, 27 insertions, 23 deletions

diff --git a/src/bts.cpp b/src/bts.cpp
index d4bb35fa..8d6f1566 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -52,7 +52,6 @@ extern "C" {
 #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 bts_rfn_to_fn(const struct gprs_rlcmac_bts *bts, uint32_t rfn)
 	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/bts.h b/src/bts.h
index 7dd85478..7a67f76c 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -196,7 +196,7 @@ struct rach_ind_params {
 	uint16_t ra;
 	uint8_t trx_nr;
 	uint8_t ts_nr;
-	uint32_t rfn;
+	uint16_t rfn;
 	int16_t qta;
 };
 
diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp
index aadc6406..cda7af7a 100644
--- a/src/pcu_l1_if.cpp
+++ b/src/pcu_l1_if.cpp
@@ -630,7 +630,7 @@ extern "C" int pcu_rx_rach_ind_ptcch(struct gprs_rlcmac_bts *bts, uint8_t trx_nr
 		.ra = 0x00,
 		.trx_nr = trx_nr,
 		.ts_nr = ts_nr,
-		.rfn = fn,
+		.rfn = fn2rfn(fn),
 		.qta = qta,
 	};
 
@@ -640,11 +640,22 @@ extern "C" int pcu_rx_rach_ind_ptcch(struct gprs_rlcmac_bts *bts, uint8_t trx_nr
 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;
+
+	/* 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, 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);
+	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 @@ static int pcu_rx_rach_ind(struct gprs_rlcmac_bts *bts, const struct gsm_pcu_if_
 		.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 1c698ca4..a6beab04 100644
--- a/src/pcu_utils.h
+++ b/src/pcu_utils.h
@@ -84,6 +84,12 @@ static inline uint8_t pcu_lsb(uint8_t x)
 	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 bbd68211..ae8b56fd 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -80,7 +80,7 @@ static int bts_handle_rach(struct gprs_rlcmac_bts *bts, uint16_t ra, uint32_t Fn
 		.ra = ra,
 		.trx_nr = 0,
 		.ts_nr = 0,
-		.rfn = Fn,
+		.rfn = fn2rfn(Fn),
 		.qta = qta,
 	};