TCP write buffering: Extend and fix some buffer dumping logic

9 files changed, 61 insertions, 46 deletions

diff --git a/nuttx/include/nuttx/net/iob.h b/nuttx/include/nuttx/net/iob.h
index 34a59cc00..1a18ac934 100644
--- a/nuttx/include/nuttx/net/iob.h
+++ b/nuttx/include/nuttx/net/iob.h
@@ -306,7 +306,7 @@ int iob_contig(FAR struct iob_s *iob, unsigned int len);
 #ifdef CONFIG_DEBUG
 void iob_dump(FAR const char *msg, FAR struct iob_s *iob);
 #else
-#  define tcp_writebuffer_dump(wrb)
+#  define iob_dump(wrb)
 #endif
 
 #endif /* _INCLUDE_NUTTX_NET_IOB_H */
diff --git a/nuttx/include/nuttx/net/uip/uip-tcp.h b/nuttx/include/nuttx/net/uip/uip-tcp.h
index 51b20ee0a..7f10e204b 100644
--- a/nuttx/include/nuttx/net/uip/uip-tcp.h
+++ b/nuttx/include/nuttx/net/uip/uip-tcp.h
@@ -139,9 +139,9 @@
   do { (wrb)->wb_iob = iob_trimhead((wrb)->wb_iob,(n)); } while (0)
 
 #ifdef CONFIG_DEBUG
-#  define WRB_DUMP(msg,wrb)       tcp_writebuffer_dump(msg,wrb)
+#  define WRB_DUMP(msg,wrb)       tcp_wrbuffer_dump(msg,wrb)
 #else
-#  define WRB_DUMP(mgs,wrb)
+#  define WRB_DUMP(msg,wrb)
 #endif
 #endif
 
diff --git a/nuttx/net/iob/iob_dump.c b/nuttx/net/iob/iob_dump.c
index a4d1cf04a..b56477071 100644
--- a/nuttx/net/iob/iob_dump.c
+++ b/nuttx/net/iob/iob_dump.c
@@ -81,50 +81,18 @@
 void iob_dump(FAR const char *msg, FAR struct iob_s *iob)
 {
   FAR struct iob_s *head = iob;
-  FAR const uint8_t *buffer;
   uint8_t data[32];
   unsigned int nbytes;
   unsigned int i;
   unsigned int j;
-  int len;
 
-  message("%s: IOB=%p pktlen=%d\n", msg, head, head->io_pktlen);
+  message("%s: iob=%p pktlen=%d\n", msg, head, head->io_pktlen);
 
-  buffer = &iob->io_data[iob->io_offset];
-  len    = iob->io_len;
-
-  for (i = 0; i < head->io_pktlen && iob; i += 32)
+  for (i = 0; i < head->io_pktlen; i += 32)
     {
       /* Copy 32-bytes into our local buffer */
 
-      for (nbytes = 0; nbytes < 32; nbytes++)
-        {
-          data[nbytes] = *buffer++;
-
-          /* If we have exhausted the data in this I/O buffer,
-           * then skip to the next I/O buffer in the chain.
-           */
-
-          if (--len <= 0)
-           {
-             iob = iob->io_flink;
-             if (!iob)
-               {
-                 /* Ooops... we are at the end of the chain.
-                  * break out with iob = NULL, len == 0, and
-                  * nbytes <= 32.
-                  */
-
-                 len = 0;
-                 break;
-               }
-
-             /* Get the data from the next I/O buffer in the chain */
-
-             buffer = &iob->io_data[iob->io_offset];
-             len = iob->io_len;
-           }
-        }
+      nbytes = iob_copyout(data, iob, 32, i);
 
       /* Make sure that we have something to print */
 
@@ -140,7 +108,7 @@ void iob_dump(FAR const char *msg, FAR struct iob_s *iob)
 
               if (i + j < head->io_pktlen)
                 {
-                  message("%02x", buffer[j]);
+                  message("%02x", data[j]);
                 }
               else
                 {
@@ -158,9 +126,9 @@ void iob_dump(FAR const char *msg, FAR struct iob_s *iob)
 
               if (i + j < head->io_pktlen)
                 {
-                  if (buffer[j] >= 0x20 && buffer[j] < 0x7f)
+                  if (data[j] >= 0x20 && data[j] < 0x7f)
                     {
-                      message("%c", buffer[j]);
+                      message("%c", data[j]);
                     }
                   else
                     {
diff --git a/nuttx/net/iob/iob_free.c b/nuttx/net/iob/iob_free.c
index 511c6438f..68c7fb264 100644
--- a/nuttx/net/iob/iob_free.c
+++ b/nuttx/net/iob/iob_free.c
@@ -48,6 +48,7 @@
 
 #include <semaphore.h>
 #include <assert.h>
+#include <debug.h>
 
 #include <nuttx/arch.h>
 #include <nuttx/net/iob.h>
@@ -88,6 +89,9 @@ FAR struct iob_s *iob_free(FAR struct iob_s *iob)
   FAR struct iob_s *next = iob->io_flink;
   irqstate_t flags;
 
+  nllvdbg("iob=%p io_pktlen=%u io_len=%u next=%p\n",
+          iob, iob->io_pktlen, iob->io_len, next);
+
   /* Copy the data that only exists in the head of a I/O buffer chain into
    * the next entry.
    */
@@ -114,6 +118,9 @@ FAR struct iob_s *iob_free(FAR struct iob_s *iob)
           next->io_pktlen = 0;
           DEBUGASSERT(next->io_len == 0 && next->io_flink == NULL);
         }
+
+      nllvdbg("next=%p io_pktlen=%u io_len=%u\n",
+               next, next->io_pktlen, next->io_len);
     }
 
   /* Free the I/O buffer by adding it to the head of the free list. We don't
diff --git a/nuttx/net/net_send_buffered.c b/nuttx/net/net_send_buffered.c
index f8e943daa..93220b64e 100644
--- a/nuttx/net/net_send_buffered.c
+++ b/nuttx/net/net_send_buffered.c
@@ -77,6 +77,16 @@
 
 #define TCPBUF ((struct uip_tcpip_hdr *)&dev->d_buf[UIP_LLH_LEN])
 
+/* Debug */
+
+#ifdef CONFIG_NET_TCP_WRBUFFER_DUMP
+#  define BUF_DUMP(msg,buf,len) lib_dumpbuffer(msg,buf,len)
+#else
+#  define BUF_DUMP(msg,buf,len)
+#  undef  WRB_DUMP
+#  define WRB_DUMP(msg,wrb)
+#endif
+
 /****************************************************************************
  * Private Types
  ****************************************************************************/
@@ -255,6 +265,8 @@ static uint16_t send_interrupt(FAR struct uip_driver_s *dev, FAR void *pvconn,
 
               if (ackno >= lastseq)
                 {
+                  nllvdbg("ACK: wrb=%p Freeing write buffer\n", wrb);
+
                   /* Yes... Remove the write buffer from ACK waiting queue */
 
                   sq_rem(entry, &conn->unacked_q);
@@ -296,7 +308,7 @@ static uint16_t send_interrupt(FAR struct uip_driver_s *dev, FAR void *pvconn,
         {
           uint32_t nacked;
 
-          /* Get the sequence number at the end of the data */
+          /* Number of bytes that were ACKed */
 
           nacked = ackno - WRB_SEQNO(wrb);
           if (nacked > WRB_SENT(wrb))
@@ -574,6 +586,8 @@ static uint16_t send_interrupt(FAR struct uip_driver_s *dev, FAR void *pvconn,
             {
               FAR struct tcp_wrbuffer_s *tmp;
 
+              nllvdbg("ACK: wrb=%p Move to unacked_q\n", wrb);
+
               tmp = (FAR struct tcp_wrbuffer_s *)sq_remfirst(&conn->write_q);
               DEBUGASSERT(tmp == wrb);
               UNUSED(tmp);
@@ -686,6 +700,10 @@ ssize_t psock_send(FAR struct socket *psock, FAR const void *buf, size_t len,
       goto errout;
     }
 
+  /* Dump the incoming buffer */
+
+  BUF_DUMP("psock_send", buf, len);
+
   /* Set the socket state to sending */
 
   psock->s_flags = _SS_SETSTATE(psock->s_flags, _SF_SEND);
@@ -734,6 +752,10 @@ ssize_t psock_send(FAR struct socket *psock, FAR const void *buf, size_t len,
               WRB_NRTX(wrb)  = 0;
               WRB_COPYIN(wrb, (FAR uint8_t *)buf, len);
 
+              /* Dump I/O buffer chain */
+
+              WRB_DUMP("I/O buffer chain", wrb);
+
               /* send_interrupt() will send data in FIFO order from the
                * conn->write_q
                */
diff --git a/nuttx/net/tcp/Kconfig b/nuttx/net/tcp/Kconfig
index 2fdb351f2..5989d3d90 100644
--- a/nuttx/net/tcp/Kconfig
+++ b/nuttx/net/tcp/Kconfig
@@ -114,6 +114,16 @@ config NET_TCP_WRBUFFER_DEBUG
 		write buffer logic and do not want to get overloaded with other
 		network-related debug output.
 
+config NET_TCP_WRBUFFER_DUMP
+	bool "Force write buffer dump"
+	default n
+	depends on DEBUG_NET || NET_TCP_WRBUFFER_DEBUG
+	select NET_IOB_DEBUG
+	---help---
+		Dump the contents of the write buffers.  You do not want to do this
+		unless you really want to analyze the write buffer transfers in
+		detail.
+
 endif # NET_TCP_WRITE_BUFFERS
 
 config NET_TCP_RECVDELAY
diff --git a/nuttx/net/tcp/tcp.h b/nuttx/net/tcp/tcp.h
index a18b78a6c..8da77d61f 100644
--- a/nuttx/net/tcp/tcp.h
+++ b/nuttx/net/tcp/tcp.h
@@ -120,7 +120,7 @@ void tcp_wrbuffer_release(FAR struct tcp_wrbuffer_s *wrb);
 #endif /* CONFIG_NET_TCP_WRITE_BUFFERS */
 
 /****************************************************************************
- * Function: tcp_writebuffer_dump
+ * Function: tcp_wrbuffer_dump
  *
  * Description:
  *   Dump the contents of a write buffer.
@@ -129,9 +129,9 @@ void tcp_wrbuffer_release(FAR struct tcp_wrbuffer_s *wrb);
 
 #ifdef CONFIG_NET_TCP_WRITE_BUFFERS
 #ifdef CONFIG_DEBUG
-void tcp_writebuffer_dump(FAR const char *msg, FAR struct tcp_wrbuffer_s *wrb);
+void tcp_wrbuffer_dump(FAR const char *msg, FAR struct tcp_wrbuffer_s *wrb);
 #else
-#  define tcp_writebuffer_dump(msg,wrb)
+#  define tcp_wrbuffer_dump(msg,wrb)
 #endif
 #endif /* CONFIG_NET_TCP_WRITE_BUFFERS */
 
diff --git a/nuttx/net/tcp/tcp_wrbuffer_dump.c b/nuttx/net/tcp/tcp_wrbuffer_dump.c
index 910eeb304..c7ac3d657 100644
--- a/nuttx/net/tcp/tcp_wrbuffer_dump.c
+++ b/nuttx/net/tcp/tcp_wrbuffer_dump.c
@@ -81,7 +81,7 @@
 
 void tcp_wrbuffer_dump(FAR const char *msg, FAR struct tcp_wrbuffer_s *wrb)
 {
-  message("%s: WRB=%p segno=%d sent=%d nrtx=%d\n",
+  message("%s: wrb=%p segno=%d sent=%d nrtx=%d\n",
           msg, wrb, WRB_SEQNO(wrb), WRB_SENT(wrb), WRB_NRTX(wrb));
   iob_dump("I/O Buffer Chain", WRB_IOB(wrb));
 }
diff --git a/nuttx/net/uip/uip_iobsend.c b/nuttx/net/uip/uip_iobsend.c
index d5ed4072b..c3bec5f99 100644
--- a/nuttx/net/uip/uip_iobsend.c
+++ b/nuttx/net/uip/uip_iobsend.c
@@ -97,8 +97,16 @@ void uip_iobsend(FAR struct uip_driver_s *dev, FAR struct iob_s *iob,
 {
   DEBUGASSERT(dev && len > 0 && len < CONFIG_NET_BUFSIZE);
 
+  /* Copy the data from the I/O buffer chain to the device buffer */
+
   iob_copyout(dev->d_snddata, iob, len, offset);
   dev->d_sndlen = len;
+
+#ifdef CONFIG_NET_TCP_WRBUFFER_DUMP
+  /* Dump the outgoing device buffer */
+
+  lib_dumpbuffer("uip_iobsend", dev->d_snddata, len);
+#endif
 }
 
 #endif /* CONFIG_NET_IOB */