[PATCH] openbsc[master]: Restructure SI2quater generation

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

Max gerrit-no-reply at lists.osmocom.org
Fri May 12 18:27:33 UTC 2017


Review at  https://gerrit.osmocom.org/2588

Restructure SI2quater generation

In preparation for extended SI2q messages:

* add SI2q-specific accessor macro
* add index, count and offset variables to gsm_bts struct
* internalize memory check while generating rest octets - introduce
  budget concept (number of bits available in a given message)
* internalize *arfcn_size() functions as they are not needed outside of
  si2q_num() anymore
* change rest octets generation to work with gsm_bts struct directly
* do not generate rest octets if no SI2q is necessary
* adjust unit tests accordingly

Change-Id: Ib554cf7ffc949a321571e1ae2ada1160e1b35fa6
Related: RT#8792
---
M openbsc/include/openbsc/gsm_data_shared.h
M openbsc/include/openbsc/rest_octets.h
M openbsc/include/openbsc/system_information.h
M openbsc/src/libbsc/bsc_vty.c
M openbsc/src/libbsc/rest_octets.c
M openbsc/src/libbsc/system_information.c
M openbsc/tests/gsm0408/gsm0408_test.c
M openbsc/tests/gsm0408/gsm0408_test.ok
8 files changed, 235 insertions(+), 161 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/88/2588/1

diff --git a/openbsc/include/openbsc/gsm_data_shared.h b/openbsc/include/openbsc/gsm_data_shared.h
index 5c96a62..5d87226 100644
--- a/openbsc/include/openbsc/gsm_data_shared.h
+++ b/openbsc/include/openbsc/gsm_data_shared.h
@@ -25,6 +25,7 @@
 #endif
 
 #include <openbsc/common_cs.h>
+#include <openbsc/rest_octets.h>
 
 struct osmo_bsc_data;
 
@@ -485,6 +486,7 @@
 	struct gsm_bts_trx_ts ts[TRX_NR_TS];
 };
 
+#define GSM_BTS_SI2Q(bts)	(struct gsm48_system_information_type_2quater *)((bts)->si_buf[SYSINFO_TYPE_2quater])
 #define GSM_BTS_SI(bts, i)	(void *)((bts)->si_buf[i])
 #define GSM_LCHAN_SI(lchan, i)	(void *)((lchan)->si.buf[i])
 
@@ -713,10 +715,13 @@
 	/* bitmask of all SI that are present/valid in si_buf */
 	uint32_t si_valid;
 	/* 3GPP TS 44.018 Table 10.5.2.33b.1 INDEX and COUNT for SI2quater */
-	uint8_t si2q_index;
-	uint8_t si2q_count;
+	uint8_t si2q_index; /* distinguish individual SI2quater messages */
+	uint8_t si2q_count; /* si2q_index for the last (highest indexed) individual SI2quater message */
 	/* buffers where we put the pre-computed SI */
 	sysinfo_buf_t si_buf[_MAX_SYSINFO_TYPE];
+	/* offsets used while generating SI2quater */
+	size_t e_offset;
+	size_t u_offset;
 
 	/* ip.accesss Unit ID's have Site/BTS/TRX layout */
 	union {
diff --git a/openbsc/include/openbsc/rest_octets.h b/openbsc/include/openbsc/rest_octets.h
index 73ce57b..7b324d3 100644
--- a/openbsc/include/openbsc/rest_octets.h
+++ b/openbsc/include/openbsc/rest_octets.h
@@ -13,8 +13,7 @@
 
 /* generate SI1 rest octets */
 int rest_octets_si1(uint8_t *data, uint8_t *nch_pos, int is1800_net);
-int rest_octets_si2quater(uint8_t *data, uint8_t index, uint8_t count, const struct osmo_earfcn_si2q *e,
-			  const uint16_t *u, const uint16_t *sc, size_t u_len);
+int rest_octets_si2quater(uint8_t *data, struct gsm_bts *bts);
 int rest_octets_si6(uint8_t *data, bool is1800_net);
 
 struct gsm48_si_selection_params {
diff --git a/openbsc/include/openbsc/system_information.h b/openbsc/include/openbsc/system_information.h
index b012107..6cee816 100644
--- a/openbsc/include/openbsc/system_information.h
+++ b/openbsc/include/openbsc/system_information.h
@@ -8,13 +8,12 @@
 struct gsm_bts;
 
 int gsm_generate_si(struct gsm_bts *bts, enum osmo_sysinfo_type type);
-unsigned uarfcn_size(const uint16_t *u, const uint16_t *sc, size_t u_len);
-unsigned earfcn_size(const struct osmo_earfcn_si2q *e);
+size_t earfcn_num(const struct osmo_earfcn_si2q *e);
 unsigned range1024_p(unsigned n);
 unsigned range512_q(unsigned m);
 int range_encode(enum gsm48_range r, int *arfcns, int arfcns_used, int *w,
 		 int f0, uint8_t *chan_list);
-uint8_t si2q_num(const struct gsm_bts *bts);
+uint8_t si2q_num(struct gsm_bts *bts);
 int bts_uarfcn_del(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble);
 int bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble,
 		   bool diversity);
diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c
index 2794d85..2c7b6c0 100644
--- a/openbsc/src/libbsc/bsc_vty.c
+++ b/openbsc/src/libbsc/bsc_vty.c
@@ -2829,11 +2829,11 @@
 		e->prio_valid = true;
 	}
 
-	if (si2q_num(bts) < 2)
+	if (si2q_num(bts) < 2) /* FIXME: use SI2Q_MAX_NUM */
 		return CMD_SUCCESS;
 
-	vty_out(vty, "Warning: not enough space in SI2quater for a given EARFCN "
-		"%u%s", arfcn, VTY_NEWLINE);
+	vty_out(vty, "Warning: not enough space in SI2quater (%u/%u used) for a given EARFCN %u%s",
+		bts->si2q_count, 2, arfcn, VTY_NEWLINE); /* FIXME: use SI2Q_MAX_NUM */
 	osmo_earfcn_del(e, arfcn);
 
 	return CMD_WARNING;
diff --git a/openbsc/src/libbsc/rest_octets.c b/openbsc/src/libbsc/rest_octets.c
index af660f1..bd9e13f 100644
--- a/openbsc/src/libbsc/rest_octets.c
+++ b/openbsc/src/libbsc/rest_octets.c
@@ -58,24 +58,55 @@
 	return bv.data_len;
 }
 
-/* Append Repeated E-UTRAN Neighbour Cell to bitvec:
- * see 3GPP TS 44.018 Table 10.5.2.33b.1
- */
-static inline void append_eutran_neib_cell(struct bitvec *bv,
-					    const struct osmo_earfcn_si2q *e)
+/* Append Repeated E-UTRAN Neighbour Cell to bitvec: see 3GPP TS 44.018 Table 10.5.2.33b.1 */
+static inline void append_eutran_neib_cell(struct bitvec *bv, struct gsm_bts *bts, uint8_t budget)
 {
-	unsigned i;
+	const struct osmo_earfcn_si2q *e = &bts->si_common.si2quater_neigh_list;
+	unsigned i, skip = 0;
+	size_t offset = bts->e_offset;
+	uint8_t rem = budget - 6, earfcn_budget; /* account for mandatory stop bit and THRESH_E-UTRAN_high */
+	/* first we have to properly adjust budget requirements */
+	if (e->prio_valid) /* E-UTRAN_PRIORITY: 3GPP TS 45.008*/
+		rem -= 4;
+	else
+		rem--;
+
+	if (e->thresh_lo_valid) /* THRESH_E-UTRAN_low: */
+		rem -= 6;
+	else
+		rem--;
+
+	if (e->qrxlm_valid) /* E-UTRAN_QRXLEVMIN: */
+		rem -= 6;
+	else
+		rem--;
+
+	/* no we can proceed with actually adding EARFCNs within adjusted budget limit */
 	for (i = 0; i < e->length; i++) {
 		if (e->arfcn[i] != OSMO_EARFCN_INVALID) {
-			bitvec_set_bit(bv, 1); /* EARFCN: */
-			bitvec_set_uint(bv, e->arfcn[i], 16);
+			if (skip < offset) {
+				skip++; /* ignore EARFCNs added on previous calls */
+			} else {
+				earfcn_budget = 17; /* computer budget per-EARFCN */
+				if (OSMO_EARFCN_MEAS_INVALID == e->meas_bw[i])
+					earfcn_budget++;
+				else
+					earfcn_budget += 4;
 
-			if (OSMO_EARFCN_MEAS_INVALID == e->meas_bw[i])
-				bitvec_set_bit(bv, 0);
-			else {
-				/* Measurement Bandwidth: 9.1.54 */
-				bitvec_set_bit(bv, 1);
-				bitvec_set_uint(bv, e->meas_bw[i], 3);
+				if (rem - earfcn_budget < 0) {
+					break;
+				} else {
+					bts->e_offset++;
+					bitvec_set_bit(bv, 1); /* EARFCN: */
+					bitvec_set_uint(bv, e->arfcn[i], 16);
+
+					if (OSMO_EARFCN_MEAS_INVALID == e->meas_bw[i])
+						bitvec_set_bit(bv, 0);
+					else { /* Measurement Bandwidth: 9.1.54 */
+						bitvec_set_bit(bv, 1);
+						bitvec_set_uint(bv, e->meas_bw[i], 3);
+					}
+				}
 			}
 		}
 	}
@@ -108,8 +139,7 @@
 		bitvec_set_bit(bv, 0);
 }
 
-static inline void append_earfcn(struct bitvec *bv,
-				const struct osmo_earfcn_si2q *e)
+static inline void append_earfcn(struct bitvec *bv, struct gsm_bts *bts, uint8_t budget)
 {
 	/* Additions in Rel-5: */
 	bitvec_set_bit(bv, H);
@@ -158,9 +188,8 @@
 	/* Repeated E-UTRAN Neighbour Cells */
 	bitvec_set_bit(bv, 1);
 
-	/* Note: we don't support different EARFCN arrays each with different
-	   priority, threshold etc. */
-	append_eutran_neib_cell(bv, e);
+	/* Note: we don't support different EARFCN arrays each with different priority, threshold etc. */
+	append_eutran_neib_cell(bv, bts, budget - 25);
 
 	/* stop bit - end of Repeated E-UTRAN Neighbour Cells sequence: */
 	bitvec_set_bit(bv, 0);
@@ -180,22 +209,43 @@
 	bitvec_set_bit(bv, L);
 }
 
-/* Append single FDD UARFCN */
-static inline int append_utran_fdd(struct bitvec *bv, uint16_t u, int *sc,
-				   size_t length)
+static inline int f0_helper(int *sc, size_t length, uint8_t *chan_list)
 {
-	int f0, w[RANGE_ENC_MAX_ARFCNS] = { 0 };
-	uint8_t chan_list[16] = {0};
+	int w[RANGE_ENC_MAX_ARFCNS] = { 0 };
+
+	return range_encode(ARFCN_RANGE_1024, sc, length, w, 0, chan_list);
+}
+
+/* Estimate how many bits it'll take to append single FDD UARFCN */
+static inline int append_utran_fdd_length(uint16_t u, int *sc, size_t sc_len, size_t length)
+{
+	uint8_t chan_list[16] = { 0 };
+	int tmp[sc_len], f0;
+
+	memcpy(tmp, sc, sizeof(tmp));
+
+	f0 = f0_helper(tmp, length, chan_list);
+	if (f0 < 0)
+		return f0;
+
+	return 21 + range1024_p(length);
+}
+
+/* Append single FDD UARFCN */
+static inline int append_utran_fdd(struct bitvec *bv, uint16_t u, int *sc, size_t length)
+{
+	uint8_t chan_list[16] = { 0 };
+	int f0 = f0_helper(sc, length, chan_list);
+
+	if (f0 < 0)
+		return f0;
+
 	/* Repeated UTRAN FDD Neighbour Cells */
 	bitvec_set_bit(bv, 1);
 
 	/* FDD-ARFCN */
 	bitvec_set_bit(bv, 0);
 	bitvec_set_uint(bv, u, 14);
-
-	f0 = range_encode(ARFCN_RANGE_1024, sc, length, w, 0, chan_list);
-	if (f0 < 0)
-		return f0;
 
 	/* FDD_Indic0: parameter value '0000000000' is a member of the set? */
 	bitvec_set_bit(bv, f0);
@@ -205,15 +255,17 @@
 	f0 = bv->cur_bit;
 	bitvec_add_range1024(bv, (struct gsm48_range_1024 *)chan_list);
 	bv->cur_bit = f0 + range1024_p(length);
-	return 0;
+
+	return 21 + range1024_p(length);
 }
 
 /* Append multiple FDD UARFCNs */
-static inline int append_uarfcns(struct bitvec *bv, const uint16_t *u,
-				 const uint16_t *sc, size_t length)
+static inline int append_uarfcns(struct bitvec *bv, struct gsm_bts *bts, uint8_t budget)
 {
-	int i, j, k, rc, st = 0, a[length];
-	uint16_t cu = u[0]; /* caller ensures that length is positive */
+	const uint16_t *u = bts->si_common.data.uarfcn_list, *sc = bts->si_common.data.scramble_list;
+	int i, j, k, rc, st = 0, a[bts->si_common.uarfcn_length];
+	uint16_t cu = u[bts->u_offset]; /* caller ensures that length is positive */
+	uint8_t rem = budget - 7; /* account for constant bits right away */
 
 	/* 3G Neighbour Cell Description */
 	bitvec_set_bit(bv, 1);
@@ -227,24 +279,41 @@
 	/* No Bandwidth_FDD */
 	bitvec_set_bit(bv, 0);
 
-	for (i = 0; i < length; i++) {
+	for (i = bts->u_offset; i < bts->si_common.uarfcn_length; i++) {
 		for (j = st, k = 0; j < i; j++)
 			a[k++] = sc[j]; /* copy corresponding SCs */
+
 		if (u[i] != cu) { /* we've reached new UARFCN */
-			rc = append_utran_fdd(bv, cu, a, k);
-			if (rc < 0)
+			rc = append_utran_fdd_length(cu, a, bts->si_common.uarfcn_length, k);
+			if (rc < 0) { /* estimate bit length requirements */
 				return rc;
+			}
+
+			if (rem - rc < 0) {
+				break; /* we have ran out of budget in current SI2q */
+			} else {
+				rem -= append_utran_fdd(bv, cu, a, k);
+				bts->u_offset++;
+			}
 			cu = u[i];
 			st = i; /* update start position */
 		}
 	}
 
-	/* add last UARFCN not covered by previous cycle */
-	for (i = st, k = 0; i < length; i++)
-		a[k++] = sc[i];
-	rc = append_utran_fdd(bv, cu, a, k);
-	if (rc < 0)
-		return rc;
+	if (rem > 22) {	/* add last UARFCN not covered by previous cycle if it could possibly fit into budget */
+		for (i = st, k = 0; i < bts->si_common.uarfcn_length; i++)
+			a[k++] = sc[i];
+
+		rc = append_utran_fdd_length(cu, a, bts->si_common.uarfcn_length, k);
+		if (rc < 0) {
+			return rc;
+		}
+
+		if (rem - rc >= 0) {
+			rem -= append_utran_fdd(bv, cu, a, k);
+			bts->u_offset++;
+		}
+	}
 
 	/* stop bit - end of Repeated UTRAN FDD Neighbour Cells */
 	bitvec_set_bit(bv, 0);
@@ -256,11 +325,9 @@
 }
 
 /* generate SI2quater rest octets: 3GPP TS 44.018 § 10.5.2.33b */
-int rest_octets_si2quater(uint8_t *data, uint8_t index, uint8_t count, const struct osmo_earfcn_si2q *e,
-			  const uint16_t *u, const uint16_t *sc, size_t u_len)
+int rest_octets_si2quater(uint8_t *data, struct gsm_bts *bts)
 {
 	int rc;
-	unsigned sz;
 	struct bitvec bv;
 	bv.data = data;
 	bv.data_len = 20;
@@ -273,11 +340,10 @@
 	/* MP_CHANGE_MARK */
 	bitvec_set_bit(&bv, 0);
 
-	/* we do not support multiple si2quater messages at the moment: */
 	/* SI2quater_INDEX */
-	bitvec_set_uint(&bv, index, 4);
+	bitvec_set_uint(&bv, bts->si2q_index, 4);
 	/* SI2quater_COUNT */
-	bitvec_set_uint(&bv, count, 4);
+	bitvec_set_uint(&bv, bts->si2q_count, 4);
 
 	/* No Measurement_Parameters Description */
 	bitvec_set_bit(&bv, 0);
@@ -294,22 +360,12 @@
 	/* No extension (length) */
 	bitvec_set_bit(&bv, 0);
 
-	if (u_len) {
-		sz = uarfcn_size(u, sc, u_len);
+	if (bts->si_common.uarfcn_length) {
 		/* Even if we do not append EARFCN we still need to set 3 bits */
-		if (sz + bv.cur_bit + 3 > SI2Q_MAX_LEN) {
-			LOGP(DRR, LOGL_ERROR, "SI2quater: not enough memory to "
-			     "add UARFCNs bits, current %u + required %u + "
-			     "reminder %u > max %u\n", bv.cur_bit, sz, 3,
-			     SI2Q_MAX_LEN);
-			return -ENOMEM;
-		}
-
-		rc = append_uarfcns(&bv, u, sc, u_len);
+		rc = append_uarfcns(&bv, bts, SI2Q_MAX_LEN - (bv.cur_bit + 3));
 		if (rc < 0) {
-			LOGP(DRR, LOGL_ERROR, "SI2quater: failed to append %zu "
-			     "UARFCNs due to range encoding failure: %s\n",
-			     u_len, strerror(-rc));
+			LOGP(DRR, LOGL_ERROR, "SI2quater: failed to append %zu UARFCNs due to range encoding failure: %s\n",
+			     bts->si_common.uarfcn_length, strerror(-rc));
 			return rc;
 		}
 	} else { /* No 3G Neighbour Cell Description */
@@ -321,15 +377,14 @@
 	/* No GPRS_3G_MEASUREMENT Parameters Descr. */
 	bitvec_set_bit(&bv, 0);
 
-	if (e) {
-		sz = earfcn_size(e);
-		if (sz + bv.cur_bit > SI2Q_MAX_LEN) {
-			LOGP(DRR, LOGL_ERROR, "SI2quater: not enough memory to "
-			     "add EARFCNs bits, current %u + required %u > max "
-			     "%u\n", bv.cur_bit, sz, SI2Q_MAX_LEN);
+	if (&bts->si_common.si2quater_neigh_list) { /* FIXME: use earfcn_num() in if */
+		append_earfcn(&bv, bts, SI2Q_MAX_LEN - bv.cur_bit);
+
+		/* FIXME: remove following check once multiple SI2q are properly supported */
+		if ((bts->e_offset != earfcn_num(&bts->si_common.si2quater_neigh_list)) ||
+		    earfcn_num(&bts->si_common.si2quater_neigh_list) > 5)
 			return -ENOMEM;
-		}
-		append_earfcn(&bv, e);
+
 	} else {
 		/* No Additions in Rel-5: */
 		bitvec_set_bit(&bv, L);
diff --git a/openbsc/src/libbsc/system_information.c b/openbsc/src/libbsc/system_information.c
index 37395f0..b226be6 100644
--- a/openbsc/src/libbsc/system_information.c
+++ b/openbsc/src/libbsc/system_information.c
@@ -122,19 +122,25 @@
 	}
 }
 
-unsigned earfcn_size(const struct osmo_earfcn_si2q *e)
+static inline unsigned earfcn_size(const struct gsm_bts *bts)
 {
+	const struct osmo_earfcn_si2q *e = &bts->si_common.si2quater_neigh_list; /* EARFCN */
+
 	/* account for all the constant bits in append_earfcn() */
+	/* FIXME: use osmo_earfcn_bit_size_ext(e, bts->e_offset) for proper offset accounting */
 	return 25 + osmo_earfcn_bit_size(e);
 }
 
-unsigned uarfcn_size(const uint16_t *u, const uint16_t *sc, size_t u_len)
+static inline unsigned uarfcn_size(const struct gsm_bts *bts)
 {
-	/*account for all the constant bits in append_uarfcns() */
+	const uint16_t *u = bts->si_common.data.uarfcn_list;
+	/* FIXME: use cu = u[bts->u_offset] for proper offset accounting */
+	uint16_t cu = u[0]; /* UARFCN */
+	/* account for all the constant bits in append_uarfcns() */
 	unsigned s = 7, append = 22, r = 0, i, st = 0, j, k;
-	uint16_t cu = u[0];
 
-	for (i = 0; i < u_len; i++) {
+	/* FIXME: start from i = bts->u_offset for proper offset accounting */
+	for (i = 0; i < bts->si_common.uarfcn_length; i++) {
 		for (j = st, k = 0; j < i; j++, k++);
 		if (u[i] != cu) { /* we've reached new UARFCN */
 			r += (append + range1024_p(k));
@@ -144,18 +150,25 @@
 	}
 
 	/* add last UARFCN not covered by previous cycle */
-	for (i = st, k = 0; i < u_len; i++, k++);
+	for (i = st, k = 0; i < bts->si_common.uarfcn_length; i++, k++);
 
 	return s + r + append + range1024_p(k);
 }
 
-uint8_t si2q_num(const struct gsm_bts *bts)
+uint8_t si2q_num(struct gsm_bts *bts)
 {
-	const struct osmo_earfcn_si2q *e = &bts->si_common.si2quater_neigh_list; /* EARFCN */
-	const uint16_t *u = bts->si_common.data.uarfcn_list, *sc = bts->si_common.data.scramble_list; /* UARFCN */
-	size_t l = bts->si_common.uarfcn_length, e_sz = e ? earfcn_size(e) : 1, u_sz = l ? uarfcn_size(u, sc, l) : 1;
+	size_t est, e_sz = 1, u_sz = 1;
+
+	if (&bts->si_common.si2quater_neigh_list) /* EARFCN */
+		e_sz = earfcn_size(bts);
+
+	if (bts->si_common.uarfcn_length) /* UARFCN */
+		u_sz = uarfcn_size(bts);
+
 	/* 2 bits are used in between UARFCN and EARFCN structs */
-	return 1 + (e_sz + u_sz) / (SI2Q_MAX_LEN - (SI2Q_MIN_LEN + 2));
+	est = 1 + (e_sz + u_sz) / (SI2Q_MAX_LEN - (SI2Q_MIN_LEN + 2));
+
+	return est;
 }
 
 /* 3GPP TS 44.018, Table 9.1.54.1 - prepend diversity bit to scrambling code */
@@ -191,8 +204,7 @@
 	return 0;
 }
 
-int bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble,
-		   bool diversity)
+int bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble, bool diversity)
 {
 	size_t len = bts->si_common.uarfcn_length, i, k = 0;
 	uint16_t scr, chk,
@@ -228,7 +240,7 @@
 	scl[k] = scr;
 	bts->si_common.uarfcn_length++;
 
-	if (si2q_num(bts) < 2)
+	if (si2q_num(bts) < 2) /* FIXME: use SI2Q_MAX_NUM */
 		return 0;
 
 	bts_uarfcn_del(bts, arfcn, scramble);
@@ -662,11 +674,40 @@
 	return sizeof(*si2t);
 }
 
+static inline bool si2quater_not_needed(struct gsm_bts *bts)
+{
+	unsigned i = MAX_EARFCN_LIST;
+
+	if (bts->si_common.si2quater_neigh_list.arfcn)
+		for (i = 0; i < MAX_EARFCN_LIST; i++)
+			if (bts->si_common.si2quater_neigh_list.arfcn[i] != OSMO_EARFCN_INVALID)
+				break;
+
+	if (!bts->si_common.uarfcn_length && i == MAX_EARFCN_LIST) {
+		bts->si_valid &= ~(1 << SYSINFO_TYPE_2quater); /* mark SI2q as invalid if no (E|U)ARFCNs are present */
+		return true;
+	}
+
+	return false;
+}
+
+size_t earfcn_num(const struct osmo_earfcn_si2q *e)
+{
+	unsigned i, ret = 0;
+	for (i = 0; i < e->length; i++)
+		if (e->arfcn[i] != OSMO_EARFCN_INVALID)
+			ret++;
+
+	return ret;
+}
+
 static int generate_si2quater(enum osmo_sysinfo_type t, struct gsm_bts *bts)
 {
-	int rc, i = MAX_EARFCN_LIST;
-	struct gsm48_system_information_type_2quater *si2q =
-		(struct gsm48_system_information_type_2quater *) GSM_BTS_SI(bts, t);
+	int rc;
+	struct gsm48_system_information_type_2quater *si2q = GSM_BTS_SI2Q(bts);
+
+	if (si2quater_not_needed(bts)) /* generate rest_octets for SI2q only when necessary */
+		return GSM_MACBLOCK_LEN;
 
 	memset(si2q, GSM_MACBLOCK_PADDING, GSM_MACBLOCK_LEN);
 
@@ -675,21 +716,9 @@
 	si2q->header.skip_indicator = 0;
 	si2q->header.system_information = GSM48_MT_RR_SYSINFO_2quater;
 
-	rc = rest_octets_si2quater(si2q->rest_octets, bts->si2q_index, bts->si2q_count,
-				   &bts->si_common.si2quater_neigh_list,
-				   bts->si_common.data.uarfcn_list,
-				   bts->si_common.data.scramble_list,
-				   bts->si_common.uarfcn_length);
+	rc = rest_octets_si2quater(si2q->rest_octets, bts);
 	if (rc < 0)
 		return rc;
-
-	if (bts->si_common.si2quater_neigh_list.arfcn)
-		for (i = 0; i < MAX_EARFCN_LIST; i++)
-			if (bts->si_common.si2quater_neigh_list.arfcn[i] !=
-			    OSMO_EARFCN_INVALID)
-				break;
-	if (!bts->si_common.uarfcn_length && i == MAX_EARFCN_LIST)
-		bts->si_valid &= ~(1 << SYSINFO_TYPE_2quater);
 
 	return sizeof(*si2q) + rc;
 }
diff --git a/openbsc/tests/gsm0408/gsm0408_test.c b/openbsc/tests/gsm0408/gsm0408_test.c
index 81dc177..9f0c328 100644
--- a/openbsc/tests/gsm0408/gsm0408_test.c
+++ b/openbsc/tests/gsm0408/gsm0408_test.c
@@ -84,18 +84,12 @@
     COMPARE(lai48.lac, ==, htons(0x000f));
 }
 
-static inline void add_arfcn_b(struct osmo_earfcn_si2q *e, uint16_t earfcn,
-			       uint8_t bw)
+static inline void gen(struct gsm_bts *bts, const char *s)
 {
-	int r = osmo_earfcn_add(e, earfcn, bw);
-	if (r)
-		printf("failed to add EARFCN %u: %s\n", earfcn, strerror(-r));
-	else
-		printf("added EARFCN %u - ", earfcn);
-}
-
-static inline void gen(struct gsm_bts *bts)
-{
+	bts->u_offset = 0;
+	bts->e_offset = 0;
+	bts->si2q_index = 0;
+	bts->si2q_count = 0;
 	bts->si_valid = 0;
 	bts->si_valid |= (1 << SYSINFO_TYPE_2quater);
 	/* should be no-op as entire buffer is filled with padding: */
@@ -106,17 +100,28 @@
 		printf("generated %s SI2quater: [%d] %s\n",
 		       v ? "valid" : "invalid", r, osmo_hexdump(GSM_BTS_SI(bts, SYSINFO_TYPE_2quater), r));
 	else
-		printf("failed to generate SI2quater: %s\n", strerror(-r));
+		printf("%s() failed to generate SI2quater: %s\n", s, strerror(-r));
 }
 
-static inline void _bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn,
-			      uint16_t scramble, bool diversity)
+static inline void add_earfcn_b(struct gsm_bts *bts, uint16_t earfcn, uint8_t bw)
+{
+	struct osmo_earfcn_si2q *e = &bts->si_common.si2quater_neigh_list;
+	int r = osmo_earfcn_add(e, earfcn, bw);
+	if (r)
+		printf("failed to add EARFCN %u: %s\n", earfcn, strerror(-r));
+	else
+		printf("added EARFCN %u - ", earfcn);
+
+	gen(bts, __func__);
+}
+
+static inline void _bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble, bool diversity)
 {
 	int r = bts_uarfcn_add(bts, arfcn, scramble, diversity);
 	if (r < 0)
 		printf("failed to add UARFCN to SI2quater: %s\n", strerror(-r));
 	else
-		gen(bts);
+		gen(bts, __func__);
 }
 
 static inline void test_si2q_segfault(void)
@@ -131,7 +136,7 @@
 
 	_bts_uarfcn_add(bts, 10564, 319, 0);
 	_bts_uarfcn_add(bts, 10612, 319, 0);
-	gen(bts);
+	gen(bts, __func__);
 }
 
 static inline void test_si2q_mu(void)
@@ -151,7 +156,7 @@
 	_bts_uarfcn_add(bts, 10613, 64, 0);
 	_bts_uarfcn_add(bts, 10613, 164, 0);
 	_bts_uarfcn_add(bts, 10613, 14, 0);
-	gen(bts);
+	gen(bts, __func__);
 }
 
 static inline void test_si2q_u(void)
@@ -165,7 +170,7 @@
 	bts = gsm_bts_alloc(network);
 
 	/* first generate invalid SI as no UARFCN added */
-	gen(bts);
+	gen(bts, __func__);
 	/* subsequent calls should produce valid SI if there's enough memory */
 	_bts_uarfcn_add(bts, 1982, 13, 1);
 	_bts_uarfcn_add(bts, 1982, 44, 0);
@@ -178,7 +183,7 @@
 	_bts_uarfcn_add(bts, 1982, 223, 1);
 	_bts_uarfcn_add(bts, 1982, 14, 0);
 	_bts_uarfcn_add(bts, 1982, 88, 0);
-	gen(bts);
+	gen(bts, __func__);
 }
 
 static inline void test_si2q_e(void)
@@ -191,40 +196,22 @@
 		exit(1);
 	bts = gsm_bts_alloc(network);
 
-	bts->si_common.si2quater_neigh_list.arfcn =
-		bts->si_common.data.earfcn_list;
-	bts->si_common.si2quater_neigh_list.meas_bw =
-		bts->si_common.data.meas_bw_list;
+	bts->si_common.si2quater_neigh_list.arfcn = bts->si_common.data.earfcn_list;
+	bts->si_common.si2quater_neigh_list.meas_bw = bts->si_common.data.meas_bw_list;
 	bts->si_common.si2quater_neigh_list.length = MAX_EARFCN_LIST;
 	bts->si_common.si2quater_neigh_list.thresh_hi = 5;
 
 	osmo_earfcn_init(&bts->si_common.si2quater_neigh_list);
 	/* first generate invalid SI as no EARFCN added */
-	gen(bts);
+	gen(bts, __func__);
 	/* subsequent calls should produce valid SI if there's enough memory */
-	add_arfcn_b(&bts->si_common.si2quater_neigh_list, 1917, 1);
-	gen(bts);
-
-	add_arfcn_b(&bts->si_common.si2quater_neigh_list, 1932,
-		    OSMO_EARFCN_MEAS_INVALID);
-	gen(bts);
-
-	add_arfcn_b(&bts->si_common.si2quater_neigh_list, 1937, 2);
-	gen(bts);
-
-	add_arfcn_b(&bts->si_common.si2quater_neigh_list, 1945,
-		    OSMO_EARFCN_MEAS_INVALID);
-	gen(bts);
-
-	add_arfcn_b(&bts->si_common.si2quater_neigh_list, 1965,
-		    OSMO_EARFCN_MEAS_INVALID);
-	gen(bts);
-
-	add_arfcn_b(&bts->si_common.si2quater_neigh_list, 1967, 4);
-	gen(bts);
-
-	add_arfcn_b(&bts->si_common.si2quater_neigh_list, 1982, 3);
-	gen(bts);
+	add_earfcn_b(bts, 1917, 1);
+	add_earfcn_b(bts, 1932, OSMO_EARFCN_MEAS_INVALID);
+	add_earfcn_b(bts, 1937, 2);
+	add_earfcn_b(bts, 1945, OSMO_EARFCN_MEAS_INVALID);
+	add_earfcn_b(bts, 1965, OSMO_EARFCN_MEAS_INVALID);
+	add_earfcn_b(bts, 1967, 4);
+	add_earfcn_b(bts, 1982, 3);
 }
 
 static void test_mi_functionality(void)
diff --git a/openbsc/tests/gsm0408/gsm0408_test.ok b/openbsc/tests/gsm0408/gsm0408_test.ok
index 1c02dfd..5f9398b 100644
--- a/openbsc/tests/gsm0408/gsm0408_test.ok
+++ b/openbsc/tests/gsm0408/gsm0408_test.ok
@@ -67,16 +67,16 @@
 generated valid SI2quater: [23] 59 06 07 c0 00 25 52 e8 0a 7f 52 88 0a 7e 10 99 64 00 0b 2b 2b 2b 2b 
 generated valid SI2quater: [23] 59 06 07 c0 00 25 52 e8 0a 7f 52 88 0a 7e 10 99 64 00 0b 2b 2b 2b 2b 
 Testing SYSINFO_TYPE_2quater EARFCN generation:
-generated invalid SI2quater: [23] 59 06 07 c0 00 04 86 59 0a 03 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
+generated invalid SI2quater: [23] ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae 
 added EARFCN 1917 - generated valid SI2quater: [23] 59 06 07 c0 00 04 86 59 83 be c8 50 0b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
 added EARFCN 1932 - generated valid SI2quater: [23] 59 06 07 c0 00 04 86 59 83 be cc 1e 30 14 03 2b 2b 2b 2b 2b 2b 2b 2b 
 added EARFCN 1937 - generated valid SI2quater: [23] 59 06 07 c0 00 04 86 59 83 be cc 1e 31 07 91 a0 a0 2b 2b 2b 2b 2b 2b 
 added EARFCN 1945 - generated valid SI2quater: [23] 59 06 07 c0 00 04 86 59 83 be cc 1e 31 07 91 a8 3c c8 28 0b 2b 2b 2b 
 added EARFCN 1965 - generated valid SI2quater: [23] 59 06 07 c0 00 04 86 59 83 be cc 1e 31 07 91 a8 3c ca 0f 5a 0a 03 2b 
-added EARFCN 1967 - failed to generate SI2quater: Cannot allocate memory
-added EARFCN 1982 - failed to generate SI2quater: Cannot allocate memory
+added EARFCN 1967 - add_earfcn_b() failed to generate SI2quater: Cannot allocate memory
+added EARFCN 1982 - add_earfcn_b() failed to generate SI2quater: Cannot allocate memory
 Testing SYSINFO_TYPE_2quater UARFCN generation:
-generated invalid SI2quater: [23] 59 06 07 c0 00 04 86 59 00 03 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
+generated invalid SI2quater: [23] ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae ae 
 generated valid SI2quater: [23] 59 06 07 c0 00 25 0f 7c 0c 1a 10 99 64 00 0b 2b 2b 2b 2b 2b 2b 2b 2b 
 generated valid SI2quater: [23] 59 06 07 c0 00 25 0f 7c 14 1a 1f 00 44 b2 00 03 2b 2b 2b 2b 2b 2b 2b 
 generated valid SI2quater: [23] 59 06 07 c0 00 25 0f 7c 18 58 12 f0 84 86 59 00 03 2b 2b 2b 2b 2b 2b 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib554cf7ffc949a321571e1ae2ada1160e1b35fa6
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>



More information about the gerrit-log mailing list