diff --git a/CHANGELOG b/CHANGELOG index 0f2bc6e5..789e1aa7 100644 --- a/CHANGELOG +++ b/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 diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index 17ed0b01..1824e92c 100644 --- a/src/core/tcp_in.c +++ b/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)); diff --git a/src/core/tcp_out.c b/src/core/tcp_out.c index 6ad1c1ae..93ed21fa 100644 --- a/src/core/tcp_out.c +++ b/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); diff --git a/test/unit/tcp/test_tcp.c b/test/unit/tcp/test_tcp.c index d6832504..52a7d00e 100644 --- a/test/unit/tcp/test_tcp.c +++ b/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 */ - for (i = 0; i < 4; i++) { + /* call tcp_timer some more times to let persist timer count up */ + 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); - EXPECT(txcounters.num_tx_bytes == 0); - } - - /* this should trigger the zero-window-probe */ - txcounters.copy_tx_packets = 1; - test_tcp_tmr(); - txcounters.copy_tx_packets = 0; - EXPECT(txcounters.num_tx_calls == 1); - EXPECT(txcounters.num_tx_bytes == 1 + 40U); - EXPECT(txcounters.tx_packets != NULL); - if (txcounters.tx_packets != NULL) { - u8_t sent; - u16_t ret; - 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; + txcounters.copy_tx_packets = 0; + EXPECT(txcounters.num_tx_calls == 1); + EXPECT(txcounters.num_tx_bytes == 1 + 40U); + EXPECT(txcounters.tx_packets != NULL); + if (txcounters.tx_packets != NULL) { + u8_t sent; + u16_t ret; + 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 */