<p>pespin <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/25542">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  fixeria: Looks good to me, approved
  Jenkins Builder: Verified

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bts_trx: Drop non-executed path in trx_link_estab()<br><br>This function is only called during sign_link_up() e1inp callback, hence<br>only the link!=NULL condition (UP) is ever executed. Let's drop the DOWN<br>path and make it a function only used to trigger events when link<br>becomes up, similar to what bts_link_estab() does with OML.<br><br>Here it becomes clear the NM_EV_RSL_DOWN was never sent. It's not much<br>of an issue though since it would only make transition RCARRIER/BBTRANSC<br>Enabled->DisabledOffline. However, since due to libosmo-abis limitation<br>we receive a sign_link_down() for the entire line when 1 of its links<br>goes down, we don't care much since we go for shutdown of the entire BTS<br>anyway. Ideally, libosmo-abis would support simply telling us 1 of the<br>links in the line went down and if it was not OML and not RSL TRX==C0,<br>then we could keep on running and simply disable the related TRX.<br><br>Change-Id: Iac553c68339c0da32fd313676995747eb4344087<br>---<br>M src/common/abis.c<br>M src/common/bts_trx.c<br>2 files changed, 13 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/common/abis.c b/src/common/abis.c</span><br><span>index 1722f3d..1b489c1 100644</span><br><span>--- a/src/common/abis.c</span><br><span>+++ b/src/common/abis.c</span><br><span>@@ -214,6 +214,14 @@</span><br><span>                     e1inp_sign_link_destroy(trx->rsl_link);</span><br><span>                   trx->rsl_link = NULL;</span><br><span>             }</span><br><span style="color: hsl(120, 100%, 40%);">+             /* Note: Here we could send NM_EV_RSL_DOWN to each</span><br><span style="color: hsl(120, 100%, 40%);">+             * trx->(bb_transc.)mo.fi, but we are starting shutdown of the</span><br><span style="color: hsl(120, 100%, 40%);">+              * entire BTS anyway through bts_model_abis_close(), so simply</span><br><span style="color: hsl(120, 100%, 40%);">+                 * let bts_shutdown FSM take care of slowly powering down all</span><br><span style="color: hsl(120, 100%, 40%);">+          * the TRX. It would make sense to send NM_EV_RSL_DOWN only if a</span><br><span style="color: hsl(120, 100%, 40%);">+               * RSL link TRX!=C0 was going down, in order to selectively stop</span><br><span style="color: hsl(120, 100%, 40%);">+               * that TRX only. But libosmo-abis expects us to drop the entire</span><br><span style="color: hsl(120, 100%, 40%);">+               * line when something goes wrong... */</span><br><span>      }</span><br><span>    bts_model_abis_close(bts);</span><br><span>   osmo_fsm_inst_state_chg(fi, ABIS_LINK_ST_WAIT_RECONNECT, OML_RETRY_TIMER, 0);</span><br><span>diff --git a/src/common/bts_trx.c b/src/common/bts_trx.c</span><br><span>index a5d7ed3..ed742d5 100644</span><br><span>--- a/src/common/bts_trx.c</span><br><span>+++ b/src/common/bts_trx.c</span><br><span>@@ -206,25 +206,16 @@</span><br><span> /* RSL link is established, send status report */</span><br><span> int trx_link_estab(struct gsm_bts_trx *trx)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-     struct e1inp_sign_link *link = trx->rsl_link;</span><br><span>     int rc;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     LOGPTRX(trx, DSUM, LOGL_INFO, "RSL link %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                link ? "up" : "down");</span><br><span style="color: hsl(120, 100%, 40%);">+    LOGPTRX(trx, DSUM, LOGL_INFO, "RSL link up\n");</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   osmo_fsm_inst_dispatch(trx->mo.fi, link ? NM_EV_RSL_UP : NM_EV_RSL_DOWN, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">-      osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, link ? NM_EV_RSL_UP : NM_EV_RSL_DOWN, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+  osmo_fsm_inst_dispatch(trx->mo.fi, NM_EV_RSL_UP, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+    osmo_fsm_inst_dispatch(trx->bb_transc.mo.fi, NM_EV_RSL_UP, NULL);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        if (link)</span><br><span style="color: hsl(0, 100%, 40%);">-               rc = rsl_tx_rf_res(trx);</span><br><span style="color: hsl(0, 100%, 40%);">-        else</span><br><span style="color: hsl(0, 100%, 40%);">-            rc = bts_model_trx_deact_rf(trx);</span><br><span style="color: hsl(0, 100%, 40%);">-       if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+      if ((rc = rsl_tx_rf_res(trx)) < 0)</span><br><span>                oml_tx_failure_event_rep(&trx->bb_transc.mo, NM_SEVER_MAJOR, OSMO_EVT_MAJ_RSL_FAIL,</span><br><span style="color: hsl(0, 100%, 40%);">-                                       link ?</span><br><span style="color: hsl(0, 100%, 40%);">-                                  "Failed to establish RSL link (%d)" :</span><br><span style="color: hsl(0, 100%, 40%);">-                                         "Failed to deactivate RF (%d)", rc);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+                                      "Failed to establish RSL link (%d)", rc);</span><br><span> </span><br><span>     return 0;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/25542">change 25542</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-bts/+/25542"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Iac553c68339c0da32fd313676995747eb4344087 </div>
<div style="display:none"> Gerrit-Change-Number: 25542 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>