abis_nm: move fail report parsing to extra func

Separate raw input parsing from handling the failure report. A follow-up
patch will use the new parsing function to print saved failure reports
to the VTY.

While at it, put struct tlv_parsed inside struct nm_fail_rep_signal_data
instead of a pointer, so we don't need an additional alloc. Also add
error handling to the abis_nm_tlv_parse() call.

Related: OS#1605
Change-Id: Ia51004faf620aa4d40435d58c70d758c9d0054d8

3 files changed, 64 insertions, 41 deletions

diff --git a/include/osmocom/bsc/abis_nm.h b/include/osmocom/bsc/abis_nm.h
index 45bbe2c02..83bc2f809 100644
--- a/include/osmocom/bsc/abis_nm.h
+++ b/include/osmocom/bsc/abis_nm.h
@@ -27,6 +27,7 @@
 #include <osmocom/gsm/protocol/gsm_12_21.h>
 
 #include <osmocom/bsc/gsm_data.h>
+#include <osmocom/bsc/signal.h>
 
 /* max number of attributes represented as 3GPP TS 52.021 §9.4.62 SW Description array */
 #define MAX_BTS_ATTR 5
@@ -170,6 +171,8 @@ void abis_nm_queue_send_next(struct gsm_bts *bts);	/* for bs11_config. */
 
 int abis_nm_select_newest_sw(const struct abis_nm_sw_desc *sw, const size_t len);
 
+struct nm_fail_rep_signal_data *abis_nm_fail_evt_rep_parse(struct msgb *mb, struct gsm_bts *bts);
+
 /* Helper functions for updating attributes */
 int abis_nm_update_max_power_red(struct gsm_bts_trx *trx);
 
diff --git a/include/osmocom/bsc/signal.h b/include/osmocom/bsc/signal.h
index 10200d75e..952e03ce1 100644
--- a/include/osmocom/bsc/signal.h
+++ b/include/osmocom/bsc/signal.h
@@ -153,7 +153,7 @@ struct nm_fail_rep_signal_data {
 	struct gsm_bts *bts;
 	/* raw data */
 	struct msgb *msg;
-	struct tlv_parsed *tp;
+	struct tlv_parsed tp;
 	/* parsed data */
 	struct {
 		const char *event_type;
diff --git a/src/osmo-bsc/abis_nm.c b/src/osmo-bsc/abis_nm.c
index 2d181e8bc..07effd38a 100644
--- a/src/osmo-bsc/abis_nm.c
+++ b/src/osmo-bsc/abis_nm.c
@@ -353,38 +353,77 @@ static inline void handle_manufact_report(struct gsm_bts *bts, const uint8_t *p_
 	};
 }
 
-static int rx_fail_evt_rep(struct msgb *mb, struct gsm_bts *bts)
+/* Parse into newly allocated struct abis_nm_fail_evt_rep, caller must free it. */
+struct nm_fail_rep_signal_data *abis_nm_fail_evt_rep_parse(struct msgb *mb, struct gsm_bts *bts)
 {
 	struct abis_om_hdr *oh = msgb_l2(mb);
 	struct abis_om_fom_hdr *foh = msgb_l3(mb);
-	struct e1inp_sign_link *sign_link = mb->dst;
-	struct nm_fail_rep_signal_data sd;
-	struct tlv_parsed tp;
-	int rc = 0;
+	struct nm_fail_rep_signal_data *sd;
 	const uint8_t *p_val = NULL;
 	char *p_text = NULL;
 	const char *e_type = NULL, *severity = NULL;
 
-	abis_nm_tlv_parse(&tp, sign_link->trx->bts, foh->data,
-			  oh->length-sizeof(*foh));
+	sd = talloc_zero(tall_bsc_ctx, struct nm_fail_rep_signal_data);
+	OSMO_ASSERT(sd);
 
-	if (TLVP_PRESENT(&tp, NM_ATT_ADD_TEXT)) {
-		const uint8_t *val = TLVP_VAL(&tp, NM_ATT_ADD_TEXT);
-		p_text = talloc_strndup(tall_bsc_ctx, (const char *) val,
-					TLVP_LEN(&tp, NM_ATT_ADD_TEXT));
+	if (abis_nm_tlv_parse(&sd->tp, bts, foh->data, oh->length-sizeof(*foh)) < 0)
+		goto fail;
+
+	if (TLVP_PRESENT(&sd->tp, NM_ATT_ADD_TEXT)) {
+		const uint8_t *val = TLVP_VAL(&sd->tp, NM_ATT_ADD_TEXT);
+		p_text = talloc_strndup(sd, (const char *) val, TLVP_LEN(&sd->tp, NM_ATT_ADD_TEXT));
 	}
 
-	if (TLVP_PRESENT(&tp, NM_ATT_EVENT_TYPE))
-		e_type = abis_nm_event_type_name(*TLVP_VAL(&tp,
-							   NM_ATT_EVENT_TYPE));
+	if (TLVP_PRESENT(&sd->tp, NM_ATT_EVENT_TYPE))
+		e_type = abis_nm_event_type_name(*TLVP_VAL(&sd->tp, NM_ATT_EVENT_TYPE));
+
+	if (TLVP_PRESENT(&sd->tp, NM_ATT_SEVERITY))
+		severity = abis_nm_severity_name(*TLVP_VAL(&sd->tp, NM_ATT_SEVERITY));
 
-	if (TLVP_PRESENT(&tp, NM_ATT_SEVERITY))
-		severity = abis_nm_severity_name(*TLVP_VAL(&tp,
-							   NM_ATT_SEVERITY));
+	if (TLVP_PRESENT(&sd->tp, NM_ATT_PROB_CAUSE))
+		p_val = TLVP_VAL(&sd->tp, NM_ATT_PROB_CAUSE);
+
+	sd->bts = bts;
+	sd->msg = mb;
+	if (e_type)
+		sd->parsed.event_type = e_type;
+	else
+		sd->parsed.event_type = talloc_strdup(sd, "<none>");
+	if (severity)
+		sd->parsed.severity = severity;
+	else
+		sd->parsed.severity = talloc_strdup(sd, "<none>");
+	if (p_text)
+		sd->parsed.additional_text = p_text;
+	else
+		sd->parsed.additional_text = talloc_strdup(sd, "<none>");
+	sd->parsed.probable_cause = p_val;
 
-	if (TLVP_PRESENT(&tp, NM_ATT_PROB_CAUSE)) {
-		p_val = TLVP_VAL(&tp, NM_ATT_PROB_CAUSE);
+	return sd;
+fail:
+	talloc_free(sd);
+	return NULL;
+}
 
+static int rx_fail_evt_rep(struct msgb *mb, struct gsm_bts *bts)
+{
+	struct abis_om_fom_hdr *foh = msgb_l3(mb);
+	struct nm_fail_rep_signal_data *sd;
+	int rc = 0;
+	const uint8_t *p_val;
+	const char *e_type, *severity, *p_text;
+
+	sd = abis_nm_fail_evt_rep_parse(mb, bts);
+	if (!sd) {
+		LOGPFOH(DNM, LOGL_ERROR, foh, "BTS%u: failed to parse Failure Event Report\n", bts->nr);
+		return -EINVAL;
+	}
+	e_type = sd->parsed.event_type;
+	severity = sd->parsed.severity;
+	p_text = sd->parsed.additional_text;
+	p_val = sd->parsed.probable_cause;
+
+	if (p_val) {
 		switch (p_val[0]) {
 		case NM_PCAUSE_T_MANUF:
 			handle_manufact_report(bts, p_val, e_type, severity,
@@ -399,27 +438,8 @@ static int rx_fail_evt_rep(struct msgb *mb, struct gsm_bts *bts)
 		rc = -EINVAL;
 	}
 
-	sd.bts = bts;
-	sd.msg = mb;
-	sd.tp = &tp;
-	if (e_type)
-		sd.parsed.event_type = e_type;
-	else
-		sd.parsed.event_type = "<none>";
-	if (severity)
-		sd.parsed.severity = severity;
-	else
-		sd.parsed.severity = "<none>";
-	if (p_text)
-		sd.parsed.additional_text = p_text;
-	else
-		sd.parsed.additional_text = "<none>";
-	sd.parsed.probable_cause = p_val;
-	osmo_signal_dispatch(SS_NM, S_NM_FAIL_REP, &sd);
-
-	if (p_text)
-		talloc_free(p_text);
-
+	osmo_signal_dispatch(SS_NM, S_NM_FAIL_REP, sd);
+	talloc_free(sd);
 	return rc;
 }