tcp_write(): optimize order of valid states checks, most common first.

added assertion: when the queues are NULL, queuelen must be zero.
remove ACK flags from a PCB, only when we could succesfully sent
an empty ACK packet.

src/core/tcp_out.c

@@ -79,10 +79,11 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t copy)
 {
   LWIP_DEBUGF(TCP_OUTPUT_DEBUG, ("tcp_write(pcb=%p, arg=%p, len=%u, copy=%d)\n", (void *)pcb,
     arg, len, (unsigned int)copy));
-  if (pcb->state == SYN_SENT ||
+  /* connection is in valid state for data transmission? */
-     pcb->state == SYN_RCVD ||
+  if (pcb->state == ESTABLISHED ||
-     pcb->state == ESTABLISHED ||
+     pcb->state == CLOSE_WAIT ||
-     pcb->state == CLOSE_WAIT) {
+     pcb->state == SYN_SENT ||
+     pcb->state == SYN_RCVD) {
     if (len > 0) {
       return tcp_enqueue(pcb, (void *)arg, len, 0, copy, NULL, 0);
     }
@@ -93,10 +94,20 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t copy)
   }
 }
 
+/**
+ * Enqueue data for tranmission
+ * 
+ * @arg pcb Protocol control block for the TCP connection to enqueue data for.
+ * @arg arg Pointer to the data to be enqueued for sending.
+ * @arg len Data length in bytes
+ * @arg flags
+ * @arg optdata
+ * @arg optlen
+ */
 err_t
 tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
-      u8_t flags, u8_t copy,
+  u8_t flags, u8_t copy,
-            u8_t *optdata, u8_t optlen)
+  u8_t *optdata, u8_t optlen)
 {
   struct pbuf *p;
   struct tcp_seg *seg, *useg, *queue;
@@ -107,19 +118,18 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
 
   LWIP_DEBUGF(TCP_OUTPUT_DEBUG, ("tcp_enqueue(pcb=%p, arg=%p, len=%u, flags=%x, copy=%u)\n",
     (void *)pcb, arg, len, (unsigned int)flags, (unsigned int)copy));
-  left = len;
-  ptr = arg;
   /* fail on too much data */
   if (len > pcb->snd_buf) {
     LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 3, ("tcp_enqueue: too much data (len=%u > snd_buf=%u)\n", len, pcb->snd_buf));
     return ERR_MEM;
   }
+  left = len;
+  ptr = arg;
 
   /* seqno will be the sequence number of the first segment enqueued
    * by the call to this function. */
   seqno = pcb->snd_lbb;
 
-  queue = NULL;
   LWIP_DEBUGF(TCP_QLEN_DEBUG, ("tcp_enqueue: queuelen: %u\n", (unsigned int)pcb->snd_queuelen));
 
   /* Check if the queue length exceeds the configured maximum queue
@@ -129,17 +139,18 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 3, ("tcp_enqueue: too long queue %u (max %u)\n", queuelen, TCP_SND_QUEUELEN));
     goto memerr;
   }
-
+  if (queuelen != 0) {
-  if (pcb->snd_queuelen != 0) {
+    LWIP_ASSERT("tcp_enqueue: queue length non-zero and at least one queue non-empty",
-    LWIP_ASSERT("tcp_enqueue: valid queue length", pcb->unacked != NULL ||
+      pcb->unacked != NULL || pcb->unsent != NULL);
-    pcb->unsent != NULL);
+  } else {
+    LWIP_ASSERT("tcp_enqueue: queue length zero and queues empty",
+      pcb->unacked == NULL && pcb->unsent == NULL);
   }
 
-  seg = useg = NULL;
-  seglen = 0;
-
   /* First, break up the data into segments and tuck them together in
    * the local "queue" variable. */
+  useg = queue = seg = NULL;
+  seglen = 0;
   while (queue == NULL || left > 0) {
 
     /* The segment length should be the MSS if the data to be enqueued
@@ -155,20 +166,26 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     seg->next = NULL;
     seg->p = NULL;
 
+    /* first segment of to-be-queued data? */
     if (queue == NULL) {
       useg = queue = seg;
     }
+    /* subsequent segments of to-be-queued data */
     else {
       /* Attach the segment to the end of the queued segments. */
       LWIP_ASSERT("useg != NULL", useg != NULL);
       useg->next = seg;
+      /* remember last segment of to-be-queued data for next iteration */
       useg = seg;
     }
+    /* { useg == seg } */
 
     /* If copy is set, memory should be allocated
      * and data copied into pbuf, otherwise data comes from
      * ROM or other static memory, and need not be copied. If
      * optdata is != NULL, we have options instead of data. */
+     
+    /* options? */
     if (optdata != NULL) {
       if ((seg->p = pbuf_alloc(PBUF_TRANSPORT, optlen, PBUF_RAM)) == NULL) {
         goto memerr;
@@ -176,6 +193,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
       ++queuelen;
       seg->dataptr = seg->p->payload;
     }
+    /* copy from volatile memory? */
     else if (copy) {
       if ((seg->p = pbuf_alloc(PBUF_TRANSPORT, seglen, PBUF_RAM)) == NULL) {
         LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 2, ("tcp_enqueue : could not allocate memory for pbuf copy size %u\n", seglen));
@@ -190,7 +208,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     /* do not copy data */
     else {
 
-      /* first, allocate a pbuf for holding the data.
+      /* First, allocate a pbuf for holding the data.
        * since the referenced data is available at least until it is sent out on the
        * link (as it has to be ACKed by the remote party) we can safely use PBUF_ROM
        * instead of PBUF_REF here.
@@ -200,6 +218,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
         goto memerr;
       }
       ++queuelen;
+      /* reference the non-volatile payload data */
       p->payload = ptr;
       seg->dataptr = ptr;
 
@@ -214,7 +233,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
       ++queuelen;
 
       /* Concatenate the headers and data pbufs together. */
-      pbuf_cat(seg->p, p);
+      pbuf_cat(seg->p/*header*/, p/*data*/);
       p = NULL;
     }
 
@@ -226,16 +245,10 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     }
 
     seg->len = seglen;
-#if 0 /* Was commented out. TODO: can someone say why this is here? */
+
-    if ((flags & TCP_SYN) || (flags & TCP_FIN)) {
+    /* build TCP header */
-      ++seg->len;
-    }
-#endif
-    /* Build TCP header. */
     if (pbuf_header(seg->p, TCP_HLEN)) {
-
       LWIP_DEBUGF(TCP_OUTPUT_DEBUG | 2, ("tcp_enqueue: no room for TCP header in pbuf.\n"));
-
       TCP_STATS_INC(tcp.err);
       goto memerr;
     }
@@ -268,7 +281,6 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     ptr = (void *)((char *)ptr + seglen);
   }
 
-
   /* Now that the data to be enqueued has been broken up into TCP
   segments in the queue variable, we add them to the end of the
   pcb->unsent queue. */
@@ -278,6 +290,7 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
   else {
     for (useg = pcb->unsent; useg->next != NULL; useg = useg->next);
   }
+  /* { useg is last segment on the unsent queue, NULL if list is empty } */
 
   /* If there is room in the last pbuf on the unsent queue,
   chain the first pbuf on the queue together with that. */
@@ -285,24 +298,27 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
     TCP_TCPLEN(useg) != 0 &&
     !(TCPH_FLAGS(useg->tcphdr) & (TCP_SYN | TCP_FIN)) &&
     !(flags & (TCP_SYN | TCP_FIN)) &&
+    /* fit within max seg size */
     useg->len + queue->len <= pcb->mss) {
-    /* Remove TCP header from first segment. */
+    /* Remove TCP header from first segment of our to-be-queued list */
     pbuf_header(queue->p, -TCP_HLEN);
     pbuf_cat(useg->p, queue->p);
     useg->len += queue->len;
     useg->next = queue->next;
 
-    LWIP_DEBUGF(TCP_OUTPUT_DEBUG | DBG_TRACE | DBG_STATE, ("tcp_enqueue: chaining, new len %u\n", useg->len));
+    LWIP_DEBUGF(TCP_OUTPUT_DEBUG | DBG_TRACE | DBG_STATE, ("tcp_enqueue: chaining segments, new len %u\n", useg->len));
     if (seg == queue) {
       seg = NULL;
     }
     memp_free(MEMP_TCP_SEG, queue);
   }
   else {
+    /* empty list */
     if (useg == NULL) {
+      /* initialize list with this segment */
       pcb->unsent = queue;
-
     }
+    /* enqueue segment */
     else {
       useg->next = queue;
     }
@@ -315,9 +331,8 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
   pcb->snd_queuelen = queuelen;
   LWIP_DEBUGF(TCP_QLEN_DEBUG, ("tcp_enqueue: %d (after enqueued)\n", pcb->snd_queuelen));
   if (pcb->snd_queuelen != 0) {
-    LWIP_ASSERT("tcp_enqueue: valid queue length", pcb->unacked != NULL ||
+    LWIP_ASSERT("tcp_enqueue: valid queue length",
-      pcb->unsent != NULL);
+      pcb->unacked != NULL || pcb->unsent != NULL);
-
   }
 
   /* Set the PSH flag in the last segment that we enqueued, but only
@@ -336,7 +351,6 @@ tcp_enqueue(struct tcp_pcb *pcb, void *arg, u16_t len,
   if (pcb->snd_queuelen != 0) {
     LWIP_ASSERT("tcp_enqueue: valid queue length", pcb->unacked != NULL ||
       pcb->unsent != NULL);
-
   }
   LWIP_DEBUGF(TCP_QLEN_DEBUG | DBG_STATE, ("tcp_enqueue: %d (with mem err)\n", pcb->snd_queuelen));
   return ERR_MEM;
@@ -364,7 +378,6 @@ tcp_output(struct tcp_pcb *pcb)
 
   wnd = LWIP_MIN(pcb->snd_wnd, pcb->cwnd);
 
-
   seg = pcb->unsent;
 
   /* useg should point to last segment on unacked queue */
@@ -372,24 +385,24 @@ tcp_output(struct tcp_pcb *pcb)
   if (useg != NULL) {
     for (; useg->next != NULL; useg = useg->next);
   }                                                                             
-
    
-  /* If the TF_ACK_NOW flag is set, we check if there is data that is
+  /* If the TF_ACK_NOW flag is set and no data will be sent (either
-     to be sent. If data is to be sent out, we'll just piggyback our
+   * because the ->unsent queue is empty or because the window does
-     acknowledgement with the outgoing segment. If no data will be
+   * not allow it), construct an empty ACK segment and send it.
-     sent (either because the ->unsent queue is empty or because the
+   *
-     window doesn't allow it) we'll have to construct an empty ACK
+   * If data is to be sent, we will just piggyback the ACK (see below).
-     segment and send it. */
+   */
   if (pcb->flags & TF_ACK_NOW &&
      (seg == NULL ||
       ntohl(seg->tcphdr->seqno) - pcb->lastack + seg->len > wnd)) {
-    pcb->flags &= ~(TF_ACK_DELAY | TF_ACK_NOW);
     p = pbuf_alloc(PBUF_IP, TCP_HLEN, PBUF_RAM);
     if (p == NULL) {
       LWIP_DEBUGF(TCP_OUTPUT_DEBUG, ("tcp_output: (ACK) could not allocate pbuf\n"));
       return ERR_BUF;
     }
     LWIP_DEBUGF(TCP_OUTPUT_DEBUG, ("tcp_output: sending ACK for %lu\n", pcb->rcv_nxt));
+    /* remove ACK flags from the PCB, as we send an empty ACK now */
+    pcb->flags &= ~(TF_ACK_DELAY | TF_ACK_NOW);
 
     tcphdr = p->payload;
     tcphdr->src = htons(pcb->local_port);
@@ -406,7 +419,6 @@ tcp_output(struct tcp_pcb *pcb)
     tcphdr->chksum = inet_chksum_pseudo(p, &(pcb->local_ip), &(pcb->remote_ip),
           IP_PROTO_TCP, p->tot_len);
 #endif
-
     ip_output(p, &(pcb->local_ip), &(pcb->remote_ip), pcb->ttl, pcb->tos,
         IP_PROTO_TCP);
     pbuf_free(p);
@@ -431,7 +443,7 @@ tcp_output(struct tcp_pcb *pcb)
                             ntohl(seg->tcphdr->seqno), pcb->lastack));
   }
 #endif /* TCP_CWND_DEBUG */
-
+  /* data available and window allows it to be sent? */
   while (seg != NULL &&
   ntohl(seg->tcphdr->seqno) - pcb->lastack + seg->len <= wnd) {
 #if TCP_CWND_DEBUG
@@ -458,21 +470,26 @@ tcp_output(struct tcp_pcb *pcb)
     /* put segment on unacknowledged list if length > 0 */
     if (TCP_TCPLEN(seg) > 0) {
       seg->next = NULL;
+      /* unacked list is empty? */
       if (pcb->unacked == NULL) {
         pcb->unacked = seg;
         useg = seg;
+      /* unacked list is not empty? */
       } else {
-        /* In the case of fast retransmit, the packet should not go to the end
+        /* In the case of fast retransmit, the packet should not go to the tail
-         * of the unacked queue, but rather at the start. We need to check for
+         * of the unacked queue, but rather at the head. We need to check for
          * this case. -STJ Jul 27, 2004 */
         if (TCP_SEQ_LT(ntohl(seg->tcphdr->seqno), ntohl(useg->tcphdr->seqno))){
+          /* add segment to head of unacked list */
           seg->next = pcb->unacked;
           pcb->unacked = seg;
         } else {
+          /* add segment to tail of unacked list */
           useg->next = seg;
           useg = useg->next;
         }
       }
+    /* do not queue empty segments on the unacked list */
     } else {
       tcp_seg_free(seg);
     }
@@ -481,6 +498,9 @@ tcp_output(struct tcp_pcb *pcb)
   return ERR_OK;
 }
 
+/**
+ * Actually send a TCP segment over IP
+ */
 static void
 tcp_output_segment(struct tcp_seg *seg, struct tcp_pcb *pcb)
 {
@@ -576,6 +596,7 @@ tcp_rst(u32_t seqno, u32_t ackno,
   LWIP_DEBUGF(TCP_RST_DEBUG, ("tcp_rst: seqno %lu ackno %lu.\n", seqno, ackno));
 }
 
+/* requeue all unacked segments for retransmission */
 void
 tcp_rexmit_rto(struct tcp_pcb *pcb)
 {
@@ -585,22 +606,24 @@ tcp_rexmit_rto(struct tcp_pcb *pcb)
     return;
   }
 
-  /* Move all unacked segments to the unsent queue. */
+  /* Move all unacked segments to the head of the unsent queue */
   for (seg = pcb->unacked; seg->next != NULL; seg = seg->next);
+  /* concatenate unsent queue after unacked queue */
   seg->next = pcb->unsent;
+  /* unsent queue is the concatenated queue (of unacked, unsent) */
   pcb->unsent = pcb->unacked;
+  /* unacked queue is now empty */
   pcb->unacked = NULL;
 
   pcb->snd_nxt = ntohl(pcb->unsent->tcphdr->seqno);
-
+  /* increment number of retransmissions */
   ++pcb->nrtx;
 
-  /* Don't take any rtt measurements after retransmitting. */
+  /* Don't take any RTT measurements after retransmitting. */
   pcb->rttest = 0;
 
-  /* Do the actual retransmission. */
+  /* Do the actual retransmission */
   tcp_output(pcb);
-
 }
 
 void