From 02fe07e785825dd2946c434c254fdacc7b7a7509 Mon Sep 17 00:00:00 2001
From: Joel Cunningham <joel.cunningham@me.com>
Date: Sat, 9 Jul 2016 12:12:26 -0500
Subject: [PATCH] Convert lwip_netconn_do_write to use a netbuf

This commit switches the internal netconn API, lwip_netconn_do_write()
to operate on data in a netbuf rather than a void * and size

In order to achieve this, the following changes were made:
  1) In struct api_msg, the w struct now takes a struct netbuf * instead
     of void *.  The len parameter has been repurposed as purely an
     output (previously it was both input and output). The pbuf tot_len
     inside the netbuf is used to get the total size of the write. w.len
     is now also used store the amount written during the write
     operation rather than purely at write termination
  2) struct netconn, the write_offset has been repurposed as offset into
     the current pbuf instead of total write size.  Since we have
     repurposed the w.len variable to track the amount written, this is
     freed up to track current pbuf offset
  3) netconn_write_partly() maintains its existing API signature, but
     internally stores the data pointer in stack/allocated struct netbuf
     in order to integrate with changed do_write()
  4) lwip_netconn_do_writemore() handles chained pbufs within the netbuf
     by checking if the current pbuf was fully consumed and loops again
     with the new pbuf.  The current pbuf is tracked by the ptr value
     of the netbuf.  The allows the ptr to slide down the chain as the
     pbufs are sent.  The p member of the netbuf always points to the
     beginning of the chain

This functionality enables the following future work:
  1) netconn_send() can be improved to support TCP netconns and the
     caller can now assemble a netbuf
  2) sendmsg() (for TCP) can be converted to now also create a netbuf out
     out of the vectors like UDP can after also completing #1
  3) This makes progress towards supporting TCP accepting pbufs at the
     top end of the stack and zero-copying them during transmit. Further
     work needs to happen for tcp_write() to operate on pbufs and future
     work #1
---
 src/api/api_lib.c               | 16 +++++++-
 src/api/api_msg.c               | 90 ++++++++++++++++++++++++++---------------
 src/include/lwip/api.h          |  4 +-
 src/include/lwip/priv/api_msg.h |  2 +-
 4 files changed, 75 insertions(+), 37 deletions(-)

diff --git a/src/api/api_lib.c b/src/api/api_lib.c
index 32f6709..a5d4d2f 100644
--- a/src/api/api_lib.c
+++ b/src/api/api_lib.c
@@ -690,6 +690,7 @@ netconn_write_partly(struct netconn *conn, const void *dataptr, size_t size,
                      u8_t apiflags, size_t *bytes_written)
 {
   API_MSG_VAR_DECLARE(msg);
+  API_VAR_DECLARE(struct netbuf, buf);
   err_t err;
   u8_t dontblock;
 
@@ -705,12 +706,21 @@ netconn_write_partly(struct netconn *conn, const void *dataptr, size_t size,
     return ERR_VAL;
   }
 
+  /* create netbuf for lwip_netconn_do_write() */
+  API_VAR_ALLOC(struct netbuf, MEMP_NETBUF, buf, ERR_MEM);
+  API_VAR_REF(buf).p = API_VAR_REF(buf).ptr = NULL;
+  err = netbuf_ref(&API_VAR_REF(buf), dataptr, (u16_t)size);
+  if (err != ERR_OK) {
+    API_VAR_FREE(MEMP_NETBUF, buf);
+    return err;
+  }
+
   API_MSG_VAR_ALLOC(msg);
   /* non-blocking write sends as much  */
   API_MSG_VAR_REF(msg).conn = conn;
-  API_MSG_VAR_REF(msg).msg.w.dataptr = dataptr;
+  API_MSG_VAR_REF(msg).msg.w.b = &API_VAR_REF(buf);
   API_MSG_VAR_REF(msg).msg.w.apiflags = apiflags;
-  API_MSG_VAR_REF(msg).msg.w.len = size;
+  API_MSG_VAR_REF(msg).msg.w.len = 0;
 #if LWIP_SO_SNDTIMEO
   if (conn->send_timeout != 0) {
     /* get the time we started, which is later compared to
@@ -739,6 +749,8 @@ netconn_write_partly(struct netconn *conn, const void *dataptr, size_t size,
     }
   }
   API_MSG_VAR_FREE(msg);
+  netbuf_free(&API_VAR_REF(buf));
+  API_VAR_FREE(MEMP_NETBUF, buf);
 
   return err;
 }
diff --git a/src/api/api_msg.c b/src/api/api_msg.c
index 9dfea64..f21e117 100644
--- a/src/api/api_msg.c
+++ b/src/api/api_msg.c
@@ -1504,13 +1504,15 @@ lwip_netconn_do_writemore(struct netconn *conn  WRITE_DELAYED_PARAM)
   size_t diff;
   u8_t dontblock;
   u8_t apiflags;
+  u8_t write_more;
 
   LWIP_ASSERT("conn != NULL", conn != NULL);
   LWIP_ASSERT("conn->state == NETCONN_WRITE", (conn->state == NETCONN_WRITE));
   LWIP_ASSERT("conn->current_msg != NULL", conn->current_msg != NULL);
   LWIP_ASSERT("conn->pcb.tcp != NULL", conn->pcb.tcp != NULL);
-  LWIP_ASSERT("conn->write_offset < conn->current_msg->msg.w.len",
-    conn->write_offset < conn->current_msg->msg.w.len);
+  LWIP_ASSERT("Current pbuf is invalid", conn->current_msg->msg.w.b->ptr != NULL);
+  LWIP_ASSERT("Current pbuf offset is invalid",
+    conn->write_offset < conn->current_msg->msg.w.b->ptr->len);
 
   dontblock = netconn_is_nonblocking(conn) ||
        (conn->current_msg->msg.w.apiflags & NETCONN_DONTBLOCK);
@@ -1520,46 +1522,72 @@ lwip_netconn_do_writemore(struct netconn *conn  WRITE_DELAYED_PARAM)
   if ((conn->send_timeout != 0) &&
       ((s32_t)(sys_now() - conn->current_msg->msg.w.time_started) >= conn->send_timeout)) {
     write_finished = 1;
-    if (conn->write_offset == 0) {
+    if ((conn->write_offset == 0) &&
+        (conn->current_msg->msg.w.b->p == conn->current_msg->msg.w.b->ptr)) {
       /* nothing has been written */
       err = ERR_WOULDBLOCK;
-      conn->current_msg->msg.w.len = 0;
     } else {
       /* partial write */
       err = ERR_OK;
-      conn->current_msg->msg.w.len = conn->write_offset;
       conn->write_offset = 0;
     }
   } else
 #endif /* LWIP_SO_SNDTIMEO */
   {
-    dataptr = (const u8_t*)conn->current_msg->msg.w.dataptr + conn->write_offset;
-    diff = conn->current_msg->msg.w.len - conn->write_offset;
-    if (diff > 0xffffUL) { /* max_u16_t */
-      len = 0xffff;
-      apiflags |= TCP_WRITE_FLAG_MORE;
-    } else {
-      len = (u16_t)diff;
-    }
-    available = tcp_sndbuf(conn->pcb.tcp);
-    if (available < len) {
-      /* don't try to write more than sendbuf */
-      len = available;
-      if (dontblock) {
-        if (!len) {
-          err = ERR_WOULDBLOCK;
-          goto err_mem;
-        }
+    do {
+      dataptr = (const u8_t*)conn->current_msg->msg.w.b->ptr->payload + conn->write_offset;
+      diff = conn->current_msg->msg.w.b->ptr->len - conn->write_offset;
+      if (diff > 0xffffUL) { /* max_u16_t restriction of tcp_write() */
+        len = 0xffff;
+        apiflags |= TCP_WRITE_FLAG_MORE;
       } else {
+        len = (u16_t)diff;
+      }
+      available = tcp_sndbuf(conn->pcb.tcp);
+      if (available < len) {
+        /* don't try to write more than sendbuf */
+        len = available;
+        if (dontblock) {
+          if (!len) {
+            /* sndbuf is closed, set error according to partial write or not */
+            if ((conn->write_offset == 0) &&
+                (conn->current_msg->msg.w.b->p == conn->current_msg->msg.w.b->ptr)) {
+              err = ERR_WOULDBLOCK;
+            } else {
+              err = ERR_OK;
+            }
+            goto err_mem;
+          }
+        } else {
+          apiflags |= TCP_WRITE_FLAG_MORE;
+        }
+      }
+      /* if we can finish this pbuf and have more, mark for another loop */
+      if ((len == (u16_t)diff) && (conn->current_msg->msg.w.b->ptr->next != NULL)) {
+        write_more = 1;
         apiflags |= TCP_WRITE_FLAG_MORE;
+      } else {
+        write_more = 0;
       }
-    }
-    LWIP_ASSERT("lwip_netconn_do_writemore: invalid length!", ((conn->write_offset + len) <= conn->current_msg->msg.w.len));
-    err = tcp_write(conn->pcb.tcp, dataptr, len, apiflags);
+      LWIP_ASSERT("lwip_netconn_do_writemore: invalid length!", ((conn->write_offset + len) <= conn->current_msg->msg.w.b->ptr->len));
+      err = tcp_write(conn->pcb.tcp, dataptr, len, apiflags);
+      if (err == ERR_OK) {
+        /* update total sent */
+        conn->current_msg->msg.w.len += len;
+        if (write_more) {
+          /* we finished this pbuf, advance to next pbuf */
+          conn->current_msg->msg.w.b->ptr = conn->current_msg->msg.w.b->ptr->next;
+          conn->write_offset = 0;
+        } else {
+          /* update offset into current pbuf */
+          conn->write_offset += len;
+        }
+      }
+    } while (write_more && err == ERR_OK);
     /* if OK or memory error, check available space */
     if ((err == ERR_OK) || (err == ERR_MEM)) {
 err_mem:
-      if (dontblock && (len < conn->current_msg->msg.w.len)) {
+      if (dontblock && (conn->current_msg->msg.w.len < conn->current_msg->msg.w.b->p->tot_len)) {
         /* non-blocking write did not write everything: mark the pcb non-writable
            and let poll_tcp check writable space to mark the pcb writable again */
         API_EVENT(conn, NETCONN_EVT_SENDMINUS, len);
@@ -1574,10 +1602,7 @@ err_mem:
 
     if (err == ERR_OK) {
       err_t out_err;
-      conn->write_offset += len;
-      if ((conn->write_offset == conn->current_msg->msg.w.len) || dontblock) {
-        /* return sent length */
-        conn->current_msg->msg.w.len = conn->write_offset;
+      if ((conn->current_msg->msg.w.len == conn->current_msg->msg.w.b->p->tot_len) || dontblock) {
         /* everything was written */
         write_finished = 1;
       }
@@ -1661,8 +1686,9 @@ lwip_netconn_do_write(void *m)
         msg->conn->state = NETCONN_WRITE;
         /* set all the variables used by lwip_netconn_do_writemore */
         LWIP_ASSERT("already writing or closing", msg->conn->current_msg == NULL &&
-          msg->conn->write_offset == 0);
-        LWIP_ASSERT("msg->msg.w.len != 0", msg->msg.w.len != 0);
+          msg->conn->write_offset == 0 && msg->msg.w.len == 0);
+        LWIP_ASSERT("invalid netbuf", msg->msg.w.b != NULL && msg->msg.w.b->p != NULL &&
+          msg->msg.w.b->p == msg->msg.w.b->ptr);
         msg->conn->current_msg = msg;
         msg->conn->write_offset = 0;
 #if LWIP_TCPIP_CORE_LOCKING
diff --git a/src/include/lwip/api.h b/src/include/lwip/api.h
index c24db85..6a0cd4d 100644
--- a/src/include/lwip/api.h
+++ b/src/include/lwip/api.h
@@ -224,8 +224,8 @@ struct netconn {
   /** flags holding more netconn-internal state, see NETCONN_FLAG_* defines */
   u8_t flags;
 #if LWIP_TCP
-  /** TCP: when data passed to netconn_write doesn't fit into the send buffer,
-      this temporarily stores how much is already sent. */
+  /** TCP: when doing a blocking netconn_write and the data doesn't fit into
+      the send buffer, this temporarily stores offset into current pbuf */
   size_t write_offset;
   /** TCP: when data passed to netconn_write doesn't fit into the send buffer,
       this temporarily stores the message.
diff --git a/src/include/lwip/priv/api_msg.h b/src/include/lwip/priv/api_msg.h
index ad38345..d898fb1 100644
--- a/src/include/lwip/priv/api_msg.h
+++ b/src/include/lwip/priv/api_msg.h
@@ -104,7 +104,7 @@ struct api_msg {
     } ad;
     /** used for lwip_netconn_do_write */
     struct {
-      const void *dataptr;
+      struct netbuf *b;
       size_t len;
       u8_t apiflags;
 #if LWIP_SO_SNDTIMEO
-- 
1.9.5.msysgit.0

