<p>lynxis lazus <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21451">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
lynxis lazus: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">oml: Delay configuring NSVC until BTS features are negotiated<br><br>This is needed in order to to proper feature support verification for<br>IPv6 when configuring the NSVC.<br>Before this patch, there could be a race condition where NSVC FSM<br>checked for BTS feature BTS_FEAT_IPV6_NSVC before it was negotiated<br>through BTS Get Attributes (Ack).<br><br>Fixes: OS#4870<br>Change-Id: I7c207eee0e331995ae04acec014fbd13d4d16280<br>---<br>M include/osmocom/bsc/bts.h<br>M include/osmocom/bsc/nm_common_fsm.h<br>M src/osmo-bsc/nm_bts_fsm.c<br>M src/osmo-bsc/nm_common_fsm.c<br>M src/osmo-bsc/nm_gprs_nsvc_fsm.c<br>5 files changed, 38 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/bts.h b/include/osmocom/bsc/bts.h</span><br><span>index fd968fa..2b05418 100644</span><br><span>--- a/include/osmocom/bsc/bts.h</span><br><span>+++ b/include/osmocom/bsc/bts.h</span><br><span>@@ -585,6 +585,11 @@</span><br><span> /* reset the state of all MO in the BTS */</span><br><span> void gsm_bts_mo_reset(struct gsm_bts *bts);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static inline bool gsm_bts_features_negotiated(struct gsm_bts *bts)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ return bts->mo.get_attr_rep_received || bts->mo.nm_state.operational == NM_OPSTATE_ENABLED;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* dependency handling */</span><br><span> void bts_depend_mark(struct gsm_bts *bts, int dep);</span><br><span> void bts_depend_clear(struct gsm_bts *bts, int dep);</span><br><span>diff --git a/include/osmocom/bsc/nm_common_fsm.h b/include/osmocom/bsc/nm_common_fsm.h</span><br><span>index bceefbe..89ec863 100644</span><br><span>--- a/include/osmocom/bsc/nm_common_fsm.h</span><br><span>+++ b/include/osmocom/bsc/nm_common_fsm.h</span><br><span>@@ -36,6 +36,7 @@</span><br><span> NM_EV_OPSTART_NACK,</span><br><span> NM_EV_OML_DOWN,</span><br><span> NM_EV_FORCE_LOCK, /* Only supported by RadioCarrier so far */</span><br><span style="color: hsl(120, 100%, 40%);">+ NM_EV_FEATURE_NEGOTIATED, /* Sent by BTS to NSVC MO */</span><br><span> };</span><br><span> extern const struct value_string nm_fsm_event_names[];</span><br><span> </span><br><span>diff --git a/src/osmo-bsc/nm_bts_fsm.c b/src/osmo-bsc/nm_bts_fsm.c</span><br><span>index 731b578..c951edd 100644</span><br><span>--- a/src/osmo-bsc/nm_bts_fsm.c</span><br><span>+++ b/src/osmo-bsc/nm_bts_fsm.c</span><br><span>@@ -127,6 +127,21 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void rx_get_attr_rep(struct gsm_bts *bts, bool allow_opstart)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_gprs_nsvc *nsvc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ bts->mo.get_attr_rep_received = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ bts->mo.get_attr_sent = false;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Announce bts_features are available to related NSVC MOs */</span><br><span style="color: hsl(120, 100%, 40%);">+ nsvc = gsm_bts_sm_nsvc_num(bts->site_mgr, 0); /* we only support NSVC0 so far */</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(nsvc->mo.fi, NM_EV_FEATURE_NEGOTIATED, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Move FSM forward */</span><br><span style="color: hsl(120, 100%, 40%);">+ configure_loop(bts, &bts->mo.nm_state, allow_opstart);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void st_op_disabled_dependency_on_enter(struct osmo_fsm_inst *fi, uint32_t prev_state)</span><br><span> {</span><br><span> struct gsm_bts *bts = (struct gsm_bts *)fi->priv;</span><br><span>@@ -149,9 +164,7 @@</span><br><span> </span><br><span> switch (event) {</span><br><span> case NM_EV_GET_ATTR_REP:</span><br><span style="color: hsl(0, 100%, 40%);">- bts->mo.get_attr_rep_received = true;</span><br><span style="color: hsl(0, 100%, 40%);">- bts->mo.get_attr_sent = false;</span><br><span style="color: hsl(0, 100%, 40%);">- configure_loop(bts, &bts->mo.nm_state, false);</span><br><span style="color: hsl(120, 100%, 40%);">+ rx_get_attr_rep(bts, false);</span><br><span> return;</span><br><span> case NM_EV_SET_ATTR_ACK:</span><br><span> bts->mo.set_attr_ack_received = true;</span><br><span>@@ -203,9 +216,7 @@</span><br><span> </span><br><span> switch (event) {</span><br><span> case NM_EV_GET_ATTR_REP:</span><br><span style="color: hsl(0, 100%, 40%);">- bts->mo.get_attr_rep_received = true;</span><br><span style="color: hsl(0, 100%, 40%);">- bts->mo.get_attr_sent = false;</span><br><span style="color: hsl(0, 100%, 40%);">- configure_loop(bts, &bts->mo.nm_state, true);</span><br><span style="color: hsl(120, 100%, 40%);">+ rx_get_attr_rep(bts, true);</span><br><span> return;</span><br><span> case NM_EV_SET_ATTR_ACK:</span><br><span> bts->mo.set_attr_ack_received = true;</span><br><span>diff --git a/src/osmo-bsc/nm_common_fsm.c b/src/osmo-bsc/nm_common_fsm.c</span><br><span>index 2a529db..2f19ed4 100644</span><br><span>--- a/src/osmo-bsc/nm_common_fsm.c</span><br><span>+++ b/src/osmo-bsc/nm_common_fsm.c</span><br><span>@@ -31,5 +31,6 @@</span><br><span> { NM_EV_OPSTART_NACK, "OPSTART_NACK" },</span><br><span> { NM_EV_OML_DOWN, "OML_DOWN" },</span><br><span> { NM_EV_FORCE_LOCK, "FORCE_LOCK_CHG" },</span><br><span style="color: hsl(120, 100%, 40%);">+ { NM_EV_FEATURE_NEGOTIATED, "FEATURE_NEGOTIATED" },</span><br><span> { 0, NULL }</span><br><span> };</span><br><span>diff --git a/src/osmo-bsc/nm_gprs_nsvc_fsm.c b/src/osmo-bsc/nm_gprs_nsvc_fsm.c</span><br><span>index ffc5659..2a57ada 100644</span><br><span>--- a/src/osmo-bsc/nm_gprs_nsvc_fsm.c</span><br><span>+++ b/src/osmo-bsc/nm_gprs_nsvc_fsm.c</span><br><span>@@ -62,6 +62,8 @@</span><br><span> struct gsm_nm_state *new_state;</span><br><span> </span><br><span> switch (event) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_FEATURE_NEGOTIATED:</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> case NM_EV_SW_ACT_REP:</span><br><span> break;</span><br><span> case NM_EV_STATE_CHG_REP:</span><br><span>@@ -94,7 +96,9 @@</span><br><span> if (nsvc->bts->gprs.mode == BTS_GPRS_NONE)</span><br><span> return;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!nsvc->mo.set_attr_sent && !nsvc->mo.set_attr_ack_received) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* We need to know BTS features in order to know if we can set IPv6 addresses */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsm_bts_features_negotiated(nsvc->bts) && !nsvc->mo.set_attr_sent &&</span><br><span style="color: hsl(120, 100%, 40%);">+ !nsvc->mo.set_attr_ack_received) {</span><br><span> if (!osmo_bts_has_feature(&nsvc->bts->features, BTS_FEAT_IPV6_NSVC) &&</span><br><span> nsvc->remote.u.sa.sa_family == AF_INET6) {</span><br><span> LOGPFSML(nsvc->mo.fi, LOGL_ERROR,</span><br><span>@@ -149,6 +153,9 @@</span><br><span> struct gsm_nm_state *new_state;</span><br><span> </span><br><span> switch (event) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_FEATURE_NEGOTIATED:</span><br><span style="color: hsl(120, 100%, 40%);">+ configure_loop(nsvc, &nsvc->mo.nm_state, false);</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span> case NM_EV_SET_ATTR_ACK:</span><br><span> nsvc->mo.set_attr_ack_received = true;</span><br><span> nsvc->mo.set_attr_sent = false;</span><br><span>@@ -198,6 +205,9 @@</span><br><span> struct gsm_nm_state *new_state;</span><br><span> </span><br><span> switch (event) {</span><br><span style="color: hsl(120, 100%, 40%);">+ case NM_EV_FEATURE_NEGOTIATED:</span><br><span style="color: hsl(120, 100%, 40%);">+ configure_loop(nsvc, &nsvc->mo.nm_state, true);</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span> case NM_EV_SET_ATTR_ACK:</span><br><span> nsvc->mo.set_attr_ack_received = true;</span><br><span> nsvc->mo.set_attr_sent = false;</span><br><span>@@ -304,6 +314,7 @@</span><br><span> [NM_GPRS_NSVC_ST_OP_DISABLED_NOTINSTALLED] = {</span><br><span> .in_event_mask =</span><br><span> X(NM_EV_SW_ACT_REP) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_FEATURE_NEGOTIATED) |</span><br><span> X(NM_EV_STATE_CHG_REP),</span><br><span> .out_state_mask =</span><br><span> X(NM_GPRS_NSVC_ST_OP_DISABLED_DEPENDENCY) |</span><br><span>@@ -316,6 +327,7 @@</span><br><span> [NM_GPRS_NSVC_ST_OP_DISABLED_DEPENDENCY] = {</span><br><span> .in_event_mask =</span><br><span> X(NM_EV_STATE_CHG_REP) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_FEATURE_NEGOTIATED) |</span><br><span> X(NM_EV_SET_ATTR_ACK),</span><br><span> .out_state_mask =</span><br><span> X(NM_GPRS_NSVC_ST_OP_DISABLED_NOTINSTALLED) |</span><br><span>@@ -328,6 +340,7 @@</span><br><span> [NM_GPRS_NSVC_ST_OP_DISABLED_OFFLINE] = {</span><br><span> .in_event_mask =</span><br><span> X(NM_EV_STATE_CHG_REP) |</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NM_EV_FEATURE_NEGOTIATED) |</span><br><span> X(NM_EV_SET_ATTR_ACK),</span><br><span> .out_state_mask =</span><br><span> X(NM_GPRS_NSVC_ST_OP_DISABLED_NOTINSTALLED) |</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/21451">change 21451</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bsc/+/21451"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I7c207eee0e331995ae04acec014fbd13d4d16280 </div>
<div style="display:none"> Gerrit-Change-Number: 21451 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>