Fixed bug #20287: tcp_output_nagle sends too early. This fixes the nagle algorithm; nagle now also works for all raw API applications and has to be explicitly disabled with 'tcp_pcb->flags |= TF_NODELAY'

CHANGELOG

@@ -486,6 +486,11 @@ HISTORY
 
   ++ Bug fixes:
 
+  2007-11-21 Simon Goldschmidt
+  * tcp.h, tcp_out.c, api_msg.c: Fixed bug #20287: tcp_output_nagle sends too early
+    Fixed the nagle algorithm; nagle now also works for all raw API applications
+    and has to be explicitly disabled with 'tcp_pcb->flags |= TF_NODELAY'
+
   2007-11-12 Frédéric Bernon
   * sockets.c, api.h, api_lib.c, api_msg.h, api_msg.c: Fixed bug #20900. Now, most
     of the netconn_peer and netconn_addr processing is done inside tcpip_thread

src/api/api_msg.c

@@ -819,6 +819,9 @@ do_writemore(struct netconn *conn)
       API_EVENT(conn, NETCONN_EVT_SENDMINUS, len);
     }
   } else if (err == ERR_MEM) {
+    /* tcp_enqueue returned ERR_MEM, try tcp_output anyway */
+    err = tcp_output(conn->pcb.tcp);
+
 #if LWIP_TCPIP_CORE_LOCKING
     conn->write_delayed = 1;
 #endif

src/core/tcp_out.c

@@ -147,6 +147,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
   /* fail on too much data */
   if (len > pcb->snd_buf) {
     LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 3, ("tcp_enqueue: too much data (len=%"U16_F" > snd_buf=%"U16_F")\n", len, pcb->snd_buf));
+    pcb->flags |= TF_NAGLEMEMERR;
     return ERR_MEM;
   }
   left = len;
@@ -165,6 +166,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
   if ((queuelen >= TCP_SND_QUEUELEN) || (queuelen > TCP_SNDQUEUELEN_OVERFLOW)) {
     LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 3, ("tcp_enqueue: too long queue %"U16_F" (max %"U16_F")\n", queuelen, TCP_SND_QUEUELEN));
     TCP_STATS_INC(tcp.memerr);
+    pcb->flags |= TF_NAGLEMEMERR;
     return ERR_MEM;
   }
   if (queuelen != 0) {
@@ -361,6 +363,9 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
   if ((flags & TCP_SYN) || (flags & TCP_FIN)) {
     ++len;
   }
+  if (flags & TCP_FIN) {
+    pcb->flags |= TF_FIN;
+  }
   pcb->snd_lbb += len;
 
   pcb->snd_buf -= len;
@@ -381,6 +386,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
 
   return ERR_OK;
 memerr:
+  pcb->flags |= TF_NAGLEMEMERR;
   TCP_STATS_INC(tcp.memerr);
 
   if (queue != NULL) {
@@ -503,6 +509,18 @@ tcp_output(struct tcp_pcb *pcb)
   /* data available and window allows it to be sent? */
   while (seg != NULL &&
   ntohl(seg->tcphdr->seqno) - pcb->lastack + seg->len <= wnd) {
+    LWIP_ASSERT("RST not expected here!", (TCPH_FLAGS(seg->tcphdr) & TCP_RST) == 0);
+    /* Stop sending if the nagle algorithm would prevent it
+     * Don't stop:
+     * - if tcp_enqueue had a memory error before (prevent delayed ACK timeout) or
+     * - if FIN was already enqueued for this PCB (SYN is always alone in a segment -
+     *   either seg->next != NULL or pcb->unacked == NULL;
+     *   RST is no sent using tcp_enqueue/tcp_output.
+     */
+    if((tcp_do_output_nagle(pcb) == 0) &&
+      ((pcb->flags & (TF_NAGLEMEMERR | TF_FIN)) == 0)){
+      break;
+    }
 #if TCP_CWND_DEBUG
     LWIP_DEBUGF(TCP_CWND_DEBUG, ("tcp_output: snd_wnd %"U16_F", cwnd %"U16_F", wnd %"U32_F", effwnd %"U32_F", seq %"U32_F", ack %"U32_F", i %"S16_F"\n",
                             pcb->snd_wnd, pcb->cwnd, wnd,
@@ -552,6 +570,7 @@ tcp_output(struct tcp_pcb *pcb)
     }
     seg = pcb->unsent;
   }
+ pcb->flags &= ~TF_NAGLEMEMERR;
  return ERR_OK;
 }
 

src/include/lwip/tcp.h

@@ -122,10 +122,12 @@ void             tcp_rexmit_rto  (struct tcp_pcb *pcb);
  * previously transmitted data on the connection remains
  * unacknowledged.
  */
-#define tcp_output_nagle(tpcb) ((((tpcb)->unacked == NULL) || \
+#define tcp_do_output_nagle(tpcb) ((((tpcb)->unacked == NULL) || \
                             ((tpcb)->flags & TF_NODELAY) || \
                             (((tpcb)->unsent != NULL) && ((tpcb)->unsent->next != NULL))) ? \
-                                tcp_output(tpcb) : ERR_OK)
+                                1 : 0)
+#define tcp_output_nagle(tpcb) (tcp_do_output_nagle(tpcb) ? tcp_output(tpcb) : ERR_OK)
+
 
 /** This returns a TCP header option for MSS in an u32_t */
 #define TCP_BUILD_MSS_OPTION()  htonl(((u32_t)2 << 24) | \
@@ -239,6 +241,12 @@ enum tcp_state {
   TIME_WAIT   = 10
 };
 
+/** Flags used on input processing, not on pcb->flags
+*/
+#define TF_RESET     (u8_t)0x08U   /* Connection was reset. */
+#define TF_CLOSED    (u8_t)0x10U   /* Connection was sucessfully closed. */
+#define TF_GOT_FIN   (u8_t)0x20U   /* Connection was closed by the remote end. */
+
 /**
  * members common to struct tcp_pcb and struct tcp_listen_pcb
  */
@@ -261,13 +269,12 @@ struct tcp_pcb {
   u16_t remote_port;
   
   u8_t flags;
-#define TF_ACK_DELAY (u8_t)0x01U   /* Delayed ACK. */
-#define TF_ACK_NOW   (u8_t)0x02U   /* Immediate ACK. */
-#define TF_INFR      (u8_t)0x04U   /* In fast recovery. */
-#define TF_RESET     (u8_t)0x08U   /* Connection was reset. */
-#define TF_CLOSED    (u8_t)0x10U   /* Connection was sucessfully closed. */
-#define TF_GOT_FIN   (u8_t)0x20U   /* Connection was closed by the remote end. */
-#define TF_NODELAY   (u8_t)0x40U   /* Disable Nagle algorithm */
+#define TF_ACK_DELAY   (u8_t)0x01U   /* Delayed ACK. */
+#define TF_ACK_NOW     (u8_t)0x02U   /* Immediate ACK. */
+#define TF_INFR        (u8_t)0x04U   /* In fast recovery. */
+#define TF_FIN         (u8_t)0x20U   /* Connection was closed locally (FIN segment enqueued). */
+#define TF_NODELAY     (u8_t)0x40U   /* Disable Nagle algorithm */
+#define TF_NAGLEMEMERR (u8_t)0x80U /* nagle enabled, memerr, try to output to prevent delayed ACK to happen */
 
   /* the rest of the fields are in host byte order
      as we have to do some math with them */