Change in osmo-bsc[master]: abis_nm: Unify Get Attribute Response 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/.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Thu Nov 8 13:43:19 UTC 2018


Pau Espin Pedrol has uploaded this change for review. ( https://gerrit.osmocom.org/11682


Change subject: abis_nm: Unify Get Attribute Response parsing
......................................................................

abis_nm: Unify Get Attribute Response parsing

After a bit more investgiation, it seems nanoBTS actually supports
regular formatting. There are a few differences with spec though:
* The attributes are directly in the message instead of being inside the
Get Attributes Response Info after the unsupported attribute ID list.
* The Get Attributes Response info can be at any position in the
attribute list, and it only contains the unsupported attribute ID list.

As a result, we can split parsing into 3 main parts or functions:
* Parsing regular (per spec) Get Attributes Response Info attr and get a
pointer to the list of attributes.
* A function that parses the list of attributes, called directly in case
of nanoBTS, and called by the former parser of Get Attributes Response
Info for regular (per spec) OML endpoints.
* A function to parse the unsupported attribute ID list, also used in the
first function to get a pointer to the list of attributes.

Related: OS#3624
Change-Id: I77574579a19f1988f82cdac81b257b8f6871b857
---
M src/osmo-bsc/abis_nm.c
1 file changed, 55 insertions(+), 68 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/82/11682/1

diff --git a/src/osmo-bsc/abis_nm.c b/src/osmo-bsc/abis_nm.c
index 61820b8..bb2b5e2 100644
--- a/src/osmo-bsc/abis_nm.c
+++ b/src/osmo-bsc/abis_nm.c
@@ -461,36 +461,41 @@
 		     bts_nr, get_value_string(abis_nm_att_names, ari[i + 1]));
 
 	/* the data starts right after the list of unreported attributes + space for length of that list */
-	*out_len = ari_len - (num_unreported + 2);
+	if (out_len)
+		*out_len = ari_len - (num_unreported + 2);
 
 	return ari + num_unreported + 1; /* we have to account for 1st byte with number of unreported attributes */
 }
 
-/* Parse Attribute Response Info content for 3GPP TS 52.021 §9.4.30 Manufacturer Id */
-static inline const uint8_t *parse_attr_resp_info_manuf_id(struct gsm_bts *bts, const uint8_t *data, uint16_t *data_len)
+/* Handle 3GPP TS 52.021 §8.11.3 Get Attribute Response (with nanoBTS speecific attribute formatting) */
+static int parse_attr_resp_info_attr(struct gsm_bts *bts, const struct gsm_bts_trx *trx, struct abis_om_fom_hdr *foh, struct tlv_parsed *tp)
 {
-	struct tlv_parsed tp;
-	uint16_t m_id_len = 0;
-	uint8_t adjust = 0, i;
+	const uint8_t* data;
+	uint16_t len;
+	uint16_t port;
+	struct in_addr ia = {0};
+	char unit_id[40];
+	int i;
+	int rc;
+	struct abis_nm_sw_desc sw_descr[MAX_BTS_ATTR];
 
-	abis_nm_tlv_parse(&tp, bts, data, *data_len);
-	if (TLVP_PRES_LEN(&tp, NM_ATT_MANUF_ID, 2)) {
-		m_id_len = TLVP_LEN(&tp, NM_ATT_MANUF_ID);
+	/* Parse Attribute Response Info content for 3GPP TS 52.021 §9.4.30 Manufacturer Id */
+	if (TLVP_PRES_LEN(tp, NM_ATT_MANUF_ID, 2)) {
+		len = TLVP_LEN(tp, NM_ATT_MANUF_ID);
 
 		/* log potential BTS feature vector overflow */
-		if (m_id_len > sizeof(bts->_features_data))
+		if (len > sizeof(bts->_features_data))
 			LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: feature vector is truncated to %u bytes\n",
 			     bts->nr, MAX_BTS_FEATURES/8);
 
 		/* check that max. expected BTS attribute is above given feature vector length */
-		if (m_id_len > OSMO_BYTES_FOR_BITS(_NUM_BTS_FEAT))
+		if (len > OSMO_BYTES_FOR_BITS(_NUM_BTS_FEAT))
 			LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: reported unexpectedly long (%u bytes) "
 			     "feature vector - most likely it was compiled against newer BSC headers. "
 			     "Consider upgrading your BSC to later version.\n",
-			     bts->nr, m_id_len);
+			     bts->nr, len);
 
-		memcpy(bts->_features_data, TLVP_VAL(&tp, NM_ATT_MANUF_ID), sizeof(bts->_features_data));
-		adjust = m_id_len + 3; /* adjust for parsed TL16V struct */
+		memcpy(bts->_features_data, TLVP_VAL(tp, NM_ATT_MANUF_ID), sizeof(bts->_features_data));
 
 		for (i = 0; i < _NUM_BTS_FEAT; i++)
 			if (osmo_bts_has_feature(&bts->features, i) != osmo_bts_has_feature(&bts->model->features, i))
@@ -500,42 +505,32 @@
 				     osmo_bts_has_feature(&bts->features, i), osmo_bts_has_feature(&bts->model->features, i));
 	}
 
-	*data_len -= adjust;
-
-	return data + adjust;
-}
-
-/* Parse Attribute Response Info content for 3GPP TS 52.021 §9.4.28 Manufacturer Dependent State */
-static inline const uint8_t *parse_attr_resp_info_manuf_state(const struct gsm_bts_trx *trx, const uint8_t *data, uint16_t *data_len)
-{
-	struct tlv_parsed tp;
-	const uint8_t *power;
-	uint8_t adjust = 0;
-
-	if (!trx) /* this attribute does not make sense on BTS level, only on TRX level */
-		return data;
-
-	abis_nm_tlv_parse(&tp, trx->bts, data, *data_len);
-	if (TLVP_PRES_LEN(&tp, NM_ATT_MANUF_STATE, 1)) {
-		power = TLVP_VAL(&tp, NM_ATT_MANUF_STATE);
-		LOGP(DNM, LOGL_NOTICE, "%s Get Attributes Response: nominal power is %u\n", gsm_trx_name(trx), *power);
-		adjust = 2; /* adjust for parsed TV struct */
+	/* Parse Attribute Response Info content for 3GPP TS 52.021 §9.4.28 Manufacturer Dependent State */
+	/* this attribute does not make sense on BTS level, only on TRX level */
+	if (trx && TLVP_PRES_LEN(tp, NM_ATT_MANUF_STATE, 1)) {
+		data = TLVP_VAL(tp, NM_ATT_MANUF_STATE);
+		LOGPFOH(DNM, LOGL_INFO, foh, "%s Get Attributes Response: nominal power is %u\n", gsm_trx_name(trx), *data);
 	}
 
-	*data_len -= adjust;
-
-	return data + adjust;
-}
-
-/* Handle 3GPP TS 52.021 §8.11.3 Get Attribute Response (with nanoBTS speecific attribute formatting) */
-static int parse_attr_resp_nanobts(struct gsm_bts *bts, struct abis_om_fom_hdr *foh, struct tlv_parsed *tp)
-{
-	const uint8_t* data;
-	uint16_t len;
-	uint16_t port;
-	struct in_addr ia = {0};
-	char unit_id[40];
-
+	/* Parse Attribute Response Info content for 3GPP TS 52.021 §9.4.61 SW Configuration */
+	if (TLVP_PRESENT(tp, NM_ATT_SW_CONFIG)) {
+		data = TLVP_VAL(tp, NM_ATT_SW_CONFIG);
+		len = TLVP_LEN(tp, NM_ATT_SW_CONFIG);
+		/* after parsing manufacturer-specific attributes there's list of replies in form of sw-conf structure: */
+		rc = abis_nm_get_sw_conf(data, len, &sw_descr[0], ARRAY_SIZE(sw_descr));
+		if (rc > 0) {
+			for (i = 0; i < rc; i++) {
+				if (!handle_attr(bts, str2btsattr((const char *)sw_descr[i].file_id),
+						 sw_descr[i].file_version, sw_descr[i].file_version_len))
+					LOGPFOH(DNM, LOGL_NOTICE, foh, "BTS%u: ARI reported sw[%d/%d]: %s "
+						"is %s\n", bts->nr, i, rc, sw_descr[i].file_id,
+						sw_descr[i].file_version);
+			}
+		} else {
+			LOGPFOH(DNM, LOGL_ERROR, foh, "BTS%u: failed to parse SW-Config part of "
+				"Get Attribute Response Info: %s\n", bts->nr, strerror(-rc));
+		}
+	}
 
 	if (TLVP_PRES_LEN(tp, NM_ATT_IPACC_PRIM_OML_CFG_LIST, 7)) {
 		data = TLVP_VAL(tp, NM_ATT_IPACC_PRIM_OML_CFG_LIST);
@@ -555,6 +550,14 @@
 		osmo_strlcpy(unit_id, (char*)data, OSMO_MIN(len, sizeof(unit_id)));
 		LOGPFOH(DNM, LOGL_ERROR, foh, "BTS%u Get Attributes Response: Unit ID is %s\n", bts->nr, unit_id);
 	}
+
+	/* nanoBTS provides Get Attribute Response Info at random position and only the unreported part of it. */
+	if (TLVP_PRES_LEN(tp, NM_ATT_GET_ARI, 1)) {
+		data = TLVP_VAL(tp, NM_ATT_GET_ARI);
+		len = TLVP_LEN(tp, NM_ATT_GET_ARI);
+		parse_attr_resp_info_unreported(bts->nr, data, len, NULL);
+	}
+
 	return 0;
 }
 
@@ -562,10 +565,7 @@
 static int parse_attr_resp_info(struct gsm_bts *bts, const struct gsm_bts_trx *trx, struct abis_om_fom_hdr *foh, struct tlv_parsed *tp)
 {
 	const uint8_t *data;
-	int i;
 	uint16_t data_len;
-	int rc;
-	struct abis_nm_sw_desc sw_descr[MAX_BTS_ATTR];
 
 	if (!TLVP_PRES_LEN(tp, NM_ATT_GET_ARI, 1)) {
 		LOGPFOH(DNM, LOGL_ERROR, foh, "BTS%u: Get Attr Response without Response Info?!\n",
@@ -576,23 +576,10 @@
 	data = parse_attr_resp_info_unreported(bts->nr, TLVP_VAL(tp, NM_ATT_GET_ARI), TLVP_LEN(tp, NM_ATT_GET_ARI),
 					       &data_len);
 
-	data = parse_attr_resp_info_manuf_state(trx, data, &data_len);
-	data = parse_attr_resp_info_manuf_id(bts, data, &data_len);
+	/* After parsing unreported part of Response info, there's a TLV of attr, same as per nanoBTS */
+	abis_nm_tlv_parse(tp, bts, data, data_len);
+	parse_attr_resp_info_attr(bts, trx, foh, tp);
 
-	/* after parsing manufacturer-specific attributes there's list of replies in form of sw-conf structure: */
-	rc = abis_nm_get_sw_conf(data, data_len, &sw_descr[0], ARRAY_SIZE(sw_descr));
-	if (rc > 0) {
-		for (i = 0; i < rc; i++) {
-			if (!handle_attr(bts, str2btsattr((const char *)sw_descr[i].file_id),
-					 sw_descr[i].file_version, sw_descr[i].file_version_len))
-				LOGPFOH(DNM, LOGL_NOTICE, foh, "BTS%u: ARI reported sw[%d/%d]: %s "
-					"is %s\n", bts->nr, i, rc, sw_descr[i].file_id,
-					sw_descr[i].file_version);
-		}
-	} else {
-		LOGPFOH(DNM, LOGL_ERROR, foh, "BTS%u: failed to parse SW-Config part of "
-			"Get Attribute Response Info: %s\n", bts->nr, strerror(-rc));
-	}
 	return 0;
 }
 
@@ -613,7 +600,7 @@
 	if (bts->type != GSM_BTS_TYPE_NANOBTS)
 		parse_attr_resp_info(bts, trx, foh, &tp);
 	else
-		parse_attr_resp_nanobts(bts, foh, &tp);
+		parse_attr_resp_info_attr(bts, trx, foh, &tp);
 
 
 	return 0;

-- 
To view, visit https://gerrit.osmocom.org/11682
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I77574579a19f1988f82cdac81b257b8f6871b857
Gerrit-Change-Number: 11682
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181108/0cb65c3a/attachment.htm>


More information about the gerrit-log mailing list