[PATCH] osmo-pcu[master]: BTS: Convert relative frame numbers to absolute frame numbers

dexter gerrit-no-reply at lists.osmocom.org
Wed Feb 22 12:21:24 UTC 2017


Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/1861

to look at the new patch set (#3).

BTS: Convert relative frame numbers to absolute frame numbers

The implementation of the method rcv_rach() in class BTS, restores
the absolute frame number of the incoming RACH-Request by using
the relative frame number (RFn = Fn mod 42432) from the rach
request and the already known internal absolute frame number
m_cur_fn, which is continusly updated by the CCU interface.

In some rare cases, a RACH request might be received by the BTS,
a very short time before the frame number wraps in its 42432.
Depending on the PCU location, RACH request might be received
by the BSC, which forwards it to the PCU. It is then likely
that, while the RACH request is being forwarded to the PCU, the
PCU internal absolute frame number wraps before the RACH
can be processed. The relative frame number from the rach
request would then be interpreted as if it were received after
the wrapping of the internal frame number modulos.

This commit adds logic to detect and resolve this race condition.
Also a unit test is added to check some cornercases.

Change-Id: I74f00c11e5739d49f370ce6c357149e81d9aa759
---
M src/bts.cpp
M src/bts.h
M tests/Makefile.am
A tests/fn/FnTest.cpp
A tests/fn/FnTest.ok
M tests/testsuite.at
6 files changed, 306 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/61/1861/3

diff --git a/src/bts.cpp b/src/bts.cpp
index 21e9d96..553b952 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -34,12 +34,16 @@
 	#include <osmocom/core/msgb.h>
 	#include <osmocom/core/stats.h>
 	#include <osmocom/gsm/protocol/gsm_04_08.h>
+	#include <osmocom/gsm/gsm_utils.h>
 }
 
 #include <arpa/inet.h>
 
 #include <errno.h>
 #include <string.h>
+
+#define RFN_MODULUS 42432
+#define RFN_THRESHOLD RFN_MODULUS / 2
 
 extern void *tall_pcu_ctx;
 
@@ -516,6 +520,62 @@
 	return 0;
 }
 
+/* Determine the full frame number from a relative frame number */
+uint32_t BTS::rfn_to_fn(uint32_t rfn)
+{
+	uint32_t m_cur_rfn;
+	uint32_t fn;
+	uint32_t fn_rounded;
+
+	/* 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 */
+	rfn = rfn % 42432;
+
+	/* Compute an internal relative frame number from the full internal
+	   frame number */
+	m_cur_rfn = m_cur_fn % RFN_MODULUS;
+
+	/* Compute a "rounded" version of the internal frame number, which
+	 * exactly fits in the RFN_MODULUS raster */
+	if (m_cur_fn - m_cur_rfn < 0) {
+		LOGP(DRLCMAC, LOGL_ERROR,
+		     "Invalid condition detected: m_cur_rfn (%u) larger than m_cur_fn (%u)\n",
+		     m_cur_rfn, m_cur_fn);
+	}
+	fn_rounded = m_cur_fn - m_cur_rfn;
+
+	/* If the delta between the internal and the external relative frame
+	 * number exceeds a certain limit, we need to assume that the incoming
+	 * rach request belongs to a the previous rfn period. To correct this,
+	 * we roll back the rounded frame number by one RFN_MODULUS */
+	if (abs(rfn - m_cur_rfn) > RFN_THRESHOLD) {
+		LOGP(DRLCMAC, LOGL_DEBUG,
+		     "Race condition between rfn (%u) and m_cur_fn (%u) detected: rfn belongs to the previos modulus %u cycle, wrappng...\n",
+		     rfn, m_cur_fn, RFN_MODULUS);
+		if (fn_rounded < RFN_MODULUS) {
+			LOGP(DRLCMAC, LOGL_DEBUG,
+			"Cornercase detected: wrapping crosses %u border\n",
+			GSM_MAX_FN);
+			fn_rounded = GSM_MAX_FN - (RFN_MODULUS - fn_rounded);
+		}
+		else
+			fn_rounded -= RFN_MODULUS;
+	}
+
+	/* The real frame number is the sum of the rounded frame number and the
+	 * relative framenumber computed via RACH */
+	fn = fn_rounded + rfn;
+
+	return fn;
+}
+
 int BTS::rcv_rach(uint16_t ra, uint32_t Fn, int16_t qta, uint8_t is_11bit,
 		enum ph_burst_type burst_type)
 {
@@ -536,20 +596,8 @@
 	if (is_11bit)
 		rach_frame_11bit();
 
-	/* 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 secenarious, so
-	 * we need to assume that Fn is a relative frame number. */
-
-	/* Ensure that all following calculations are performed with the
-	 * relative frame number */
-	Fn = Fn % 42432;
-
-	/* Restore the full frame number
-	 * (See also 3GPP TS 44.018, section 10.5.2.38) */
-	Fn = Fn + m_cur_fn - m_cur_fn % 42432;
+	/* Determine full frame number */
+	Fn = rfn_to_fn(Fn);
 
 	LOGP(DRLCMAC, LOGL_DEBUG, "MS requests UL TBF on RACH, "
 		"so we provide one: ra=0x%02x Fn=%u qta=%d is_11bit=%d:\n",
diff --git a/src/bts.h b/src/bts.h
index 2932154..5d8dc59 100644
--- a/src/bts.h
+++ b/src/bts.h
@@ -349,6 +349,8 @@
 	int rcv_imm_ass_cnf(const uint8_t *data, uint32_t fn);
 	uint8_t is_single_block(uint16_t ra, enum ph_burst_type burst_type,
 		uint8_t is_11bit, uint16_t *ms_class, uint16_t *priority);
+
+	uint32_t rfn_to_fn(uint32_t rfn);
 	int rcv_rach(uint16_t ra, uint32_t Fn, int16_t qta, uint8_t is_11bit,
 		enum ph_burst_type burst_type);
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a24f4ea..a372354 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,7 +1,7 @@
 AM_CPPFLAGS = $(STD_DEFINES_AND_INCLUDES) $(LIBOSMOCORE_CFLAGS) $(LIBOSMOGB_CFLAGS) $(LIBOSMOGSM_CFLAGS) -I$(top_srcdir)/src/
 AM_LDFLAGS = -lrt
 
-check_PROGRAMS = rlcmac/RLCMACTest alloc/AllocTest tbf/TbfTest types/TypesTest ms/MsTest llist/LListTest llc/LlcTest codel/codel_test edge/EdgeTest bitcomp/BitcompTest
+check_PROGRAMS = rlcmac/RLCMACTest alloc/AllocTest tbf/TbfTest types/TypesTest ms/MsTest llist/LListTest llc/LlcTest codel/codel_test edge/EdgeTest bitcomp/BitcompTest fn/FnTest
 noinst_PROGRAMS = emu/pcu_emu
 
 rlcmac_RLCMACTest_SOURCES = rlcmac/RLCMACTest.cpp
@@ -90,6 +90,14 @@
 	$(LIBOSMOCORE_LIBS) \
 	$(COMMON_LA)
 
+fn_FnTest_SOURCES = fn/FnTest.cpp
+fn_FnTest_LDADD = \
+	$(top_builddir)/src/libgprs.la \
+	$(LIBOSMOGB_LIBS) \
+	$(LIBOSMOGSM_LIBS) \
+	$(LIBOSMOCORE_LIBS) \
+	$(COMMON_LA)
+
 # The `:;' works around a Bash 3.2 bug when the output is not writeable.
 $(srcdir)/package.m4: $(top_srcdir)/configure.ac
 	:;{ \
@@ -119,7 +127,8 @@
 	llc/LlcTest.ok llc/LlcTest.err \
 	llist/LListTest.ok llist/LListTest.err \
 	codel/codel_test.ok \
-	edge/EdgeTest.ok
+	edge/EdgeTest.ok \
+	fn/FnTest.ok fn/FnTest.err
 
 DISTCLEANFILES = atconfig
 
diff --git a/tests/fn/FnTest.cpp b/tests/fn/FnTest.cpp
new file mode 100644
index 0000000..279903c
--- /dev/null
+++ b/tests/fn/FnTest.cpp
@@ -0,0 +1,175 @@
+/* Frame number calculation test */
+
+/* (C) 2016 by sysmocom s.f.m.c. GmbH <info at sysmocom.de>
+ * All Rights Reserved
+ *
+ * Author: Philipp Maier
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "bts.h"
+#include <string.h>
+#include <stdio.h>
+
+extern "C" {
+#include <osmocom/core/application.h>
+#include <osmocom/gsm/gsm_utils.h>
+#include <osmocom/core/talloc.h>
+#include <osmocom/core/utils.h>
+}
+
+#define RFN_MODULUS 42432
+
+/* globals used by the code */ void *tall_pcu_ctx;
+int16_t spoof_mnc = 0, spoof_mcc = 0;
+
+static uint32_t calc_fn(BTS * bts, uint32_t rfn)
+{
+	uint32_t fn;
+	fn = bts->rfn_to_fn(rfn);
+	printf("rfn=%i ==> fn=%i\n", rfn, fn);
+	return fn;
+}
+
+static void set_fn(BTS * bts, uint32_t fn)
+{
+	printf("\n");
+	bts->set_current_frame_number(fn);
+	printf("bts: fn=%i\n", fn);
+}
+
+static void run_test()
+{
+	BTS bts;
+	uint32_t fn;
+
+	printf("RFN_MODULUS=%i\n",RFN_MODULUS);
+	printf("GSM_MAX_FN=%i\n",GSM_MAX_FN);
+
+
+	/* Test with a collection of real world examples,
+	 * all all of them are not critical and do not
+	 * assume the occurence of any race contions */
+	set_fn(&bts, 1320462);
+	fn = calc_fn(&bts, 5066);
+	OSMO_ASSERT(fn == 1320458);
+
+	set_fn(&bts, 8246);
+	fn = calc_fn(&bts, 8244);
+	OSMO_ASSERT(fn == 8244);
+
+	set_fn(&bts, 10270);
+	fn = calc_fn(&bts, 10269);
+	OSMO_ASSERT(fn == 10269);
+
+	set_fn(&bts, 311276);
+	fn = calc_fn(&bts, 14250);
+	OSMO_ASSERT(fn == 311274);
+
+
+	/* Now lets assume a case where the frame number
+	 * just wrapped over a little bit above the
+	 * modulo 42432 raster, but the rach request
+	 * occurred before the wrapping */
+	set_fn(&bts, RFN_MODULUS + 30);
+	fn = calc_fn(&bts, RFN_MODULUS - 10);
+	OSMO_ASSERT(fn == 42422);
+
+	set_fn(&bts, RFN_MODULUS + 1);
+	fn = calc_fn(&bts, RFN_MODULUS - 1);
+	OSMO_ASSERT(fn == 42431);
+
+	set_fn(&bts, RFN_MODULUS * 123 + 16);
+	fn = calc_fn(&bts, RFN_MODULUS - 4);
+	OSMO_ASSERT(fn == 5219132);
+
+	set_fn(&bts, RFN_MODULUS * 123 + 451);
+	fn = calc_fn(&bts, RFN_MODULUS - 175);
+	OSMO_ASSERT(fn == 5218961);
+
+
+	/* Lets check a special cornercase. We assume that
+	 * the BTS just wrapped its internal frame number
+	 * but we still get rach requests with high relative
+	 * frame numbers. */
+	set_fn(&bts, 0);
+	fn = calc_fn(&bts, RFN_MODULUS - 13);
+	OSMO_ASSERT(fn == 2715635);
+
+	set_fn(&bts, 453);
+	fn = calc_fn(&bts, RFN_MODULUS - 102);
+	OSMO_ASSERT(fn == 2715546);
+
+	set_fn(&bts, 10);
+	fn = calc_fn(&bts, RFN_MODULUS - 10);
+	OSMO_ASSERT(fn == 2715638);
+
+	set_fn(&bts, 23);
+	fn = calc_fn(&bts, RFN_MODULUS - 42);
+	OSMO_ASSERT(fn == 2715606);
+
+
+	/* Also check with some corner case
+	 * values where Fn and RFn reach its
+	 * maximum/minimum valid range */
+	set_fn(&bts, GSM_MAX_FN);
+	fn = calc_fn(&bts, RFN_MODULUS-1);
+	OSMO_ASSERT(fn == GSM_MAX_FN-1);
+
+	set_fn(&bts, 0);
+	fn = calc_fn(&bts, RFN_MODULUS-1);
+	OSMO_ASSERT(fn == GSM_MAX_FN-1);
+
+	set_fn(&bts, GSM_MAX_FN);
+	fn = calc_fn(&bts, 0);
+	OSMO_ASSERT(fn == GSM_MAX_FN);
+
+	set_fn(&bts, 0);
+	fn = calc_fn(&bts, 0);
+	OSMO_ASSERT(fn == 0);
+}
+
+int main(int argc, char **argv)
+{
+	tall_pcu_ctx = talloc_named_const(NULL, 1, "fn test context");
+	if (!tall_pcu_ctx)
+		abort();
+
+	msgb_talloc_ctx_init(tall_pcu_ctx, 0);
+	osmo_init_logging(&gprs_log_info);
+	log_set_use_color(osmo_stderr_target, 0);
+	log_set_print_filename(osmo_stderr_target, 0);
+	log_set_log_level(osmo_stderr_target, LOGL_DEBUG);
+
+	run_test();
+	return EXIT_SUCCESS;
+}
+
+/*
+ * stubs that should not be reached
+ */
+extern "C" {
+	void l1if_pdch_req() {
+		abort();
+	} void l1if_connect_pdch() {
+		abort();
+	}
+	void l1if_close_pdch() {
+		abort();
+	}
+	void l1if_open_pdch() {
+		abort();
+	}
+}
diff --git a/tests/fn/FnTest.ok b/tests/fn/FnTest.ok
new file mode 100644
index 0000000..be6400f
--- /dev/null
+++ b/tests/fn/FnTest.ok
@@ -0,0 +1,50 @@
+RFN_MODULUS=42432
+GSM_MAX_FN=2715648
+
+bts: fn=1320462
+rfn=5066 ==> fn=1320458
+
+bts: fn=8246
+rfn=8244 ==> fn=8244
+
+bts: fn=10270
+rfn=10269 ==> fn=10269
+
+bts: fn=311276
+rfn=14250 ==> fn=311274
+
+bts: fn=42462
+rfn=42422 ==> fn=42422
+
+bts: fn=42433
+rfn=42431 ==> fn=42431
+
+bts: fn=5219152
+rfn=42428 ==> fn=5219132
+
+bts: fn=5219587
+rfn=42257 ==> fn=5218961
+
+bts: fn=0
+rfn=42419 ==> fn=2715635
+
+bts: fn=453
+rfn=42330 ==> fn=2715546
+
+bts: fn=10
+rfn=42422 ==> fn=2715638
+
+bts: fn=23
+rfn=42390 ==> fn=2715606
+
+bts: fn=2715648
+rfn=42431 ==> fn=2715647
+
+bts: fn=0
+rfn=42431 ==> fn=2715647
+
+bts: fn=2715648
+rfn=0 ==> fn=2715648
+
+bts: fn=0
+rfn=0 ==> fn=0
diff --git a/tests/testsuite.at b/tests/testsuite.at
index e42a0fd..d8f8f9a 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -70,3 +70,9 @@
 cat $abs_srcdir/codel/codel_test.ok > expout
 AT_CHECK([$OSMO_QEMU $abs_top_builddir/tests/codel/codel_test], [0], [expout], [ignore])
 AT_CLEANUP
+
+AT_SETUP([fn])
+AT_KEYWORDS([fn])
+cat $abs_srcdir/fn/FnTest.ok > expout
+AT_CHECK([$OSMO_QEMU $abs_top_builddir/tests/fn/FnTest], [0], [expout], [ignore])
+AT_CLEANUP

-- 
To view, visit https://gerrit.osmocom.org/1861
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I74f00c11e5739d49f370ce6c357149e81d9aa759
Gerrit-PatchSet: 3
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder


More information about the gerrit-log mailing list