From 067f8f963d64b1cbc70f2b78ebd2c6d3791e7d22 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Sat, 4 Jan 2020 00:53:04 +0000 Subject: tcp: fix duplicate sack whith reneging Type: fix Signed-off-by: Florin Coras Change-Id: I6f7fb91e059996ff702eb9c36e3abaed237fe221 --- src/plugins/unittest/tcp_test.c | 110 ++++++++++++++++++++++++++++++++++++---- src/vnet/tcp/tcp_input.c | 15 +++--- 2 files changed, 108 insertions(+), 17 deletions(-) diff --git a/src/plugins/unittest/tcp_test.c b/src/plugins/unittest/tcp_test.c index dbdb5a977f6..5474f8dc511 100644 --- a/src/plugins/unittest/tcp_test.c +++ b/src/plugins/unittest/tcp_test.c @@ -85,6 +85,7 @@ tcp_test_sack_rx (vlib_main_t * vm, unformat_input_t * input) clib_memset (tc, 0, sizeof (*tc)); + tc->flags |= TCP_CONN_FAST_RECOVERY | TCP_CONN_RECOVERY; tc->snd_una = 0; tc->snd_una_max = 1000; tc->snd_nxt = 1000; @@ -132,11 +133,12 @@ tcp_test_sack_rx (vlib_main_t * vm, unformat_input_t * input) TCP_TEST ((sb->lost_bytes == 300), "lost bytes %u", sb->lost_bytes); /* - * Inject odd blocks + * Inject odd blocks except the last + * */ vec_reset_length (tc->rcv_opts.sacks); - for (i = 0; i < 1000 / 200; i++) + for (i = 0; i < 800 / 200; i++) { vec_add1 (tc->rcv_opts.sacks, sacks[i * 2 + 1]); } @@ -148,33 +150,120 @@ tcp_test_sack_rx (vlib_main_t * vm, unformat_input_t * input) sb, tc); hole = scoreboard_first_hole (sb); - TCP_TEST ((pool_elts (sb->holes) == 1), + TCP_TEST ((pool_elts (sb->holes) == 2), "scoreboard has %d holes", pool_elts (sb->holes)); TCP_TEST ((hole->start == 0 && hole->end == 100), "first hole start %u end %u", hole->start, hole->end); - TCP_TEST ((sb->sacked_bytes == 900), "sacked bytes %d", sb->sacked_bytes); + TCP_TEST ((sb->sacked_bytes == 800), "sacked bytes %d", sb->sacked_bytes); TCP_TEST ((!sb->is_reneging), "is not reneging"); - TCP_TEST ((sb->high_sacked == 1000), "high sacked %u", sb->high_sacked); - TCP_TEST ((sb->last_sacked_bytes == 500), + TCP_TEST ((sb->high_sacked == 900), "high sacked %u", sb->high_sacked); + TCP_TEST ((sb->last_sacked_bytes == 400), "last sacked bytes %d", sb->last_sacked_bytes); TCP_TEST ((sb->lost_bytes == 100), "lost bytes %u", sb->lost_bytes); /* - * Ack until byte 100 - this is reneging because we should ack until 1000 + * Ack until byte 100 - this is reneging because we should ack until 900 */ tcp_rcv_sacks (tc, 100); if (verbose) vlib_cli_output (vm, "\nack until byte 100:\n%U", format_tcp_scoreboard, sb, tc); - TCP_TEST ((pool_elts (sb->holes) == 0), "scoreboard has %d elements", + TCP_TEST ((pool_elts (sb->holes) == 1), "scoreboard has %d elements", pool_elts (sb->holes)); TCP_TEST ((sb->is_reneging), "is reneging"); /* - * Sack all up to 1000 + * Make sure we accept duplicate acks while reneging. */ tc->snd_una = 100; + sb->high_rxt = 950; + + block.start = 900; + block.end = 950; + vec_add1 (tc->rcv_opts.sacks, block); + + tcp_rcv_sacks (tc, 100); + TCP_TEST ((pool_elts (sb->holes) == 1), "scoreboard has %d elements", + pool_elts (sb->holes)); + TCP_TEST ((sb->is_reneging), "is reneging"); + TCP_TEST ((sb->last_sacked_bytes == 50), "last sacked bytes %d", + sb->last_sacked_bytes); + TCP_TEST ((sb->rxt_sacked == 50), "last rxt sacked bytes %d", + sb->rxt_sacked); + + /* + * Sack all up to 950 + */ + tcp_rcv_sacks (tc, 950); + TCP_TEST ((sb->high_sacked == 950), "max sacked byte %u", sb->high_sacked); + TCP_TEST ((sb->sacked_bytes == 0), "sacked bytes %d", sb->sacked_bytes); + TCP_TEST ((sb->last_sacked_bytes == 0), + "last sacked bytes %d", sb->last_sacked_bytes); + TCP_TEST ((sb->lost_bytes == 0), "lost bytes %u", sb->lost_bytes); + TCP_TEST ((!sb->is_reneging), "is not reneging"); + + /* + * Sack [960 970] [980 990] + */ + sb->high_rxt = 985; + + tc->snd_una = 950; + vec_reset_length (tc->rcv_opts.sacks); + block.start = 960; + block.end = 970; + vec_add1 (tc->rcv_opts.sacks, block); + + block.start = 980; + block.end = 990; + vec_add1 (tc->rcv_opts.sacks, block); + + tcp_rcv_sacks (tc, 950); + TCP_TEST ((sb->high_sacked == 990), "max sacked byte %u", sb->high_sacked); + TCP_TEST ((sb->sacked_bytes == 20), "sacked bytes %d", sb->sacked_bytes); + TCP_TEST ((sb->last_sacked_bytes == 20), + "last sacked bytes %d", sb->last_sacked_bytes); + TCP_TEST ((sb->lost_bytes == 0), "lost bytes %u", sb->lost_bytes); + TCP_TEST ((!sb->is_reneging), "is not reneging"); + TCP_TEST ((sb->rxt_sacked == 15), "last rxt sacked bytes %d", + sb->rxt_sacked); + + /* + * Ack up to 960 (reneging) + [961 971] + */ + tc->rcv_opts.sacks[0].start = 961; + tc->rcv_opts.sacks[0].end = 971; + + tcp_rcv_sacks (tc, 960); + + TCP_TEST ((sb->is_reneging), "is reneging"); + TCP_TEST ((sb->sacked_bytes == 21), "sacked bytes %d", sb->sacked_bytes); + TCP_TEST ((sb->last_sacked_bytes == 1), + "last sacked bytes %d", sb->last_sacked_bytes); + TCP_TEST ((sb->rxt_sacked == 11), "last rxt sacked bytes %d", + sb->rxt_sacked); + TCP_TEST ((sb->last_bytes_delivered == 0), "last bytes delivered %d", + sb->last_bytes_delivered); + + /* + * Ack up to 960 (reneging) + [961 990] + */ + tc->snd_una = 960; + tc->rcv_opts.sacks[0].start = 961; + tc->rcv_opts.sacks[0].end = 990; + + tcp_rcv_sacks (tc, 960); + + TCP_TEST ((sb->is_reneging), "is reneging"); + TCP_TEST ((sb->sacked_bytes == 30), "sacked bytes %d", sb->sacked_bytes); + TCP_TEST ((sb->last_sacked_bytes == 9), + "last sacked bytes %d", sb->last_sacked_bytes); + TCP_TEST ((sb->rxt_sacked == 9), "last rxt sacked bytes %d", + sb->rxt_sacked); + + /* + * Sack all up to 1000 + */ tcp_rcv_sacks (tc, 1000); TCP_TEST ((sb->high_sacked == 1000), "max sacked byte %u", sb->high_sacked); TCP_TEST ((sb->sacked_bytes == 0), "sacked bytes %d", sb->sacked_bytes); @@ -183,11 +272,10 @@ tcp_test_sack_rx (vlib_main_t * vm, unformat_input_t * input) TCP_TEST ((sb->lost_bytes == 0), "lost bytes %u", sb->lost_bytes); TCP_TEST ((!sb->is_reneging), "is not reneging"); - /* * Add new block */ - + tc->flags = 0; vec_reset_length (tc->rcv_opts.sacks); block.start = 1200; diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c index e4bd156391f..ef5a16f22a6 100755 --- a/src/vnet/tcp/tcp_input.c +++ b/src/vnet/tcp/tcp_input.c @@ -1097,7 +1097,11 @@ tcp_rcv_sacks (tcp_connection_t * tc, u32 ack) hole = pool_elt_at_index (sb->holes, sb->head); if (PREDICT_FALSE (sb->is_reneging)) - sb->last_bytes_delivered += hole->start - tc->snd_una; + { + sb->last_bytes_delivered += clib_min (hole->start - tc->snd_una, + ack - tc->snd_una); + sb->is_reneging = seq_lt (ack, hole->start); + } while (hole && blk_index < vec_len (rcv_sacks)) { @@ -1180,7 +1184,7 @@ tcp_rcv_sacks (tcp_connection_t * tc, u32 ack) || sb->is_reneging || sb->holes[sb->head].start == ack); ASSERT (sb->last_lost_bytes <= sb->lost_bytes); ASSERT ((ack - tc->snd_una) + sb->last_sacked_bytes - - sb->last_bytes_delivered >= sb->rxt_sacked); + - sb->last_bytes_delivered >= sb->rxt_sacked || sb->is_reneging); ASSERT ((ack - tc->snd_una) >= tc->sack_sb.last_bytes_delivered || (tc->flags & TCP_CONN_FINSNT)); @@ -1510,14 +1514,13 @@ tcp_cc_handle_event (tcp_connection_t * tc, tcp_rate_sample_t * rs, } static void -tcp_handle_old_ack (tcp_connection_t * tc, vlib_buffer_t * b, - tcp_rate_sample_t * rs) +tcp_handle_old_ack (tcp_connection_t * tc, tcp_rate_sample_t * rs) { if (!tcp_in_cong_recovery (tc)) return; if (tcp_opts_sack_permitted (&tc->rcv_opts)) - tcp_rcv_sacks (tc, vnet_buffer (b)->tcp.ack_number); + tcp_rcv_sacks (tc, tc->snd_una); tc->bytes_acked = 0; @@ -1596,7 +1599,7 @@ tcp_rcv_ack (tcp_worker_ctx_t * wrk, tcp_connection_t * tc, vlib_buffer_t * b, if (seq_lt (vnet_buffer (b)->tcp.ack_number, tc->snd_una - tc->rcv_wnd)) return -1; - tcp_handle_old_ack (tc, b, &rs); + tcp_handle_old_ack (tc, &rs); /* Don't drop yet */ return 0; -- cgit 1.2.3-korg