oml: Delay configuring NSVC until BTS features are negotiated

This is needed in order to to proper feature support verification for
IPv6 when configuring the NSVC.
Before this patch, there could be a race condition where NSVC FSM
checked for BTS feature BTS_FEAT_IPV6_NSVC before it was negotiated
through BTS Get Attributes (Ack).

Fixes: OS#4870
Change-Id: I7c207eee0e331995ae04acec014fbd13d4d16280

5 files changed, 38 insertions, 7 deletions

diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h
index fd968fa1c..2b05418fe 100644
--- a/include/osmocom/bsc/bts.h
+++ b/include/osmocom/bsc/bts.h
@@ -585,6 +585,11 @@ char *get_model_oml_status(const struct gsm_bts *bts);
 /* reset the state of all MO in the BTS */
 void gsm_bts_mo_reset(struct gsm_bts *bts);
 
+static inline bool gsm_bts_features_negotiated(struct gsm_bts *bts)
+{
+	return bts->mo.get_attr_rep_received || bts->mo.nm_state.operational == NM_OPSTATE_ENABLED;
+}
+
 /* dependency handling */
 void bts_depend_mark(struct gsm_bts *bts, int dep);
 void bts_depend_clear(struct gsm_bts *bts, int dep);
diff --git a/include/osmocom/bsc/nm_common_fsm.h b/include/osmocom/bsc/nm_common_fsm.h
index bceefbe73..89ec863aa 100644
--- a/include/osmocom/bsc/nm_common_fsm.h
+++ b/include/osmocom/bsc/nm_common_fsm.h
@@ -36,6 +36,7 @@ enum nm_fsm_events {
 	NM_EV_OPSTART_NACK,
 	NM_EV_OML_DOWN,
 	NM_EV_FORCE_LOCK, /* Only supported by RadioCarrier so far */
+	NM_EV_FEATURE_NEGOTIATED, /* Sent by BTS to NSVC MO */
 };
 extern const struct value_string nm_fsm_event_names[];
 
diff --git a/src/osmo-bsc/nm_bts_fsm.c b/src/osmo-bsc/nm_bts_fsm.c
index 731b5785b..c951edd61 100644
--- a/src/osmo-bsc/nm_bts_fsm.c
+++ b/src/osmo-bsc/nm_bts_fsm.c
@@ -127,6 +127,21 @@ static void configure_loop(struct gsm_bts *bts, struct gsm_nm_state *state, bool
 	}
 }
 
+static void rx_get_attr_rep(struct gsm_bts *bts, bool allow_opstart)
+{
+	struct gsm_gprs_nsvc *nsvc;
+
+	bts->mo.get_attr_rep_received = true;
+	bts->mo.get_attr_sent = false;
+
+	/* Announce bts_features are available to related NSVC MOs */
+	nsvc = gsm_bts_sm_nsvc_num(bts->site_mgr, 0); /* we only support NSVC0 so far */
+	osmo_fsm_inst_dispatch(nsvc->mo.fi, NM_EV_FEATURE_NEGOTIATED, NULL);
+
+	/* Move FSM forward */
+	configure_loop(bts, &bts->mo.nm_state, allow_opstart);
+}
+
 static void st_op_disabled_dependency_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)
 {
 	struct gsm_bts *bts = (struct gsm_bts *)fi->priv;
@@ -149,9 +164,7 @@ static void st_op_disabled_dependency(struct osmo_fsm_inst *fi, uint32_t event,
 
 	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);
+		rx_get_attr_rep(bts, false);
 		return;
 	case NM_EV_SET_ATTR_ACK:
 		bts->mo.set_attr_ack_received = true;
@@ -203,9 +216,7 @@ static void st_op_disabled_offline(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	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);
+		rx_get_attr_rep(bts, true);
 		return;
 	case NM_EV_SET_ATTR_ACK:
 		bts->mo.set_attr_ack_received = true;
diff --git a/src/osmo-bsc/nm_common_fsm.c b/src/osmo-bsc/nm_common_fsm.c
index 2a529dba6..2f19ed411 100644
--- a/src/osmo-bsc/nm_common_fsm.c
+++ b/src/osmo-bsc/nm_common_fsm.c
@@ -31,5 +31,6 @@ const struct value_string nm_fsm_event_names[] = {
 	{ NM_EV_OPSTART_NACK, "OPSTART_NACK" },
 	{ NM_EV_OML_DOWN, "OML_DOWN" },
 	{ NM_EV_FORCE_LOCK, "FORCE_LOCK_CHG" },
+	{ NM_EV_FEATURE_NEGOTIATED, "FEATURE_NEGOTIATED" },
 	{ 0, NULL }
 };
diff --git a/src/osmo-bsc/nm_gprs_nsvc_fsm.c b/src/osmo-bsc/nm_gprs_nsvc_fsm.c
index ffc565944..2a57ada7f 100644
--- a/src/osmo-bsc/nm_gprs_nsvc_fsm.c
+++ b/src/osmo-bsc/nm_gprs_nsvc_fsm.c
@@ -62,6 +62,8 @@ static void st_op_disabled_notinstalled(struct osmo_fsm_inst *fi, uint32_t event
 	struct gsm_nm_state *new_state;
 
 	switch (event) {
+	case NM_EV_FEATURE_NEGOTIATED:
+		break;
 	case NM_EV_SW_ACT_REP:
 		break;
 	case NM_EV_STATE_CHG_REP:
@@ -94,7 +96,9 @@ static void configure_loop(struct gsm_gprs_nsvc *nsvc, struct gsm_nm_state *stat
 	if (nsvc->bts->gprs.mode == BTS_GPRS_NONE)
 		return;
 
-	if (!nsvc->mo.set_attr_sent && !nsvc->mo.set_attr_ack_received) {
+	/* 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) {
 		if (!osmo_bts_has_feature(&nsvc->bts->features, BTS_FEAT_IPV6_NSVC) &&
 		    nsvc->remote.u.sa.sa_family == AF_INET6) {
 			LOGPFSML(nsvc->mo.fi, LOGL_ERROR,
@@ -149,6 +153,9 @@ 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_FEATURE_NEGOTIATED:
+		configure_loop(nsvc, &nsvc->mo.nm_state, false);
+		return;
 	case NM_EV_SET_ATTR_ACK:
 		nsvc->mo.set_attr_ack_received = true;
 		nsvc->mo.set_attr_sent = false;
@@ -198,6 +205,9 @@ 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_FEATURE_NEGOTIATED:
+		configure_loop(nsvc, &nsvc->mo.nm_state, true);
+		return;
 	case NM_EV_SET_ATTR_ACK:
 		nsvc->mo.set_attr_ack_received = true;
 		nsvc->mo.set_attr_sent = false;
@@ -304,6 +314,7 @@ static struct osmo_fsm_state nm_gprs_nsvc_fsm_states[] = {
 	[NM_GPRS_NSVC_ST_OP_DISABLED_NOTINSTALLED] = {
 		.in_event_mask =
 			X(NM_EV_SW_ACT_REP) |
+			X(NM_EV_FEATURE_NEGOTIATED) |
 			X(NM_EV_STATE_CHG_REP),
 		.out_state_mask =
 			X(NM_GPRS_NSVC_ST_OP_DISABLED_DEPENDENCY) |
@@ -316,6 +327,7 @@ static struct osmo_fsm_state nm_gprs_nsvc_fsm_states[] = {
 	[NM_GPRS_NSVC_ST_OP_DISABLED_DEPENDENCY] = {
 		.in_event_mask =
 			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_FEATURE_NEGOTIATED) |
 			X(NM_EV_SET_ATTR_ACK),
 		.out_state_mask =
 			X(NM_GPRS_NSVC_ST_OP_DISABLED_NOTINSTALLED) |
@@ -328,6 +340,7 @@ static struct osmo_fsm_state nm_gprs_nsvc_fsm_states[] = {
 	[NM_GPRS_NSVC_ST_OP_DISABLED_OFFLINE] = {
 		.in_event_mask =
 			X(NM_EV_STATE_CHG_REP) |
+			X(NM_EV_FEATURE_NEGOTIATED) |
 			X(NM_EV_SET_ATTR_ACK),
 		.out_state_mask =
 			X(NM_GPRS_NSVC_ST_OP_DISABLED_NOTINSTALLED) |