[MERGED] osmo-bts[master]: OML: internalize failure reporting

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 Jan 25 19:44:20 UTC 2017


Harald Welte has submitted this change and it was merged.

Change subject: OML: internalize failure reporting
......................................................................


OML: internalize failure reporting

* make oml_tx_failure_event_rep() static and use osmo_signal_dispatch()
  wrapped into oml_fail_rep() to trigger event reports outside of oml.c
  instead of directly calling into OML layer
* remove unnecessary formatting from text messages

Related: OS#1615
Change-Id: I738555c547926e97b325ab53763c0076c42309bc
---
M include/osmo-bts/oml.h
M include/osmo-bts/signal.h
M src/common/abis.c
M src/common/bts.c
M src/common/l1sap.c
M src/common/main.c
M src/common/oml.c
M src/common/rsl.c
8 files changed, 83 insertions(+), 58 deletions(-)

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



diff --git a/include/osmo-bts/oml.h b/include/osmo-bts/oml.h
index 217ec64..139464e 100644
--- a/include/osmo-bts/oml.h
+++ b/include/osmo-bts/oml.h
@@ -9,7 +9,7 @@
 struct gsm_lchan;
 
 
-int oml_init(void);
+int oml_init(struct gsm_abis_mo *mo);
 int down_oml(struct gsm_bts *bts, struct msgb *msg);
 
 struct msgb *oml_msgb_alloc(void);
@@ -45,7 +45,6 @@
 extern const unsigned int oml_default_t200_ms[7];
 
 /* Transmit failure event report */
-int oml_tx_failure_event_rep(struct gsm_abis_mo *mo, uint16_t cause_value,
-			     const char *fmt, ...);
+void oml_fail_rep(uint16_t cause_value, const char *fmt, ...);
 
 #endif // _OML_H */
diff --git a/include/osmo-bts/signal.h b/include/osmo-bts/signal.h
index c8168a2..01d4099 100644
--- a/include/osmo-bts/signal.h
+++ b/include/osmo-bts/signal.h
@@ -5,6 +5,7 @@
 
 enum sig_subsys {
 	SS_GLOBAL,
+	SS_FAIL,
 };
 
 enum signals_global {
diff --git a/src/common/abis.c b/src/common/abis.c
index 6753149..768d261 100644
--- a/src/common/abis.c
+++ b/src/common/abis.c
@@ -235,7 +235,7 @@
 {
 	g_bts = bts;
 
-	oml_init();
+	oml_init(&bts->mo);
 	libosmo_abis_init(NULL);
 
 	osmo_signal_register_handler(SS_L_INPUT, &inp_s_cbfn, bts);
diff --git a/src/common/bts.c b/src/common/bts.c
index efefb86..9915e1c 100644
--- a/src/common/bts.c
+++ b/src/common/bts.c
@@ -268,9 +268,9 @@
 	else
 		rc = bts_model_trx_deact_rf(trx);
 	if (rc < 0)
-		oml_tx_failure_event_rep(&trx->mo, OSMO_EVT_MAJ_RSL_FAIL, link ?
-					 "Failed to establish RSL link (%d)\n" :
-					 "Failed to deactivate RF (%d)\n", rc);
+		oml_fail_rep(OSMO_EVT_MAJ_RSL_FAIL,
+			     link ? "Failed to establish RSL link (%d)" :
+			     "Failed to deactivate RF (%d)", rc);
 	return 0;
 }
 
diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index 9116e23..b2f18d1 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -1065,10 +1065,8 @@
 	default:
 		LOGP(DL1P, LOGL_NOTICE, "unknown prim %d op %d\n",
 			l1sap->oph.primitive, l1sap->oph.operation);
-		oml_tx_failure_event_rep(&trx->mo, OSMO_EVT_MAJ_UKWN_MSG,
-					 "unknown prim %d op %d\n",
-					 l1sap->oph.primitive,
-					 l1sap->oph.operation);
+		oml_fail_rep(OSMO_EVT_MAJ_UKWN_MSG, "unknown prim %d op %d",
+			     l1sap->oph.primitive, l1sap->oph.operation);
 		break;
 	}
 
diff --git a/src/common/main.c b/src/common/main.c
index 1af4f34..4c51848 100644
--- a/src/common/main.c
+++ b/src/common/main.c
@@ -184,12 +184,9 @@
 
 	switch (signal) {
 	case SIGINT:
-		//osmo_signal_dispatch(SS_GLOBAL, S_GLOBAL_SHUTDOWN, NULL);
 		if (!quit) {
-			oml_tx_failure_event_rep(&bts->mo,
-						 OSMO_EVT_CRIT_PROC_STOP,
-						 "BTS: SIGINT received -> "
-						 "shutdown\n");
+			oml_fail_rep(OSMO_EVT_CRIT_PROC_STOP,
+				     "BTS: SIGINT received -> shutdown");
 			bts_shutdown(bts, "SIGINT");
 		}
 		quit++;
@@ -197,9 +194,9 @@
 	case SIGABRT:
 	case SIGUSR1:
 	case SIGUSR2:
-		oml_tx_failure_event_rep(&bts->mo, OSMO_EVT_CRIT_PROC_STOP,
-					 "BTS: signal %s received\n",
-					 strsignal(signal));
+		oml_fail_rep(OSMO_EVT_CRIT_PROC_STOP,
+			     "BTS: signal %d (%s) received", signal,
+			     strsignal(signal));
 		talloc_report_full(tall_bts_ctx, stderr);
 		break;
 	default:
diff --git a/src/common/oml.c b/src/common/oml.c
index 1482410..272123e 100644
--- a/src/common/oml.c
+++ b/src/common/oml.c
@@ -25,6 +25,8 @@
  */
 
 #include <errno.h>
+#include <stdarg.h>
+#include <string.h>
 #include <sys/types.h>
 #include <arpa/inet.h>
 
@@ -56,6 +58,42 @@
 struct msgb *oml_msgb_alloc(void)
 {
 	return msgb_alloc_headroom(1024, 128, "OML");
+}
+
+/* 3GPP TS 12.21 § 8.8.2 */
+static int oml_tx_failure_event_rep(struct gsm_abis_mo *mo, uint16_t cause_value,
+				    const char *fmt, ...)
+{
+	struct msgb *nmsg;
+	va_list ap;
+
+	LOGP(DOML, LOGL_NOTICE, "Reporting FAILURE to BSC: ");
+	va_start(ap, fmt);
+	osmo_vlogp(DOML, LOGL_NOTICE, __FILE__, __LINE__, 1, fmt, ap);
+	nmsg = abis_nm_fail_evt_vrep(NM_EVT_PROC_FAIL, NM_SEVER_CRITICAL,
+				     NM_PCAUSE_T_MANUF, cause_value, fmt, ap);
+	va_end(ap);
+	LOGPC(DOML, LOGL_NOTICE, "\n");
+
+	if (!nmsg)
+		return -ENOMEM;
+
+	return oml_mo_send_msg(mo, nmsg, NM_MT_FAILURE_EVENT_REP);
+}
+
+void oml_fail_rep(uint16_t cause_value, const char *fmt, ...)
+{
+	va_list ap;
+	char *rep;
+
+	va_start(ap, fmt);
+	rep = talloc_asprintf(tall_bts_ctx, fmt, ap);
+	va_end(ap);
+
+	osmo_signal_dispatch(SS_FAIL, cause_value, rep);
+	/* signal dispatch is synchronous so all the signal handlers are
+	   finished already: we're free to free */
+	talloc_free(rep);
 }
 
 int oml_send_msg(struct msgb *msg, int is_manuf)
@@ -347,7 +385,7 @@
 	rc = oml_tlv_parse(&tp, foh->data, msgb_l3len(msg) - sizeof(*foh));
 	if (rc < 0) {
 		oml_tx_failure_event_rep(&bts->mo, OSMO_EVT_MAJ_UNSUP_ATTR,
-					 "New value for Attribute not supported\n");
+					 "New value for Attribute not supported");
 		return oml_fom_ack_nack(msg, NM_NACK_INCORR_STRUCT);
 	}
 
@@ -356,7 +394,7 @@
 		uint16_t arfcn = ntohs(tlvp_val16_unal(&tp, NM_ATT_BCCH_ARFCN));
 		if (arfcn > 1024) {
 			oml_tx_failure_event_rep(&bts->mo, OSMO_EVT_WARN_SW_WARN,
-						 "Given ARFCN %d is not supported.\n",
+						 "Given ARFCN %u is not supported",
 						 arfcn);
 			LOGP(DOML, LOGL_NOTICE, "Given ARFCN %d is not supported.\n", arfcn);
 			return oml_fom_ack_nack(msg, NM_NACK_FREQ_NOTAVAIL);
@@ -366,7 +404,7 @@
 	if (TLVP_PRESENT(&tp, NM_ATT_START_TIME)) {
 		oml_tx_failure_event_rep(&bts->mo, OSMO_EVT_MAJ_UNSUP_ATTR,
 					 "NM_ATT_START_TIME Attribute not "
-					 "supported\n");
+					 "supported");
 		return oml_fom_ack_nack(msg, NM_NACK_SPEC_IMPL_NOTSUPP);
 	}
 
@@ -506,7 +544,7 @@
 	if (rc < 0) {
 		oml_tx_failure_event_rep(&trx->mo, OSMO_EVT_MAJ_UNSUP_ATTR,
 					 "New value for Set Radio Attribute not"
-					 " supported\n");
+					 " supported");
 		return oml_fom_ack_nack(msg, NM_NACK_INCORR_STRUCT);
 	}
 
@@ -568,8 +606,15 @@
 		memcpy(&_value, value, 2);
 		arfcn = ntohs(_value);
 		value += 2;
-		if (arfcn > 1024)
+		if (arfcn > 1024) {
+			oml_tx_failure_event_rep(&trx->bts->mo,
+						 OSMO_EVT_WARN_SW_WARN,
+						 "Given ARFCN %u is unsupported",
+						 arfcn);
+			LOGP(DOML, LOGL_NOTICE,
+			     "Given ARFCN %u is unsupported.\n", arfcn);
 			return oml_fom_ack_nack(msg, NM_NACK_FREQ_NOTAVAIL);
+		}
 		trx->arfcn = arfcn;
 	}
 #endif
@@ -672,7 +717,7 @@
 	if (rc < 0) {
 		oml_tx_failure_event_rep(&ts->mo, OSMO_EVT_MAJ_UNSUP_ATTR,
 					 "New value for Set Channel Attribute "
-					 "not supported\n");
+					 "not supported");
 		return oml_fom_ack_nack(msg, NM_NACK_INCORR_STRUCT);
 	}
 
@@ -814,7 +859,7 @@
 			trx->mo.obj_inst.trx_nr = foh->obj_inst.trx_nr;
 			trx->mo.obj_inst.ts_nr = 0xff;
 			oml_tx_failure_event_rep(&trx->mo, OSMO_EVT_MAJ_UKWN_MSG,
-						 "Formatted O&M message too short\n");
+						 "Formatted O&M message too short");
 		}
 		return -EIO;
 	}
@@ -828,7 +873,7 @@
 			trx->mo.obj_inst.ts_nr = 0xff;
 			oml_tx_failure_event_rep(&trx->mo, OSMO_EVT_MAJ_UKWN_MSG,
 						 "Formatted O&M with BTS %d out"
-						 " of range (0:0xFF).\n",
+						 " of range (0:0xFF)",
 						 foh->obj_inst.bts_nr);
 		}
 		return oml_fom_ack_nack(msg, NM_NACK_BTSNR_UNKN);
@@ -871,12 +916,12 @@
 			trx->mo.obj_inst.ts_nr = 0xff;
 			oml_tx_failure_event_rep(&trx->mo, OSMO_EVT_MAJ_UKWN_MSG,
 						 "unknown Formatted O&M "
-						 "msg_type 0x%02x\n",
+						 "msg_type 0x%02x",
 						 foh->msg_type);
 		} else
 			oml_tx_failure_event_rep(&bts->mo, OSMO_EVT_MAJ_UKWN_MSG,
 						 "unknown Formatted O&M "
-						 "msg_type 0x%02x\n",
+						 "msg_type 0x%02x",
 						 foh->msg_type);
 		ret = oml_fom_ack_nack(msg, NM_NACK_MSGTYPE_INVAL);
 	}
@@ -1191,29 +1236,19 @@
 	return ret;
 }
 
-/* 3GPP TS 12.21 § 8.8.2 */
-int oml_tx_failure_event_rep(struct gsm_abis_mo *mo, uint16_t cause_value,
-			     const char *fmt, ...)
+static int handle_fail_sig(unsigned int subsys, unsigned int signal, void *handle,
+			   void *signal_data)
 {
-	struct msgb *nmsg;
-	va_list ap;
+	oml_tx_failure_event_rep(handle, signal, "%s", signal_data);
 
-	va_start(ap, fmt);
-	nmsg = abis_nm_fail_evt_rep(NM_EVT_PROC_FAIL, NM_SEVER_CRITICAL,
-				    NM_PCAUSE_T_MANUF, cause_value, fmt, ap);
-	LOGP(DOML, LOGL_INFO, fmt, ap);
-	va_end(ap);
-
-	if (!nmsg)
-		return -ENOMEM;
-
-	return oml_mo_send_msg(mo, nmsg, NM_MT_FAILURE_EVENT_REP);
+	return 0;
 }
 
-int oml_init(void)
+int oml_init(struct gsm_abis_mo *mo)
 {
 	DEBUGP(DOML, "Initializing OML attribute definitions\n");
 	tlv_def_patch(&abis_nm_att_tlvdef_ipa, &abis_nm_att_tlvdef);
+	osmo_signal_register_handler(SS_FAIL, handle_fail_sig, mo);
 
 	return 0;
 }
diff --git a/src/common/rsl.c b/src/common/rsl.c
index a34c455..b3e9afb 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -403,9 +403,8 @@
 	if (rc < 0) {
 		/* FIXME: notfiy the BSC on other errors? */
 		if (rc == -ENOSPC)
-			oml_tx_failure_event_rep(&trx->mo,
-						 OSMO_EVT_MIN_PAG_TAB_FULL,
-						 "BTS paging table is full\n");
+			oml_fail_rep(OSMO_EVT_MIN_PAG_TAB_FULL,
+				     "BTS paging table is full");
 	}
 
 	pcu_tx_pag_req(identity_lv, chan_needed);
@@ -1653,11 +1652,9 @@
 			LOGP(DRSL, LOGL_ERROR,
 			     "%s IPAC Failed to create RTP/RTCP sockets\n",
 			     gsm_lchan_name(lchan));
-			oml_tx_failure_event_rep(&lchan->ts->trx->mo,
-						 OSMO_EVT_CRIT_RTP_TOUT,
-						 "%s IPAC Failed to create "
-						 "RTP/RTCP sockets\n",
-						 gsm_lchan_name(lchan));
+			oml_fail_rep(OSMO_EVT_CRIT_RTP_TOUT,
+				     "%s IPAC Failed to create RTP/RTCP sockets",
+				     gsm_lchan_name(lchan));
 			return tx_ipac_XXcx_nack(lchan, RSL_ERR_RES_UNAVAIL,
 						 inc_ip_port, dch->c.msg_type);
 		}
@@ -1697,11 +1694,9 @@
 			LOGP(DRSL, LOGL_ERROR,
 			     "%s IPAC Failed to bind RTP/RTCP sockets\n",
 			     gsm_lchan_name(lchan));
-			oml_tx_failure_event_rep(&lchan->ts->trx->mo,
-						 OSMO_EVT_CRIT_RTP_TOUT,
-						 "%s IPAC Failed to bind "
-						 "RTP/RTCP sockets\n",
-						 gsm_lchan_name(lchan));
+			oml_fail_rep(OSMO_EVT_CRIT_RTP_TOUT,
+				     "%s IPAC Failed to bind RTP/RTCP sockets",
+				     gsm_lchan_name(lchan));
 			osmo_rtp_socket_free(lchan->abis_ip.rtp_socket);
 			lchan->abis_ip.rtp_socket = NULL;
 			msgb_queue_flush(&lchan->dl_tch_queue);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I738555c547926e97b325ab53763c0076c42309bc
Gerrit-PatchSet: 6
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>



More information about the gerrit-log mailing list