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

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">abis.c: Fix mess with priv->bsc_oml_host<br><br>The pointer was used as "struct bsc_oml_host" sometimes, and other times<br>as "struct llist_head". It just worked because bsc_oml_host->list is the<br>first item in the script. The code was really confusing, also because<br>the bts list of items has a name really similar to the one currently<br>assigned. Let's rename the currently assigned address to "current_bsc",<br>store it always as "struct bsc_oml_host*" and finally use llist_entry<br>helpers when needed.<br><br>The related code is also moved to a helper function to enclose there the<br>logic to get next BSC in list. This change actually changes the logic<br>where a remote address is removed from VTY, since now the next address<br>in list is picked at the time, and later when reconnecting the list is<br>forwarded another time, meaning one address will be skipped.<br>This could be considered a bug, but this situation is really special<br>and anyway the entire logic will be changed in new commits where we'll<br>keep reconnecting in loop without exiting when reaching the end of the<br>list, so we are fine with it. Think of this commit as a preparation<br>commit for next ones.<br><br>Change-Id: I3cc8a4148b3d63bc185b72dab8108105a6644910<br>---<br>M include/osmo-bts/abis.h<br>M src/common/abis.c<br>2 files changed, 32 insertions(+), 20 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmo-bts/abis.h b/include/osmo-bts/abis.h</span><br><span>index 1939faf..40707cd 100644</span><br><span>--- a/include/osmo-bts/abis.h</span><br><span>+++ b/include/osmo-bts/abis.h</span><br><span>@@ -9,7 +9,7 @@</span><br><span> enum abis_link_fsm_event {</span><br><span>        ABIS_LINK_EV_SIGN_LINK_OML_UP,</span><br><span>       ABIS_LINK_EV_SIGN_LINK_DOWN,</span><br><span style="color: hsl(0, 100%, 40%);">-    ABIS_LINK_EV_VTY_RM_ADDR,</span><br><span style="color: hsl(120, 100%, 40%);">+     ABIS_LINK_EV_VTY_RM_ADDR, /* data: struct bsc_oml_host* being removed */</span><br><span> };</span><br><span> </span><br><span> void abis_init(struct gsm_bts *bts);</span><br><span>diff --git a/src/common/abis.c b/src/common/abis.c</span><br><span>index 551670c..c230a4b 100644</span><br><span>--- a/src/common/abis.c</span><br><span>+++ b/src/common/abis.c</span><br><span>@@ -76,7 +76,7 @@</span><br><span> };</span><br><span> </span><br><span> struct abis_link_fsm_priv {</span><br><span style="color: hsl(0, 100%, 40%);">-        struct llist_head *bsc_oml_host;</span><br><span style="color: hsl(120, 100%, 40%);">+      struct bsc_oml_host *current_bsc;</span><br><span>    struct gsm_bts *bts;</span><br><span>         char *model_name;</span><br><span>    int line_ctr;</span><br><span>@@ -115,30 +115,45 @@</span><br><span>        }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static int pick_next_bsc(struct osmo_fsm_inst *fi)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+     struct abis_link_fsm_priv *priv = fi->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+        struct gsm_bts *bts = priv->bts;</span><br><span style="color: hsl(120, 100%, 40%);">+   struct bsc_oml_host *last;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+  if (llist_empty(&bts->bsc_oml_hosts)) {</span><br><span style="color: hsl(120, 100%, 40%);">+                LOGPFSML(fi, LOGL_ERROR, "List of BSCs to connect to is empty!\n");</span><br><span style="color: hsl(120, 100%, 40%);">+         return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+    }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   last = (struct bsc_oml_host *)llist_last_entry(&bts->bsc_oml_hosts, struct bsc_oml_host, list);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+      if (!priv->current_bsc) /* Pick first one: */</span><br><span style="color: hsl(120, 100%, 40%);">+              priv->current_bsc = (struct bsc_oml_host *)llist_first_entry(&bts->bsc_oml_hosts, struct bsc_oml_host, list);</span><br><span style="color: hsl(120, 100%, 40%);">+       else if (priv->current_bsc != last)</span><br><span style="color: hsl(120, 100%, 40%);">+                priv->current_bsc = (struct bsc_oml_host *)llist_entry(priv->current_bsc->list.next, struct bsc_oml_host, list);</span><br><span style="color: hsl(120, 100%, 40%);">+     else</span><br><span style="color: hsl(120, 100%, 40%);">+          return -1; /* We are so far not starting over the list when we reach the list, but only exit */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void abis_link_connecting_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state)</span><br><span> {</span><br><span>       struct e1inp_line *line;</span><br><span>     struct abis_link_fsm_priv *priv = fi->priv;</span><br><span>       struct gsm_bts *bts = priv->bts;</span><br><span style="color: hsl(0, 100%, 40%);">-     struct bsc_oml_host *bsc_oml_host;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  if (priv->bsc_oml_host) {</span><br><span style="color: hsl(0, 100%, 40%);">-            /* Get a BSC host from the list and move the list head one position forward. */</span><br><span style="color: hsl(0, 100%, 40%);">-         bsc_oml_host = (struct bsc_oml_host *)priv->bsc_oml_host;</span><br><span style="color: hsl(0, 100%, 40%);">-            if (priv->bsc_oml_host == llist_last(&bts->bsc_oml_hosts))</span><br><span style="color: hsl(0, 100%, 40%);">-                    priv->bsc_oml_host = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-           else</span><br><span style="color: hsl(0, 100%, 40%);">-                    priv->bsc_oml_host = priv->bsc_oml_host->next;</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">-                LOGP(DABIS, LOGL_FATAL, "No BSC available, A-bis connection establishment failed\n");</span><br><span style="color: hsl(120, 100%, 40%);">+       if (pick_next_bsc(fi) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+               LOGPFSML(fi, LOGL_FATAL, "No BSC available, A-bis connection establishment failed\n");</span><br><span>             osmo_fsm_inst_state_chg(fi, ABIS_LINK_ST_FAILED, 0, 0);</span><br><span>              return;</span><br><span>      }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   LOGP(DABIS, LOGL_NOTICE, "A-bis connection establishment to BSC (%s) in progress...\n", bsc_oml_host->addr);</span><br><span style="color: hsl(120, 100%, 40%);">+     LOGP(DABIS, LOGL_NOTICE, "A-bis connection establishment to BSC (%s) in progress...\n", priv->current_bsc->addr);</span><br><span> </span><br><span>        /* patch in various data from VTY and other sources */</span><br><span style="color: hsl(0, 100%, 40%);">-  line_ops.cfg.ipa.addr = bsc_oml_host->addr;</span><br><span style="color: hsl(120, 100%, 40%);">+        line_ops.cfg.ipa.addr = priv->current_bsc->addr;</span><br><span>       osmo_get_macaddr(bts_dev_info.mac_addr, "eth0");</span><br><span>   bts_dev_info.site_id = bts->ip_access.site_id;</span><br><span>    bts_dev_info.bts_id = bts->ip_access.bts_id;</span><br><span>@@ -239,13 +254,11 @@</span><br><span> </span><br><span>  OSMO_ASSERT(event == ABIS_LINK_EV_VTY_RM_ADDR);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     if (priv->bsc_oml_host == data) {</span><br><span style="color: hsl(120, 100%, 40%);">+  if (priv->current_bsc == data) {</span><br><span>          if (llist_count(&bts->bsc_oml_hosts) <= 1)</span><br><span style="color: hsl(0, 100%, 40%);">-                    priv->bsc_oml_host = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-           else if (priv->bsc_oml_host == llist_last(&bts->bsc_oml_hosts))</span><br><span style="color: hsl(0, 100%, 40%);">-                       priv->bsc_oml_host = priv->bsc_oml_host->prev;</span><br><span style="color: hsl(120, 100%, 40%);">+                       priv->current_bsc = NULL;</span><br><span>                 else</span><br><span style="color: hsl(0, 100%, 40%);">-                    priv->bsc_oml_host = priv->bsc_oml_host->next;</span><br><span style="color: hsl(120, 100%, 40%);">+                       pick_next_bsc(fi);</span><br><span>   }</span><br><span> }</span><br><span> </span><br><span>@@ -477,7 +490,6 @@</span><br><span> </span><br><span>         abis_link_fsm_priv = talloc_zero(bts->abis_link_fi, struct abis_link_fsm_priv);</span><br><span>   OSMO_ASSERT(abis_link_fsm_priv);</span><br><span style="color: hsl(0, 100%, 40%);">-        abis_link_fsm_priv->bsc_oml_host = bts->bsc_oml_hosts.next;</span><br><span>    abis_link_fsm_priv->bts = bts;</span><br><span>    abis_link_fsm_priv->model_name = model_name;</span><br><span>      bts->abis_link_fi->priv = abis_link_fsm_priv;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/25458">change 25458</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/+/25458"/><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: I3cc8a4148b3d63bc185b72dab8108105a6644910 </div>
<div style="display:none"> Gerrit-Change-Number: 25458 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </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: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>