Change in ...osmo-pcu[master]: Encoding: ACK/NACK: always encode with length field present

[lynxis lazus]

lynxis lazus has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/14697


Change subject: Encoding: ACK/NACK: always encode with length field present
......................................................................

Encoding: ACK/NACK: always encode with length field present

In most cases the length field was present and this field takes 7
bits of the maximum available 110 rest bits.
The length field was only removed when encoding huge bitmaps usually
only happen on lossy connections with packet lost.
However the cases without length field were encoded incorrect,
because all remaining bits must be used by the uncompressed bitmaps,
but the PCU violates this by encoding allways the "release 5" bit.

Change-Id: I7bc2e18d647b72b8f17ba7a5c9c5e421d88275fb
---
M src/encoding.cpp
1 file changed, 28 insertions(+), 43 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/97/14697/1

diff --git a/src/encoding.cpp b/src/encoding.cpp
index 99c1018..3908744 100644
--- a/src/encoding.cpp
+++ b/src/encoding.cpp
@@ -849,12 +849,12 @@
 	bitvec ucmp_vec;
 	bitvec crbb_vec;
 	uint8_t uclen_crbb = 0;
-	bool len_coded = true;
 	uint8_t crbb_start_clr_code;
 	uint8_t i;
 
 	/* static size of 16 bits
-	 ..0. .... = ACKNACK:  (Union)
+	 ..1. .... = ACKNACK:  (Union)
+	    0 0000 000 Length
         Desc
 
             ...0 .... = FINAL_ACK_INDICATION: False
@@ -865,8 +865,9 @@
 
             .... ..10  0101 0001  1... .... = STARTING_SEQUENCE_NUMBER: 1187
 
-	    .0.. .... = CRBB Exist: 0 */
-	rest_bits -= 16;
+	    .0.. .... = CRBB Exist: 0
+	minimal size is 24 rest_bits */
+	rest_bits -= 24;
 
 	if (num_blocks > 0)
 		/* V(Q) is NACK and omitted -> SSN = V(Q) + 1 */
@@ -903,47 +904,31 @@
 		crbb_len = crbb_vec.cur_bit;
 	}
 
-	if (is_compressed == 0) {
-		/* length field takes 8 bits*/
-		if (num_blocks > rest_bits - 8) {
-			eow = false;
-			urbb_len = rest_bits;
-			len_coded = false;
-		} else if (num_blocks == rest_bits) {
-			urbb_len = rest_bits;
-			len_coded = false;
-		} else
-			urbb_len = num_blocks;
 
-		len = urbb_len + 15;
+	if (is_compressed) {
+		/* 8 = 7 (CRBBlength) + 1 (CRBB starting color code) */
+		rest_bits -= 8;
 	} else {
-		if (num_blocks > uclen_crbb) {
-			eow = false;
-			urbb_len = num_blocks - uclen_crbb;
-		}
-		/* Union bit takes 1 bit */
-		/* Other fields in descr of compresed bitmap takes 23 bits
-		 * -8 = CRBB_STARTING_COLOR_CODE + CRBB_LENGTH */
-		if (urbb_len > (rest_bits - crbb_len - 8)) {
-			eow = false;
-			len_coded = false;
-			urbb_len = rest_bits - crbb_len - 8;
-		/* -16 =  ACKNACK Dissector length + CRBB_STARTING_COLOR_CODE + CRBB_LENGTH */
-		} else if (urbb_len > (rest_bits - crbb_len - 16)) {
-			eow = false;
-			len_coded = false;
-			urbb_len = rest_bits - crbb_len - 16;
-		}
+		uclen_crbb = 0;
+		crbb_len = 0;
+	}
+
+	if (num_blocks > uclen_crbb + rest_bits) {
+		eow = false;
+		urbb_len = rest_bits - crbb_len;
+	} else
+		urbb_len = num_blocks - uclen_crbb;
+
+	if (is_compressed)
 		len = urbb_len + crbb_len + 23;
-	}
+	else
+		len = urbb_len + 15;
 
-	/* EGPRS Ack/Nack Description IE */
-	if (len_coded == false) {
-		bitvec_write_field(dest, &wp, 0, 1); // 0: don't have length
-	} else {
-		bitvec_write_field(dest, &wp, 1, 1); // 1: have length
-		bitvec_write_field(dest, &wp, len, 8); // length
-	}
+
+	/* EGPRS Ack/Nack Description IE
+	 * do not support Ack/Nack without length */
+	bitvec_write_field(dest, &wp, 1, 1); // 1: have length
+	bitvec_write_field(dest, &wp, len, 8); // length
 
 	bitvec_write_field(dest, &wp, is_final, 1); // FINAL_ACK_INDICATION
 	bitvec_write_field(dest, &wp, bow, 1); // BEGINNING_OF_WINDOW
@@ -973,9 +958,9 @@
 	}
 	LOGP(DRLCMACUL, LOGL_DEBUG,
 		"EGPRS URBB, urbb len = %d, SSN = %d, ESN_CRBB = %d, "
-		"len present = %s,desc len = %d, "
+		"desc len = %d, "
 		"SNS = %d, WS = %d, V(Q) = %d, V(R) = %d%s%s\n",
-		urbb_len, ssn, esn_crbb, len_coded ? "yes" : "No" , len,
+		urbb_len, ssn, esn_crbb, len,
 		window->sns(), window->ws(), window->v_q(), window->v_r(),
 		bow ? ", BOW" : "", eow ? ", EOW" : "");
 

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

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I7bc2e18d647b72b8f17ba7a5c9c5e421d88275fb
Gerrit-Change-Number: 14697
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190708/26fb0654/attachment.htm>