Fixed bug #28106: dup ack for fast retransmit could have non-zero length

CHANGELOG

@@ -46,6 +46,10 @@ HISTORY
 
   ++ Bugfixes:
 
+  2009-12-03: Simon Goldschmidt
+  * tcp.h, tcp_in.c, tcp_out.c: Fixed bug #28106: dup ack for fast retransmit
+    could have non-zero length
+
   2009-12-02: Simon Goldschmidt
   * tcp_in.c: Fixed bug #27904: TCP sends too many ACKs: delay resetting
     tcp_input_pcb until after calling the pcb's callbacks

src/core/tcp_in.c

@@ -1267,7 +1267,7 @@ tcp_receive(struct tcp_pcb *pcb)
 
       } else {
         /* We get here if the incoming segment is out-of-sequence. */
-        tcp_ack_now(pcb);
+        tcp_send_empty_ack(pcb);
 #if TCP_QUEUE_OOSEQ
         /* We queue the segment on the ->ooseq queue. */
         if (pcb->ooseq == NULL) {
@@ -1376,7 +1376,7 @@ tcp_receive(struct tcp_pcb *pcb)
       }
     } else {
       /* The incoming segment is not withing the window. */
-      tcp_ack_now(pcb);
+      tcp_send_empty_ack(pcb);
     }
   } else {
     /* Segments with length 0 is taken care of here. Segments that

src/core/tcp_out.c

@@ -454,6 +454,58 @@ tcp_build_timestamp_option(struct tcp_pcb *pcb, u32_t *opts)
 }
 #endif
 
+/** Send an ACK without data.
+ *
+ * @param pcb Protocol control block for the TCP connection to send the ACK
+ */
+err_t
+tcp_send_empty_ack(struct tcp_pcb *pcb)
+{
+  struct pbuf *p;
+  struct tcp_hdr *tcphdr;
+  u8_t optlen = 0;
+
+#if LWIP_TCP_TIMESTAMPS
+  if (pcb->flags & TF_TIMESTAMP) {
+    optlen = LWIP_TCP_OPT_LENGTH(TF_SEG_OPTS_TS);
+  }
+#endif
+  p = pbuf_alloc(PBUF_IP, TCP_HLEN + optlen, 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 %"U32_F"\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 = tcp_output_set_header(pcb, p, optlen, htonl(pcb->snd_nxt));
+
+  /* NB. MSS option is only sent on SYNs, so ignore it here */
+#if LWIP_TCP_TIMESTAMPS
+  pcb->ts_lastacksent = pcb->rcv_nxt;
+
+  if (pcb->flags & TF_TIMESTAMP) {
+    tcp_build_timestamp_option(pcb, (u32_t *)(tcphdr + 1));
+  }
+#endif 
+
+#if CHECKSUM_GEN_TCP
+  tcphdr->chksum = inet_chksum_pseudo(p, &(pcb->local_ip), &(pcb->remote_ip),
+        IP_PROTO_TCP, p->tot_len);
+#endif
+#if LWIP_NETIF_HWADDRHINT
+  ip_output_hinted(p, &(pcb->local_ip), &(pcb->remote_ip), pcb->ttl, pcb->tos,
+      IP_PROTO_TCP, &(pcb->addr_hint));
+#else /* LWIP_NETIF_HWADDRHINT*/
+  ip_output(p, &(pcb->local_ip), &(pcb->remote_ip), pcb->ttl, pcb->tos,
+      IP_PROTO_TCP);
+#endif /* LWIP_NETIF_HWADDRHINT*/
+  pbuf_free(p);
+
+  return ERR_OK;
+}
 
 /**
  * Find out what we can send and send it
@@ -465,14 +517,11 @@ tcp_build_timestamp_option(struct tcp_pcb *pcb, u32_t *opts)
 err_t
 tcp_output(struct tcp_pcb *pcb)
 {
-  struct pbuf *p;
-  struct tcp_hdr *tcphdr;
   struct tcp_seg *seg, *useg;
   u32_t wnd, snd_nxt;
 #if TCP_CWND_DEBUG
   s16_t i = 0;
 #endif /* TCP_CWND_DEBUG */
-  u8_t optlen = 0;
 
   /* First, check if we are invoked by the TCP input processing
      code. If so, we do not output anything. Instead, we rely on the
@@ -486,12 +535,6 @@ tcp_output(struct tcp_pcb *pcb)
 
   seg = pcb->unsent;
 
-  /* useg should point to last segment on unacked queue */
-  useg = pcb->unacked;
-  if (useg != NULL) {
-    for (; useg->next != NULL; useg = useg->next);
-  }
-
   /* If the TF_ACK_NOW flag is set and no data will be sent (either
    * because the ->unsent queue is empty or because the window does
    * not allow it), construct an empty ACK segment and send it.
@@ -501,44 +544,13 @@ tcp_output(struct tcp_pcb *pcb)
   if (pcb->flags & TF_ACK_NOW &&
      (seg == NULL ||
       ntohl(seg->tcphdr->seqno) - pcb->lastack + seg->len > wnd)) {
-#if LWIP_TCP_TIMESTAMPS
+     return tcp_send_empty_ack(pcb);
-    if (pcb->flags & TF_TIMESTAMP)
+  }
-      optlen = LWIP_TCP_OPT_LENGTH(TF_SEG_OPTS_TS);
-#endif
-    p = pbuf_alloc(PBUF_IP, TCP_HLEN + optlen, 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 %"U32_F"\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 = tcp_output_set_header(pcb, p, optlen, htonl(pcb->snd_nxt));
+  /* useg should point to last segment on unacked queue */
-
+  useg = pcb->unacked;
-    /* NB. MSS option is only sent on SYNs, so ignore it here */
+  if (useg != NULL) {
-#if LWIP_TCP_TIMESTAMPS
+    for (; useg->next != NULL; useg = useg->next);
-    pcb->ts_lastacksent = pcb->rcv_nxt;
-
-    if (pcb->flags & TF_TIMESTAMP)
-      tcp_build_timestamp_option(pcb, (u32_t *)(tcphdr + 1));
-#endif 
-
-#if CHECKSUM_GEN_TCP
-    tcphdr->chksum = inet_chksum_pseudo(p, &(pcb->local_ip), &(pcb->remote_ip),
-          IP_PROTO_TCP, p->tot_len);
-#endif
-#if LWIP_NETIF_HWADDRHINT
-    ip_output_hinted(p, &(pcb->local_ip), &(pcb->remote_ip), pcb->ttl, pcb->tos,
-        IP_PROTO_TCP, &(pcb->addr_hint));
-#else /* LWIP_NETIF_HWADDRHINT*/
-    ip_output(p, &(pcb->local_ip), &(pcb->remote_ip), pcb->ttl, pcb->tos,
-        IP_PROTO_TCP);
-#endif /* LWIP_NETIF_HWADDRHINT*/
-    pbuf_free(p);
-
-    return ERR_OK;
   }
 
 #if TCP_OUTPUT_DEBUG

src/include/lwip/tcp.h

@@ -125,6 +125,7 @@ void             tcp_fasttmr (void);
 /* Only used by IP to pass a TCP segment to TCP: */
 void             tcp_input   (struct pbuf *p, struct netif *inp);
 /* Used within the TCP code only: */
+err_t            tcp_send_empty_ack(struct tcp_pcb *pcb);
 err_t            tcp_output  (struct tcp_pcb *pcb);
 void             tcp_rexmit  (struct tcp_pcb *pcb);
 void             tcp_rexmit_rto  (struct tcp_pcb *pcb);