Handle BTS/BBTRANSC Get Attributes (Ack) in NM FSMs

Before this patch, Get Attributes was sent quicklyafter the OML link
became up, even if the BTS/BB_TRANSC objects were still powered off,
which is wrong since attributes should only be available after the
objects transition out of the Power off state.

Furthermore, information about get attr response already received will
be required in future patches to delay NSVC setting.

Related: OS#4870
Change-Id: I8ec39c7e1f956ffce9aecd58a5590c43200ba086

7 files changed, 97 insertions, 24 deletions

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index d8f8be25c..6904266ec 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -399,6 +399,8 @@ struct gsm_abis_mo {
 	struct osmo_fsm_inst *fi;
 	bool opstart_sent;
 	bool adm_unlock_sent;
+	bool get_attr_sent;
+	bool get_attr_rep_received;
 	bool set_attr_sent;
 	bool set_attr_ack_received;
 	bool force_rf_lock;
diff --git a/include/osmocom/bsc/nm_common_fsm.h b/include/osmocom/bsc/nm_common_fsm.h
index 7ad3df631..bceefbe73 100644
--- a/include/osmocom/bsc/nm_common_fsm.h
+++ b/include/osmocom/bsc/nm_common_fsm.h
@@ -30,6 +30,7 @@
 enum nm_fsm_events {
 	NM_EV_SW_ACT_REP,
 	NM_EV_STATE_CHG_REP,
+	NM_EV_GET_ATTR_REP,
 	NM_EV_SET_ATTR_ACK,
 	NM_EV_OPSTART_ACK,
 	NM_EV_OPSTART_NACK,
diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c
index 6d8f91fbf..c84d75084 100644
--- a/src/osmo-bsc/bts_ipaccess_nanobts.c
+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c
@@ -339,6 +339,27 @@ static void nm_rx_opstart_nack(struct msgb *oml_msg)
 	}
 }
 
+static void nm_rx_get_attr_rep(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;
+
+	switch (foh->obj_class) {
+	case NM_OC_BTS:
+		osmo_fsm_inst_dispatch(bts->mo.fi, NM_EV_GET_ATTR_REP, NULL);
+		break;
+	case NM_OC_BASEB_TRANSC:
+		if (!(trx = gsm_bts_trx_num(bts, foh->obj_inst.trx_nr)))
+			return;
+		osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_GET_ATTR_REP, NULL);
+		break;
+	default:
+		LOGPFOH(DNM, LOGL_ERROR, foh, "Get Attributes Response received on incorrect object class %d!\n", foh->obj_class);
+	}
+}
+
 static void nm_rx_set_bts_attr_ack(struct msgb *oml_msg)
 {
 	struct abis_om_fom_hdr *foh = msgb_l3(oml_msg);
@@ -432,6 +453,9 @@ static int bts_ipa_nm_sig_cb(unsigned int subsys, unsigned int signal,
 	case S_NM_OPSTART_NACK:
 		nm_rx_opstart_nack(signal_data);
 		return 0;
+	case S_NM_GET_ATTR_REP:
+		nm_rx_get_attr_rep(signal_data);
+		return 0;
 	case S_NM_SET_BTS_ATTR_ACK:
 		nm_rx_set_bts_attr_ack(signal_data);
 		return 0;
diff --git a/src/osmo-bsc/nm_bb_transc_fsm.c b/src/osmo-bsc/nm_bb_transc_fsm.c
index e7132e85e..31e5f3410 100644
--- a/src/osmo-bsc/nm_bb_transc_fsm.c
+++ b/src/osmo-bsc/nm_bb_transc_fsm.c
@@ -48,6 +48,8 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 {
 	struct gsm_bts_bb_trx *bb_transc = (struct gsm_bts_bb_trx *)fi->priv;
 
+	bb_transc->mo.get_attr_sent = false;
+	bb_transc->mo.get_attr_rep_received = false;
 	bb_transc->mo.adm_unlock_sent = false;
 	bb_transc->mo.opstart_sent = false;
 }
@@ -87,7 +89,20 @@ static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event
 static void configure_loop(struct gsm_bts_bb_trx *bb_transc, struct gsm_nm_state *state, bool allow_opstart)
 {
 	struct gsm_bts_trx *trx = gsm_bts_bb_trx_get_trx(bb_transc);
-	if (state->administrative != NM_STATE_UNLOCKED && !bb_transc->mo.adm_unlock_sent) {
+
+	/* Request TRX-level attributes */
+	if (!bb_transc->mo.get_attr_sent && !bb_transc->mo.get_attr_rep_received) {
+		bb_transc->mo.get_attr_sent = true;
+		/* N. B: we rely on attribute order when parsing response in abis_nm_rx_get_attr_resp() */
+		const uint8_t trx_attr[] = { NM_ATT_MANUF_STATE, NM_ATT_SW_CONFIG, };
+		/* we should not request more attributes than we're ready to handle */
+		OSMO_ASSERT(sizeof(trx_attr) < MAX_BTS_ATTR);
+		abis_nm_get_attr(trx->bts, NM_OC_BASEB_TRANSC, 0, trx->nr, 0xff,
+				 trx_attr, sizeof(trx_attr));
+	}
+
+	if (bb_transc->mo.get_attr_rep_received &&
+	    state->administrative != NM_STATE_UNLOCKED && !bb_transc->mo.adm_unlock_sent) {
 		bb_transc->mo.adm_unlock_sent = true;
 		/* Note: nanoBTS sometimes fails NACKing the BaseBand
 		   Transceiver Unlock command while in Dependency, specially
@@ -105,7 +120,7 @@ static void configure_loop(struct gsm_bts_bb_trx *bb_transc, struct gsm_nm_state
 		/* TRX software is active, tell it to initiate RSL Link */
 		abis_nm_ipaccess_rsl_connect(trx, trx->bts->ip_access.rsl_ip,
 					     3003, trx->rsl_tei);
-	    }
+	}
 }
 
 static void st_op_disabled_dependency_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
@@ -127,6 +142,11 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bb_transc->mo.get_attr_rep_received = true;
+		bb_transc->mo.get_attr_sent = false;
+		configure_loop(bb_transc, &bb_transc->mo.nm_state, false);
+		return;
 	case NM_EV_STATE_CHG_REP:
 		nsd = (struct nm_statechg_signal_data *)data;
 		new_state = nsd->new_state;
@@ -172,6 +192,11 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bb_transc->mo.get_attr_rep_received = true;
+		bb_transc->mo.get_attr_sent = false;
+		configure_loop(bb_transc, &bb_transc->mo.nm_state, true);
+		return;
 	case NM_EV_STATE_CHG_REP:
 		nsd = (struct nm_statechg_signal_data *)data;
 		new_state = nsd->new_state;
@@ -214,6 +239,8 @@ static void st_op_enabled_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state
 
 	/* Reset state, we don't need it in this state and it will need to be
 	  reused as soon as we move back to Disabled */
+	bb_transc->mo.get_attr_sent = false;
+	bb_transc->mo.get_attr_rep_received = false;
 	bb_transc->mo.opstart_sent = false;
 	bb_transc->mo.adm_unlock_sent = false;
 }
@@ -282,7 +309,8 @@ static struct osmo_fsm_state nm_bb_transc_fsm_states[] = {
 	},
 	[NM_BB_TRANSC_ST_OP_DISABLED_DEPENDENCY] = {
 		.in_event_mask =
-			X(NM_EV_STATE_CHG_REP),
+			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP),
 		.out_state_mask =
 			X(NM_BB_TRANSC_ST_OP_DISABLED_NOTINSTALLED) |
 			X(NM_BB_TRANSC_ST_OP_DISABLED_OFFLINE) |
@@ -293,7 +321,8 @@ static struct osmo_fsm_state nm_bb_transc_fsm_states[] = {
 	},
 	[NM_BB_TRANSC_ST_OP_DISABLED_OFFLINE] = {
 		.in_event_mask =
-			X(NM_EV_STATE_CHG_REP),
+			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP),
 		.out_state_mask =
 			X(NM_BB_TRANSC_ST_OP_DISABLED_NOTINSTALLED) |
 			X(NM_BB_TRANSC_ST_OP_DISABLED_DEPENDENCY) |
diff --git a/src/osmo-bsc/nm_bts_fsm.c b/src/osmo-bsc/nm_bts_fsm.c
index 6c577bd7c..731b5785b 100644
--- a/src/osmo-bsc/nm_bts_fsm.c
+++ b/src/osmo-bsc/nm_bts_fsm.c
@@ -48,6 +48,8 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 {
 	struct gsm_bts *bts = (struct gsm_bts *)fi->priv;
 
+	bts->mo.get_attr_sent = false;
+	bts->mo.get_attr_rep_received = false;
 	bts->mo.set_attr_sent = false;
 	bts->mo.set_attr_ack_received = false;
 	bts->mo.adm_unlock_sent = false;
@@ -89,14 +91,27 @@ static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event
 static void configure_loop(struct gsm_bts *bts, struct gsm_nm_state *state, bool allow_opstart) {
 	struct msgb *msgb;
 
-	if (!bts->mo.set_attr_sent && !bts->mo.set_attr_ack_received) {
+	/* Request generic BTS-level attributes */
+	if (!bts->mo.get_attr_sent && !bts->mo.get_attr_rep_received) {
+		bts->mo.get_attr_sent = true;
+		/* N. B: we rely on attribute order when parsing response in abis_nm_rx_get_attr_resp() */
+		const uint8_t bts_attr[] = { NM_ATT_MANUF_ID, NM_ATT_SW_CONFIG, };
+		/* we should not request more attributes than we're ready to handle */
+		OSMO_ASSERT(sizeof(bts_attr) < MAX_BTS_ATTR);
+		abis_nm_get_attr(bts, NM_OC_BTS, 0, 0xff, 0xff,
+				 bts_attr, sizeof(bts_attr));
+	}
+
+	if (bts->mo.get_attr_rep_received &&
+	    !bts->mo.set_attr_sent && !bts->mo.set_attr_ack_received) {
 		bts->mo.set_attr_sent = true;
 		msgb = nanobts_attr_bts_get(bts);
 		abis_nm_set_bts_attr(bts, msgb->data, msgb->len);
 		msgb_free(msgb);
 	}
 
-	if (state->administrative != NM_STATE_UNLOCKED && !bts->mo.adm_unlock_sent) {
+	if (bts->mo.set_attr_ack_received &&
+	    state->administrative != NM_STATE_UNLOCKED && !bts->mo.adm_unlock_sent) {
 		bts->mo.adm_unlock_sent = true;
 		abis_nm_chg_adm_state(bts, NM_OC_BTS,
 				      bts->bts_nr, 0xff, 0xff,
@@ -105,11 +120,11 @@ static void configure_loop(struct gsm_bts *bts, struct gsm_nm_state *state, bool
 
 	if (allow_opstart && state->administrative == NM_STATE_UNLOCKED &&
 	    bts->mo.set_attr_ack_received) {
-		    if (!bts->mo.opstart_sent) {
-			    bts->mo.opstart_sent = true;
-			    abis_nm_opstart(bts, NM_OC_BTS, bts->bts_nr, 0xff, 0xff);
-		    }
-	    }
+		if (!bts->mo.opstart_sent) {
+		    bts->mo.opstart_sent = true;
+		    abis_nm_opstart(bts, NM_OC_BTS, bts->bts_nr, 0xff, 0xff);
+		}
+	}
 }
 
 static void st_op_disabled_dependency_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
@@ -133,6 +148,11 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bts->mo.get_attr_rep_received = true;
+		bts->mo.get_attr_sent = false;
+		configure_loop(bts, &bts->mo.nm_state, false);
+		return;
 	case NM_EV_SET_ATTR_ACK:
 		bts->mo.set_attr_ack_received = true;
 		bts->mo.set_attr_sent = false;
@@ -182,6 +202,11 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_GET_ATTR_REP:
+		bts->mo.get_attr_rep_received = true;
+		bts->mo.get_attr_sent = false;
+		configure_loop(bts, &bts->mo.nm_state, true);
+		return;
 	case NM_EV_SET_ATTR_ACK:
 		bts->mo.set_attr_ack_received = true;
 		bts->mo.set_attr_sent = false;
@@ -231,6 +256,8 @@ static void st_op_enabled_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state
 	  reused as soon as we move back to Disabled */
 	bts->mo.opstart_sent = false;
 	bts->mo.adm_unlock_sent = false;
+	bts->mo.get_attr_sent = false;
+	bts->mo.get_attr_rep_received = false;
 	bts->mo.set_attr_sent = false;
 	bts->mo.set_attr_ack_received = false;
 }
@@ -300,6 +327,7 @@ static struct osmo_fsm_state nm_bts_fsm_states[] = {
 	[NM_BTS_ST_OP_DISABLED_DEPENDENCY] = {
 		.in_event_mask =
 			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP) |
 			X(NM_EV_SET_ATTR_ACK),
 		.out_state_mask =
 			X(NM_BTS_ST_OP_DISABLED_NOTINSTALLED) |
@@ -312,6 +340,7 @@ static struct osmo_fsm_state nm_bts_fsm_states[] = {
 	[NM_BTS_ST_OP_DISABLED_OFFLINE] = {
 		.in_event_mask =
 			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_GET_ATTR_REP) |
 			X(NM_EV_SET_ATTR_ACK),
 		.out_state_mask =
 			X(NM_BTS_ST_OP_DISABLED_NOTINSTALLED) |
diff --git a/src/osmo-bsc/nm_common_fsm.c b/src/osmo-bsc/nm_common_fsm.c
index 42c01e0b1..2a529dba6 100644
--- a/src/osmo-bsc/nm_common_fsm.c
+++ b/src/osmo-bsc/nm_common_fsm.c
@@ -25,6 +25,7 @@
 const struct value_string nm_fsm_event_names[] = {
 	{ NM_EV_SW_ACT_REP, "SW_ACT_REP" },
 	{ NM_EV_STATE_CHG_REP, "STATE_CHG_REP" },
+	{ NM_EV_GET_ATTR_REP, "GET_ATTR_REP" },
 	{ NM_EV_SET_ATTR_ACK, "SET_ATTR_ACK" },
 	{ NM_EV_OPSTART_ACK, "OPSTART_ACK" },
 	{ NM_EV_OPSTART_NACK, "OPSTART_NACK" },
diff --git a/src/osmo-bsc/osmo_bsc_main.c b/src/osmo-bsc/osmo_bsc_main.c
index 3be859391..ca00ba0b5 100644
--- a/src/osmo-bsc/osmo_bsc_main.c
+++ b/src/osmo-bsc/osmo_bsc_main.c
@@ -371,13 +371,6 @@ static int inp_sig_cb(unsigned int subsys, unsigned int signal,
 {
 	struct input_signal_data *isd = signal_data;
 	struct gsm_bts_trx *trx = isd->trx;
-	/* N. B: we rely on attribute order when parsing response in abis_nm_rx_get_attr_resp() */
-	const uint8_t bts_attr[] = { NM_ATT_MANUF_ID, NM_ATT_SW_CONFIG, };
-	const uint8_t trx_attr[] = { NM_ATT_MANUF_STATE, NM_ATT_SW_CONFIG, };
-
-	/* we should not request more attributes than we're ready to handle */
-	OSMO_ASSERT(sizeof(bts_attr) < MAX_BTS_ATTR);
-	OSMO_ASSERT(sizeof(trx_attr) < MAX_BTS_ATTR);
 
 	if (subsys != SS_L_INPUT)
 		return -EINVAL;
@@ -395,14 +388,8 @@ static int inp_sig_cb(unsigned int subsys, unsigned int signal,
 			  set bts->si_common.cell_alloc */
 			generate_cell_chan_list(ca, trx->bts);
 
-			/* Request generic BTS-level attributes */
-			abis_nm_get_attr(trx->bts, NM_OC_BTS, 0xFF, 0xFF, 0xFF, bts_attr, sizeof(bts_attr));
-
 			llist_for_each_entry(cur_trx, &trx->bts->trx_list, list) {
 				int i;
-				/* Request TRX-level attributes */
-				abis_nm_get_attr(cur_trx->bts, NM_OC_BASEB_TRANSC, 0, cur_trx->nr, 0xFF,
-						 trx_attr, sizeof(trx_attr));
 				for (i = 0; i < ARRAY_SIZE(cur_trx->ts); i++)
 					generate_ma_for_ts(&cur_trx->ts[i]);
 			}