Fixed bug #20021: Moved sendbuf-processing in netconn_write from api_lib.c to api_msg.c to also prevent multiple context-changes on low memory or empty send-buffer.

CHANGELOG

@@ -19,6 +19,11 @@ HISTORY
 
   ++ New features:
 
+  2007-06-21 Simon Goldschmidt
+  * api.h, api_lib.c, api_msg.c: Fixed bug #20021: Moved sendbuf-processing in
+    netconn_write from api_lib.c to api_msg.c to also prevent multiple context-
+    changes on low memory or empty send-buffer.
+
   2007-06-18 Simon Goldschmidt
   * etharp.c, etharp.h: Changed etharp to use a defined hardware address length
     of 6 to avoid loading netif->hwaddr_len every time (since this file is only

src/api/api_lib.c

@@ -778,9 +778,9 @@ err_t
 netconn_write(struct netconn *conn, const void *dataptr, u16_t size, u8_t copy)
 {
   struct api_msg msg;
-  u16_t len, sndbuf;
 
   LWIP_ERROR("netconn_write: invalid conn",  (conn == NULL), return ERR_ARG;);
+  LWIP_ERROR("netconn_write: invalid conn->type",  (conn->type != NETCONN_TCP), return ERR_VAL;);
 
   if (conn->err != ERR_OK) {
     return conn->err;
@@ -788,44 +788,13 @@ netconn_write(struct netconn *conn, const void *dataptr, u16_t size, u8_t copy)
 
   msg.function = do_write;
   msg.msg.conn = conn;
-
-  conn->state = NETCONN_WRITE;
-  while (conn->err == ERR_OK && size > 0) {
   msg.msg.msg.w.dataptr = dataptr;
   msg.msg.msg.w.copy = copy;
-    
+  msg.msg.msg.w.len = size;
-    if (conn->type == NETCONN_TCP) {
+  /* For locking the core: this _can_ be delayed on low memory/low send buffer,
-      while ((sndbuf = tcp_sndbuf(conn->pcb.tcp)) == 0) {
+     but if it is, this is done inside api_msg.c:do_write(), so we can use the
-        sys_sem_wait(conn->sem);
+     non-blocking version here. */
-        if (conn->err != ERR_OK) {
-          goto ret;
-        }
-      }
-      if (size > sndbuf) {
-        /* We cannot send more than one send buffer's worth of data at a time. */
-        len = sndbuf;
-      } else {
-        len = size;
-      }
-    } else {
-      len = size;
-    }
-
-    LWIP_DEBUGF(API_LIB_DEBUG, ("netconn_write: writing %d bytes (%d)\n", len, copy));
-    msg.msg.msg.w.len = len;
   TCPIP_APIMSG(&msg);
-    if (conn->err == ERR_OK) {
-      dataptr = (void *)((u8_t *)dataptr + len);
-      size -= len;
-    } else if (conn->err == ERR_MEM) {
-      conn->err = ERR_OK;
-      sys_sem_wait(conn->sem);
-    } else {
-      goto ret;
-    }
-  }
- ret:
-  conn->state = NETCONN_NONE;
 
   return conn->err;
 }

src/api/api_msg.c

@@ -40,6 +40,11 @@
 
 #if !NO_SYS
 
+/* forward declarations */
+#if LWIP_TCP
+static err_t do_writemore(struct netconn *conn);
+#endif
+
 #if LWIP_RAW
 /**
  * Receive callback function for RAW netconns.
@@ -176,17 +181,17 @@ recv_tcp(void *arg, struct tcp_pcb *pcb, struct pbuf *p, err_t err)
 static err_t
 poll_tcp(void *arg, struct tcp_pcb *pcb)
 {
-  struct netconn *conn;
+  struct netconn *conn = arg;
 
   LWIP_UNUSED_ARG(pcb);
+  LWIP_ASSERT("conn != NULL", (conn != NULL));
 
-  conn = arg;
+  if (conn->state == NETCONN_WRITE) {
-
+    do_writemore(conn);
-  if (conn != NULL &&
+  } else if ((conn->state == NETCONN_CLOSE) && (conn->sem != SYS_SEM_NULL)) {
-     (conn->state == NETCONN_WRITE || conn->state == NETCONN_CLOSE) &&
-     conn->sem != SYS_SEM_NULL) {
     sys_sem_signal(conn->sem);
   }
+
   return ERR_OK;
 }
 
@@ -200,13 +205,14 @@ poll_tcp(void *arg, struct tcp_pcb *pcb)
 static err_t
 sent_tcp(void *arg, struct tcp_pcb *pcb, u16_t len)
 {
-  struct netconn *conn;
+  struct netconn *conn = arg;
 
   LWIP_UNUSED_ARG(pcb);
+  LWIP_ASSERT("conn != NULL", (conn != NULL));
 
-  conn = arg;
+  if (conn->state == NETCONN_WRITE) {
-
+    do_writemore(conn);
-  if (conn != NULL && conn->sem != SYS_SEM_NULL) {
+  } else if ((conn->state == NETCONN_CLOSE) && (conn->sem != SYS_SEM_NULL)) {
     sys_sem_signal(conn->sem);
   }
 
@@ -230,6 +236,7 @@ err_tcp(void *arg, err_t err)
   struct netconn *conn;
 
   conn = arg;
+  LWIP_ASSERT("conn != NULL", (conn != NULL));
 
   conn->pcb.tcp = NULL;
 
@@ -250,7 +257,15 @@ err_tcp(void *arg, err_t err)
       (*conn->callback)(conn, NETCONN_EVT_RCVPLUS, 0);
     sys_mbox_post(conn->acceptmbox, NULL);
   }
-  if (conn->sem != SYS_SEM_NULL) {
+  if (conn->state == NETCONN_WRITE) {
+    /* calling do_writemore is not necessary
+       since the pcb has already been deleted! */
+    conn->state = NETCONN_NONE;
+    /* wake up the waiting task */
+    sys_mbox_post(conn->mbox, NULL);
+  } else
+    if ((conn->sem != SYS_SEM_NULL) &&
+      (conn->state == NETCONN_CLOSE)) {
       sys_sem_signal(conn->sem);
     }
 }
@@ -694,6 +709,94 @@ do_recv(struct api_msg_msg *msg)
   TCPIP_APIMSG_ACK(msg);
 }
 
+#if LWIP_TCP
+/**
+ * See if more data needs to be written from a previous call to netconn_write.
+ * Called initially from do_write. If the first call can't send all data
+ * (because of low memory or empty send-buffer), this function is called again
+ * from sent_tcp() or poll_tcp() to send more data. If all data is sent, the
+ * blocking application thread (waiting in netconn_write) is released.
+ *
+ * @param conn netconn (that is currently in state NETCONN_WRITE) to process
+ * @return ERR_OK
+ *         ERR_MEM if LWIP_TCPIP_CORE_LOCKING=1 and sending hasn't yet finished
+ */
+err_t
+do_writemore(struct netconn *conn)
+{
+  err_t err;
+  void *dataptr;
+  u16_t len, available;
+  u8_t write_finished = 0;
+
+  LWIP_ASSERT("conn->state != NETCONN_WRITE", (conn->state != NETCONN_WRITE));
+
+  dataptr = (u8_t*)conn->write_msg->msg.w.dataptr + conn->write_offset;
+  len = conn->write_msg->msg.w.len - conn->write_offset;
+  available = tcp_sndbuf(conn->pcb.tcp);
+  if (available < len) {
+    /* don't try to write more than sendbuf */
+    len = available;
+#if LWIP_TCPIP_CORE_LOCKING
+    conn->write_delayed = 1;
+#endif
+  }
+
+  err = tcp_write(conn->pcb.tcp, dataptr, len, conn->write_msg->msg.w.copy);
+  LWIP_ASSERT("do_writemore: invalid length!", ((conn->write_offset + len) <= conn->write_msg->msg.w.len));
+  if (err == ERR_OK) {
+    conn->write_offset += len;
+    if (conn->write_offset == conn->write_msg->msg.w.len) {
+      /* everything was written */
+      write_finished = 1;
+      conn->write_msg = NULL;
+      conn->write_offset = 0;
+    }
+    /* This is the Nagle algorithm: inhibit the sending of new TCP
+       segments when new outgoing data arrives from the user if any
+       previously transmitted data on the connection remains
+       unacknowledged. */
+    if ((conn->pcb.tcp->unacked == NULL ||
+        (conn->pcb.tcp->flags & TF_NODELAY) || 
+        (conn->pcb.tcp->snd_queuelen) > 1)) {
+      err = tcp_output(conn->pcb.tcp);
+    }
+    conn->err = err;
+    if ((conn->callback) && (err == ERR_OK) &&
+        (tcp_sndbuf(conn->pcb.tcp) <= TCP_SNDLOWAT)) {
+      (*conn->callback)(conn, NETCONN_EVT_SENDMINUS, len);
+    }
+  } else if (err == ERR_MEM) {
+#if LWIP_TCPIP_CORE_LOCKING
+    conn->write_delayed = 1;
+#endif
+  } else {
+    /* if ERR_MEM, we wait for sent_tcp or poll_tcp to be called */
+    conn->err = err;
+    /* since we've had an error (except temporary running out of memory,
+       we don't try writing any more */
+    write_finished = 1;
+  }
+
+  if (write_finished) {
+    /* everything was written: set back connection state
+       and back to application task */
+    conn->state = NETCONN_NONE;
+#if LWIP_TCPIP_CORE_LOCKING
+    if (conn->write_delayed != 0)
+#endif
+    {
+      sys_mbox_post(conn->mbox, NULL);
+    }
+  }
+#if LWIP_TCPIP_CORE_LOCKING
+  else
+    return ERR_MEM;
+#endif
+  return ERR_OK;
+}
+#endif /* LWIP_TCP */
+
 /**
  * Send some data on a TCP pcb contained in a netconn
  * Called from netconn_write
@@ -703,30 +806,27 @@ do_recv(struct api_msg_msg *msg)
 void
 do_write(struct api_msg_msg *msg)
 {
-#if LWIP_TCP
-  err_t err;
-#endif /* LWIP_TCP */
   if (msg->conn->err == ERR_OK) {
-    if (msg->conn->pcb.tcp != NULL) {
+    if ((msg->conn->pcb.tcp != NULL) && (msg->conn->type == NETCONN_TCP)) {
-      if (msg->conn->type == NETCONN_TCP) {
 #if LWIP_TCP
-        err = tcp_write(msg->conn->pcb.tcp, msg->msg.w.dataptr,
+      msg->conn->state = NETCONN_WRITE;
-                        msg->msg.w.len, msg->msg.w.copy);
+      /* set all the variables used by do_writemore */
-        /* This is the Nagle algorithm: inhibit the sending of new TCP
+      msg->conn->write_msg = msg;
-           segments when new outgoing data arrives from the user if any
+      msg->conn->write_offset = 0;
-           previously transmitted data on the connection remains
+#if LWIP_TCPIP_CORE_LOCKING
-           unacknowledged. */
+      msg->conn->write_delayed = 0;
-        if(err == ERR_OK && (msg->conn->pcb.tcp->unacked == NULL ||
+      while (do_writemore(msg->conn) != ERR_OK) {
-          (msg->conn->pcb.tcp->flags & TF_NODELAY) || 
+        LWIP_ASSERT("state!", msg->conn->state == NETCONN_WRITE);
-          (msg->conn->pcb.tcp->snd_queuelen) > 1)) {
+        UNLOCK_TCPIP_CORE();
-            err = tcp_output(msg->conn->pcb.tcp);
+        sys_arch_mbox_fetch(msg->conn->mbox, NULL, 0);
-        }
+        LOCK_TCPIP_CORE();
-        msg->conn->err = err;
+        LWIP_ASSERT("state!", msg->conn->state == NETCONN_WRITE);
-        if (msg->conn->callback)
-          if (err == ERR_OK) {
-            if (tcp_sndbuf(msg->conn->pcb.tcp) <= TCP_SNDLOWAT)
-              (*msg->conn->callback)(msg->conn, NETCONN_EVT_SENDMINUS, msg->msg.w.len);
       }
+#else
+      do_writemore(msg->conn);
+#endif
+      /* for both cases: if do_writemore was called, don't ACK the APIMSG! */
+      return;
 #endif /* LWIP_TCP */
 #if (LWIP_UDP || LWIP_RAW)
     } else {
@@ -734,7 +834,6 @@ do_write(struct api_msg_msg *msg)
 #endif /* (LWIP_UDP || LWIP_RAW) */
     }
   }
-  }
   TCPIP_APIMSG_ACK(msg);
 }
 

src/include/lwip/api.h

@@ -118,6 +118,19 @@ struct netconn {
   int recv_timeout;
 #endif /* LWIP_SO_RCVTIMEO */
   u16_t recv_avail;
+  /** TCP: when data passed to netconn_write doesn't fit into the send buffer,
+      this temporarily stores the message. */
+  struct api_msg_msg *write_msg;
+  /** TCP: when data passed to netconn_write doesn't fit into the send buffer,
+      this temporarily stores how much is already sent. */
+  u16_t write_offset;
+#if LWIP_TCPIP_CORE_LOCKING
+  /** TCP: when data passed to netconn_write doesn't fit into the send buffer,
+      this temporarily stores whether to wake up the original application task
+      if data couldn't be sent in the first try. */
+  u8_t write_delayed;
+#endif
+
   void (* callback)(struct netconn *, enum netconn_evt, u16_t len);
 };