Attention is currently required from: laforge, pespin, dexter.

pespin has uploaded this change for review.

View Change

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 change 33119. To unsubscribe, or for help writing mail filters, visit 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