oml: Fix premature Opstart to Radio Carrier

During the A-bis/OML bootstrapping, osmo-bsc sends Opstart to the
Radio Carrier MO twice.  The first Opstart is triggered by the
State Changed Event Report, originated by the Radio Carrier itself.
The second is triggered by Software Activated Report.

According to 3GPP TS 12.21, figure 2, we shall send it only once,
after the "Attribute setting" step.  Therefore, the first Opstart
is premature, and we shall not send it.

Related: SYS#5063, OS#4755
Change-Id: If69393551117266ecb726d8961153560b2b3cc59

3 files changed, 30 insertions, 9 deletions

diff --git a/include/osmocom/bsc/signal.h b/include/osmocom/bsc/signal.h
index 952e03ce1..c7d7fe129 100644
--- a/include/osmocom/bsc/signal.h
+++ b/include/osmocom/bsc/signal.h
@@ -73,6 +73,7 @@ enum signal_nm {
 	S_NM_OM2K_CONF_RES,	/* OM2K Configuration Result */
 	S_NM_OPSTART_ACK,	/* Received OPSTART ACK, arg is struct msgb *oml_msg */
 	S_NM_GET_ATTR_REP,	/* Received Get Attributes Response, arg is struct msgb *oml_msg */
+	S_NM_SET_RADIO_ATTR_ACK, /* Received Set Radio Carrier Attributes Ack, arg is struct msgb *oml_msg */
 };
 
 /* SS_LCHAN signals */
diff --git a/src/osmo-bsc/abis_nm.c b/src/osmo-bsc/abis_nm.c
index 7e637df89..ec5238088 100644
--- a/src/osmo-bsc/abis_nm.c
+++ b/src/osmo-bsc/abis_nm.c
@@ -821,6 +821,14 @@ static int abis_nm_rx_opstart_ack(struct msgb *mb)
 	return 0;
 }
 
+static int abis_nm_rx_set_radio_attr_ack(struct msgb *mb)
+{
+	struct abis_om_fom_hdr *foh = msgb_l3(mb);
+	DEBUGPFOH(DNM, foh, "Set Radio Carrier Attributes ACK\n");
+	osmo_signal_dispatch(SS_NM, S_NM_SET_RADIO_ATTR_ACK, mb);
+	return 0;
+}
+
 bool all_trx_rsl_connected_unlocked(const struct gsm_bts *bts)
 {
 	const struct gsm_bts_trx *trx;
@@ -941,6 +949,7 @@ static int abis_nm_rcvmsg_fom(struct msgb *mb)
 		break;
 	case NM_MT_SET_RADIO_ATTR_ACK:
 		DEBUGPFOH(DNM, foh, "Set Radio Carrier Attributes ACK\n");
+		abis_nm_rx_set_radio_attr_ack(mb);
 		break;
 	case NM_MT_CONN_MDROP_LINK_ACK:
 		DEBUGPFOH(DNM, foh, "CONN MDROP LINK ACK\n");
diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c
index 49720e5c2..796d20812 100644
--- a/src/osmo-bsc/bts_ipaccess_nanobts.c
+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c
@@ -185,11 +185,7 @@ static int nm_statechg_event(int evt, struct nm_statechg_signal_data *nsd)
 		}
 		break;
 	case NM_OC_RADIO_CARRIER:
-		trx = obj;
-		if (new_state->operational == NM_OPSTATE_DISABLED &&
-		    new_state->availability == NM_AVSTATE_OK)
-			abis_nm_opstart(trx->bts, obj_class, trx->bts->bts_nr,
-					trx->nr, 0xff);
+		/* OPSTART done after Set Radio Carrier Attributes ACK is received */
 		break;
 	case NM_OC_GPRS_NSE:
 		bts = container_of(obj, struct gsm_bts, gprs.nse);
@@ -285,16 +281,13 @@ static int sw_activ_rep(struct msgb *mb)
 		 * This code is here to make sure that on start
 		 * a TRX remains locked.
 		 */
-		int rc_state = trx->mo.nm_state.administrative;
 		/* Patch ARFCN into radio attribute */
 		struct msgb *msgb = nanobts_attr_radio_get(trx->bts, trx);
 		abis_nm_set_radio_attr(trx, msgb->data, msgb->len);
 		msgb_free(msgb);
 		abis_nm_chg_adm_state(trx->bts, foh->obj_class,
 				      trx->bts->bts_nr, trx->nr, 0xff,
-				      rc_state);
-		abis_nm_opstart(trx->bts, foh->obj_class, trx->bts->bts_nr,
-				trx->nr, 0xff);
+				      trx->mo.nm_state.administrative);
 		break;
 		}
 	}
@@ -328,6 +321,21 @@ static void nm_rx_opstart_ack(struct msgb *oml_msg)
 	}
 }
 
+static void nm_rx_set_radio_attr_ack(struct msgb *oml_msg)
+{
+	struct abis_om_fom_hdr *foh = msgb_l3(oml_msg);
+	struct e1inp_sign_link *sign_link = oml_msg->dst;
+	struct gsm_bts *bts = sign_link->trx->bts;
+	struct gsm_bts_trx *trx = gsm_bts_trx_num(bts, foh->obj_inst.trx_nr);
+
+	if (foh->obj_class != NM_OC_RADIO_CARRIER) {
+		LOG_TRX(trx, DNM, LOGL_ERROR, "Set Radio Carrier Attr Ack received on non Radio Carrier object!\n");
+		return;
+	}
+	abis_nm_opstart(trx->bts, foh->obj_class, trx->bts->bts_nr,
+			trx->nr, 0xff);
+}
+
 /* Callback function to be called every time we receive a signal from NM */
 static int bts_ipa_nm_sig_cb(unsigned int subsys, unsigned int signal,
 		     void *handler_data, void *signal_data)
@@ -344,6 +352,9 @@ static int bts_ipa_nm_sig_cb(unsigned int subsys, unsigned int signal,
 	case S_NM_OPSTART_ACK:
 		nm_rx_opstart_ack(signal_data);
 		return 0;
+	case S_NM_SET_RADIO_ATTR_ACK:
+		nm_rx_set_radio_attr_ack(signal_data);
+		return 0;
 	default:
 		break;
 	}