Change in osmo-pcu[master]: ulc: Fix FN store order upon wrap around

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Tue Mar 16 10:02:58 UTC 2021 

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/23357 )

Change subject: ulc: Fix FN store order upon wrap around
......................................................................

ulc: Fix FN store order upon wrap around

Related: OS#5020
Change-Id: I0a742f7fa1541b1837739207b9383772f981fb25
---
M src/pdch_ul_controller.c
M tests/ulc/PdchUlcTest.cpp
M tests/ulc/PdchUlcTest.err
M tests/ulc/PdchUlcTest.ok
4 files changed, 99 insertions(+), 55 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, but someone else must approve
  osmith: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved



diff --git a/src/pdch_ul_controller.c b/src/pdch_ul_controller.c
index 1fa91ae..173fd8f 100644
--- a/src/pdch_ul_controller.c
+++ b/src/pdch_ul_controller.c
@@ -37,6 +37,20 @@
 	{ 0, NULL }
 };
 
+#define GSM_MAX_FN_THRESH (GSM_MAX_FN >> 1)
+/* 0: equal, -1: fn1 BEFORE fn2, 1: fn1 AFTER fn2 */
+static inline int fn_cmp(uint32_t fn1, uint32_t fn2)
+{
+	if (fn1 == fn2)
+		return 0;
+	/* FN1 goes before FN2: */
+	if ((fn1 < fn2 && (fn2 - fn1) < GSM_MAX_FN_THRESH) ||
+	    (fn1 > fn2 && (fn1 - fn2) > GSM_MAX_FN_THRESH))
+		return -1;
+	/* FN1 goes after FN2: */
+	return 1;
+}
+
 struct pdch_ulc *pdch_ulc_alloc(struct gprs_rlcmac_pdch *pdch, void *ctx)
 {
 	struct pdch_ulc* ulc;
@@ -53,11 +67,13 @@
 {
 	struct rb_node *node;
 	struct pdch_ulc_node *it;
+	int res;
 	for (node = rb_first(&ulc->tree_root); node; node = rb_next(node)) {
 		it = container_of(node, struct pdch_ulc_node, node);
-		if (it->fn == fn)
+		res = fn_cmp(it->fn, fn);
+		if (res == 0) /* it->fn == fn */
 			return it;
-		if (it->fn > fn)
+		if (res > 0) /* it->fn AFTER fn */
 			break;
 	}
 	return NULL;
@@ -123,13 +139,15 @@
 
 	while (*n) {
 		struct pdch_ulc_node *it;
+		int res;
 
 		it = container_of(*n, struct pdch_ulc_node, node);
 
 		parent = *n;
-		if (item->fn < it->fn) {
+		res = fn_cmp(item->fn, it->fn);
+		if (res < 0) { /* item->fn "BEFORE" it->fn */
 			n = &((*n)->rb_left);
-		} else if (item->fn > it->fn) {
+		} else if (res > 0) { /* item->fn "AFTER" it->fn */
 			n = &((*n)->rb_right);
 		} else {
 			LOGPDCH(ulc->pdch, DRLCMAC, LOGL_ERROR,
@@ -213,13 +231,15 @@
 {
 	struct gprs_rlcmac_sba *sba;
 	struct pdch_ulc_node *item;
+	int res;
 
 	struct rb_node *first;
 	while((first = rb_first(&ulc->tree_root))) {
 		item = container_of(first, struct pdch_ulc_node, node);
-		if (item->fn > fn)
+		res = fn_cmp(item->fn, fn);
+		if (res > 0) /* item->fn AFTER fn */
 			break;
-		if (item->fn < fn) {
+		if (res < 0) { /* item->fn BEFORE fn */
 			/* Sanity check: */
 			LOGPDCH(ulc->pdch, DRLCMAC, LOGL_ERROR,
 				"Expiring FN=%" PRIu32 " but previous FN=%" PRIu32 " is still reserved!\n",
diff --git a/tests/ulc/PdchUlcTest.cpp b/tests/ulc/PdchUlcTest.cpp
index 320a7bf..0099101 100644
--- a/tests/ulc/PdchUlcTest.cpp
+++ b/tests/ulc/PdchUlcTest.cpp
@@ -199,14 +199,14 @@
 	printf("*** RELEASE fn=%" PRIu32 ":\n", 0);
 	rc = pdch_ulc_release_fn(pdch->ulc, 0);
 	print_ulc_nodes(pdch->ulc);
-	//OSMO_ASSERT(rc == 0); FIXME: DISABLED DUE TO BUG!
+	OSMO_ASSERT(rc == 0);
 
 	/* Expiring last FN should expire all entries */
 	printf("*** EXPIRE FN=%" PRIu32 ":\n", last_fn);
 	pdch_ulc_expire_fn(pdch->ulc, last_fn);
 	print_ulc_nodes(pdch->ulc);
 	/* Make sure the store is empty now: */
-	//OSMO_ASSERT(!rb_first(&pdch->ulc->tree_root)); FIXME: DISABLED DUE TO BUG!
+	OSMO_ASSERT(!rb_first(&pdch->ulc->tree_root));
 
 	talloc_free(bts);
 	printf("=== end: %s ===\n", __FUNCTION__);
diff --git a/tests/ulc/PdchUlcTest.err b/tests/ulc/PdchUlcTest.err
index 88e4644..34a090b 100644
--- a/tests/ulc/PdchUlcTest.err
+++ b/tests/ulc/PdchUlcTest.err
@@ -6,26 +6,38 @@
 PDCH(bts=0,trx=0,ts=0) Timeout for registered SBA (FN=72, TA=0)
 Creating MS object, TLLI = 0x12345678
 MS(TLLI=0x12345678, IMSI=, TA=220, 0/0,) Attaching DL TBF: TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=0 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=0): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=4 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=4): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=8 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=8): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=13 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=13): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=17 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=17): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=21 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=21): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=26 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=26): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=30 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=30): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=34 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=34): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
-PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=39 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=39): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
 PDCH(bts=0,trx=0,ts=0) Expiring FN=2715613 but previous FN=2715608 is still reserved!
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715608): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
-PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715613): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715608): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715613): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715617 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715617): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715622 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715622): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715626 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715626): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715630 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715630): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715635 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715635): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715639 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715639): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=NULL)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=2715643 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=2715643): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=4 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=4): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=8 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=8): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=13 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=13): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=17 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=17): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=21 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=21): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=26 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=26): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=30 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=30): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=34 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=34): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
+PDCH(bts=0,trx=0,ts=0) Expiring FN=43 but previous FN=39 is still reserved!
+PDCH(bts=0,trx=0,ts=0) Timeout for registered POLL (FN=39): TBF(TFI=0 TLLI=0x12345678 DIR=DL STATE=RELEASING)
diff --git a/tests/ulc/PdchUlcTest.ok b/tests/ulc/PdchUlcTest.ok
index 7c17f01..f20fb30 100644
--- a/tests/ulc/PdchUlcTest.ok
+++ b/tests/ulc/PdchUlcTest.ok
@@ -56,7 +56,6 @@
 FN=2715639 type=POLL
 FN=2715643 type=POLL
 *** RESERVE FN=0:
-FN=0 type=POLL
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -66,9 +65,20 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
+FN=0 type=POLL
 *** RESERVE FN=4:
+FN=2715608 type=POLL
+FN=2715613 type=POLL
+FN=2715617 type=POLL
+FN=2715622 type=POLL
+FN=2715626 type=POLL
+FN=2715630 type=POLL
+FN=2715635 type=POLL
+FN=2715639 type=POLL
+FN=2715643 type=POLL
 FN=0 type=POLL
 FN=4 type=POLL
+*** RESERVE FN=8:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -78,10 +88,10 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=8:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
+*** RESERVE FN=13:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -91,11 +101,11 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=13:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
 FN=13 type=POLL
+*** RESERVE FN=17:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -105,12 +115,12 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=17:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
 FN=13 type=POLL
 FN=17 type=POLL
+*** RESERVE FN=21:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -120,13 +130,13 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=21:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
 FN=13 type=POLL
 FN=17 type=POLL
 FN=21 type=POLL
+*** RESERVE FN=26:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -136,7 +146,6 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=26:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
@@ -144,6 +153,7 @@
 FN=17 type=POLL
 FN=21 type=POLL
 FN=26 type=POLL
+*** RESERVE FN=30:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -153,7 +163,6 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=30:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
@@ -162,6 +171,7 @@
 FN=21 type=POLL
 FN=26 type=POLL
 FN=30 type=POLL
+*** RESERVE FN=34:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -171,7 +181,6 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=34:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
@@ -181,6 +190,7 @@
 FN=26 type=POLL
 FN=30 type=POLL
 FN=34 type=POLL
+*** RESERVE FN=39:
 FN=2715608 type=POLL
 FN=2715613 type=POLL
 FN=2715617 type=POLL
@@ -190,7 +200,6 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
-*** RESERVE FN=39:
 FN=0 type=POLL
 FN=4 type=POLL
 FN=8 type=POLL
@@ -201,15 +210,6 @@
 FN=30 type=POLL
 FN=34 type=POLL
 FN=39 type=POLL
-FN=2715608 type=POLL
-FN=2715613 type=POLL
-FN=2715617 type=POLL
-FN=2715622 type=POLL
-FN=2715626 type=POLL
-FN=2715630 type=POLL
-FN=2715635 type=POLL
-FN=2715639 type=POLL
-FN=2715643 type=POLL
 *** EXPIRE FN=2715613:
 FN=2715617 type=POLL
 FN=2715622 type=POLL
@@ -218,6 +218,16 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
+FN=0 type=POLL
+FN=4 type=POLL
+FN=8 type=POLL
+FN=13 type=POLL
+FN=17 type=POLL
+FN=21 type=POLL
+FN=26 type=POLL
+FN=30 type=POLL
+FN=34 type=POLL
+FN=39 type=POLL
 *** RELEASE fn=0:
 FN=2715617 type=POLL
 FN=2715622 type=POLL
@@ -226,12 +236,14 @@
 FN=2715635 type=POLL
 FN=2715639 type=POLL
 FN=2715643 type=POLL
+FN=4 type=POLL
+FN=8 type=POLL
+FN=13 type=POLL
+FN=17 type=POLL
+FN=21 type=POLL
+FN=26 type=POLL
+FN=30 type=POLL
+FN=34 type=POLL
+FN=39 type=POLL
 *** EXPIRE FN=43:
-FN=2715617 type=POLL
-FN=2715622 type=POLL
-FN=2715626 type=POLL
-FN=2715630 type=POLL
-FN=2715635 type=POLL
-FN=2715639 type=POLL
-FN=2715643 type=POLL
 === end: test_fn_wrap_around ===

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/23357
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I0a742f7fa1541b1837739207b9383772f981fb25
Gerrit-Change-Number: 23357
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210316/c1defdb0/attachment.htm>

More information about the gerrit-log mailing list