[PATCH] osmo-pcu[master]: EGPRS: fix for EPDAN out of window

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/.

arvind.sirsikar gerrit-no-reply at lists.osmocom.org
Mon Oct 24 14:06:17 UTC 2016


Hello Neels Hofmeyr, Jenkins Builder,

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

    https://gerrit.osmocom.org/862

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

EGPRS: fix for EPDAN out of window

Fix alignment of EPDAN outside the RLC transmit window,
according to section 9.1.8.2.4 in 44.060 version 7.27.0 Release 7.
The specification explains that a bit within the uncompressed bitmap
whose corresponding BSN is not within the transmit window shall be
ignored. Without this fix PCU was dropping the EPDAN message and not
updating the status of BSNs which are inside the RLC window. This patch
updates the status of the BSNs which are inside the window and ignores
the remaining bits.

Related: OS#1789

Change-Id: Id07d178970f168f5389016c1eea31eb6b82057b6
---
M src/rlc.cpp
M src/rlc.h
M src/tbf_dl.cpp
M tests/tbf/TbfTest.cpp
M tests/tbf/TbfTest.err
5 files changed, 138 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/62/862/6

diff --git a/src/rlc.cpp b/src/rlc.cpp
index ee2635a..320c3d0 100644
--- a/src/rlc.cpp
+++ b/src/rlc.cpp
@@ -105,7 +105,10 @@
 			uint16_t first_bsn, uint16_t *lost,
 			uint16_t *received)
 {
-	unsigned num_blocks = rbb->cur_bit;
+	/* distance() is guaranteed to be positive because it does &mod_sns() */
+	uint16_t dist = distance();
+	unsigned num_blocks = rbb->cur_bit > (unsigned)dist
+				? dist : rbb->cur_bit;
 	unsigned bsn;
 
 	/* first_bsn is in range V(A)..V(S) */
diff --git a/src/rlc.h b/src/rlc.h
index b693418..b2fcd95 100644
--- a/src/rlc.h
+++ b/src/rlc.h
@@ -300,7 +300,7 @@
 	const uint16_t v_s() const;
 	const uint16_t v_s_mod(int offset) const;
 	const uint16_t v_a() const;
-	const int16_t distance() const;
+	const uint16_t distance() const;
 
 	/* Methods to manage reception */
 	int resend_needed();
@@ -539,7 +539,7 @@
 	m_v_a = (m_v_a + moves) & mod_sns();
 }
 
-inline const int16_t gprs_rlc_dl_window::distance() const
+inline const uint16_t gprs_rlc_dl_window::distance() const
 {
 	return (m_v_s - m_v_a) & mod_sns();
 }
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index c89cd03..f3ab0f4 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -847,6 +847,12 @@
 	unsigned received_packets = 0, lost_packets = 0;
 	unsigned num_blocks = strlen(show_rbb);
 
+	/* distance() is guaranteed to be positive because it does &mod_sns() */
+	uint16_t distance = m_window.distance();
+
+	num_blocks = num_blocks > (unsigned)distance
+				? distance : num_blocks;
+
 	/* SSN - 1 is in range V(A)..V(S)-1 */
 	for (unsigned int bitpos = 0; bitpos < num_blocks; bitpos++) {
 		bool is_received;
@@ -925,7 +931,10 @@
 	char show_rbb[RLC_MAX_SNS + 1];
 	int error_rate;
 	struct ana_result ana_res;
-	unsigned num_blocks = rbb->cur_bit;
+	/* distance() is guaranteed to be positive because it does &mod_sns() */
+	dist = m_window.distance();
+	unsigned num_blocks = rbb->cur_bit > (unsigned)dist
+				? dist : rbb->cur_bit;
 	unsigned behind_last_bsn = m_window.mod_sns(first_bsn + num_blocks);
 
 	Decoding::extract_rbb(rbb, show_rbb);
@@ -933,25 +942,6 @@
 	LOGP(DRLCMACDL, LOGL_DEBUG, "- ack:  (BSN=%d)\"%s\""
 		"(BSN=%d)  R=ACK I=NACK\n", first_bsn,
 		show_rbb, m_window.mod_sns(behind_last_bsn - 1));
-
-	/* apply received array to receive state (first_bsn..behind_last_bsn-1) */
-	if (num_blocks > 0) {
-		/* calculate distance of ssn from V(S) */
-		dist = m_window.mod_sns(m_window.v_s() - behind_last_bsn);
-		/* check if distance is less than distance V(A)..V(S) */
-		if (dist >= m_window.distance()) {
-			/* this might happpen, if the downlink assignment
-			 * was not received by ms and the ack refers
-			 * to previous TBF
-			 * FIXME: we should implement polling for
-			 * control ack!
-			 * TODO: check whether this FIXME still makes sense
-			 */
-			LOGP(DRLCMACDL, LOGL_NOTICE, "- ack range is out of "
-				"V(A)..V(S) range %s Free TBF!\n", tbf_name(this));
-			return 1; /* indicate to free TBF */
-		}
-	}
 
 	error_rate = analyse_errors(show_rbb, behind_last_bsn, &ana_res);
 
diff --git a/tests/tbf/TbfTest.cpp b/tests/tbf/TbfTest.cpp
index 8fb8bfe..cc48392 100644
--- a/tests/tbf/TbfTest.cpp
+++ b/tests/tbf/TbfTest.cpp
@@ -2101,17 +2101,11 @@
 	dl_tbf->rcvd_dl_ack(
 		ack_nack->EGPRS_AckNack.Desc.FINAL_ACK_INDICATION,
 		bsn_begin, &bits);
-	/*
-	 * TODO:status of BSN:1176,1177 shall be invalid
-	 * status of BSN:1286,1287 shall be acked.
-	 * both condition fails because of existing bug. Which shall be
-	 * fixed in subsequent commit
-	 */
 
-	OSMO_ASSERT(prlcmvb->is_unacked(1176));
-	OSMO_ASSERT(prlcmvb->is_unacked(1177));
-	OSMO_ASSERT(prlcmvb->is_unacked(1286));
-	OSMO_ASSERT(prlcmvb->is_unacked(1287));
+	OSMO_ASSERT(prlcmvb->is_invalid(1176));
+	OSMO_ASSERT(prlcmvb->is_invalid(1177));
+	OSMO_ASSERT(prlcmvb->is_acked(1286));
+	OSMO_ASSERT(prlcmvb->is_acked(1287));
 
 	bitvec_free(block);
 	tbf_free(dl_tbf);
diff --git a/tests/tbf/TbfTest.err b/tests/tbf/TbfTest.err
index f8e6503..fc3a113 100644
--- a/tests/tbf/TbfTest.err
+++ b/tests/tbf/TbfTest.err
@@ -446,7 +446,7 @@
 Sending data request: trx=0 ts=4 sapi=5 arfcn=0 fn=91 block=9 data=07 00 28 0a 41 c6 c7 43 c0 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) downlink acknowledge
 - ack:  (BSN=85)"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR"(BSN=20)  R=ACK I=NACK
-TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) DL analysis, range=0:21, lost=0, recv=21, skipped=0, bsn=127, info='RRRRRRRRRRRRRRRRRRRRR$..........................................'
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) DL analysis, range=0:21, lost=0, recv=21, skipped=0, bsn=0, info='RRRRRRRRRRRRRRRRRRRRR...........................................'
 - got ack for BSN=20
 - got ack for BSN=19
 - got ack for BSN=18
@@ -485,7 +485,7 @@
 Sending data request: trx=0 ts=4 sapi=5 arfcn=0 fn=95 block=10 data=07 00 2a 4d 43 c0 01 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) downlink acknowledge
 - ack:  (BSN=86)"RRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRRR"(BSN=21)  R=ACK I=NACK
-TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) DL analysis, range=21:22, lost=0, recv=1, skipped=0, bsn=20, info='R$..............................................................'
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW) DL analysis, range=21:22, lost=0, recv=1, skipped=0, bsn=21, info='R...............................................................'
 - got ack for BSN=21
 - V(B): (V(A)=22)""(V(S)-1=21)  A=Acked N=Nacked U=Unacked X=Resend-Unacked I=Invalid
 Scheduling data message at RTS for DL TFI=0 (TRX=0, TS=4) prio=4
@@ -6504,9 +6504,122 @@
 TBF(TFI=0 TLLI=0x00000000 DIR=DL STATE=NULL EGPRS) changes state from NULL to FLOW
 The MS object cannot fully confirm an unexpected TLLI: 0xffeeddcc, partly confirmed
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW EGPRS) downlink acknowledge
-- ack:  (BSN=1176)"RRRRRRRRRRIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIRRRIRRRRRRRRRRRRRRRRRRRRRRRRRRI"(BSN=1288)  R=ACK I=NACK
-- ack range is out of V(A)..V(S) range TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW EGPRS) Free TBF!
-DL packet loss of IMSI= / TLLI=0xffeeddcc: 100%
+- ack:  (BSN=1176)"RRRRRRRRRRIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIRRRIRRRRRRRRRRRRRRRRRRRRRRRRRRI"(BSN=1287)  R=ACK I=NACK
+TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW EGPRS) DL analysis, range=1176:1288, lost=73, recv=39, skipped=0, bsn=1944, info='RRRRRRRRRRRRRRRRRRRRRRRRRRLRRRLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLLRRRRRRRRRR................................................................................................................................................................................................................................................................................................................................................................................'
+- got ack for BSN=1176
+- got ack for BSN=1177
+- got ack for BSN=1178
+- got ack for BSN=1179
+- got ack for BSN=1180
+- got ack for BSN=1181
+- got ack for BSN=1182
+- got ack for BSN=1183
+- got ack for BSN=1184
+- got ack for BSN=1185
+- got NACK for BSN=1186
+- got NACK for BSN=1187
+- got NACK for BSN=1188
+- got NACK for BSN=1189
+- got NACK for BSN=1190
+- got NACK for BSN=1191
+- got NACK for BSN=1192
+- got NACK for BSN=1193
+- got NACK for BSN=1194
+- got NACK for BSN=1195
+- got NACK for BSN=1196
+- got NACK for BSN=1197
+- got NACK for BSN=1198
+- got NACK for BSN=1199
+- got NACK for BSN=1200
+- got NACK for BSN=1201
+- got NACK for BSN=1202
+- got NACK for BSN=1203
+- got NACK for BSN=1204
+- got NACK for BSN=1205
+- got NACK for BSN=1206
+- got NACK for BSN=1207
+- got NACK for BSN=1208
+- got NACK for BSN=1209
+- got NACK for BSN=1210
+- got NACK for BSN=1211
+- got NACK for BSN=1212
+- got NACK for BSN=1213
+- got NACK for BSN=1214
+- got NACK for BSN=1215
+- got NACK for BSN=1216
+- got NACK for BSN=1217
+- got NACK for BSN=1218
+- got NACK for BSN=1219
+- got NACK for BSN=1220
+- got NACK for BSN=1221
+- got NACK for BSN=1222
+- got NACK for BSN=1223
+- got NACK for BSN=1224
+- got NACK for BSN=1225
+- got NACK for BSN=1226
+- got NACK for BSN=1227
+- got NACK for BSN=1228
+- got NACK for BSN=1229
+- got NACK for BSN=1230
+- got NACK for BSN=1231
+- got NACK for BSN=1232
+- got NACK for BSN=1233
+- got NACK for BSN=1234
+- got NACK for BSN=1235
+- got NACK for BSN=1236
+- got NACK for BSN=1237
+- got NACK for BSN=1238
+- got NACK for BSN=1239
+- got NACK for BSN=1240
+- got NACK for BSN=1241
+- got NACK for BSN=1242
+- got NACK for BSN=1243
+- got NACK for BSN=1244
+- got NACK for BSN=1245
+- got NACK for BSN=1246
+- got NACK for BSN=1247
+- got NACK for BSN=1248
+- got NACK for BSN=1249
+- got NACK for BSN=1250
+- got NACK for BSN=1251
+- got NACK for BSN=1252
+- got NACK for BSN=1253
+- got NACK for BSN=1254
+- got NACK for BSN=1255
+- got NACK for BSN=1256
+- got NACK for BSN=1257
+- got ack for BSN=1258
+- got ack for BSN=1259
+- got ack for BSN=1260
+- got NACK for BSN=1261
+- got ack for BSN=1262
+- got ack for BSN=1263
+- got ack for BSN=1264
+- got ack for BSN=1265
+- got ack for BSN=1266
+- got ack for BSN=1267
+- got ack for BSN=1268
+- got ack for BSN=1269
+- got ack for BSN=1270
+- got ack for BSN=1271
+- got ack for BSN=1272
+- got ack for BSN=1273
+- got ack for BSN=1274
+- got ack for BSN=1275
+- got ack for BSN=1276
+- got ack for BSN=1277
+- got ack for BSN=1278
+- got ack for BSN=1279
+- got ack for BSN=1280
+- got ack for BSN=1281
+- got ack for BSN=1282
+- got ack for BSN=1283
+- got ack for BSN=1284
+- got ack for BSN=1285
+- got ack for BSN=1286
+- got ack for BSN=1287
+- V(B): (V(A)=1186)"NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNAAANAAAAAAAAAAAAAAAAAAAAAAAAAA"(V(S)-1=1287)  A=Acked N=Nacked U=Unacked X=Resend-Unacked I=Invalid
+DL packet loss of IMSI= / TLLI=0xffeeddcc: 78%
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=FLOW EGPRS) changes state from FLOW to RELEASING
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=RELEASING EGPRS) free
 TBF(TFI=0 TLLI=0xffeeddcc DIR=DL STATE=RELEASING EGPRS) Software error: Pending downlink assignment. This may not happen, because the assignment message never gets transmitted. Please be sure not to free in this state. PLEASE FIX!

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id07d178970f168f5389016c1eea31eb6b82057b6
Gerrit-PatchSet: 6
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: arvind.sirsikar <arvind.sirsikar at radisys.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com>



More information about the gerrit-log mailing list