Change in osmo-bsc[master]: oml: Delay configuring NSVC until BTS features are negotiated

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

lynxis lazus gerrit-no-reply at lists.osmocom.org
Sat Dec 5 19:38:44 UTC 2020


lynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/21451 )

Change subject: oml: Delay configuring NSVC until BTS features are negotiated
......................................................................

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
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/nm_common_fsm.h
M src/osmo-bsc/nm_bts_fsm.c
M src/osmo-bsc/nm_common_fsm.c
M src/osmo-bsc/nm_gprs_nsvc_fsm.c
5 files changed, 38 insertions(+), 7 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, but someone else must approve
  lynxis lazus: Looks good to me, approved



diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h
index fd968fa..2b05418 100644
--- a/include/osmocom/bsc/bts.h
+++ b/include/osmocom/bsc/bts.h
@@ -585,6 +585,11 @@
 /* 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 bceefbe..89ec863 100644
--- a/include/osmocom/bsc/nm_common_fsm.h
+++ b/include/osmocom/bsc/nm_common_fsm.h
@@ -36,6 +36,7 @@
 	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 731b578..c951edd 100644
--- a/src/osmo-bsc/nm_bts_fsm.c
+++ b/src/osmo-bsc/nm_bts_fsm.c
@@ -127,6 +127,21 @@
 	}
 }
 
+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 @@
 
 	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 @@
 
 	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 2a529db..2f19ed4 100644
--- a/src/osmo-bsc/nm_common_fsm.c
+++ b/src/osmo-bsc/nm_common_fsm.c
@@ -31,5 +31,6 @@
 	{ 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 ffc5659..2a57ada 100644
--- a/src/osmo-bsc/nm_gprs_nsvc_fsm.c
+++ b/src/osmo-bsc/nm_gprs_nsvc_fsm.c
@@ -62,6 +62,8 @@
 	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 @@
 	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 @@
 	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 @@
 	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 @@
 	[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 @@
 	[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 @@
 	[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) |

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/21451
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I7c207eee0e331995ae04acec014fbd13d4d16280
Gerrit-Change-Number: 21451
Gerrit-PatchSet: 3
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201205/3562cb7e/attachment.htm>


More information about the gerrit-log mailing list