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

Max gerrit-no-reply at lists.osmocom.org
Wed Jan 11 17:23:08 UTC 2017


Hello Jenkins Builder,

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

    https://gerrit.osmocom.org/1570

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

OML: internalize failure reporting

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

Change-Id: I738555c547926e97b325ab53763c0076c42309bc
Related: OS#1615
---
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, 84 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/70/1570/2

diff --git a/include/osmo-bts/oml.h b/include/osmo-bts/oml.h
index 217ec64..29e0791 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);
@@ -43,9 +43,5 @@
 /* Configure LAPDm T200 timers for this lchan according to OML */
 int oml_set_lchan_t200(struct gsm_lchan *lchan);
 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, ...);
 
 #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..147cc11 100644
--- a/src/common/bts.c
+++ b/src/common/bts.c
@@ -257,20 +257,24 @@
 	struct e1inp_sign_link *link = trx->rsl_link;
 	uint8_t radio_state = link ?  NM_OPSTATE_ENABLED : NM_OPSTATE_DISABLED;
 	int rc;
+	char *rep;
 
 	LOGP(DSUM, LOGL_INFO, "RSL link (TRX %02x) state changed to %s, sending Status'.\n",
 		trx->nr, link ? "up" : "down");
 
 	oml_mo_state_chg(&trx->mo, radio_state, NM_AVSTATE_OK);
 
-	if (link)
+	if (link) {
 		rc = rsl_tx_rf_res(trx);
-	else
+		rep = talloc_asprintf(tall_bts_ctx,
+				      "Failed to establish RSL link (%d)", rc);
+	} else {
 		rc = bts_model_trx_deact_rf(trx);
+		rep = talloc_asprintf(tall_bts_ctx,
+				      "Failed to deactivate RF (%d)", rc);
+	}
 	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);
+		osmo_signal_dispatch(SS_FAIL, OSMO_EVT_MAJ_RSL_FAIL, rep);
 	return 0;
 }
 
diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index 968237f..3dcdf74 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -49,6 +49,7 @@
 #include <osmo-bts/handover.h>
 #include <osmo-bts/power_control.h>
 #include <osmo-bts/msg_utils.h>
+#include <osmo-bts/signal.h>
 
 struct gsm_lchan *get_lchan_by_chan_nr(struct gsm_bts_trx *trx,
 				       unsigned int chan_nr)
@@ -1025,10 +1026,11 @@
 	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);
+		osmo_signal_dispatch(SS_FAIL, OSMO_EVT_MAJ_UKWN_MSG,
+				     talloc_asprintf(tall_bts_ctx,
+						     "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 00c8b23..c62d152 100644
--- a/src/common/main.c
+++ b/src/common/main.c
@@ -54,6 +54,7 @@
 #include <osmocom/ctrl/ports.h>
 #include <osmocom/ctrl/control_vty.h>
 #include <openbsc/ctrl.h>
+#include <osmo-bts/signal.h>
 #include <osmo-bts/oml.h>
 
 int quit = 0;
@@ -181,16 +182,15 @@
 
 static void signal_handler(int signal)
 {
-	fprintf(stderr, "signal %u received\n", signal);
+	char *rep = talloc_asprintf(tall_bts_ctx, "BTS: signal %d (%s) received",
+				    signal, strsignal(signal));
+	fprintf(stderr, "%s\n", rep);
 
 	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");
+			osmo_signal_dispatch(SS_FAIL, OSMO_EVT_CRIT_PROC_STOP,
+					     rep);
 			bts_shutdown(bts, "SIGINT");
 		}
 		quit++;
@@ -198,9 +198,7 @@
 	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));
+		osmo_signal_dispatch(SS_FAIL, OSMO_EVT_CRIT_PROC_STOP, rep);
 		talloc_report_full(tall_bts_ctx, stderr);
 		break;
 	default:
diff --git a/src/common/oml.c b/src/common/oml.c
index 1482410..d3a47a6 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,27 @@
 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);
 }
 
 int oml_send_msg(struct msgb *msg, int is_manuf)
@@ -347,7 +370,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 +379,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 +389,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 +529,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 +591,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 +702,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 +844,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 +858,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 +901,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 +1221,21 @@
 	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);
+	if (signal_data)
+		talloc_free(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..6e516dc 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -403,9 +403,9 @@
 	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");
+			osmo_signal_dispatch(SS_FAIL, OSMO_EVT_MIN_PAG_TAB_FULL,
+					     talloc_asprintf(tall_bts_ctx,
+							     "BTS paging table is full"));
 	}
 
 	pcu_tx_pag_req(identity_lv, chan_needed);
@@ -1653,11 +1653,10 @@
 			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));
+			osmo_signal_dispatch(SS_FAIL, OSMO_EVT_CRIT_RTP_TOUT,
+					     talloc_asprintf(tall_bts_ctx,
+							     "%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 +1696,10 @@
 			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));
+			osmo_signal_dispatch(SS_FAIL, OSMO_EVT_CRIT_RTP_TOUT,
+					     talloc_asprintf(tall_bts_ctx,
+							     "%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: newpatchset
Gerrit-Change-Id: I738555c547926e97b325ab53763c0076c42309bc
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder


More information about the gerrit-log mailing list