<p>Neels Hofmeyr <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/10186">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fix nanobts: timeslot FSM: use flags to remember OML,RSL status<br><br>Before this patch, the timeslot FSM receives OML and RSL ready events.<br>Afterwards, it relies on examining the RSL and OML status to match the received<br>events. This doesn't work for the ip.access nanobts, which fails to change the<br>CHANNEL OM's operational status even though it has sent an Opstart ACK.  We<br>receive OML CHANNEL Opstart ACK, but the mo's state left at OP_STATE=Disabled.<br>We apparently cannot rely on the gsm_abis_mo state as assumed before this<br>patch, since changing the state depends on each BTS vendor's OML<br>implementation.<br><br>Also, implementation wise, it is better to not include assumptions on RSL and<br>OML implementations in the timeslot FSM. Simply receive the OML and RSL ready<br>events and remember that they arrived in dedicated flags.<br><br>Remove the no longer needed oml_is_ts_ready() callback from struct<br>gsm_bts_model added in:<br><br>  commit 91aa68f762218906e45be4817c6ea54b480da5e1<br>  "dyn TS: init only when both RSL and the Channel OM are established"<br>  I99f29d2ba079f6f4b77f0af12d9784588d2f56b3<br><br>This keeps osmo-bts operational while fixing ip.access nanobts, where the<br>CHANNEL OM's state prevented the timeslot FSM from entering operation.<br><br>Change-Id: I4843d03b3237cdcca0ad2041ef6895ff253d8419<br>---<br>M include/osmocom/bsc/gsm_data.h<br>M src/osmo-bsc/bts_ericsson_rbs2000.c<br>M src/osmo-bsc/bts_ipaccess_nanobts.c<br>M src/osmo-bsc/timeslot_fsm.c<br>M tests/handover/handover_test.c<br>5 files changed, 12 insertions(+), 17 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h</span><br><span>index ce563ef..750c027 100644</span><br><span>--- a/include/osmocom/bsc/gsm_data.h</span><br><span>+++ b/include/osmocom/bsc/gsm_data.h</span><br><span>@@ -605,6 +605,11 @@</span><br><span>   * enters IN_USE state, i.e. after each TCH use we try to PDCH ACT once again. */</span><br><span>    bool pdch_act_allowed;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+    /* Whether TS_EV_OML_READY was received */</span><br><span style="color: hsl(120, 100%, 40%);">+    bool is_oml_ready;</span><br><span style="color: hsl(120, 100%, 40%);">+    /* Whether TS_EV_RSL_READY was received */</span><br><span style="color: hsl(120, 100%, 40%);">+    bool is_rsl_ready;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>         struct gsm_abis_mo mo;</span><br><span>       struct tlv_parsed nm_attr;</span><br><span>   uint8_t nm_chan_comb;</span><br><span>@@ -739,7 +744,6 @@</span><br><span>  int (*start)(struct gsm_network *net);</span><br><span>       int (*oml_rcvmsg)(struct msgb *msg);</span><br><span>         char * (*oml_status)(const struct gsm_bts *bts);</span><br><span style="color: hsl(0, 100%, 40%);">-        bool (*oml_is_ts_ready)(const struct gsm_bts_trx_ts *ts);</span><br><span> </span><br><span>        void (*e1line_bind_ops)(struct e1inp_line *line);</span><br><span> </span><br><span>diff --git a/src/osmo-bsc/bts_ericsson_rbs2000.c b/src/osmo-bsc/bts_ericsson_rbs2000.c</span><br><span>index c2975f4..ed3db93 100644</span><br><span>--- a/src/osmo-bsc/bts_ericsson_rbs2000.c</span><br><span>+++ b/src/osmo-bsc/bts_ericsson_rbs2000.c</span><br><span>@@ -175,17 +175,11 @@</span><br><span>       e1inp_line_bind_ops(line, &bts_isdn_e1inp_line_ops);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static bool bts_model_rbs2k_is_ts_ready(const struct gsm_bts_trx_ts *ts)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">-      return ts && ts->mo.nm_state.operational == NM_OPSTATE_ENABLED;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static struct gsm_bts_model model_rbs2k = {</span><br><span>   .type = GSM_BTS_TYPE_RBS2000,</span><br><span>        .name = "rbs2000",</span><br><span>         .start = bts_model_rbs2k_start,</span><br><span>      .oml_rcvmsg = &abis_om2k_rcvmsg,</span><br><span style="color: hsl(0, 100%, 40%);">-    .oml_is_ts_ready = bts_model_rbs2k_is_ts_ready,</span><br><span>      .config_write_bts = &config_write_bts,</span><br><span>   .e1line_bind_ops = &bts_model_rbs2k_e1line_bind_ops,</span><br><span> };</span><br><span>diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c</span><br><span>index 6b6e265..80f7c9c 100644</span><br><span>--- a/src/osmo-bsc/bts_ipaccess_nanobts.c</span><br><span>+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c</span><br><span>@@ -55,18 +55,12 @@</span><br><span>        return "disconnected";</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static bool oml_is_ts_ready(const struct gsm_bts_trx_ts *ts)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">-  return ts && ts->mo.nm_state.operational == NM_OPSTATE_ENABLED;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> struct gsm_bts_model bts_model_nanobts = {</span><br><span>    .type = GSM_BTS_TYPE_NANOBTS,</span><br><span>        .name = "nanobts",</span><br><span>         .start = bts_model_nanobts_start,</span><br><span>    .oml_rcvmsg = &abis_nm_rcvmsg,</span><br><span>   .oml_status = &get_oml_status,</span><br><span style="color: hsl(0, 100%, 40%);">-      .oml_is_ts_ready = oml_is_ts_ready,</span><br><span>  .e1line_bind_ops = bts_model_nanobts_e1line_bind_ops, </span><br><span>       .nm_att_tlvdef = {</span><br><span>           .def = {</span><br><span>diff --git a/src/osmo-bsc/timeslot_fsm.c b/src/osmo-bsc/timeslot_fsm.c</span><br><span>index 84d80f8..245ce76 100644</span><br><span>--- a/src/osmo-bsc/timeslot_fsm.c</span><br><span>+++ b/src/osmo-bsc/timeslot_fsm.c</span><br><span>@@ -220,13 +220,13 @@</span><br><span> static void ts_fsm_not_initialized(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span> {</span><br><span>   struct gsm_bts_trx_ts *ts = ts_fi_ts(fi);</span><br><span style="color: hsl(0, 100%, 40%);">-       struct gsm_bts *bts = ts->trx->bts;</span><br><span>    switch (event) {</span><br><span> </span><br><span>         case TS_EV_OML_READY:</span><br><span>                ts->pdch_act_allowed = true;</span><br><span style="color: hsl(120, 100%, 40%);">+               ts->is_oml_ready = true;</span><br><span>          ts_setup_lchans(ts);</span><br><span style="color: hsl(0, 100%, 40%);">-            if (!ts->trx->rsl_link) {</span><br><span style="color: hsl(120, 100%, 40%);">+               if (!ts->is_rsl_ready) {</span><br><span>                  LOG_TS(ts, LOGL_DEBUG, "No RSL link yet\n");</span><br><span>                       return;</span><br><span>              }</span><br><span>@@ -235,8 +235,8 @@</span><br><span> </span><br><span>  case TS_EV_RSL_READY:</span><br><span>                ts->pdch_act_allowed = true;</span><br><span style="color: hsl(0, 100%, 40%);">-         if (bts->model->oml_is_ts_ready</span><br><span style="color: hsl(0, 100%, 40%);">-               && !bts->model->oml_is_ts_ready(ts)) {</span><br><span style="color: hsl(120, 100%, 40%);">+              ts->is_rsl_ready = true;</span><br><span style="color: hsl(120, 100%, 40%);">+           if (!ts->is_oml_ready) {</span><br><span>                  LOG_TS(ts, LOGL_DEBUG, "OML not ready yet\n");</span><br><span>                     return;</span><br><span>              }</span><br><span>@@ -680,6 +680,7 @@</span><br><span>      struct gsm_bts_trx_ts *ts = ts_fi_ts(fi);</span><br><span>    switch (event) {</span><br><span>     case TS_EV_OML_DOWN:</span><br><span style="color: hsl(120, 100%, 40%);">+          ts->is_oml_ready = false;</span><br><span>                 if (fi->state != TS_ST_NOT_INITIALIZED)</span><br><span>                   osmo_fsm_inst_state_chg(fi, TS_ST_NOT_INITIALIZED, 0, 0);</span><br><span>            OSMO_ASSERT(fi->state == TS_ST_NOT_INITIALIZED);</span><br><span>@@ -689,6 +690,7 @@</span><br><span>            break;</span><br><span> </span><br><span>   case TS_EV_RSL_DOWN:</span><br><span style="color: hsl(120, 100%, 40%);">+          ts->is_rsl_ready = false;</span><br><span>                 if (fi->state != TS_ST_NOT_INITIALIZED)</span><br><span>                   osmo_fsm_inst_state_chg(fi, TS_ST_NOT_INITIALIZED, 0, 0);</span><br><span>            OSMO_ASSERT(fi->state == TS_ST_NOT_INITIALIZED);</span><br><span>diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c</span><br><span>index 3a5748e..172e620 100644</span><br><span>--- a/tests/handover/handover_test.c</span><br><span>+++ b/tests/handover/handover_test.c</span><br><span>@@ -210,6 +210,7 @@</span><br><span> </span><br><span>   for (i = 0; i < ARRAY_SIZE(bts->c0->ts); i++) {</span><br><span>             /* make sure ts->lchans[] get initialized */</span><br><span style="color: hsl(120, 100%, 40%);">+               osmo_fsm_inst_dispatch(bts->c0->ts[i].fi, TS_EV_RSL_READY, 0);</span><br><span>                 osmo_fsm_inst_dispatch(bts->c0->ts[i].fi, TS_EV_OML_READY, 0);</span><br><span>         }</span><br><span>    return bts;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/10186">change 10186</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/10186"/><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-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I4843d03b3237cdcca0ad2041ef6895ff253d8419 </div>
<div style="display:none"> Gerrit-Change-Number: 10186 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>