abis_nm: delay configure_loop() until NM_MT_SW_ACTIVATED_REP

Even though the Abis/OML message flow looks the way it should look
on the wire, it does not actually reflect the sequence/flow of events
and actions in the NM FSMs.  For example (extracted from a PCAP):

    GPRS Cell(00,00,ff) State Changed Event Report
    GPRS Cell(00,00,ff) Software Activate Request
    GPRS Cell(00,00,ff) Software Activate Request ACK
    GPRS Cell(00,00,ff) Activate Software
    GPRS Cell(00,00,ff) Activate Software ACK
[a] GPRS Cell(00,00,ff) State Changed Event Report
[b] GPRS Cell(00,00,ff) Software Activated Report
[c] GPRS Cell(00,00,ff) Get Attributes
    GPRS Cell(00,00,ff) Get Attributes Response
[d] GPRS Cell(00,00,ff) IPA Set Attributes
    GPRS Cell(00,00,ff) IPA Set Attributes ACK
    GPRS Cell(00,00,ff) Change Administrative State
    GPRS Cell(00,00,ff) Change Administrative State ACK
    GPRS Cell(00,00,ff) State Changed Event Report
    GPRS Cell(00,00,ff) Opstart
    GPRS Cell(00,00,ff) Opstart ACK

A follow-up patch [1] changes the logic generating message [d],
so that the IPA Object Version of the GPRS Cell MO is taken into
account when adding the attributes.

The problem is that both messages [c] and [d] are generated and
queued for transmission on the receipt of message [a], but *before*
message [b] has been processed.  So the IPA Object Version is not
known and assumed to be 0 at that point in time.

This patch delays configure_loop() until message [b] is received.
So far only for nanoBTS and only for those MOs, for which Figure 2
in 3GPP TS 52.021 explicitly mentions that the SW downloading and
activation procedures may be required, plus for the ip.access
specific MOs which all seem to support the SW activation.

osmo-bts does send SW Activated Report only for a subset of MOs,
which does not include Baseband Transceiver, Radio Carrier, and
Radio Channel.  3GPP TS 52.021 is not clear on whether this
message shall be sent by all MOs either, so we consider it
optional and delay configure_loop() only for nanoBTS.

Change-Id: I3953a5e41eb27165f9ff203cac7447ee9d311abf
Related: [1] Ie0fb3eaf76e1f70e5a19bb088e1674b7e553d32a

7 files changed, 82 insertions, 1 deletions

diff --git a/src/osmo-bsc/nm_bb_transc_fsm.c b/src/osmo-bsc/nm_bb_transc_fsm.c
index ce2f3e951..0243c0b0a 100644
--- a/src/osmo-bsc/nm_bb_transc_fsm.c
+++ b/src/osmo-bsc/nm_bb_transc_fsm.c
@@ -61,6 +61,7 @@ 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.sw_act_rep_received = false;
 	bb_transc->mo.get_attr_sent = false;
 	bb_transc->mo.get_attr_rep_received = false;
 	bb_transc->mo.adm_unlock_sent = false;
@@ -71,11 +72,14 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 
 static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_bts_bb_trx *bb_transc = (struct gsm_bts_bb_trx *)fi->priv;
 	struct nm_statechg_signal_data *nsd;
 	const struct gsm_nm_state *new_state;
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		bb_transc->mo.sw_act_rep_received = true;
+		break;
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -109,6 +113,11 @@ static void configure_loop(struct gsm_bts_bb_trx *bb_transc, const struct gsm_nm
 	if (bts_setup_ramp_wait(trx->bts))
 		return;
 
+	/* nanoBTS only: delay until SW Activated Report is received, which
+	 * tells us the IPA Object version (may be used to set attr conditionally). */
+	if (is_nanobts(trx->bts) && !bb_transc->mo.sw_act_rep_received)
+		return;
+
 	/* Request TRX-level attributes */
 	if (!bb_transc->mo.get_attr_sent && !bb_transc->mo.get_attr_rep_received) {
 		uint8_t attr_buf[3]; /* enlarge if needed */
@@ -176,6 +185,7 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		bb_transc->mo.sw_act_rep_received = true;
 		configure_loop(bb_transc, &bb_transc->mo.nm_state, false);
 		break;
 	case NM_EV_GET_ATTR_REP:
@@ -238,6 +248,7 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		bb_transc->mo.sw_act_rep_received = true;
 		configure_loop(bb_transc, &bb_transc->mo.nm_state, true);
 		break;
 	case NM_EV_GET_ATTR_REP:
diff --git a/src/osmo-bsc/nm_bts_fsm.c b/src/osmo-bsc/nm_bts_fsm.c
index 937b63db1..0bcc8e7b1 100644
--- a/src/osmo-bsc/nm_bts_fsm.c
+++ b/src/osmo-bsc/nm_bts_fsm.c
@@ -48,6 +48,7 @@ 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.sw_act_rep_received = false;
 	bts->mo.get_attr_sent = false;
 	bts->mo.get_attr_rep_received = false;
 	bts->mo.set_attr_sent = false;
@@ -58,11 +59,14 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 
 static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_bts *bts = (struct gsm_bts *)fi->priv;
 	struct nm_statechg_signal_data *nsd;
 	const struct gsm_nm_state *new_state;
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		bts->mo.sw_act_rep_received = true;
+		break;
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -96,6 +100,11 @@ static void configure_loop(struct gsm_bts *bts, const struct gsm_nm_state *state
 	if (bts_setup_ramp_wait(bts))
 		return;
 
+	/* nanoBTS only: delay until SW Activated Report is received, which
+	 * tells us the IPA Object version (may be used to set attr conditionally). */
+	if (is_nanobts(bts) && !bts->mo.sw_act_rep_received)
+		return;
+
 	/* Request generic BTS-level attributes */
 	if (!bts->mo.get_attr_sent && !bts->mo.get_attr_rep_received) {
 		uint8_t attr_buf[3]; /* enlarge if needed */
@@ -180,6 +189,7 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		bts->mo.sw_act_rep_received = true;
 		configure_loop(bts, &bts->mo.nm_state, false);
 		break;
 	case NM_EV_GET_ATTR_REP:
@@ -236,6 +246,7 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		bts->mo.sw_act_rep_received = true;
 		configure_loop(bts, &bts->mo.nm_state, true);
 		break;
 	case NM_EV_GET_ATTR_REP:
diff --git a/src/osmo-bsc/nm_bts_sm_fsm.c b/src/osmo-bsc/nm_bts_sm_fsm.c
index 70b065620..5e7b55875 100644
--- a/src/osmo-bsc/nm_bts_sm_fsm.c
+++ b/src/osmo-bsc/nm_bts_sm_fsm.c
@@ -49,6 +49,7 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 	struct gsm_bts *bts = gsm_bts_sm_get_bts(site_mgr);
 
 	site_mgr->peer_has_no_avstate_offline = (bts->type == GSM_BTS_TYPE_NANOBTS);
+	site_mgr->mo.sw_act_rep_received = false;
 	site_mgr->mo.opstart_sent = false;
 }
 
@@ -61,6 +62,8 @@ static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		site_mgr->mo.sw_act_rep_received = true;
+		break;
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -107,6 +110,11 @@ static void configure_loop(struct gsm_bts_sm *site_mgr, const struct gsm_nm_stat
 	if (bts_setup_ramp_wait(bts))
 		return;
 
+	/* nanoBTS only: delay until SW Activated Report is received, which
+	 * tells us the IPA Object version (may be used to set attr conditionally). */
+	if (is_nanobts(bts) && !site_mgr->mo.sw_act_rep_received)
+		return;
+
 	if (allow_opstart && !site_mgr->mo.opstart_sent) {
 		site_mgr->mo.opstart_sent = true;
 		abis_nm_opstart(bts, NM_OC_SITE_MANAGER, 0xff, 0xff, 0xff);
@@ -116,11 +124,15 @@ static void configure_loop(struct gsm_bts_sm *site_mgr, const struct gsm_nm_stat
 
 static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_bts_sm *site_mgr = (struct gsm_bts_sm *)fi->priv;
 	struct nm_statechg_signal_data *nsd;
 	const struct gsm_nm_state *new_state;
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		site_mgr->mo.sw_act_rep_received = true;
+		configure_loop(site_mgr, &site_mgr->mo.nm_state, false);
+		break;
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -162,6 +174,7 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		site_mgr->mo.sw_act_rep_received = true;
 		configure_loop(site_mgr, &site_mgr->mo.nm_state, true);
 		break;
 	case NM_EV_STATE_CHG_REP:
diff --git a/src/osmo-bsc/nm_gprs_cell_fsm.c b/src/osmo-bsc/nm_gprs_cell_fsm.c
index 40514a81c..308e7e0bf 100644
--- a/src/osmo-bsc/nm_gprs_cell_fsm.c
+++ b/src/osmo-bsc/nm_gprs_cell_fsm.c
@@ -48,6 +48,7 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 {
 	struct gsm_gprs_cell *cell = (struct gsm_gprs_cell *)fi->priv;
 
+	cell->mo.sw_act_rep_received = false;
 	cell->mo.set_attr_sent = false;
 	cell->mo.set_attr_ack_received = false;
 	cell->mo.adm_unlock_sent = false;
@@ -56,11 +57,14 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 
 static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_gprs_cell *cell = (struct gsm_gprs_cell *)fi->priv;
 	struct nm_statechg_signal_data *nsd;
 	const struct gsm_nm_state *new_state;
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		cell->mo.sw_act_rep_received = true;
+		break;
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -98,6 +102,11 @@ static void configure_loop(struct gsm_gprs_cell *cell, const struct gsm_nm_state
 	if (bts_setup_ramp_wait(bts))
 		return;
 
+	/* nanoBTS only: delay until SW Activated Report is received, which
+	 * tells us the IPA Object version (may be used to set attr conditionally). */
+	if (is_nanobts(bts) && !cell->mo.sw_act_rep_received)
+		return;
+
 	if (!cell->mo.set_attr_sent && !cell->mo.set_attr_ack_received) {
 		cell->mo.set_attr_sent = true;
 		msgb = nanobts_gen_set_cell_attr(bts);
@@ -146,6 +155,7 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		cell->mo.sw_act_rep_received = true;
 		configure_loop(cell, &cell->mo.nm_state, false);
 		break;
 	case NM_EV_SET_ATTR_ACK:
@@ -200,6 +210,7 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		cell->mo.sw_act_rep_received = true;
 		configure_loop(cell, &cell->mo.nm_state, true);
 		break;
 	case NM_EV_SET_ATTR_ACK:
diff --git a/src/osmo-bsc/nm_gprs_nse_fsm.c b/src/osmo-bsc/nm_gprs_nse_fsm.c
index df4113c98..319775c49 100644
--- a/src/osmo-bsc/nm_gprs_nse_fsm.c
+++ b/src/osmo-bsc/nm_gprs_nse_fsm.c
@@ -49,6 +49,7 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 {
 	struct gsm_gprs_nse *nse = (struct gsm_gprs_nse *)fi->priv;
 
+	nse->mo.sw_act_rep_received = false;
 	nse->mo.set_attr_sent = false;
 	nse->mo.set_attr_ack_received = false;
 	nse->mo.adm_unlock_sent = false;
@@ -57,11 +58,14 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 
 static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_gprs_nse *nse = (struct gsm_gprs_nse *)fi->priv;
 	struct nm_statechg_signal_data *nsd;
 	const struct gsm_nm_state *new_state;
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		nse->mo.sw_act_rep_received = true;
+		break;
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -97,6 +101,11 @@ static void configure_loop(struct gsm_gprs_nse *nse, const struct gsm_nm_state *
 	if (bts_setup_ramp_wait(bts))
 		return;
 
+	/* nanoBTS only: delay until SW Activated Report is received, which
+	 * tells us the IPA Object version (may be used to set attr conditionally). */
+	if (is_nanobts(bts) && !nse->mo.sw_act_rep_received)
+		return;
+
 	if (!nse->mo.set_attr_sent && !nse->mo.set_attr_ack_received) {
 		nse->mo.set_attr_sent = true;
 		msgb = nanobts_gen_set_nse_attr(bts_sm);
@@ -147,6 +156,7 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		nse->mo.sw_act_rep_received = true;
 		configure_loop(nse, &nse->mo.nm_state, false);
 		break;
 	case NM_EV_SET_ATTR_ACK:
@@ -201,6 +211,7 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		nse->mo.sw_act_rep_received = true;
 		configure_loop(nse, &nse->mo.nm_state, true);
 		break;
 	case NM_EV_SET_ATTR_ACK:
diff --git a/src/osmo-bsc/nm_gprs_nsvc_fsm.c b/src/osmo-bsc/nm_gprs_nsvc_fsm.c
index e6e3b3987..a3555aa93 100644
--- a/src/osmo-bsc/nm_gprs_nsvc_fsm.c
+++ b/src/osmo-bsc/nm_gprs_nsvc_fsm.c
@@ -49,6 +49,7 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 {
 	struct gsm_gprs_nsvc *nsvc = (struct gsm_gprs_nsvc *)fi->priv;
 
+	nsvc->mo.sw_act_rep_received = false;
 	nsvc->mo.set_attr_sent = false;
 	nsvc->mo.set_attr_sent = false;
 	nsvc->mo.set_attr_ack_received = false;
@@ -58,12 +59,15 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 
 static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_gprs_nsvc *nsvc = (struct gsm_gprs_nsvc *)fi->priv;
 	struct nm_statechg_signal_data *nsd;
 	const struct gsm_nm_state *new_state;
 
 	switch (event) {
-	case NM_EV_FEATURE_NEGOTIATED:
 	case NM_EV_SW_ACT_REP:
+		nsvc->mo.sw_act_rep_received = true;
+		break;
+	case NM_EV_FEATURE_NEGOTIATED:
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -116,6 +120,11 @@ static void configure_loop(struct gsm_gprs_nsvc *nsvc, const struct gsm_nm_state
 	if (bts_setup_ramp_wait(nsvc->bts))
 		return;
 
+	/* nanoBTS only: delay until SW Activated Report is received, which
+	 * tells us the IPA Object version (may be used to set attr conditionally). */
+	if (is_nanobts(nsvc->bts) && !nsvc->mo.sw_act_rep_received)
+		return;
+
 	/* We need to know BTS features in order to know if we can set IPv6 addresses */
 	if (gsm_bts_features_negotiated(nsvc->bts) && !nsvc->mo.set_attr_sent &&
 	    !nsvc->mo.set_attr_ack_received) {
@@ -177,6 +186,8 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		nsvc->mo.sw_act_rep_received = true;
+		/* fall-through */
 	case NM_EV_FEATURE_NEGOTIATED:
 		configure_loop(nsvc, &nsvc->mo.nm_state, false);
 		return;
@@ -231,6 +242,8 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		nsvc->mo.sw_act_rep_received = true;
+		/* fall-through */
 	case NM_EV_FEATURE_NEGOTIATED:
 		configure_loop(nsvc, &nsvc->mo.nm_state, true);
 		return;
diff --git a/src/osmo-bsc/nm_rcarrier_fsm.c b/src/osmo-bsc/nm_rcarrier_fsm.c
index 15e7c9f35..9f8f5f396 100644
--- a/src/osmo-bsc/nm_rcarrier_fsm.c
+++ b/src/osmo-bsc/nm_rcarrier_fsm.c
@@ -58,6 +58,7 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 {
 	struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;
 
+	trx->mo.sw_act_rep_received = false;
 	trx->mo.set_attr_sent = false;
 	trx->mo.set_attr_ack_received = false;
 	trx->mo.adm_unlock_sent = false;
@@ -66,11 +67,14 @@ static void st_op_disabled_notinstalled_on_enter(struct osmo_fsm_inst *fi, uint3
 
 static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
+	struct gsm_bts_trx *trx = (struct gsm_bts_trx *)fi->priv;
 	struct nm_statechg_signal_data *nsd;
 	const struct gsm_nm_state *new_state;
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		trx->mo.sw_act_rep_received = true;
+		break;
 	case NM_EV_SETUP_RAMP_READY:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -104,6 +108,11 @@ static void configure_loop(struct gsm_bts_trx *trx, const struct gsm_nm_state *s
 	if (bts_setup_ramp_wait(trx->bts))
 		return;
 
+	/* nanoBTS only: delay until SW Activated Report is received, which
+	 * tells us the IPA Object version (may be used to set attr conditionally). */
+	if (is_nanobts(trx->bts) && !trx->mo.sw_act_rep_received)
+		return;
+
 	if (!trx->mo.set_attr_sent && !trx->mo.set_attr_ack_received) {
 		trx->mo.set_attr_sent = true;
 		msgb = nanobts_gen_set_radio_attr(trx->bts, trx);
@@ -152,6 +161,7 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		trx->mo.sw_act_rep_received = true;
 		configure_loop(trx, &trx->mo.nm_state, false);
 		break;
 	case NM_EV_SET_ATTR_ACK:
@@ -205,6 +215,7 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	switch (event) {
 	case NM_EV_SW_ACT_REP:
+		trx->mo.sw_act_rep_received = true;
 		configure_loop(trx, &trx->mo.nm_state, true);
 		break;
 	case NM_EV_SET_ATTR_ACK: