TCP write buffering: Fix an error in the trim logic

22 files changed, 164 insertions, 21 deletions

diff --git a/nuttx/include/nuttx/net/uip/uip-tcp.h b/nuttx/include/nuttx/net/uip/uip-tcp.h
index a2234c60d..51b20ee0a 100644
--- a/nuttx/include/nuttx/net/uip/uip-tcp.h
+++ b/nuttx/include/nuttx/net/uip/uip-tcp.h
@@ -134,7 +134,9 @@
 #  define WRB_IOB(wrb)            ((wrb)->wb_iob)
 #  define WRB_COPYOUT(wrb,dest,n) (iob_copyout(dest,(wrb)->wb_iob,(n),0))
 #  define WRB_COPYIN(wrb,src,n)   (iob_copyin((wrb)->wb_iob,src,(n),0))
-#  define WRB_TRIM(wrb,n)         (iob_trimhead((wrb)->wb_iob,(n)))
+
+#  define WRB_TRIM(wrb,n) \
+  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)
diff --git a/nuttx/net/iob/Kconfig b/nuttx/net/iob/Kconfig
index 3ae153dac..ea6a851cd 100644
--- a/nuttx/net/iob/Kconfig
+++ b/nuttx/net/iob/Kconfig
@@ -42,4 +42,15 @@ config IOB_NCHAINS
 		I/O buffer chain containers that also carry a payload of usage
 		specific information.
 
+config NET_IOB_DEBUG
+	bool "Force I/O buffer debug"
+	default n
+	depends on DEBUG
+	---help---
+		This option will force debug output from I/O buffer logic,
+		even without network debug output.  This is not normally something
+		that would want to do but is convenient if you are debugging the
+		I/O buffer logic and do not want to get overloaded with other
+		network-related debug output.
+
 endif # NET_IOB
diff --git a/nuttx/net/iob/iob_add_queue.c b/nuttx/net/iob/iob_add_queue.c
index 8ecc556ea..f915b49a0 100644
--- a/nuttx/net/iob/iob_add_queue.c
+++ b/nuttx/net/iob/iob_add_queue.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <assert.h>
 #include <errno.h>
 #include <debug.h>
diff --git a/nuttx/net/iob/iob_alloc.c b/nuttx/net/iob/iob_alloc.c
index 68e3fb602..be6491dbb 100644
--- a/nuttx/net/iob/iob_alloc.c
+++ b/nuttx/net/iob/iob_alloc.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <semaphore.h>
 #include <assert.h>
 
diff --git a/nuttx/net/iob/iob_alloc_qentry.c b/nuttx/net/iob/iob_alloc_qentry.c
index 21314c011..43813a269 100644
--- a/nuttx/net/iob/iob_alloc_qentry.c
+++ b/nuttx/net/iob/iob_alloc_qentry.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <semaphore.h>
 #include <assert.h>
 
diff --git a/nuttx/net/iob/iob_clone.c b/nuttx/net/iob/iob_clone.c
index 9d4b849f8..ce83400d4 100644
--- a/nuttx/net/iob/iob_clone.c
+++ b/nuttx/net/iob/iob_clone.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 #include <assert.h>
 #include <errno.h>
diff --git a/nuttx/net/iob/iob_concat.c b/nuttx/net/iob/iob_concat.c
index 6875984ff..68d35c480 100644
--- a/nuttx/net/iob/iob_concat.c
+++ b/nuttx/net/iob/iob_concat.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 
 #include <nuttx/net/iob.h>
diff --git a/nuttx/net/iob/iob_contig.c b/nuttx/net/iob/iob_contig.c
index ff9831a21..6af1b9c99 100755
--- a/nuttx/net/iob/iob_contig.c
+++ b/nuttx/net/iob/iob_contig.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 #include <assert.h>
 #include <errno.h>
diff --git a/nuttx/net/iob/iob_copyin.c b/nuttx/net/iob/iob_copyin.c
index 0c9b3f037..fedd37098 100644
--- a/nuttx/net/iob/iob_copyin.c
+++ b/nuttx/net/iob/iob_copyin.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <stdint.h>
 #include <string.h>
 #include <errno.h>
@@ -91,13 +98,14 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src,
   unsigned int ncopy;
   unsigned int avail;
 
+  nllvdbg("iob=%p len=%u offset=%u\n", iob, len, offset);
   DEBUGASSERT(iob && src);
 
   /* The offset must applied to data that is already in the I/O buffer chain */
 
   if (offset > iob->io_pktlen)
     {
-      ndbg("ERROR: offset is past the end of data: %d > %d\n",
+      ndbg("ERROR: offset is past the end of data: %u > %u\n",
            offset, iob->io_pktlen);
       return -ESPIPE;
     }
@@ -123,6 +131,8 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src,
       dest  = &iob->io_data[iob->io_offset + offset];
       avail = iob->io_len - offset;
 
+      nllvdbg("iob=%p avail=%u len=%u next=%p\n", iob, avail, len, next);
+
       /* Will the rest of the copy fit into this buffer, overwriting
        * existing data.
        */
@@ -146,7 +156,7 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src,
 
               /* Yes.. We can extend this buffer to the up to the very end. */
 
-              maxlen  = CONFIG_IOB_BUFSIZE - iob->io_offset;
+              maxlen = CONFIG_IOB_BUFSIZE - iob->io_offset;
 
               /* This is the new buffer length that we need.  Of course,
                * clipped to the maximum possible size in this buffer.
@@ -178,6 +188,8 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src,
       /* Copy from the user buffer to the I/O buffer.  */
 
       memcpy(dest, src, ncopy);
+      nllvdbg("iob=%p, Copy %u bytes, new len=%u\n",
+              iob, ncopy, iob->io_len);
 
       /* Adjust the total length of the copy and the destination address in
        * the user buffer.
@@ -204,6 +216,7 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src,
           /* Add the new, empty I/O buffer to the end of the buffer chain. */
 
           iob->io_flink = next;
+          nllvdbg("iob=%p added to the chain\n", iob);
         }
 
       iob = next;
diff --git a/nuttx/net/iob/iob_copyout.c b/nuttx/net/iob/iob_copyout.c
index 5695eb2e9..2db050d31 100644
--- a/nuttx/net/iob/iob_copyout.c
+++ b/nuttx/net/iob/iob_copyout.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <stdint.h>
 #include <string.h>
 #include <assert.h>
diff --git a/nuttx/net/iob/iob_free.c b/nuttx/net/iob/iob_free.c
index e2c95aae0..511c6438f 100644
--- a/nuttx/net/iob/iob_free.c
+++ b/nuttx/net/iob/iob_free.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <semaphore.h>
 #include <assert.h>
 
diff --git a/nuttx/net/iob/iob_free_chain.c b/nuttx/net/iob/iob_free_chain.c
index 320cdfd07..d09a0ca97 100644
--- a/nuttx/net/iob/iob_free_chain.c
+++ b/nuttx/net/iob/iob_free_chain.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <nuttx/arch.h>
 #include <nuttx/net/iob.h>
 
diff --git a/nuttx/net/iob/iob_free_qentry.c b/nuttx/net/iob/iob_free_qentry.c
index db0f3d78a..3f5a517b9 100644
--- a/nuttx/net/iob/iob_free_qentry.c
+++ b/nuttx/net/iob/iob_free_qentry.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <semaphore.h>
 #include <assert.h>
 
diff --git a/nuttx/net/iob/iob_free_queue.c b/nuttx/net/iob/iob_free_queue.c
index d9af47d6c..f23394d45 100644
--- a/nuttx/net/iob/iob_free_queue.c
+++ b/nuttx/net/iob/iob_free_queue.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <assert.h>
 
 #include <nuttx/net/iob.h>
diff --git a/nuttx/net/iob/iob_initialize.c b/nuttx/net/iob/iob_initialize.c
index 2a9b527c0..319e75b8a 100644
--- a/nuttx/net/iob/iob_initialize.c
+++ b/nuttx/net/iob/iob_initialize.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <stdbool.h>
 #include <semaphore.h>
 
diff --git a/nuttx/net/iob/iob_pack.c b/nuttx/net/iob/iob_pack.c
index eb2c772c4..f129e7357 100644
--- a/nuttx/net/iob/iob_pack.c
+++ b/nuttx/net/iob/iob_pack.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 
 #include <nuttx/net/iob.h>
diff --git a/nuttx/net/iob/iob_remove_queue.c b/nuttx/net/iob/iob_remove_queue.c
index 88752a2e8..e8ee794e3 100644
--- a/nuttx/net/iob/iob_remove_queue.c
+++ b/nuttx/net/iob/iob_remove_queue.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <debug.h>
 
 #include <nuttx/net/iob.h>
diff --git a/nuttx/net/iob/iob_trimhead.c b/nuttx/net/iob/iob_trimhead.c
index 23d8adf8a..6ea684b9a 100644
--- a/nuttx/net/iob/iob_trimhead.c
+++ b/nuttx/net/iob/iob_trimhead.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <assert.h>
 #include <debug.h>
 
@@ -82,23 +89,22 @@
 FAR struct iob_s *iob_trimhead(FAR struct iob_s *iob, unsigned int trimlen)
 {
   uint16_t pktlen;
-  unsigned int len;
 
-  nllvdbg("iob=%p pktlen=%d trimlen=%d\n", iob, iob->io_pktlen, trimlen);
+  nllvdbg("iob=%p trimlen=%d\n", iob, trimlen);
 
   if (iob && trimlen > 0)
     {
       /* Trim from the head of the I/IO buffer chain */
 
       pktlen = iob->io_pktlen;
-      len    = trimlen;
-
-      while (len > 0 && iob != NULL)
+      while (trimlen > 0 && iob != NULL)
         {
           /* Do we trim this entire I/O buffer away? */
 
-          nllvdbg("iob=%p len=%d vs %d\n", iob, iob->io_len, len);
-          if (iob->io_len <= len)
+          nllvdbg("iob=%p io_len=%d pktlen=%d trimlen=%d\n",
+                  iob, iob->io_len, pktlen, trimlen);
+
+          if (iob->io_len <= trimlen)
             {
               FAR struct iob_s *next;
 
@@ -107,7 +113,7 @@ FAR struct iob_s *iob_trimhead(FAR struct iob_s *iob, unsigned int trimlen)
                */
 
               pktlen        -= iob->io_len;
-              len           -= iob->io_len;
+              trimlen       -= iob->io_len;
               iob->io_len    = 0;
               iob->io_offset = 0;
 
@@ -126,8 +132,9 @@ FAR struct iob_s *iob_trimhead(FAR struct iob_s *iob, unsigned int trimlen)
 
               /* Free this entry and set the next I/O buffer as the head */
 
+              nllvdbg("iob=%p: Freeing\n", iob);
               (void)iob_free(iob);
-              iob   = next;
+              iob = next;
             }
           else
             {
@@ -135,10 +142,10 @@ FAR struct iob_s *iob_trimhead(FAR struct iob_s *iob, unsigned int trimlen)
                * stop the trim.
                */
 
-              pktlen         -= len;
-              iob->io_len    -= len;
-              iob->io_offset += len;
-              len             = 0;
+              pktlen         -= trimlen;
+              iob->io_len    -= trimlen;
+              iob->io_offset += trimlen;
+              trimlen         = 0;
             }
         }
 
diff --git a/nuttx/net/iob/iob_trimtail.c b/nuttx/net/iob/iob_trimtail.c
index 61f536644..c87de712f 100644
--- a/nuttx/net/iob/iob_trimtail.c
+++ b/nuttx/net/iob/iob_trimtail.c
@@ -39,6 +39,13 @@
 
 #include <nuttx/config.h>
 
+#if defined(CONFIG_DEBUG) && defined(CONFIG_NET_IOB_DEBUG)
+/* Force debug output (from this file only) */
+
+#  undef  CONFIG_DEBUG_NET
+#  define CONFIG_DEBUG_NET 1
+#endif
+
 #include <string.h>
 #include <debug.h>
 
diff --git a/nuttx/net/net_send_buffered.c b/nuttx/net/net_send_buffered.c
index a9346574e..32278594c 100644
--- a/nuttx/net/net_send_buffered.c
+++ b/nuttx/net/net_send_buffered.c
@@ -548,18 +548,21 @@ static uint16_t send_interrupt(FAR struct uip_driver_s *dev, FAR void *pvconn,
               conn->sent    += sndlen;
             }
 
-          nllvdbg("SEND: nrtx=%d unacked=%d sent=%d\n",
-                  WRB_NRTX(wrb), conn->unacked, conn->sent);
+          nllvdbg("SEND: wrb=%p nrtx=%d unacked=%d sent=%d\n",
+                  wrb, WRB_NRTX(wrb), conn->unacked, conn->sent);
 
           /* Increment the count of bytes sent from this write buffer */
 
           WRB_SENT(wrb) += sndlen;
-          DEBUGASSERT(WRB_SENT(wrb) <= WRB_PKTLEN(wrb));
+
+          nllvdbg("SEND: wrb=%p sent=%d pktlen=%d\n",
+                  wrb, WRB_SENT(wrb), WRB_PKTLEN(wrb));
 
           /* Remove the write buffer from the write queue if the
            * last of the data has been sent from the buffer.
            */
 
+          DEBUGASSERT(WRB_SENT(wrb) <= WRB_PKTLEN(wrb));
           if (WRB_SENT(wrb) >= WRB_PKTLEN(wrb))
             {
               FAR struct tcp_wrbuffer_s *tmp;
diff --git a/nuttx/net/net_sockets.c b/nuttx/net/net_sockets.c
index 488531356..c6962915d 100644
--- a/nuttx/net/net_sockets.c
+++ b/nuttx/net/net_sockets.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * net/net_sockets.c
  *
- *   Copyright (C) 2007-2009, 2011-2013 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2007-2009, 2011-2014 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -47,6 +47,7 @@
 #include <errno.h>
 #include <debug.h>
 
+#include <nuttx/net/iob.h>
 #include <nuttx/net/arp.h>
 #include <nuttx/net/uip/uip.h>
 #include <nuttx/net/net.h>
diff --git a/nuttx/net/tcp/Kconfig b/nuttx/net/tcp/Kconfig
index 64e13781d..2fdb351f2 100644
--- a/nuttx/net/tcp/Kconfig
+++ b/nuttx/net/tcp/Kconfig
@@ -106,9 +106,10 @@ config NET_TCP_WRBUFFER_DEBUG
 	bool "Force write buffer debug"
 	default n
 	depends on DEBUG
+	select NET_IOB_DEBUG
 	---help---
 		This option will force debug output from TCP write buffer logic,
-		even with network debug output.  This is not normally something
+		even without network debug output.  This is not normally something
 		that would want to do but is convenient if you are debugging the
 		write buffer logic and do not want to get overloaded with other
 		network-related debug output.