[MERGED] libosmocore[master]: Distinguish between unsupported and invalid MCS

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

Harald Welte gerrit-no-reply at lists.osmocom.org
Wed May 24 22:12:56 UTC 2017 

Harald Welte has submitted this change and it was merged.

Change subject: Distinguish between unsupported and invalid MCS
......................................................................


Distinguish between unsupported and invalid MCS

Previously MCS0 was incorrectly set for some of type1 header values
while according to 3GPP TS 44.060 it can only be set for type3. Fix
this:

* use EGPRS_MCS* constants instead of magic values
* do not set MCS0 for reserved bits values in EGPRS header type1
* return different error codes for invalid and unsupported MCS as well
  as for other decoding errors

Note: there's no need to adjust tests because MCS0 decoding is not
supported but it's better to explicitly distinguish between unsupported
and invalid values nevertheless.

Change-Id: Id665d5c0cf50efa18b1bcbf4f17359418a380f9e
Related: OS#1524
---
M src/coding/gsm0503_coding.c
M src/gsm/gprs_rlc.c
2 files changed, 63 insertions(+), 59 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/coding/gsm0503_coding.c b/src/coding/gsm0503_coding.c
index 30ec387..baf1ce1 100644
--- a/src/coding/gsm0503_coding.c
+++ b/src/coding/gsm0503_coding.c
@@ -24,6 +24,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #include <osmocom/core/bits.h>
 #include <osmocom/core/conv.h>
@@ -820,15 +821,17 @@
 	if ((nbits != GSM0503_GPRS_BURSTS_NBITS) &&
 		(nbits != GSM0503_EGPRS_BURSTS_NBITS)) {
 		/* Invalid EGPRS bit length */
-		return -1;
+		return -EOVERFLOW;
 	}
 
 	hdr = (union gprs_rlc_ul_hdr_egprs *) l2_data;
 	type = egprs_decode_hdr(hdr, bursts, nbits);
 	if (egprs_parse_ul_cps(&cps, hdr, type) < 0)
-		return -1;
+		return -EIO;
 
 	switch (cps.mcs) {
+	case EGPRS_MCS0:
+		return -ENOTSUP;
 	case EGPRS_MCS1:
 	case EGPRS_MCS2:
 	case EGPRS_MCS3:
@@ -846,7 +849,7 @@
 		break;
 	default:
 		/* Invalid MCS-X */
-		return -1;
+		return -EINVAL;
 	}
 
 	/* Decode MCS-X block, where X = cps.mcs */
@@ -854,19 +857,19 @@
 		rc = egprs_decode_data(l2_data, dc, cps.mcs, cps.p[0],
 			0, n_errors, n_bits_total);
 		if (rc < 0)
-			return -1;
+			return -EFAULT;
 	} else {
 		/* MCS-7,8,9 block 1 */
 		rc = egprs_decode_data(l2_data, c1, cps.mcs, cps.p[0],
 			0, n_errors, n_bits_total);
 		if (rc < 0)
-			return -1;
+			return -EFAULT;
 
 		/* MCS-7,8,9 block 2 */
 		rc = egprs_decode_data(l2_data, c2, cps.mcs, cps.p[1],
 			1, n_errors, n_bits_total);
 		if (rc < 0)
-			return -1;
+			return -EFAULT;
 	}
 
 	return rc;
diff --git a/src/gsm/gprs_rlc.c b/src/gsm/gprs_rlc.c
index d6ccbbe..a4053ef 100644
--- a/src/gsm/gprs_rlc.c
+++ b/src/gsm/gprs_rlc.c
@@ -2,6 +2,7 @@
 #include <string.h>
 
 #include <osmocom/gprs/gprs_rlc.h>
+#include <osmocom/coding/gsm0503_coding.h>
 #include <osmocom/gprs/protocol/gsm_04_60.h>
 
 #define EGPRS_CPS_TYPE1_TBL_SZ		29
@@ -10,35 +11,35 @@
 
 /* 3GPP TS 44.060 10.4.8a.1.1 "Header type 1" */
 static const struct egprs_cps egprs_cps_table_type1[EGPRS_CPS_TYPE1_TBL_SZ] = {
-	{ .bits =  0, .mcs = 9, .p = { EGPRS_CPS_P1, EGPRS_CPS_P1 } },
-	{ .bits =  1, .mcs = 9, .p = { EGPRS_CPS_P1, EGPRS_CPS_P2 } },
-	{ .bits =  2, .mcs = 9, .p = { EGPRS_CPS_P1, EGPRS_CPS_P3 } },
-	{ .bits =  3, .mcs = 0, .p = { EGPRS_CPS_NONE, EGPRS_CPS_NONE } },
-	{ .bits =  4, .mcs = 9, .p = { EGPRS_CPS_P2, EGPRS_CPS_P1 } },
-	{ .bits =  5, .mcs = 9, .p = { EGPRS_CPS_P2, EGPRS_CPS_P2 } },
-	{ .bits =  6, .mcs = 9, .p = { EGPRS_CPS_P2, EGPRS_CPS_P3 } },
-	{ .bits =  7, .mcs = 0, .p = { EGPRS_CPS_NONE, EGPRS_CPS_NONE } },
-	{ .bits =  8, .mcs = 9, .p = { EGPRS_CPS_P3, EGPRS_CPS_P1 } },
-	{ .bits =  9, .mcs = 9, .p = { EGPRS_CPS_P3, EGPRS_CPS_P2 } },
-	{ .bits = 10, .mcs = 9, .p = { EGPRS_CPS_P3, EGPRS_CPS_P3 } },
-	{ .bits = 11, .mcs = 8, .p = { EGPRS_CPS_P1, EGPRS_CPS_P1 } },
-	{ .bits = 12, .mcs = 8, .p = { EGPRS_CPS_P1, EGPRS_CPS_P2 } },
-	{ .bits = 13, .mcs = 8, .p = { EGPRS_CPS_P1, EGPRS_CPS_P3 } },
-	{ .bits = 14, .mcs = 8, .p = { EGPRS_CPS_P2, EGPRS_CPS_P1 } },
-	{ .bits = 15, .mcs = 8, .p = { EGPRS_CPS_P2, EGPRS_CPS_P2 } },
-	{ .bits = 16, .mcs = 8, .p = { EGPRS_CPS_P2, EGPRS_CPS_P3 } },
-	{ .bits = 17, .mcs = 8, .p = { EGPRS_CPS_P3, EGPRS_CPS_P1 } },
-	{ .bits = 18, .mcs = 8, .p = { EGPRS_CPS_P3, EGPRS_CPS_P2 } },
-	{ .bits = 19, .mcs = 8, .p = { EGPRS_CPS_P3, EGPRS_CPS_P3 } },
-	{ .bits = 20, .mcs = 7, .p = { EGPRS_CPS_P1, EGPRS_CPS_P1 } },
-	{ .bits = 21, .mcs = 7, .p = { EGPRS_CPS_P1, EGPRS_CPS_P2 } },
-	{ .bits = 22, .mcs = 7, .p = { EGPRS_CPS_P1, EGPRS_CPS_P3 } },
-	{ .bits = 23, .mcs = 7, .p = { EGPRS_CPS_P2, EGPRS_CPS_P1 } },
-	{ .bits = 24, .mcs = 7, .p = { EGPRS_CPS_P2, EGPRS_CPS_P2 } },
-	{ .bits = 25, .mcs = 7, .p = { EGPRS_CPS_P2, EGPRS_CPS_P3 } },
-	{ .bits = 26, .mcs = 7, .p = { EGPRS_CPS_P3, EGPRS_CPS_P1 } },
-	{ .bits = 27, .mcs = 7, .p = { EGPRS_CPS_P3, EGPRS_CPS_P2 } },
-	{ .bits = 28, .mcs = 7, .p = { EGPRS_CPS_P3, EGPRS_CPS_P3 } },
+	{ .bits =  0, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P1, EGPRS_CPS_P1 } },
+	{ .bits =  1, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P1, EGPRS_CPS_P2 } },
+	{ .bits =  2, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P1, EGPRS_CPS_P3 } },
+	{ .bits =  3, .mcs = EGPRS_NUM_MCS, .p = { EGPRS_CPS_NONE, EGPRS_CPS_NONE } }, /* reserved for future use */
+	{ .bits =  4, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P2, EGPRS_CPS_P1 } },
+	{ .bits =  5, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P2, EGPRS_CPS_P2 } },
+	{ .bits =  6, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P2, EGPRS_CPS_P3 } },
+	{ .bits =  7, .mcs = EGPRS_NUM_MCS, .p = { EGPRS_CPS_NONE, EGPRS_CPS_NONE } }, /* reserved for future use */
+	{ .bits =  8, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P3, EGPRS_CPS_P1 } },
+	{ .bits =  9, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P3, EGPRS_CPS_P2 } },
+	{ .bits = 10, .mcs = EGPRS_MCS9, .p = { EGPRS_CPS_P3, EGPRS_CPS_P3 } },
+	{ .bits = 11, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P1, EGPRS_CPS_P1 } },
+	{ .bits = 12, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P1, EGPRS_CPS_P2 } },
+	{ .bits = 13, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P1, EGPRS_CPS_P3 } },
+	{ .bits = 14, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P2, EGPRS_CPS_P1 } },
+	{ .bits = 15, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P2, EGPRS_CPS_P2 } },
+	{ .bits = 16, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P2, EGPRS_CPS_P3 } },
+	{ .bits = 17, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P3, EGPRS_CPS_P1 } },
+	{ .bits = 18, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P3, EGPRS_CPS_P2 } },
+	{ .bits = 19, .mcs = EGPRS_MCS8, .p = { EGPRS_CPS_P3, EGPRS_CPS_P3 } },
+	{ .bits = 20, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P1, EGPRS_CPS_P1 } },
+	{ .bits = 21, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P1, EGPRS_CPS_P2 } },
+	{ .bits = 22, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P1, EGPRS_CPS_P3 } },
+	{ .bits = 23, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P2, EGPRS_CPS_P1 } },
+	{ .bits = 24, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P2, EGPRS_CPS_P2 } },
+	{ .bits = 25, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P2, EGPRS_CPS_P3 } },
+	{ .bits = 26, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P3, EGPRS_CPS_P1 } },
+	{ .bits = 27, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P3, EGPRS_CPS_P2 } },
+	{ .bits = 28, .mcs = EGPRS_MCS7, .p = { EGPRS_CPS_P3, EGPRS_CPS_P3 } },
 };
 
 /*
@@ -46,34 +47,34 @@
  * "Header type 2 in EGPRS TBF or uplink EGPRS2-A TBF"
  */
 static const struct egprs_cps egprs_cps_table_type2[EGPRS_CPS_TYPE2_TBL_SZ] = {
-	{ .bits =  0, .mcs = 6, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits =  1, .mcs = 6, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits =  2, .mcs = 6, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits =  3, .mcs = 6, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits =  4, .mcs = 5, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits =  5, .mcs = 5, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits =  6, .mcs = 6, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits =  7, .mcs = 6, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits =  0, .mcs = EGPRS_MCS6, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits =  1, .mcs = EGPRS_MCS6, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits =  2, .mcs = EGPRS_MCS6, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits =  3, .mcs = EGPRS_MCS6, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits =  4, .mcs = EGPRS_MCS5, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits =  5, .mcs = EGPRS_MCS5, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits =  6, .mcs = EGPRS_MCS6, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits =  7, .mcs = EGPRS_MCS6, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
 };
 
 /* 3GPP TS 44.060 10.4.8a.3 "Header type 3" */
 static const struct egprs_cps egprs_cps_table_type3[EGPRS_CPS_TYPE3_TBL_SZ] = {
-	{ .bits =  0, .mcs = 4, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits =  1, .mcs = 4, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits =  2, .mcs = 4, .p = { EGPRS_CPS_P3, EGPRS_CPS_NONE } },
-	{ .bits =  3, .mcs = 3, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits =  4, .mcs = 3, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits =  5, .mcs = 3, .p = { EGPRS_CPS_P3, EGPRS_CPS_NONE } },
-	{ .bits =  6, .mcs = 3, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits =  7, .mcs = 3, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits =  8, .mcs = 3, .p = { EGPRS_CPS_P3, EGPRS_CPS_NONE } },
-	{ .bits =  9, .mcs = 2, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits = 10, .mcs = 2, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits = 11, .mcs = 1, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits = 12, .mcs = 1, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits = 13, .mcs = 2, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
-	{ .bits = 14, .mcs = 2, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
-	{ .bits = 15, .mcs = 0, .p = { EGPRS_CPS_NONE, EGPRS_CPS_NONE } },
+	{ .bits =  0, .mcs = EGPRS_MCS4, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits =  1, .mcs = EGPRS_MCS4, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits =  2, .mcs = EGPRS_MCS4, .p = { EGPRS_CPS_P3, EGPRS_CPS_NONE } },
+	{ .bits =  3, .mcs = EGPRS_MCS3, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits =  4, .mcs = EGPRS_MCS3, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits =  5, .mcs = EGPRS_MCS3, .p = { EGPRS_CPS_P3, EGPRS_CPS_NONE } },
+	{ .bits =  6, .mcs = EGPRS_MCS3, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits =  7, .mcs = EGPRS_MCS3, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits =  8, .mcs = EGPRS_MCS3, .p = { EGPRS_CPS_P3, EGPRS_CPS_NONE } },
+	{ .bits =  9, .mcs = EGPRS_MCS2, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits = 10, .mcs = EGPRS_MCS2, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits = 11, .mcs = EGPRS_MCS1, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits = 12, .mcs = EGPRS_MCS1, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits = 13, .mcs = EGPRS_MCS2, .p = { EGPRS_CPS_P1, EGPRS_CPS_NONE } },
+	{ .bits = 14, .mcs = EGPRS_MCS2, .p = { EGPRS_CPS_P2, EGPRS_CPS_NONE } },
+	{ .bits = 15, .mcs = EGPRS_MCS0, .p = { EGPRS_CPS_NONE, EGPRS_CPS_NONE } },
 };
 
 int egprs_get_cps(struct egprs_cps *cps, uint8_t type, uint8_t bits)

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id665d5c0cf50efa18b1bcbf4f17359418a380f9e
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Tom Tsou <tom at tsou.cc>
Gerrit-Reviewer: fixeria <axilirator at gmail.com>

More information about the gerrit-log mailing list