tcp: simplify readahead

Do not bother to preserve segment boundaries in the tcp
readahead queues.

* Avoid wasting the tail IOB space for each segments.
  Instead, pack the newly received data into the tail space
  of the last IOB. Also, advertise the tail space as
  a part of the window.

* Use IOB chain directly. Eliminate IOB queue overhead.

* Allow to accept only a part of a segment.

* This change improves the memory efficiency.
  And probably more importantly, allows less-confusing
  recv window advertisement behavior.
  Previously, even when we sdvertise N bytes window,
  we often couldn't actually accept N bytes. Depending on
  the segment size, IOB configurations, and the peer tcp,
  it was causing the poor behavior.
  Also, the previous code was moving the right edge of the
  window back and forth too often, even when nothing in
  the system was competing on the IOBs. Shrinking the
  window that way is a kinda well known recipe to confuse
  the peer stack.

net/tcp/tcp.h

@@ -209,7 +209,7 @@ struct tcp_conn_s
    *               where the TCP/IP read-ahead data is retained.
    */
 
-  struct iob_queue_s readahead;   /* Read-ahead buffering */
+  struct iob_s *readahead;   /* Read-ahead buffering */
 
 #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
   /* Write buffering

net/tcp/tcp_callback.c

@@ -27,6 +27,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <debug.h>
+#include <assert.h>
 
 #include <nuttx/mm/iob.h>
 #include <nuttx/net/netconfig.h>
@@ -234,62 +235,82 @@ uint16_t tcp_datahandler(FAR struct tcp_conn_s *conn, FAR uint8_t *buffer,
                          uint16_t buflen)
 {
   FAR struct iob_s *iob;
-  bool throttled = true;
+  uint16_t copied = 0;
   int ret;
+  unsigned int i;
 
   /* Try to allocate I/O buffers and copy the data into them
    * without waiting (and throttling as necessary).
    */
 
-  while (true)
+  iob = conn->readahead;
+  for (i = 0; i < 2; i++)
     {
-      iob = iob_tryalloc(throttled, IOBUSER_NET_TCP_READAHEAD);
+      bool throttled = i == 0; /* try throttled=true first */
+
+      if (!throttled)
+        {
+#if CONFIG_IOB_THROTTLE > 0
+          if (conn->readahead != NULL)
+            {
+              ninfo("Do not use throttled=false because of "
+                    "non-empty readahead\n");
+              break;
+            }
+#else
+          break;
+#endif
+        }
+
+      if (iob == NULL)
+        {
+          iob = iob_tryalloc(throttled, IOBUSER_NET_TCP_READAHEAD);
+          if (iob == NULL)
+            {
+              continue;
+            }
+
+          iob->io_pktlen = 0;
+        }
+
       if (iob != NULL)
         {
-          ret = iob_trycopyin(iob, buffer, buflen, 0, throttled,
+          uint32_t olen = iob->io_pktlen;
+
+          ret = iob_trycopyin(iob, buffer + copied, buflen - copied,
+                              olen, throttled,
                               IOBUSER_NET_TCP_READAHEAD);
+          copied += iob->io_pktlen - olen;
           if (ret < 0)
             {
               /* On a failure, iob_copyin return a negated error value but
                * does not free any I/O buffers.
                */
 
-              iob_free_chain(iob, IOBUSER_NET_TCP_READAHEAD);
-              iob = NULL;
+              continue;
             }
         }
 
-#if CONFIG_IOB_THROTTLE > 0
-      if (iob == NULL && throttled && IOB_QEMPTY(&conn->readahead))
-        {
-          /* Fallback out of the throttled entry */
-
-          throttled = false;
-          continue;
-        }
-#endif
-
       break;
     }
 
+  DEBUGASSERT(conn->readahead == iob || conn->readahead == NULL);
   if (iob == NULL)
     {
       nerr("ERROR: Failed to create new I/O buffer chain\n");
+      DEBUGASSERT(copied == 0);
       return 0;
     }
 
-  /* Add the new I/O buffer chain to the tail of the read-ahead queue (again
-   * without waiting).
-   */
-
-  ret = iob_tryadd_queue(iob, &conn->readahead);
-  if (ret < 0)
+  if (copied == 0)
     {
-      nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret);
-      iob_free_chain(iob, IOBUSER_NET_TCP_READAHEAD);
+      nerr("ERROR: Failed to append new I/O buffer\n");
+      DEBUGASSERT(conn->readahead == iob);
       return 0;
     }
 
+  conn->readahead = iob;
+
 #ifdef CONFIG_NET_TCP_NOTIFIER
   /* Provide notification(s) that additional TCP read-ahead data is
    * available.
@@ -298,8 +319,8 @@ uint16_t tcp_datahandler(FAR struct tcp_conn_s *conn, FAR uint8_t *buffer,
   tcp_readahead_signal(conn);
 #endif
 
-  ninfo("Buffered %d bytes\n", buflen);
-  return buflen;
+  ninfo("Buffered %" PRIu16 " bytes\n", copied);
+  return copied;
 }
 
 #endif /* NET_TCP_HAVE_STACK */

net/tcp/tcp_conn.c

@@ -726,7 +726,8 @@ void tcp_free(FAR struct tcp_conn_s *conn)
 
   /* Release any read-ahead buffers attached to the connection */
 
-  iob_free_queue(&conn->readahead, IOBUSER_NET_TCP_READAHEAD);
+  iob_free_chain(conn->readahead, IOBUSER_NET_TCP_READAHEAD);
+  conn->readahead = NULL;
 
 #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
   /* Release any write buffers attached to the connection */
@@ -970,7 +971,7 @@ FAR struct tcp_conn_s *tcp_alloc_accept(FAR struct net_driver_s *dev,
 
       /* Initialize the list of TCP read-ahead buffers */
 
-      IOB_QINIT(&conn->readahead);
+      conn->readahead = NULL;
 
 #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
       /* Initialize the write buffer lists */
@@ -1244,7 +1245,7 @@ int tcp_connect(FAR struct tcp_conn_s *conn, FAR const struct sockaddr *addr)
 
   /* Initialize the list of TCP read-ahead buffers */
 
-  IOB_QINIT(&conn->readahead);
+  conn->readahead = NULL;
 
 #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
   /* Initialize the TCP write buffer lists */

net/tcp/tcp_netpoll.c

@@ -229,7 +229,7 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
 
   /* Check for read data or backlogged connection availability now */
 
-  if (!IOB_QEMPTY(&conn->readahead) || tcp_backlogavailable(conn))
+  if (conn->readahead != NULL || tcp_backlogavailable(conn))
     {
       /* Normal data may be read without blocking. */
 

net/tcp/tcp_notifier.c

@@ -78,7 +78,7 @@ int tcp_readahead_notifier_setup(worker_t worker,
    * setting up the notification.
    */
 
-  if (conn->readahead.qh_head != NULL)
+  if (conn->readahead != NULL)
     {
       return 0;
     }

net/tcp/tcp_recvfrom.c

@@ -255,7 +255,7 @@ static inline void tcp_readahead(struct tcp_recvfrom_s *pstate)
    * buffer.
    */
 
-  while ((iob = iob_peek_queue(&conn->readahead)) != NULL &&
+  while ((iob = conn->readahead) != NULL &&
           pstate->ir_buflen > 0)
     {
       DEBUGASSERT(iob->io_pktlen > 0);
@@ -279,29 +279,19 @@ static inline void tcp_readahead(struct tcp_recvfrom_s *pstate)
 
       if (recvlen >= iob->io_pktlen)
         {
-          FAR struct iob_s *tmp;
-
-          /* Remove the I/O buffer chain from the head of the read-ahead
-           * buffer queue.
-           */
-
-          tmp = iob_remove_queue(&conn->readahead);
-          DEBUGASSERT(tmp == iob);
-          UNUSED(tmp);
-
-          /* And free the I/O buffer chain */
+          /* Free free the I/O buffer chain */
 
           iob_free_chain(iob, IOBUSER_NET_TCP_READAHEAD);
+          conn->readahead = NULL;
         }
       else
         {
           /* The bytes that we have received from the head of the I/O
-           * buffer chain (probably changing the head of the I/O
-           * buffer queue).
+           * buffer chain.
            */
 
-          iob_trimhead_queue(&conn->readahead, recvlen,
-                             IOBUSER_NET_TCP_READAHEAD);
+          conn->readahead = iob_trimhead(iob, recvlen,
+                                         IOBUSER_NET_TCP_READAHEAD);
         }
     }
 }

net/tcp/tcp_recvwindow.c

@@ -97,23 +97,41 @@ static uint16_t tcp_maxrcvwin(FAR struct tcp_conn_s *conn)
 uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
                             FAR struct tcp_conn_s *conn)
 {
+  uint16_t tailroom;
   uint16_t recvwndo;
   int niob_avail;
-  int nqentry_avail;
 
   /* Update the TCP received window based on read-ahead I/O buffer
-   * and IOB chain availability.  At least one queue entry is required.
-   * If one queue entry is available, then the amount of read-ahead
+   * and IOB chain availability.
+   * The amount of read-ahead
    * data that can be buffered is given by the number of IOBs available
    * (ignoring competition with other IOB consumers).
    */
 
-  niob_avail    = iob_navail(true);
-  nqentry_avail = iob_qentry_navail();
+  if (conn->readahead != NULL)
+    {
+      /* XXX move this to mm/iob */
+
+      const struct iob_s *iob;
+
+      iob = conn->readahead;
+      while (iob->io_flink != NULL)
+        {
+          iob = iob->io_flink;
+        }
+
+      tailroom = CONFIG_IOB_BUFSIZE - (iob->io_offset + iob->io_len);
+    }
+  else
+    {
+      tailroom = 0;
+    }
+
+  niob_avail = iob_navail(true);
 
   /* Is there a a queue entry and IOBs available for read-ahead buffering? */
 
-  if (nqentry_avail > 0 && niob_avail > 0)
+  if (niob_avail > 0)
     {
       uint32_t rwnd;
 
@@ -123,8 +141,7 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
        * TCP buffering.
        *
        * Assume that all of the available IOBs are can be used for buffering
-       * on this connection.  Also assume that at least one chain is
-       * available concatenate the IOBs.
+       * on this connection.
        *
        * REVISIT:  In an environment with multiple, active read-ahead TCP
        * sockets (and perhaps multiple network devices) or if there are
@@ -133,7 +150,7 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
        * buffering for this connection.
        */
 
-      rwnd = (niob_avail * CONFIG_IOB_BUFSIZE);
+      rwnd = tailroom + (niob_avail * CONFIG_IOB_BUFSIZE);
       if (rwnd > UINT16_MAX)
         {
           rwnd = UINT16_MAX;
@@ -144,7 +161,7 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
       recvwndo = (uint16_t)rwnd;
     }
 #if CONFIG_IOB_THROTTLE > 0
-  else if (IOB_QEMPTY(&conn->readahead))
+  else if (conn->readahead == NULL)
     {
       /* Advertise maximum segment size for window edge if here is no
        * available iobs on current "free" connection.
@@ -162,16 +179,16 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
         }
     }
 #endif
-  else /* nqentry_avail == 0 || niob_avail == 0 */
+  else /* niob_avail == 0 */
     {
-      /* No IOB chains or noIOBs are available.
+      /* No IOBs are available.
        * Advertise the edge of window to zero.
        *
        * NOTE:  If no IOBs are available, then the next packet will be
        * lost if there is no listener on the connection.
        */
 
-      recvwndo = 0;
+      recvwndo = tailroom;
     }
 
   return recvwndo;