[PATCH] openbsc[master]: Use libosmocore for SW Description parsing

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
Tue Apr 4 16:35:14 UTC 2017


Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/2166

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

Use libosmocore for SW Description parsing

Requires libosmocore with Ib63b6b5e83b8914864fc7edd789f8958cdc993cd.

Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
---
M openbsc/include/openbsc/abis_nm.h
M openbsc/src/ipaccess/ipaccess-config.c
M openbsc/src/libbsc/abis_nm.c
M openbsc/tests/abis/abis_test.c
M openbsc/tests/abis/abis_test.ok
5 files changed, 36 insertions(+), 213 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/66/2166/5

diff --git a/openbsc/include/openbsc/abis_nm.h b/openbsc/include/openbsc/abis_nm.h
index 2465452..09e0b23 100644
--- a/openbsc/include/openbsc/abis_nm.h
+++ b/openbsc/include/openbsc/abis_nm.h
@@ -68,18 +68,6 @@
 	int (*sw_act_req)(struct msgb *);
 };
 
-struct abis_nm_sw_descr {
-	/* where does it start? how long is it? */
-	const uint8_t	*start;
-	size_t		len;
-
-	/* the parsed data */
-	const uint8_t 	*file_id;
-	uint16_t	file_id_len;
-	const uint8_t	*file_ver;
-	uint16_t	file_ver_len;
-};
-
 extern int abis_nm_rcvmsg(struct msgb *msg);
 
 int abis_nm_tlv_parse(struct tlv_parsed *tp, struct gsm_bts *bts, const uint8_t *buf, int len);
@@ -181,8 +169,6 @@
 
 void abis_nm_queue_send_next(struct gsm_bts *bts);	/* for bs11_config. */
 
-int abis_nm_parse_sw_config(const uint8_t *data, const size_t len,
-			struct abis_nm_sw_descr *res, const int res_len);
 int abis_nm_select_newest_sw(const struct abis_nm_sw_descr *sw, const size_t len);
 
 /* Helper functions for updating attributes */
diff --git a/openbsc/src/ipaccess/ipaccess-config.c b/openbsc/src/ipaccess/ipaccess-config.c
index 0c3f888..ca5f195 100644
--- a/openbsc/src/ipaccess/ipaccess-config.c
+++ b/openbsc/src/ipaccess/ipaccess-config.c
@@ -52,6 +52,7 @@
 #include <openbsc/network_listen.h>
 #include <osmocom/core/talloc.h>
 #include <osmocom/abis/abis.h>
+#include <osmocom/gsm/protocol/gsm_12_21.h>
 
 struct gsm_network *bsc_gsmnet;
 
@@ -70,17 +71,9 @@
 static int found_trx = 0;
 static int loop_tests = 0;
 
-struct sw_load {
-	uint8_t file_id[255];
-	uint8_t file_id_len;
-
-	uint8_t file_version[255];
-	uint8_t file_version_len;
-};
-
 static void *tall_ctx_config = NULL;
-static struct sw_load *sw_load1 = NULL;
-static struct sw_load *sw_load2 = NULL;
+static struct abis_nm_sw_descr *sw_load1 = NULL;
+static struct abis_nm_sw_descr *sw_load2 = NULL;
 
 /*
 static uint8_t prim_oml_attr[] = { 0x95, 0x00, 7, 0x88, 192, 168, 100, 11, 0x00, 0x00 };
@@ -344,19 +337,11 @@
 		msg->l3h = &msg->l2h[3];
 
 		/* activate software */
-		if (sw_load1) {
-			msgb_v_put(msg, NM_ATT_SW_DESCR);
-			msgb_tl16v_put(msg, NM_ATT_FILE_ID, sw_load1->file_id_len, sw_load1->file_id);
-			msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, sw_load1->file_version_len,
-					sw_load1->file_version);
-		}
+		if (sw_load1)
+			abis_nm_put_sw_descr(msg, sw_load1, true);
 
-		if (sw_load2) {
-			msgb_v_put(msg, NM_ATT_SW_DESCR);
-			msgb_tl16v_put(msg, NM_ATT_FILE_ID, sw_load2->file_id_len, sw_load2->file_id);
-			msgb_tl16v_put(msg, NM_ATT_FILE_VERSION, sw_load2->file_version_len,
-					sw_load2->file_version);
-		}
+		if (sw_load2)
+			abis_nm_put_sw_descr(msg, sw_load2, true);
 
 		/* fill in the data */
 		msg->l2h[0] = NM_ATT_IPACC_CUR_SW_CFG;
@@ -618,11 +603,11 @@
 	return 0;
 }
 
-static struct sw_load *create_swload(struct sdp_header *header)
+static struct abis_nm_sw_descr *create_swload(struct sdp_header *header)
 {
-	struct sw_load *load;
+	struct abis_nm_sw_descr *load;
 
-	load = talloc_zero(tall_ctx_config, struct sw_load);
+	load = talloc_zero(tall_ctx_config, struct abis_nm_sw_descr);
 
 	strncpy((char *)load->file_id, header->firmware_info.sw_part, 20);
 	load->file_id_len = strlen(header->firmware_info.sw_part) + 1;
diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 33a23a2..cd848b0 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -413,93 +413,29 @@
 
 /* Activate the specified software into the BTS */
 static int ipacc_sw_activate(struct gsm_bts *bts, uint8_t obj_class, uint8_t i0, uint8_t i1,
-			     uint8_t i2, const uint8_t *sw_desc, uint8_t swdesc_len)
+			     uint8_t i2, const struct abis_nm_sw_descr *sw_desc)
 {
 	struct abis_om_hdr *oh;
 	struct msgb *msg = nm_msgb_alloc();
-	uint8_t len = swdesc_len;
-	uint8_t *trailer;
+	uint16_t len = abis_nm_sw_descr_len(sw_desc, true);
 
 	oh = (struct abis_om_hdr *) msgb_put(msg, ABIS_OM_FOM_HDR_SIZE);
 	fill_om_fom_hdr(oh, len, NM_MT_ACTIVATE_SW, obj_class, i0, i1, i2);
-
-	trailer = msgb_put(msg, swdesc_len);
-	memcpy(trailer, sw_desc, swdesc_len);
+	abis_nm_put_sw_descr(msg, sw_desc, true);
 
 	return abis_nm_sendmsg(bts, msg);
 }
 
-int abis_nm_parse_sw_config(const uint8_t *sw_descr, const size_t sw_descr_len,
-			struct abis_nm_sw_descr *desc, const int res_len)
-{
-	static const struct tlv_definition sw_descr_def = {
-		.def = {
-			[NM_ATT_FILE_ID] =		{ TLV_TYPE_TL16V, },
-			[NM_ATT_FILE_VERSION] =		{ TLV_TYPE_TL16V, },
-		},
-	};
-
-	size_t pos = 0;
-	int desc_pos = 0;
-
-	for (pos = 0; pos < sw_descr_len && desc_pos < res_len; ++desc_pos) {
-		uint8_t tag;
-		uint16_t tag_len;
-		const uint8_t *val;
-		int len;
-
-		memset(&desc[desc_pos], 0, sizeof(desc[desc_pos]));
-		desc[desc_pos].start = &sw_descr[pos];
-
-		/* Classic TLV parsing doesn't work well with SW_DESCR because of it's
-		 * nested nature and the fact you have to assume it contains only two sub
-		 * tags NM_ATT_FILE_VERSION & NM_ATT_FILE_ID to parse it */
-		if (sw_descr[pos] != NM_ATT_SW_DESCR) {
-			LOGP(DNM, LOGL_ERROR,
-				"SW_DESCR attribute identifier not found!\n");
-			return -1;
-		}
-
-		pos += 1;
-		len = tlv_parse_one(&tag, &tag_len, &val,
-			&sw_descr_def, &sw_descr[pos], sw_descr_len - pos);
-		if (len < 0 || (tag != NM_ATT_FILE_ID)) {
-			LOGP(DNM, LOGL_ERROR,
-				"FILE_ID attribute identifier not found!\n");
-			return -2;
-		}
-		desc[desc_pos].file_id = val;
-		desc[desc_pos].file_id_len = tag_len;
-		pos += len;
-
-
-		len = tlv_parse_one(&tag, &tag_len, &val,
-			&sw_descr_def, &sw_descr[pos], sw_descr_len - pos);
-		if (len < 0 || (tag != NM_ATT_FILE_VERSION)) {
-			LOGP(DNM, LOGL_ERROR,
-				"FILE_VERSION attribute identifier not found!\n");
-			return -3;
-		}
-		desc[desc_pos].file_ver = val;
-		desc[desc_pos].file_ver_len = tag_len;
-		pos += len;
-
-		/* final size */
-		desc[desc_pos].len = &sw_descr[pos] - desc[desc_pos].start;
-	}
-
-	return desc_pos;
-}
-
 int abis_nm_select_newest_sw(const struct abis_nm_sw_descr *sw_descr,
-				const size_t size)
+			     const size_t size)
 {
 	int res = 0;
 	int i;
 
 	for (i = 1; i < size; ++i) {
-		if (memcmp(sw_descr[res].file_ver, sw_descr[i].file_ver,
-			OSMO_MIN(sw_descr[i].file_ver_len, sw_descr[res].file_ver_len)) < 0) {
+		if (memcmp(sw_descr[res].file_version, sw_descr[i].file_version,
+			   OSMO_MIN(sw_descr[i].file_version_len,
+				    sw_descr[res].file_version_len)) < 0) {
 			res = i;
 		}
 	}
@@ -546,8 +482,8 @@
 	}
 
 	/* Parse up to two sw descriptions from the data */
-	len = abis_nm_parse_sw_config(sw_config, sw_config_len,
-				&sw_descr[0], ARRAY_SIZE(sw_descr));
+	len = abis_nm_get_sw_conf(sw_config, sw_config_len, &sw_descr[0],
+				  ARRAY_SIZE(sw_descr));
 	if (len <= 0) {
 		LOGP(DNM, LOGL_ERROR, "Failed to parse SW Config.\n");
 		return -EINVAL;
@@ -560,7 +496,7 @@
 				 foh->obj_inst.bts_nr,
 				 foh->obj_inst.trx_nr,
 				 foh->obj_inst.ts_nr,
-				 sw_descr[ret].start, sw_descr[ret].len);
+				 &sw_descr[ret]);
 }
 
 /* Receive a CHANGE_ADM_STATE_ACK, parse the TLV and update local state */
diff --git a/openbsc/tests/abis/abis_test.c b/openbsc/tests/abis/abis_test.c
index 496267f..e288e30 100644
--- a/openbsc/tests/abis/abis_test.c
+++ b/openbsc/tests/abis/abis_test.c
@@ -22,21 +22,11 @@
 
 #include <osmocom/core/application.h>
 #include <osmocom/core/utils.h>
+#include <osmocom/gsm/protocol/gsm_12_21.h>
 
 #include <openbsc/gsm_data.h>
 #include <openbsc/abis_nm.h>
 #include <openbsc/debug.h>
-
-static const uint8_t simple_config[] = {
-	/*0, 13, */
-	66, 18, 0, 3, 1, 2, 3, 19, 0, 3, 3, 4, 5,
-};
-
-static const uint8_t dual_config[] = {
-	/*0, 26, */
-	66, 18, 0, 3, 1, 2, 3, 19, 0, 3, 3, 4, 5,
-	66, 18, 0, 3, 9, 7, 5, 19, 0, 3, 6, 7, 8,
-};
 
 static const uint8_t load_config[] = {
 	0x42, 0x12, 0x00, 0x08, 0x31, 0x36, 0x38, 0x64,
@@ -48,94 +38,29 @@
 	0x33, 0x64, 0x31, 0x00
 };
 
-static void test_simple_sw_config(void)
-{
-	struct abis_nm_sw_descr descr[1];
-	int rc;
-
-	rc = abis_nm_parse_sw_config(simple_config, ARRAY_SIZE(simple_config),
-				&descr[0], ARRAY_SIZE(descr));
-	if (rc != 1) {
-		printf("FAILED to parse the File Id/File version\n");
-		abort();
-	}
-
-	if (descr[0].len != 13) {
-		printf("WRONG SIZE: %zu\n", descr[0].len);
-		abort();
-	}
-
-	printf("Start: %td len: %zu\n", descr[0].start - simple_config, descr[0].len);
-	printf("file_id:  %s\n", osmo_hexdump(descr[0].file_id, descr[0].file_id_len));
-	printf("file_ver: %s\n", osmo_hexdump(descr[0].file_ver, descr[0].file_ver_len));
-}
-
-static void test_simple_sw_short(void)
-{
-	struct abis_nm_sw_descr descr[1];
-	int i;
-
-	for (i = 1; i < ARRAY_SIZE(simple_config); ++i) {
-		int rc = abis_nm_parse_sw_config(simple_config,
-				ARRAY_SIZE(simple_config) - i, &descr[0],
-				ARRAY_SIZE(descr));
-		if (rc >= 1) {
-			printf("SHOULD not have parsed: %d\n", rc);
-			abort();
-		}
-	}
-}
-
-static void test_dual_sw_config(void)
-{
-	struct abis_nm_sw_descr descr[2];
-	int rc;
-
-	rc = abis_nm_parse_sw_config(dual_config, ARRAY_SIZE(dual_config),
-				&descr[0], ARRAY_SIZE(descr));
-	if (rc != 2) {
-		printf("FAILED to parse the File Id/File version\n");
-		abort();
-	}
-
-	if (descr[0].len != 13) {
-		printf("WRONG SIZE0: %zu\n", descr[0].len);
-		abort();
-	}
-
-	if (descr[1].len != 13) {
-		printf("WRONG SIZE1: %zu\n", descr[1].len);
-		abort();
-	}
-
-	printf("Start: %td len: %zu\n", descr[0].start - dual_config, descr[0].len);
-	printf("file_id:  %s\n", osmo_hexdump(descr[0].file_id, descr[0].file_id_len));
-	printf("file_ver: %s\n", osmo_hexdump(descr[0].file_ver, descr[0].file_ver_len));
-
-	printf("Start: %td len: %zu\n", descr[1].start - dual_config, descr[1].len);
-	printf("file_id:  %s\n", osmo_hexdump(descr[1].file_id, descr[1].file_id_len));
-	printf("file_ver: %s\n", osmo_hexdump(descr[1].file_ver, descr[1].file_ver_len));
-}
-
 static void test_sw_selection(void)
 {
 	struct abis_nm_sw_descr descr[8], tmp;
+	uint16_t len0, len1;
 	int rc, pos;
 
-	rc = abis_nm_parse_sw_config(load_config, ARRAY_SIZE(load_config),
+	rc = abis_nm_get_sw_conf(load_config, ARRAY_SIZE(load_config),
 				&descr[0], ARRAY_SIZE(descr));
 	if (rc != 2) {
-		printf("FAILED to parse the File Id/File version\n");
+		printf("%s(): FAILED to parse the File Id/File version: %d\n",
+		       __func__, rc);
 		abort();
 	}
 
-	printf("Start: %td len: %zu\n", descr[0].start - load_config, descr[0].len);
+	len0 = abis_nm_sw_descr_len(&descr[0], true);
+	printf("len: %u\n", len0);
 	printf("file_id:  %s\n", osmo_hexdump(descr[0].file_id, descr[0].file_id_len));
-	printf("file_ver: %s\n", osmo_hexdump(descr[0].file_ver, descr[0].file_ver_len));
+	printf("file_ver: %s\n", osmo_hexdump(descr[0].file_version, descr[0].file_version_len));
 
-	printf("Start: %td len: %zu\n", descr[1].start - load_config, descr[1].len);
+	len1 = abis_nm_sw_descr_len(&descr[1], true);
+	printf("len: %u\n", len1);
 	printf("file_id:  %s\n", osmo_hexdump(descr[1].file_id, descr[1].file_id_len));
-	printf("file_ver: %s\n", osmo_hexdump(descr[1].file_ver, descr[1].file_ver_len));
+	printf("file_ver: %s\n", osmo_hexdump(descr[1].file_version, descr[1].file_version_len));
 
 	/* start */
 	pos = abis_nm_select_newest_sw(descr, rc);
@@ -155,14 +80,13 @@
 		abort();
 	}
 	printf("SELECTED: %d\n", pos);
+	printf("%s(): OK\n", __func__);
 }
 
 int main(int argc, char **argv)
 {
 	osmo_init_logging(&log_info);
-	test_simple_sw_config();
-	test_simple_sw_short();
-	test_dual_sw_config();
+
 	test_sw_selection();
 
 	return EXIT_SUCCESS;
diff --git a/openbsc/tests/abis/abis_test.ok b/openbsc/tests/abis/abis_test.ok
index 2f99f9d..8418cad 100644
--- a/openbsc/tests/abis/abis_test.ok
+++ b/openbsc/tests/abis/abis_test.ok
@@ -1,17 +1,9 @@
-Start: 0 len: 13
-file_id:  01 02 03 
-file_ver: 03 04 05 
-Start: 0 len: 13
-file_id:  01 02 03 
-file_ver: 03 04 05 
-Start: 13 len: 13
-file_id:  09 07 05 
-file_ver: 06 07 08 
-Start: 0 len: 26
+len: 26
 file_id:  31 36 38 64 34 37 32 00 
 file_ver: 76 32 30 30 62 31 34 33 64 30 00 
-Start: 26 len: 26
+len: 26
 file_id:  31 36 38 64 34 37 32 00 
 file_ver: 76 32 30 30 62 31 34 33 64 31 00 
 SELECTED: 1
 SELECTED: 0
+test_sw_selection(): OK

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib94db414e94a2a1f234ac6f1cb346dca1c7a8be3
Gerrit-PatchSet: 5
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder



More information about the gerrit-log mailing list