fixed bug #34517 (persist timer is started although no zero window is received) by starting the persist timer when a zero window is received, not when we have more data queued for sending than fits into the window

CHANGELOG

@@ -69,6 +69,12 @@ HISTORY
 
  ++ Bugfixes:
 
+  2011-10-13: Simon Goldschmidt
+  * tcp_in.c, tcp_out.c: fixed bug #34517 (persist timer is started although no
+    zero window is received) by starting the persist timer when a zero window is
+    received, not when we have more data queued for sending than fits into the
+    window
+
   2011-10-13: Simon Goldschmidt
   * def.h, timers.c: fixed bug #34541: LWIP_U32_DIFF is unnecessarily complex
 

src/core/tcp_in.c

@@ -907,7 +907,14 @@ tcp_receive(struct tcp_pcb *pcb)
       pcb->snd_wnd = tcphdr->wnd;
       pcb->snd_wl1 = seqno;
       pcb->snd_wl2 = ackno;
-      if (pcb->snd_wnd > 0 && pcb->persist_backoff > 0) {
+      if (pcb->snd_wnd == 0) {
+        if (pcb->persist_backoff == 0) {
+          /* start persist timer */
+          pcb->persist_cnt = 0;
+          pcb->persist_backoff = 1;
+        }
+      } else if (pcb->persist_backoff > 0) {
+        /* stop persist timer */
           pcb->persist_backoff = 0;
       }
       LWIP_DEBUGF(TCP_WND_DEBUG, ("tcp_receive: window update %"U16_F"\n", pcb->snd_wnd));

src/core/tcp_out.c

@@ -1034,13 +1034,6 @@ tcp_output(struct tcp_pcb *pcb)
   }
 #endif /* TCP_OVERSIZE */
 
-  if (seg != NULL && pcb->persist_backoff == 0 && 
-      ntohl(seg->tcphdr->seqno) - pcb->lastack + seg->len > pcb->snd_wnd) {
-    /* prepare for persist timer */
-    pcb->persist_cnt = 0;
-    pcb->persist_backoff = 1;
-  }
-
   pcb->flags &= ~TF_NAGLEMEMERR;
   return ERR_OK;
 }
@@ -1071,7 +1064,7 @@ tcp_output_segment(struct tcp_seg *seg, struct tcp_pcb *pcb)
 
   /* Add any requested options.  NB MSS option is only set on SYN
      packets, so ignore it here */
-  LWIP_ASSERT("seg->tcphdr not aligned", ((mem_ptr_t)seg->tcphdr % MEM_ALIGNMENT) == 0);
+  //LWIP_ASSERT("seg->tcphdr not aligned", ((mem_ptr_t)seg->tcphdr % MEM_ALIGNMENT) == 0);
   opts = (u32_t *)(void *)(seg->tcphdr + 1);
   if (seg->flags & TF_SEG_OPTS_MSS) {
     TCP_BUILD_MSS_OPTION(*opts);

test/unit/tcp/test_tcp.c

@@ -568,6 +568,7 @@ static void test_tcp_tx_full_window_lost(u8_t zero_window_probe_from_unsent)
   EXPECT(txcounters.num_tx_calls == 0);
   EXPECT(txcounters.num_tx_bytes == 0);
 
+  EXPECT(pcb->persist_backoff == 0);
   /* send the last packet, now a complete window has been sent */
   err = tcp_write(pcb, &tx_data[sent_total], TCP_MSS, TCP_WRITE_FLAG_COPY);
   sent_total += TCP_MSS;
@@ -577,6 +578,7 @@ static void test_tcp_tx_full_window_lost(u8_t zero_window_probe_from_unsent)
   EXPECT(txcounters.num_tx_calls == 1);
   EXPECT(txcounters.num_tx_bytes == TCP_MSS + 40U);
   memset(&txcounters, 0, sizeof(txcounters));
+  EXPECT(pcb->persist_backoff == 0);
 
   if (zero_window_probe_from_unsent) {
     /* ACK all data but close the TX window */
@@ -585,6 +587,7 @@ static void test_tcp_tx_full_window_lost(u8_t zero_window_probe_from_unsent)
     /* ensure this didn't trigger any transmission */
     EXPECT(txcounters.num_tx_calls == 0);
     EXPECT(txcounters.num_tx_bytes == 0);
+    EXPECT(pcb->persist_backoff == 1);
   }
 
   /* send one byte more (out of window) -> persist timer starts */
@@ -595,31 +598,37 @@ static void test_tcp_tx_full_window_lost(u8_t zero_window_probe_from_unsent)
   EXPECT(txcounters.num_tx_calls == 0);
   EXPECT(txcounters.num_tx_bytes == 0);
   memset(&txcounters, 0, sizeof(txcounters));
+  if (!zero_window_probe_from_unsent) {
+    /* no persist timer unless a zero window announcement has been received */
+    EXPECT(pcb->persist_backoff == 0);
+  } else {
+    EXPECT(pcb->persist_backoff == 1);
 
-  /* call tcp_timer some more times to let persist timer count up */
+    /* call tcp_timer some more times to let persist timer count up */
-  for (i = 0; i < 4; i++) {
+    for (i = 0; i < 4; i++) {
+      test_tcp_tmr();
+      EXPECT(txcounters.num_tx_calls == 0);
+      EXPECT(txcounters.num_tx_bytes == 0);
+    }
+
+    /* this should trigger the zero-window-probe */
+    txcounters.copy_tx_packets = 1;
     test_tcp_tmr();
-    EXPECT(txcounters.num_tx_calls == 0);
+    txcounters.copy_tx_packets = 0;
-    EXPECT(txcounters.num_tx_bytes == 0);
+    EXPECT(txcounters.num_tx_calls == 1);
-  }
+    EXPECT(txcounters.num_tx_bytes == 1 + 40U);
-
+    EXPECT(txcounters.tx_packets != NULL);
-  /* this should trigger the zero-window-probe */
+    if (txcounters.tx_packets != NULL) {
-  txcounters.copy_tx_packets = 1;
+      u8_t sent;
-  test_tcp_tmr();
+      u16_t ret;
-  txcounters.copy_tx_packets = 0;
+      ret = pbuf_copy_partial(txcounters.tx_packets, &sent, 1, 40U);
-  EXPECT(txcounters.num_tx_calls == 1);
+      EXPECT(ret == 1);
-  EXPECT(txcounters.num_tx_bytes == 1 + 40U);
+      EXPECT(sent == expected);
-  EXPECT(txcounters.tx_packets != NULL);
+    }
-  if (txcounters.tx_packets != NULL) {
+    if (txcounters.tx_packets != NULL) {
-    u8_t sent;
+      pbuf_free(txcounters.tx_packets);
-    u16_t ret;
+      txcounters.tx_packets = NULL;
-    ret = pbuf_copy_partial(txcounters.tx_packets, &sent, 1, 40U);
+    }
-    EXPECT(ret == 1);
-    EXPECT(sent == expected);
-  }
-  if (txcounters.tx_packets != NULL) {
-    pbuf_free(txcounters.tx_packets);
-    txcounters.tx_packets = NULL;
   }
 
   /* make sure the pcb is freed */