<p>laforge has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/22555">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ns2: Work around AF_PACKET socket ENOBUFS problems<br><br>AF_PACKET sockets cannot be written-to using select(), as they<br>will always return "writable" but then still fail with ENOBUFS.<br><br>This also means that we cannot use osmo_wqueue() as it assumes<br>that a writable socket can actually be written to.<br><br>As there's no way to figure out when exactly we can again perform<br>a successful write, we have no other option but to start a timer<br>and re-try at a later time.<br><br>We will scale that timer based on the estimated duration of transmission<br>for the just-failed PDU on the line rate of a 31TS E1 Line.<br><br>Furthermore, with this patch, we stop queueing anything but signaling<br>traffic (NS-BVCI=0) or Q.933 LMI.  User data (NS-BVCI != 0) will<br>instead be dropped in order to avoid buffer-bloat.<br><br>Change-Id: I4f79a246236c94175ceffa5f3f556c8931c6bc62<br>Closes: OS#4995<br>---<br>M src/gb/gprs_ns2_fr.c<br>1 file changed, 132 insertions(+), 24 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/55/22555/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/gb/gprs_ns2_fr.c b/src/gb/gprs_ns2_fr.c</span><br><span>index 096e150..854eccf 100644</span><br><span>--- a/src/gb/gprs_ns2_fr.c</span><br><span>+++ b/src/gb/gprs_ns2_fr.c</span><br><span>@@ -4,7 +4,7 @@</span><br><span>  * 3GPP TS 08.16 version 8.0.1 Release 1999 / ETSI TS 101 299 V8.0.1 (2002-05)</span><br><span>  * as well as its successor 3GPP TS 48.016 */</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* (C) 2009-2010,2014,2017 by Harald Welte <laforge@gnumonks.org></span><br><span style="color: hsl(120, 100%, 40%);">+/* (C) 2009-2021 by Harald Welte <laforge@gnumonks.org></span><br><span>  * (C) 2020 sysmocom - s.f.m.c. GmbH</span><br><span>  * Author: Alexander Couzens <lynxis@fe80.eu></span><br><span>  *</span><br><span>@@ -51,9 +51,11 @@</span><br><span> #include <osmocom/core/msgb.h></span><br><span> #include <osmocom/core/select.h></span><br><span> #include <osmocom/core/socket.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/timer.h></span><br><span> #include <osmocom/core/talloc.h></span><br><span> #include <osmocom/core/write_queue.h></span><br><span> #include <osmocom/gprs/gprs_ns2.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/gprs/protocol/gsm_08_16.h></span><br><span> </span><br><span> #ifdef ENABLE_LIBMNL</span><br><span> #include <osmocom/core/mnl.h></span><br><span>@@ -72,6 +74,14 @@</span><br><span> # define IPPROTO_GRE 47</span><br><span> #endif</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define E1_LINERATE 2048000</span><br><span style="color: hsl(120, 100%, 40%);">+#define E1_SLOTS_TOTAL 32</span><br><span style="color: hsl(120, 100%, 40%);">+#define E1_SLOTS_USED 31</span><br><span style="color: hsl(120, 100%, 40%);">+/* usable bitrate of the E1 superchannel with 31 of 32 timeslots */</span><br><span style="color: hsl(120, 100%, 40%);">+#define SUPERCHANNEL_LINERATE (E1_LINERATE*E1_SLOTS_USED)/E1_SLOTS_TOTAL</span><br><span style="color: hsl(120, 100%, 40%);">+/* nanoseconds per bit (504) */</span><br><span style="color: hsl(120, 100%, 40%);">+#define BIT_DURATION_NS (1000000000 / SUPERCHANNEL_LINERATE)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct gre_hdr {</span><br><span>    uint16_t flags;</span><br><span>      uint16_t ptype;</span><br><span>@@ -88,9 +98,19 @@</span><br><span> struct priv_bind {</span><br><span>   char netif[IFNAMSIZ];</span><br><span>        struct osmo_fr_link *link;</span><br><span style="color: hsl(0, 100%, 40%);">-      struct osmo_wqueue wqueue;</span><br><span>   int ifindex;</span><br><span>         bool if_running;</span><br><span style="color: hsl(120, 100%, 40%);">+      /* backlog queue for AF_PACKET / ENOBUFS handling (see OS#4993) */</span><br><span style="color: hsl(120, 100%, 40%);">+    struct {</span><br><span style="color: hsl(120, 100%, 40%);">+              /* file-descriptor for AF_PACKET socket */</span><br><span style="color: hsl(120, 100%, 40%);">+            struct osmo_fd ofd;</span><br><span style="color: hsl(120, 100%, 40%);">+           /* list of msgb (backlog) */</span><br><span style="color: hsl(120, 100%, 40%);">+          struct llist_head list;</span><br><span style="color: hsl(120, 100%, 40%);">+               /* timer to trigger next attempt of AF_PACKET write */</span><br><span style="color: hsl(120, 100%, 40%);">+                struct osmo_timer_list timer;</span><br><span style="color: hsl(120, 100%, 40%);">+         /* re-try after that many micro-seconds */</span><br><span style="color: hsl(120, 100%, 40%);">+            uint32_t retry_us;</span><br><span style="color: hsl(120, 100%, 40%);">+    } backlog;</span><br><span> };</span><br><span> </span><br><span> struct priv_vc {</span><br><span>@@ -148,7 +168,7 @@</span><br><span>       OSMO_ASSERT(llist_empty(&bind->nsvc));</span><br><span> </span><br><span>    osmo_fr_link_free(priv->link);</span><br><span style="color: hsl(0, 100%, 40%);">-       osmo_fd_close(&priv->wqueue.bfd);</span><br><span style="color: hsl(120, 100%, 40%);">+      osmo_fd_close(&priv->backlog.ofd);</span><br><span>    talloc_free(priv);</span><br><span> }</span><br><span> </span><br><span>@@ -200,15 +220,21 @@</span><br><span> }</span><br><span> </span><br><span> /* PDU from the network interface towards the fr layer (upwards) */</span><br><span style="color: hsl(0, 100%, 40%);">-static int handle_netif_read(struct osmo_fd *bfd)</span><br><span style="color: hsl(120, 100%, 40%);">+static int fr_netif_ofd_cb(struct osmo_fd *bfd, uint32_t what)</span><br><span> {</span><br><span>    struct gprs_ns2_vc_bind *bind = bfd->data;</span><br><span>        struct priv_bind *priv = bind->priv;</span><br><span style="color: hsl(0, 100%, 40%);">- struct msgb *msg = msgb_alloc(NS_ALLOC_SIZE, "Gb/NS/FR/GRE Rx");</span><br><span style="color: hsl(120, 100%, 40%);">+    struct msgb *msg;</span><br><span>    struct sockaddr_ll sll;</span><br><span>      socklen_t sll_len = sizeof(sll);</span><br><span>     int rc = 0;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+       /* we only handle read here. write to AF_PACKET sockets cannot be triggered</span><br><span style="color: hsl(120, 100%, 40%);">+    * by select or poll, see OS#4995 */</span><br><span style="color: hsl(120, 100%, 40%);">+  if (!(what & OSMO_FD_READ))</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%);">+   msg = msgb_alloc(NS_ALLOC_SIZE, "Gb/NS/FR/GRE Rx");</span><br><span>        if (!msg)</span><br><span>            return -ENOMEM;</span><br><span> </span><br><span>@@ -245,18 +271,35 @@</span><br><span>  return rc;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static int handle_netif_write(struct osmo_fd *ofd, struct msgb *msg)</span><br><span style="color: hsl(120, 100%, 40%);">+static int fr_netif_write_one(struct gprs_ns2_vc_bind *bind, struct msgb *msg)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-    struct gprs_ns2_vc_bind *bind = ofd->data;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct priv_bind *priv = bind->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+       unsigned int len = msgb_length(msg);</span><br><span>         int rc;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     rc = write(ofd->fd, msgb_data(msg), msgb_length(msg));</span><br><span style="color: hsl(0, 100%, 40%);">-       /* write_queue expects a "-errno" type return value in case of failure */</span><br><span style="color: hsl(0, 100%, 40%);">-     if (rc == -1) {</span><br><span style="color: hsl(0, 100%, 40%);">-         LOGBIND(bind, LOGL_ERROR, "error during write to AF_PACKET: %s\n", strerror(errno));</span><br><span style="color: hsl(0, 100%, 40%);">-          return -errno;</span><br><span style="color: hsl(0, 100%, 40%);">-  } else</span><br><span style="color: hsl(0, 100%, 40%);">-          return rc;</span><br><span style="color: hsl(120, 100%, 40%);">+    /* estimate the retry time based on the data rate it takes to transmit */</span><br><span style="color: hsl(120, 100%, 40%);">+     priv->backlog.retry_us = (BIT_DURATION_NS * 8 * len) / 1000;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     rc = write(priv->backlog.ofd.fd, msgb_data(msg), len);</span><br><span style="color: hsl(120, 100%, 40%);">+     if (rc == len) {</span><br><span style="color: hsl(120, 100%, 40%);">+              msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+               return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     } else if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+               /* don't free, the caller might want to re-transmit */</span><br><span style="color: hsl(120, 100%, 40%);">+            switch (errno) {</span><br><span style="color: hsl(120, 100%, 40%);">+              case EAGAIN:</span><br><span style="color: hsl(120, 100%, 40%);">+          case ENOBUFS:</span><br><span style="color: hsl(120, 100%, 40%);">+                 return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+                default:</span><br><span style="color: hsl(120, 100%, 40%);">+                      LOGBIND(bind, LOGL_ERROR, "error during write to AF_PACKET: %s\n", strerror(errno));</span><br><span style="color: hsl(120, 100%, 40%);">+                        return -errno;</span><br><span style="color: hsl(120, 100%, 40%);">+                }</span><br><span style="color: hsl(120, 100%, 40%);">+     } else {</span><br><span style="color: hsl(120, 100%, 40%);">+              /* short write */</span><br><span style="color: hsl(120, 100%, 40%);">+             LOGBIND(bind, LOGL_ERROR, "short write on AF_PACKET: %d < %d\n", rc, len);</span><br><span style="color: hsl(120, 100%, 40%);">+               msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+               return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     }</span><br><span> }</span><br><span> </span><br><span> /*! determine if given bind is for FR-GRE encapsulation. */</span><br><span>@@ -274,16 +317,81 @@</span><br><span>    return osmo_fr_tx_dlc(msg);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define LMI_Q933A_DLCI 0</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* enqueue to backlog (LMI, signaling) or drop (userdata msg) */</span><br><span style="color: hsl(120, 100%, 40%);">+static void backlog_enqueue_or_free(struct priv_bind *priv, struct msgb *msg)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+  uint8_t dlci = msg->data[0];</span><br><span style="color: hsl(120, 100%, 40%);">+       uint8_t ns_pdu_type;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+        if (msgb_length(msg) < 1)</span><br><span style="color: hsl(120, 100%, 40%);">+          goto out_free;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+      /* we want to enqueue only Q.933 LMI traffic or NS signaling; NOT user traffic */</span><br><span style="color: hsl(120, 100%, 40%);">+     if (dlci == LMI_Q933A_DLCI) {</span><br><span style="color: hsl(120, 100%, 40%);">+         /* enqueue Q.933 LMI at head of queue */</span><br><span style="color: hsl(120, 100%, 40%);">+              llist_add(&msg->list, &priv->backlog.list);</span><br><span style="color: hsl(120, 100%, 40%);">+             osmo_timer_schedule(&priv->backlog.timer, 0, priv->backlog.retry_us);</span><br><span style="color: hsl(120, 100%, 40%);">+               return;</span><br><span style="color: hsl(120, 100%, 40%);">+       }</span><br><span style="color: hsl(120, 100%, 40%);">+     if (msgb_length(msg) < 3)</span><br><span style="color: hsl(120, 100%, 40%);">+          goto out_free;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+      ns_pdu_type = msg->data[2];</span><br><span style="color: hsl(120, 100%, 40%);">+        if (ns_pdu_type != NS_PDUT_UNITDATA) {</span><br><span style="color: hsl(120, 100%, 40%);">+                /* enqueue NS signaling traffic at tail of queue */</span><br><span style="color: hsl(120, 100%, 40%);">+           llist_add_tail(&msg->list, &priv->backlog.list);</span><br><span style="color: hsl(120, 100%, 40%);">+                osmo_timer_schedule(&priv->backlog.timer, 0, priv->backlog.retry_us);</span><br><span style="color: hsl(120, 100%, 40%);">+               return;</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%);">+out_free:</span><br><span style="color: hsl(120, 100%, 40%);">+        msgb_free(msg);</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%);">+static void fr_backlog_timer_cb(void *data)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+  struct gprs_ns2_vc_bind *bind = data;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct priv_bind *priv = bind->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+       int i, rc;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+  /* attempt to send up to 10 messages in every timer */</span><br><span style="color: hsl(120, 100%, 40%);">+        for (i = 0; i < 10; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+         struct msgb *msg = msgb_dequeue(&priv->backlog.list);</span><br><span style="color: hsl(120, 100%, 40%);">+          if (!msg)</span><br><span style="color: hsl(120, 100%, 40%);">+                     break;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+              rc = fr_netif_write_one(bind, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+           if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+                      /* re-add at head of list */</span><br><span style="color: hsl(120, 100%, 40%);">+                  llist_add(&msg->list, &priv->backlog.list);</span><br><span style="color: hsl(120, 100%, 40%);">+                     break;</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%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   /* re-start timer if we still have data in the queue */</span><br><span style="color: hsl(120, 100%, 40%);">+       if (!llist_empty(&priv->backlog.list))</span><br><span style="color: hsl(120, 100%, 40%);">+         osmo_timer_schedule(&priv->backlog.timer, 0, priv->backlog.retry_us);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* PDU from the frame relay layer towards the network interface (downwards) */</span><br><span> int fr_tx_cb(void *data, struct msgb *msg)</span><br><span> {</span><br><span>    struct gprs_ns2_vc_bind *bind = data;</span><br><span>        struct priv_bind *priv = bind->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+       int rc;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     if (osmo_wqueue_enqueue(&priv->wqueue, msg)) {</span><br><span style="color: hsl(0, 100%, 40%);">-           LOGBIND(bind, LOGL_ERROR, "frame relay %s: failed to enqueue message\n", priv->netif);</span><br><span style="color: hsl(0, 100%, 40%);">-             msgb_free(msg);</span><br><span style="color: hsl(0, 100%, 40%);">-         return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+       if (llist_empty(&priv->backlog.list)) {</span><br><span style="color: hsl(120, 100%, 40%);">+                /* attempt to transmit right now */</span><br><span style="color: hsl(120, 100%, 40%);">+           rc = fr_netif_write_one(bind, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+           if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+                      /* enqueue to backlog in case it fails */</span><br><span style="color: hsl(120, 100%, 40%);">+                     backlog_enqueue_or_free(priv, msg);</span><br><span style="color: hsl(120, 100%, 40%);">+           }</span><br><span style="color: hsl(120, 100%, 40%);">+     } else {</span><br><span style="color: hsl(120, 100%, 40%);">+              /* enqueue to backlog */</span><br><span style="color: hsl(120, 100%, 40%);">+              backlog_enqueue_or_free(priv, msg);</span><br><span>  }</span><br><span> </span><br><span>        return 0;</span><br><span>@@ -643,11 +751,11 @@</span><br><span>    rc = open_socket(priv->ifindex, bind);</span><br><span>    if (rc < 0)</span><br><span>               goto err_fr;</span><br><span style="color: hsl(0, 100%, 40%);">-    osmo_wqueue_init(&priv->wqueue, 10);</span><br><span style="color: hsl(0, 100%, 40%);">-     priv->wqueue.write_cb = handle_netif_write;</span><br><span style="color: hsl(0, 100%, 40%);">-  priv->wqueue.read_cb = handle_netif_read;</span><br><span style="color: hsl(0, 100%, 40%);">-    osmo_fd_setup(&priv->wqueue.bfd, rc, OSMO_FD_READ, osmo_wqueue_bfd_cb, bind, 0);</span><br><span style="color: hsl(0, 100%, 40%);">- rc = osmo_fd_register(&priv->wqueue.bfd);</span><br><span style="color: hsl(120, 100%, 40%);">+      INIT_LLIST_HEAD(&priv->backlog.list);</span><br><span style="color: hsl(120, 100%, 40%);">+  priv->backlog.retry_us = 2500; /* start with some non-zero value; this corrsponds to 496 bytes */</span><br><span style="color: hsl(120, 100%, 40%);">+  osmo_timer_setup(&priv->backlog.timer, fr_backlog_timer_cb, bind);</span><br><span style="color: hsl(120, 100%, 40%);">+     osmo_fd_setup(&priv->backlog.ofd, rc, OSMO_FD_READ, fr_netif_ofd_cb, bind, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = osmo_fd_register(&priv->backlog.ofd);</span><br><span>    if (rc < 0)</span><br><span>               goto err_fd;</span><br><span> </span><br><span>@@ -667,7 +775,7 @@</span><br><span>       return rc;</span><br><span> </span><br><span> err_fd:</span><br><span style="color: hsl(0, 100%, 40%);">-       close(priv->wqueue.bfd.fd);</span><br><span style="color: hsl(120, 100%, 40%);">+        close(priv->backlog.ofd.fd);</span><br><span> err_fr:</span><br><span>   osmo_fr_link_free(fr_link);</span><br><span> err_priv:</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/22555">change 22555</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/libosmocore/+/22555"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4f79a246236c94175ceffa5f3f556c8931c6bc62 </div>
<div style="display:none"> Gerrit-Change-Number: 22555 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>