Update to net_close() improvement from Max Holtzberg

5 files changed, 44 insertions, 56 deletions

diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog
index 4c39924a8..9857e955c 100644
--- a/nuttx/ChangeLog
+++ b/nuttx/ChangeLog
@@ -5720,3 +5720,6 @@
 	* net/net_close.c, net/uip/uip_tcpcon, and include/nuttx/net/uip/uip-tcp.h:
 	  Make net_close() nonblocking and free unestablished connections if no
 	  free connections available.  From Max Holtzberg (2013-10-6).
+	* net/net_close.c and other:  Update of change of 2013-10-6 from
+	  Max Holtzberg (2013-10-8).
+
diff --git a/nuttx/include/nuttx/net/uip/uip-tcp.h b/nuttx/include/nuttx/net/uip/uip-tcp.h
index 4c9f767e9..b11de05d7 100644
--- a/nuttx/include/nuttx/net/uip/uip-tcp.h
+++ b/nuttx/include/nuttx/net/uip/uip-tcp.h
@@ -199,13 +199,6 @@ struct uip_conn
 
   FAR struct uip_callback_s *list;
 
-  /* Close callback. The socket close logic allocates this callback and lets
-   * the connection handle close itself. So the application won't be blocked
-   * on the close call.  The callback has to be freed together with this.
-   */
-
-  FAR struct uip_callback_s *closecb;
-
   /* accept() is called when the TCP logic has created a connection */
 
   FAR void *accept_private;
diff --git a/nuttx/net/net_close.c b/nuttx/net/net_close.c
index 6e14cafb0..1efe3094d 100644
--- a/nuttx/net/net_close.c
+++ b/nuttx/net/net_close.c
@@ -56,15 +56,6 @@
  * Private Types
  ****************************************************************************/
 
-#ifdef CONFIG_NET_TCP
-struct tcp_close_s
-{
-  FAR struct socket         *cl_psock; /* Reference to the TCP socket */
-  FAR struct uip_callback_s *cl_cb;    /* Reference to TCP callback instance */
-  sem_t                       cl_sem;   /* Semaphore signals disconnect completion */
-};
-#endif
-
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
@@ -95,7 +86,7 @@ static uint16_t netclose_interrupt(FAR struct uip_driver_s *dev,
 
   DEBUGASSERT(conn != NULL);
 
-  nlldbg("flags: %04x\n", flags);
+  nlldbg("conn: %p flags: %04x\n", conn, flags);
 
   /* UIP_CLOSE:    The remote host has closed the connection
    * UIP_ABORT:    The remote host has aborted the connection
@@ -104,15 +95,13 @@ static uint16_t netclose_interrupt(FAR struct uip_driver_s *dev,
 
   if ((flags & (UIP_CLOSE|UIP_ABORT|UIP_TIMEDOUT)) != 0)
     {
-      /* The disconnection is complete */
-
-      conn->closecb->flags = 0;
-      conn->closecb->priv  = NULL;
-      conn->closecb->event = NULL;
-
       /* Free connection resources */
 
       uip_tcpfree(conn);
+
+      /* Stop further callbacks */
+
+      flags = 0;
     }
   else
     {
@@ -121,7 +110,7 @@ static uint16_t netclose_interrupt(FAR struct uip_driver_s *dev,
        */
 
       dev->d_len = 0;
-      return (flags & ~UIP_NEWDATA) | UIP_CLOSE;
+      flags = (flags & ~UIP_NEWDATA) | UIP_CLOSE;
     }
 
   return flags;
@@ -148,40 +137,36 @@ static uint16_t netclose_interrupt(FAR struct uip_driver_s *dev,
 #ifdef CONFIG_NET_TCP
 static inline void netclose_disconnect(FAR struct socket *psock)
 {
-  struct tcp_close_s state;
+  FAR struct uip_callback_s *cb;
   uip_lock_t flags;
 
   /* Interrupts are disabled here to avoid race conditions */
 
   flags = uip_lock();
 
-  /* Is the TCP socket in a connected state? */
-
-  if (_SS_ISCONNECTED(psock->s_flags))
-    {
-       struct uip_conn *conn = (struct uip_conn*)psock->s_conn;
+  struct uip_conn *conn = (struct uip_conn*)psock->s_conn;
 
-       DEBUGASSERT(conn->closecb == NULL);
+  /* There shouldn't be any callbacks registered */
 
-       /* Check for the case where the host beat us and disconnected first */
+  DEBUGASSERT(conn->list == NULL);
 
-       if (conn->tcpstateflags == UIP_ESTABLISHED)
-         {
-           /* This callback will be freed together with conn */
+  /* Check for the case where the host beat us and disconnected first */
 
-           conn->closecb = uip_tcpcallbackalloc(conn);
-           if (conn->closecb)
-             {
-               /* Set up to receive TCP data event callbacks */
+  if (conn->tcpstateflags == UIP_ESTABLISHED && 
+      (cb = uip_tcpcallbackalloc(conn)) != NULL)
+    {
+      /* Set up to receive TCP data event callbacks */
 
-               conn->closecb->flags   = UIP_NEWDATA|UIP_POLL|UIP_CLOSE|UIP_ABORT|UIP_TIMEDOUT;
-               conn->closecb->event   = netclose_interrupt;
+      cb->flags = UIP_NEWDATA|UIP_POLL|UIP_CLOSE|UIP_ABORT|UIP_TIMEDOUT;
+      cb->event = netclose_interrupt;
 
-               /* Notify the device driver of the availaibilty of TX data */
+      /* Notify the device driver of the availaibilty of TX data */
 
-               netdev_txnotify(conn->ripaddr);
-            }
-        }
+      netdev_txnotify(conn->ripaddr);
+    }
+  else
+    {
+      uip_tcpfree(conn);
     }
 
   uip_unlock(flags);
@@ -244,8 +229,8 @@ int psock_close(FAR struct socket *psock)
                   /* Yes... then perform the disconnection now */
 
                   uip_unlisten(conn);          /* No longer accepting connections */
-                  netclose_disconnect(psock);  /* Break any current connections */
                   conn->crefs = 0;             /* No more references on the connection */
+                  netclose_disconnect(psock);  /* Break any current connections */
                 }
               else
                 {
diff --git a/nuttx/net/uip/uip_tcpcallback.c b/nuttx/net/uip/uip_tcpcallback.c
index 8ac1351f7..671f9d53e 100644
--- a/nuttx/net/uip/uip_tcpcallback.c
+++ b/nuttx/net/uip/uip_tcpcallback.c
@@ -242,7 +242,7 @@ uint16_t uip_tcpcallback(struct uip_driver_s *dev, struct uip_conn *conn,
    * callback.
    */
 
-  if (((flags & UIP_CONN_EVENTS) != 0) && conn->connection_event)
+  if (ret != 0 && ((flags & UIP_CONN_EVENTS) != 0) && conn->connection_event)
     {
       /* Perform the callback */
 
diff --git a/nuttx/net/uip/uip_tcpconn.c b/nuttx/net/uip/uip_tcpconn.c
index 7724eacbd..9ab3c38fe 100644
--- a/nuttx/net/uip/uip_tcpconn.c
+++ b/nuttx/net/uip/uip_tcpconn.c
@@ -221,11 +221,10 @@ struct uip_conn *uip_tcpalloc(void)
 
   if (!conn)
     {
-      /* As a fallback, check for connection structures which are not
-       * established yet.
+      /* As a fallback, check for connection structures which can be stalled.
        *
        * Search the active connection list for the oldest connection
-       * that is not in the UIP_ESTABLISHED state.
+       * that is in the UIP_TIME_WAIT or UIP_FIN_WAIT_1 state.
        */
 
       struct uip_conn *tmp = g_active_tcp_connections.head;
@@ -233,11 +232,13 @@ struct uip_conn *uip_tcpalloc(void)
         {
           nllvdbg("conn: %p state: %02x\n", tmp, tmp->tcpstateflags);
 
-          /* Is this connection in some state other than UIP_ESTABLISHED
-           * state?
+          /* Is this connection in a state we can sacrifice.
+           * REVISIT: maybe UIP_FIN_WAIT_1 is too harsh? There should be a
+           *          higher priority for UIP_TIME_WAIT
            */
 
-          if (tmp->tcpstateflags != UIP_ESTABLISHED)
+          if (tmp->tcpstateflags == UIP_TIME_WAIT ||
+              tmp->tcpstateflags == UIP_FIN_WAIT_1)
             {
               /* Yes.. Is it the oldest one we have seen so far? */
 
@@ -300,6 +301,9 @@ struct uip_conn *uip_tcpalloc(void)
 
 void uip_tcpfree(struct uip_conn *conn)
 {
+  FAR struct uip_callback_s *cb;
+  FAR struct uip_callback_s *next;
+
 #if CONFIG_NET_NTCP_READAHEAD_BUFFERS > 0
   struct uip_readahead_s *readahead;
 #endif
@@ -313,11 +317,14 @@ void uip_tcpfree(struct uip_conn *conn)
   DEBUGASSERT(conn->crefs == 0);
   flags = uip_lock();
 
-  /* Check if there is an allocated close callback structure */
+  /* Free remaining callbacks, actually there should be only the close callback
+   * left.
+   */
 
-  if (conn->closecb != NULL)
+  for (cb = conn->list; cb; cb = next)
     {
-      uip_tcpcallbackfree(conn, conn->closecb);
+      next = cb->flink;
+      uip_tcpcallbackfree(conn, cb);
     }
 
   /* UIP_ALLOCATED means that that the connection is not in the active list