<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/11203">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;">ipaccess: Simplify handling of ipaccess e1line ts<br><br>Handle encoding specifics behind a macro to make code easier to<br>understand and follow.<br><br>Change-Id: Ibf251673bff95b7a0b066b19ef4dc6c0f94fff6b<br>---<br>M include/osmocom/abis/e1_input.h<br>M src/input/ipaccess.c<br>M tests/e1inp_ipa_bsc_test.c<br>M tests/e1inp_ipa_bts_test.c<br>4 files changed, 36 insertions(+), 22 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/abis/e1_input.h b/include/osmocom/abis/e1_input.h</span><br><span>index 79455e1..f9b4850 100644</span><br><span>--- a/include/osmocom/abis/e1_input.h</span><br><span>+++ b/include/osmocom/abis/e1_input.h</span><br><span>@@ -201,6 +201,8 @@</span><br><span>         struct e1inp_driver *driver;</span><br><span>         void *driver_data;</span><br><span> };</span><br><span style="color: hsl(120, 100%, 40%);">+#define e1inp_line_ipa_oml_ts(line) (&line->ts[0])</span><br><span style="color: hsl(120, 100%, 40%);">+#define e1inp_line_ipa_rsl_ts(line, trx_id) (&line->ts[1 + (trx_id)])</span><br><span> </span><br><span> /* SS_L_INPUT signals */</span><br><span> enum e1inp_signal_input {</span><br><span>diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c</span><br><span>index f8e8e09..8c02996 100644</span><br><span>--- a/src/input/ipaccess.c</span><br><span>+++ b/src/input/ipaccess.c</span><br><span>@@ -62,8 +62,11 @@</span><br><span> static int ipaccess_drop(struct osmo_fd *bfd, struct e1inp_line *line)</span><br><span> {</span><br><span>      int ret = 1;</span><br><span style="color: hsl(0, 100%, 40%);">-    unsigned int ts_nr = bfd->priv_nr;</span><br><span style="color: hsl(0, 100%, 40%);">-   struct e1inp_ts *e1i_ts = &line->ts[ts_nr-1];</span><br><span style="color: hsl(120, 100%, 40%);">+  struct e1inp_ts *e1i_ts;</span><br><span style="color: hsl(120, 100%, 40%);">+      if (bfd->priv_nr == E1INP_SIGN_OML)</span><br><span style="color: hsl(120, 100%, 40%);">+                e1i_ts = e1inp_line_ipa_oml_ts(line);</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+          e1i_ts = e1inp_line_ipa_rsl_ts(line, bfd->priv_nr - E1INP_SIGN_RSL);</span><br><span> </span><br><span>  /* Error case: we did not see any ID_RESP yet for this socket. */</span><br><span>    if (bfd->fd != -1) {</span><br><span>@@ -185,7 +188,7 @@</span><br><span>                         * this RSL link, attach it to this socket. */</span><br><span>                       bfd->data = new_line = sign_link->ts->line;</span><br><span>                         e1inp_line_get(new_line);</span><br><span style="color: hsl(0, 100%, 40%);">-                       ts = &new_line->ts[E1INP_SIGN_RSL+unit_data.trx_id-1];</span><br><span style="color: hsl(120, 100%, 40%);">+                 ts = e1inp_line_ipa_rsl_ts(new_line, unit_data.trx_id);</span><br><span>                      newbfd = &ts->driver.ipaccess.fd;</span><br><span> </span><br><span>                         /* get rid of our old temporary bfd */</span><br><span>@@ -223,12 +226,17 @@</span><br><span> {</span><br><span>  struct e1inp_line *line = bfd->data;</span><br><span>      unsigned int ts_nr = bfd->priv_nr;</span><br><span style="color: hsl(0, 100%, 40%);">-   struct e1inp_ts *e1i_ts = &line->ts[ts_nr-1];</span><br><span style="color: hsl(120, 100%, 40%);">+  struct e1inp_ts *e1i_ts;</span><br><span>     struct e1inp_sign_link *link;</span><br><span>        struct ipaccess_head *hh;</span><br><span>    struct msgb *msg = NULL;</span><br><span>     int ret, rc;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+      if (bfd->priv_nr == E1INP_SIGN_OML)</span><br><span style="color: hsl(120, 100%, 40%);">+                e1i_ts = e1inp_line_ipa_oml_ts(line);</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+          e1i_ts = e1inp_line_ipa_rsl_ts(line, bfd->priv_nr - E1INP_SIGN_RSL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>    ret = ipa_msg_recv_buffered(bfd->fd, &msg, &e1i_ts->pending_msg);</span><br><span>      if (ret < 0) {</span><br><span>            if (ret == -EAGAIN)</span><br><span>@@ -312,11 +320,16 @@</span><br><span> static int __handle_ts1_write(struct osmo_fd *bfd, struct e1inp_line *line)</span><br><span> {</span><br><span>      unsigned int ts_nr = bfd->priv_nr;</span><br><span style="color: hsl(0, 100%, 40%);">-   struct e1inp_ts *e1i_ts = &line->ts[ts_nr-1];</span><br><span style="color: hsl(120, 100%, 40%);">+  struct e1inp_ts *e1i_ts;</span><br><span>     struct e1inp_sign_link *sign_link;</span><br><span>   struct msgb *msg;</span><br><span>    int ret;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+  if (bfd->priv_nr == E1INP_SIGN_OML)</span><br><span style="color: hsl(120, 100%, 40%);">+                e1i_ts = e1inp_line_ipa_oml_ts(line);</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+          e1i_ts = e1inp_line_ipa_rsl_ts(line, bfd->priv_nr - E1INP_SIGN_RSL);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span>    bfd->when &= ~BSC_FD_WRITE;</span><br><span> </span><br><span>       /* get the next msg for this timeslot */</span><br><span>@@ -454,7 +467,6 @@</span><br><span> static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd)</span><br><span> {</span><br><span>  int ret;</span><br><span style="color: hsl(0, 100%, 40%);">-        int idx = 0;</span><br><span>         int i;</span><br><span>       struct e1inp_line *line;</span><br><span>     struct e1inp_ts *e1i_ts;</span><br><span>@@ -468,13 +480,13 @@</span><br><span>     }</span><br><span> </span><br><span>        /* create virrtual E1 timeslots for signalling */</span><br><span style="color: hsl(0, 100%, 40%);">-       e1inp_ts_config_sign(&line->ts[E1INP_SIGN_OML-1], line);</span><br><span style="color: hsl(120, 100%, 40%);">+       e1inp_ts_config_sign(e1inp_line_ipa_oml_ts(line), line);</span><br><span> </span><br><span>         /* initialize the fds */</span><br><span>     for (i = 0; i < ARRAY_SIZE(line->ts); ++i)</span><br><span>             line->ts[i].driver.ipaccess.fd.fd = -1;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-  e1i_ts = &line->ts[idx];</span><br><span style="color: hsl(120, 100%, 40%);">+       e1i_ts = e1inp_line_ipa_oml_ts(line);</span><br><span> </span><br><span>    bfd = &e1i_ts->driver.ipaccess.fd;</span><br><span>    osmo_fd_setup(bfd, fd, BSC_FD_READ, ipaccess_fd_cb, line, E1INP_SIGN_OML);</span><br><span>@@ -524,9 +536,9 @@</span><br><span> </span><br><span>         /* we need this to initialize this in case to avoid crashes in case</span><br><span>   * that the socket is closed before we've seen an ID_RESP. */</span><br><span style="color: hsl(0, 100%, 40%);">-       e1inp_ts_config_sign(&line->ts[E1INP_SIGN_OML-1], line);</span><br><span style="color: hsl(120, 100%, 40%);">+       e1inp_ts_config_sign(e1inp_line_ipa_oml_ts(line), line);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    e1i_ts = &line->ts[E1INP_SIGN_RSL-1];</span><br><span style="color: hsl(120, 100%, 40%);">+  e1i_ts = e1inp_line_ipa_rsl_ts(line, 0);</span><br><span> </span><br><span>         bfd = &e1i_ts->driver.ipaccess.fd;</span><br><span>    osmo_fd_setup(bfd, fd, BSC_FD_READ, ipaccess_fd_cb, line, E1INP_SIGN_RSL);</span><br><span>@@ -763,9 +775,9 @@</span><br><span>             msgb_free(msg);</span><br><span>              return ret;</span><br><span>  } else if (link->port == IPA_TCP_PORT_OML)</span><br><span style="color: hsl(0, 100%, 40%);">-           e1i_ts = &link->line->ts[0];</span><br><span style="color: hsl(120, 100%, 40%);">+                e1i_ts = e1inp_line_ipa_oml_ts(link->line);</span><br><span>       else if (link->port == IPA_TCP_PORT_RSL)</span><br><span style="color: hsl(0, 100%, 40%);">-             e1i_ts = &link->line->ts[link->ofd->priv_nr-1];</span><br><span style="color: hsl(120, 100%, 40%);">+               e1i_ts = e1inp_line_ipa_rsl_ts(link->line, link->ofd->priv_nr - E1INP_SIGN_RSL);</span><br><span> </span><br><span>        OSMO_ASSERT(e1i_ts != NULL);</span><br><span> </span><br><span>@@ -870,7 +882,7 @@</span><br><span>                    IPA_TCP_PORT_OML);</span><br><span> </span><br><span>          link = ipa_client_conn_create(tall_ipa_ctx,</span><br><span style="color: hsl(0, 100%, 40%);">-                                           &line->ts[E1INP_SIGN_OML-1],</span><br><span style="color: hsl(120, 100%, 40%);">+                                           e1inp_line_ipa_oml_ts(line),</span><br><span>                                         E1INP_SIGN_OML,</span><br><span>                                              line->ops->cfg.ipa.addr,</span><br><span>                                               IPA_TCP_PORT_OML,</span><br><span>@@ -920,7 +932,7 @@</span><br><span>        }</span><br><span> </span><br><span>        rsl_link = ipa_client_conn_create(tall_ipa_ctx,</span><br><span style="color: hsl(0, 100%, 40%);">-                                   &line->ts[E1INP_SIGN_RSL+trx_nr-1],</span><br><span style="color: hsl(120, 100%, 40%);">+                                    e1inp_line_ipa_rsl_ts(line, trx_nr),</span><br><span>                                         E1INP_SIGN_RSL+trx_nr,</span><br><span>                                       rem_addr, rem_port,</span><br><span>                                          ipaccess_bts_updown_cb,</span><br><span>diff --git a/tests/e1inp_ipa_bsc_test.c b/tests/e1inp_ipa_bsc_test.c</span><br><span>index 9064d0a..a7317f8 100644</span><br><span>--- a/tests/e1inp_ipa_bsc_test.c</span><br><span>+++ b/tests/e1inp_ipa_bsc_test.c</span><br><span>@@ -30,9 +30,9 @@</span><br><span>   switch(type) {</span><br><span>       case E1INP_SIGN_OML:</span><br><span>                 LOGP(DBSCTEST, LOGL_NOTICE, "OML link up request received.\n");</span><br><span style="color: hsl(0, 100%, 40%);">-               e1inp_ts_config_sign(&line->ts[E1INP_SIGN_OML - 1], line);</span><br><span style="color: hsl(120, 100%, 40%);">+             e1inp_ts_config_sign(e1inp_line_ipa_oml_ts(line), line);</span><br><span>             sign_link = oml_sign_link =</span><br><span style="color: hsl(0, 100%, 40%);">-                     e1inp_sign_link_create(&line->ts[E1INP_SIGN_OML - 1],</span><br><span style="color: hsl(120, 100%, 40%);">+                  e1inp_sign_link_create(e1inp_line_ipa_oml_ts(line),</span><br><span>                                         E1INP_SIGN_OML, NULL, 255, 0);</span><br><span>                break;</span><br><span>       case E1INP_SIGN_RSL:</span><br><span>@@ -45,10 +45,10 @@</span><br><span> </span><br><span>               /* We have to use the same line that the OML link. */</span><br><span>                oml_line = oml_sign_link->ts->line;</span><br><span style="color: hsl(0, 100%, 40%);">-               e1inp_ts_config_sign(&oml_line->ts[E1INP_SIGN_RSL - 1],</span><br><span style="color: hsl(120, 100%, 40%);">+                e1inp_ts_config_sign(e1inp_line_ipa_rsl_ts(oml_line, 0),</span><br><span>                                  oml_line);</span><br><span>              sign_link = rsl_sign_link =</span><br><span style="color: hsl(0, 100%, 40%);">-                  e1inp_sign_link_create(&oml_line->ts[E1INP_SIGN_RSL - 1],</span><br><span style="color: hsl(120, 100%, 40%);">+              e1inp_sign_link_create(e1inp_line_ipa_rsl_ts(oml_line, 0),</span><br><span>                                          E1INP_SIGN_RSL, NULL, 0, 0);</span><br><span>             break;</span><br><span>       default:</span><br><span>diff --git a/tests/e1inp_ipa_bts_test.c b/tests/e1inp_ipa_bts_test.c</span><br><span>index 48a5c59..10fe0e8 100644</span><br><span>--- a/tests/e1inp_ipa_bts_test.c</span><br><span>+++ b/tests/e1inp_ipa_bts_test.c</span><br><span>@@ -51,9 +51,9 @@</span><br><span>    case E1INP_SIGN_OML:</span><br><span>                 LOGP(DBTSTEST, LOGL_NOTICE, "OML link up request received.\n");</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-           e1inp_ts_config_sign(&line->ts[E1INP_SIGN_OML - 1], line);</span><br><span style="color: hsl(120, 100%, 40%);">+             e1inp_ts_config_sign(e1inp_line_ipa_oml_ts(line), line);</span><br><span>             sign_link = oml_sign_link =</span><br><span style="color: hsl(0, 100%, 40%);">-                     e1inp_sign_link_create(&line->ts[E1INP_SIGN_OML - 1],</span><br><span style="color: hsl(120, 100%, 40%);">+                  e1inp_sign_link_create(e1inp_line_ipa_oml_ts(line),</span><br><span>                                          E1INP_SIGN_OML, NULL, 255, 0);</span><br><span>               if (!oml_sign_link) {</span><br><span>                        LOGP(DBTSTEST, LOGL_ERROR,</span><br><span>@@ -76,10 +76,10 @@</span><br><span>     case E1INP_SIGN_RSL:</span><br><span>                 LOGP(DBTSTEST, LOGL_NOTICE, "RSL link up request received.\n");</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-           e1inp_ts_config_sign(&line->ts[E1INP_SIGN_RSL - 1], line);</span><br><span style="color: hsl(120, 100%, 40%);">+             e1inp_ts_config_sign(e1inp_line_ipa_rsl_ts(line, 0), line);</span><br><span> </span><br><span>              sign_link = rsl_sign_link =</span><br><span style="color: hsl(0, 100%, 40%);">-                     e1inp_sign_link_create(&line->ts[E1INP_SIGN_RSL - 1],</span><br><span style="color: hsl(120, 100%, 40%);">+                  e1inp_sign_link_create(e1inp_line_ipa_rsl_ts(line, 0),</span><br><span>                                               E1INP_SIGN_RSL, NULL, 0, 0);</span><br><span>                 if (!rsl_sign_link) {</span><br><span>                        LOGP(DBTSTEST, LOGL_ERROR,</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11203">change 11203</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/11203"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmo-abis </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ibf251673bff95b7a0b066b19ef4dc6c0f94fff6b </div>
<div style="display:none"> Gerrit-Change-Number: 11203 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>