diff --git a/CHANGELOG b/CHANGELOG index 701bd75d..fcd86b45 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -52,6 +52,9 @@ HISTORY ++ Bugfixes: + 2017-05-09: Joel Cunningham + * tcp: add zero-window probe timeout (bug #50837) + 2017-04-11: Simon Goldschmidt * sockets.c: task #14420 (Remove sys_sem_signal from inside SYS_ARCH_PROTECT crit section) done for LWIP_TCPIP_CORE_LOCKING==1 diff --git a/src/core/tcp.c b/src/core/tcp.c index 82002be6..dcd2c5ac 100644 --- a/src/core/tcp.c +++ b/src/core/tcp.c @@ -1072,17 +1072,21 @@ tcp_slowtmr_start: LWIP_DEBUGF(TCP_DEBUG, ("tcp_slowtmr: max DATA retries reached\n")); } else { if (pcb->persist_backoff > 0) { - /* If snd_wnd is zero, use persist timer to send 1 byte probes - * instead of using the standard retransmission mechanism. */ - u8_t backoff_cnt = tcp_persist_backoff[pcb->persist_backoff-1]; - if (pcb->persist_cnt < backoff_cnt) { - pcb->persist_cnt++; - } - if (pcb->persist_cnt >= backoff_cnt) { - if (tcp_zero_window_probe(pcb) == ERR_OK) { - pcb->persist_cnt = 0; - if (pcb->persist_backoff < sizeof(tcp_persist_backoff)) { - pcb->persist_backoff++; + if (pcb->persist_probe >= TCP_MAXRTX) { + ++pcb_remove; /* max probes reached */ + } else { + /* If snd_wnd is zero, use persist timer to send 1 byte probes + * instead of using the standard retransmission mechanism. */ + u8_t backoff_cnt = tcp_persist_backoff[pcb->persist_backoff-1]; + if (pcb->persist_cnt < backoff_cnt) { + pcb->persist_cnt++; + } + if (pcb->persist_cnt >= backoff_cnt) { + if (tcp_zero_window_probe(pcb) == ERR_OK) { + pcb->persist_cnt = 0; + if (pcb->persist_backoff < sizeof(tcp_persist_backoff)) { + pcb->persist_backoff++; + } } } } diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index fcfbf3b1..3c94763a 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -765,6 +765,7 @@ tcp_process(struct tcp_pcb *pcb) pcb->tmr = tcp_ticks; } pcb->keep_cnt_sent = 0; + pcb->persist_probe = 0; tcp_parseopt(pcb); @@ -1093,10 +1094,11 @@ tcp_receive(struct tcp_pcb *pcb) /* start persist timer */ pcb->persist_cnt = 0; pcb->persist_backoff = 1; + pcb->persist_probe = 0; } } else if (pcb->persist_backoff > 0) { /* stop persist timer */ - pcb->persist_backoff = 0; + pcb->persist_backoff = 0; } LWIP_DEBUGF(TCP_WND_DEBUG, ("tcp_receive: window update %"TCPWNDSIZE_F"\n", pcb->snd_wnd)); #if TCP_WND_DEBUG diff --git a/src/core/tcp_out.c b/src/core/tcp_out.c index c87b6a9a..dff3f210 100644 --- a/src/core/tcp_out.c +++ b/src/core/tcp_out.c @@ -1103,6 +1103,7 @@ tcp_output(struct tcp_pcb *pcb) if (pcb->persist_backoff == 0) { pcb->persist_cnt = 0; pcb->persist_backoff = 1; + pcb->persist_probe = 0; } goto output_done; } @@ -1638,6 +1639,14 @@ tcp_zero_window_probe(struct tcp_pcb *pcb) return ERR_OK; } + /* increment probe count. NOTE: we record probe even if it fails + to actually transmit due to an error. This ensures memory exhaustion/ + routing problem doesn't leave a zero-window pcb as an indefinite zombie. + RTO mechanism has similar behavior, see pcb->nrtx */ + if (pcb->persist_probe < 0xFF) { + ++pcb->persist_probe; + } + is_fin = ((TCPH_FLAGS(seg->tcphdr) & TCP_FIN) != 0) && (seg->len == 0); /* we want to send one seqno: either FIN or data (no options) */ len = is_fin ? 0 : 1; diff --git a/src/include/lwip/tcp.h b/src/include/lwip/tcp.h index 9c187299..ad192390 100644 --- a/src/include/lwip/tcp.h +++ b/src/include/lwip/tcp.h @@ -310,6 +310,8 @@ struct tcp_pcb { u8_t persist_cnt; /* Persist timer back-off */ u8_t persist_backoff; + /* Number of persist probes */ + u8_t persist_probe; /* KEEPALIVE counter */ u8_t keep_cnt_sent; diff --git a/test/unit/tcp/test_tcp.c b/test/unit/tcp/test_tcp.c index 2b3a3ebd..3edde8b6 100644 --- a/test/unit/tcp/test_tcp.c +++ b/test/unit/tcp/test_tcp.c @@ -816,6 +816,185 @@ START_TEST(test_tcp_rto_tracking) } END_TEST +START_TEST(test_tcp_rto_timeout) +{ + struct netif netif; + struct test_tcp_txcounters txcounters; + struct test_tcp_counters counters; + struct tcp_pcb *pcb, *cur; + err_t err; + u16_t i; + LWIP_UNUSED_ARG(_i); + + /* Setup data for a single segment */ + for (i = 0; i < TCP_MSS; i++) { + tx_data[i] = (u8_t)i; + } + + /* initialize local vars */ + test_tcp_init_netif(&netif, &txcounters, &test_local_ip, &test_netmask); + memset(&counters, 0, sizeof(counters)); + + /* create and initialize the pcb */ + tcp_ticks = SEQNO1 - ISS; + pcb = test_tcp_new_counters_pcb(&counters); + EXPECT_RET(pcb != NULL); + tcp_set_state(pcb, ESTABLISHED, &test_local_ip, &test_remote_ip, TEST_LOCAL_PORT, TEST_REMOTE_PORT); + pcb->mss = TCP_MSS; + pcb->cwnd = TCP_MSS; + + /* send our segment */ + err = tcp_write(pcb, &tx_data[0], TCP_MSS, TCP_WRITE_FLAG_COPY); + EXPECT_RET(err == ERR_OK); + err = tcp_output(pcb); + EXPECT(txcounters.num_tx_calls == 1); + EXPECT(txcounters.num_tx_bytes == 1 * (TCP_MSS + 40U)); + memset(&txcounters, 0, sizeof(txcounters)); + + /* ensure no errors have been recorded */ + EXPECT(counters.err_calls == 0); + EXPECT(counters.last_err == ERR_OK); + + /* Force us into retransmisson timeout */ + while (!(pcb->flags & TF_RTO)) { + test_tcp_tmr(); + } + + /* check first rexmit */ + EXPECT(pcb->nrtx == 1); + EXPECT(txcounters.num_tx_calls == 1); + EXPECT(txcounters.num_tx_bytes == 1 * (TCP_MSS + 40U)); + + /* still no error expected */ + EXPECT(counters.err_calls == 0); + EXPECT(counters.last_err == ERR_OK); + + /* keep running the timer till we hit our maximum RTO */ + while (counters.last_err == ERR_OK) { + test_tcp_tmr(); + } + + /* check number of retransmissions */ + EXPECT(txcounters.num_tx_calls == TCP_MAXRTX); + EXPECT(txcounters.num_tx_bytes == TCP_MAXRTX * (TCP_MSS + 40U)); + + /* check the connection (pcb) has been aborted */ + EXPECT(counters.err_calls == 1); + EXPECT(counters.last_err == ERR_ABRT); + /* check our pcb is no longer active */ + for (cur = tcp_active_pcbs; cur != NULL; cur = cur->next) { + EXPECT(cur != pcb); + } + EXPECT_RET(MEMP_STATS_GET(used, MEMP_TCP_PCB) == 0); +} +END_TEST + +START_TEST(test_tcp_zwp_timeout) +{ + struct netif netif; + struct test_tcp_txcounters txcounters; + struct test_tcp_counters counters; + struct tcp_pcb *pcb, *cur; + struct pbuf* p; + err_t err; + u16_t i; + LWIP_UNUSED_ARG(_i); + + /* Setup data for two segments */ + for (i = 0; i < 2*TCP_MSS; i++) { + tx_data[i] = (u8_t)i; + } + + /* initialize local vars */ + test_tcp_init_netif(&netif, &txcounters, &test_local_ip, &test_netmask); + memset(&counters, 0, sizeof(counters)); + + /* create and initialize the pcb */ + tcp_ticks = SEQNO1 - ISS; + pcb = test_tcp_new_counters_pcb(&counters); + EXPECT_RET(pcb != NULL); + tcp_set_state(pcb, ESTABLISHED, &test_local_ip, &test_remote_ip, TEST_LOCAL_PORT, TEST_REMOTE_PORT); + pcb->mss = TCP_MSS; + pcb->cwnd = TCP_MSS; + + /* send first segment */ + err = tcp_write(pcb, &tx_data[0], TCP_MSS, TCP_WRITE_FLAG_COPY); + EXPECT(err == ERR_OK); + err = tcp_output(pcb); + EXPECT(err == ERR_OK); + + /* verify segment is in-flight */ + EXPECT(pcb->unsent == NULL); + check_seqnos(pcb->unacked, 1, seqnos); + EXPECT(txcounters.num_tx_calls == 1); + EXPECT(txcounters.num_tx_bytes == 1 * (TCP_MSS + 40U)); + memset(&txcounters, 0, sizeof(txcounters)); + + /* ACK the segment and close the TX window */ + p = tcp_create_rx_segment_wnd(pcb, NULL, 0, 0, TCP_MSS, TCP_ACK, 0); + test_tcp_input(p, &netif); + EXPECT(pcb->unacked == NULL); + EXPECT(pcb->unsent == NULL); + EXPECT(pcb->persist_backoff == 1); + EXPECT(pcb->snd_wnd == 0); + + /* send second segment, should be buffered */ + err = tcp_write(pcb, &tx_data[TCP_MSS], TCP_MSS, TCP_WRITE_FLAG_COPY); + EXPECT(err == ERR_OK); + err = tcp_output(pcb); + EXPECT(err == ERR_OK); + + /* ensure it is buffered */ + EXPECT(pcb->unacked == NULL); + check_seqnos(pcb->unsent, 1, &seqnos[1]); + EXPECT(txcounters.num_tx_calls == 0); + EXPECT(txcounters.num_tx_bytes == 0); + + /* ensure no errors have been recorded */ + EXPECT(counters.err_calls == 0); + EXPECT(counters.last_err == ERR_OK); + + /* run timer till first probe */ + EXPECT(pcb->persist_probe == 0); + while (pcb->persist_probe == 0) { + test_tcp_tmr(); + } + EXPECT(txcounters.num_tx_calls == 1); + EXPECT(txcounters.num_tx_bytes == (1 + 40U)); + memset(&txcounters, 0, sizeof(txcounters)); + + /* respond to probe with remote's current SEQ, ACK, and zero-window */ + p = tcp_create_rx_segment_wnd(pcb, NULL, 0, 0, 0, TCP_ACK, 0); + test_tcp_input(p, &netif); + /* ensure zero-window is still active, but probe count reset */ + EXPECT(pcb->persist_backoff > 1); + EXPECT(pcb->persist_probe == 0); + EXPECT(pcb->snd_wnd == 0); + + /* ensure no errors have been recorded */ + EXPECT(counters.err_calls == 0); + EXPECT(counters.last_err == ERR_OK); + + /* now run the timer till we hit our maximum probe count */ + while (counters.last_err == ERR_OK) { + test_tcp_tmr(); + } + + /* check maximum number of 1 byte probes were sent */ + EXPECT(txcounters.num_tx_calls == TCP_MAXRTX); + EXPECT(txcounters.num_tx_bytes == TCP_MAXRTX * (1 + 40U)); + + /* check the connection (pcb) has been aborted */ + EXPECT(counters.err_calls == 1); + EXPECT(counters.last_err == ERR_ABRT); + /* check our pcb is no longer active */ + for (cur = tcp_active_pcbs; cur != NULL; cur = cur->next) { + EXPECT(cur != pcb); + } + EXPECT_RET(MEMP_STATS_GET(used, MEMP_TCP_PCB) == 0); +} +END_TEST + /** Create the suite including all tests for this module */ Suite * tcp_suite(void) @@ -829,7 +1008,9 @@ tcp_suite(void) TESTFUNC(test_tcp_rto_rexmit_wraparound), TESTFUNC(test_tcp_tx_full_window_lost_from_unacked), TESTFUNC(test_tcp_tx_full_window_lost_from_unsent), - TESTFUNC(test_tcp_rto_tracking) + TESTFUNC(test_tcp_rto_tracking), + TESTFUNC(test_tcp_rto_timeout), + TESTFUNC(test_tcp_zwp_timeout) }; return create_suite("TCP", tests, sizeof(tests)/sizeof(testfunc), tcp_setup, tcp_teardown); }