From c28764fd356632763614ea579f678d8f55eca4c7 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Wed, 26 Apr 2017 00:08:42 -0700 Subject: TCP ooo reception fixes - Improve svm fifo handling of out-of-order segments - Ensure tsval_recent is updated only if rcv_las falls withing the segments's sequence space - Avoid directly dropping old ACKs - Improve debugging Change-Id: I88dbe2394a0ad7eb389a4cc12d013a13733953aa Signed-off-by: Florin Coras --- src/vnet/tcp/tcp_debug.h | 15 ++++++ src/vnet/tcp/tcp_error.def | 3 +- src/vnet/tcp/tcp_format.c | 6 ++- src/vnet/tcp/tcp_input.c | 81 +++++++++++++++++++++----------- src/vnet/tcp/tcp_output.c | 4 +- src/vnet/tcp/tcp_test.c | 114 ++++++++++++++++++++++++++++++++++++++++++--- 6 files changed, 186 insertions(+), 37 deletions(-) (limited to 'src/vnet/tcp') diff --git a/src/vnet/tcp/tcp_debug.h b/src/vnet/tcp/tcp_debug.h index ecbf78874fb..b4497a3b8d8 100755 --- a/src/vnet/tcp/tcp_debug.h +++ b/src/vnet/tcp/tcp_debug.h @@ -50,6 +50,7 @@ _(CC_EVT, "cc event") \ _(CC_PACK, "cc partial ack") \ _(SEG_INVALID, "invalid segment") \ + _(PAWS_FAIL, "failed paws check") \ _(ACK_RCV_ERR, "invalid ack") \ _(RCV_WND_SHRUNK, "shrunk rcv_wnd") \ @@ -382,6 +383,20 @@ typedef enum _tcp_dbg_evt ed->data[4] = _tc->rcv_wnd; \ } +#define TCP_EVT_PAWS_FAIL_HANDLER(_tc, _seq, _end, ...) \ +{ \ + ELOG_TYPE_DECLARE (_e) = \ + { \ + .format = "paws fail: seq %u end %u tsval %u tsval_recent %u", \ + .format_args = "i4i4i4i4", \ + }; \ + DECLARE_ETD(_tc, _e, 4); \ + ed->data[0] = _seq - _tc->irs; \ + ed->data[1] = _end - _tc->irs; \ + ed->data[2] = _tc->opt.tsval; \ + ed->data[3] = _tc->tsval_recent; \ +} + #define TCP_EVT_ACK_RCV_ERR_HANDLER(_tc, _type, _ack, ...) \ { \ ELOG_TYPE_DECLARE (_e) = \ diff --git a/src/vnet/tcp/tcp_error.def b/src/vnet/tcp/tcp_error.def index 0d75d9751d7..a4e46d64629 100644 --- a/src/vnet/tcp/tcp_error.def +++ b/src/vnet/tcp/tcp_error.def @@ -37,4 +37,5 @@ tcp_error (PKTS_SENT, "Packets sent") tcp_error (FILTERED_DUPACKS, "Filtered duplicate ACKs") tcp_error (RST_SENT, "Resets sent") tcp_error (INVALID_CONNECTION, "Invalid connection") -tcp_error (NO_WND, "No window") \ No newline at end of file +tcp_error (NO_WND, "No window") +tcp_error (CONNECTION_CLOSED, "Connection closed") \ No newline at end of file diff --git a/src/vnet/tcp/tcp_format.c b/src/vnet/tcp/tcp_format.c index 3148fd406c9..4de99235934 100644 --- a/src/vnet/tcp/tcp_format.c +++ b/src/vnet/tcp/tcp_format.c @@ -131,11 +131,13 @@ format_tcp_header (u8 * s, va_list * args) u8 * format_tcp_sacks (u8 * s, va_list * args) { - sack_block_t *sacks = va_arg (*args, sack_block_t *); + tcp_connection_t *tc = va_arg (*args, tcp_connection_t *); + sack_block_t *sacks = tc->snd_sacks; sack_block_t *block; vec_foreach (block, sacks) { - s = format (s, " start %u end %u\n", block->start, block->end); + s = format (s, " start %u end %u\n", block->start - tc->irs, + block->end - tc->irs); } return s; } diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c index 3c65a5ea73d..0030cfe24ab 100644 --- a/src/vnet/tcp/tcp_input.c +++ b/src/vnet/tcp/tcp_input.c @@ -208,6 +208,15 @@ tcp_options_parse (tcp_header_t * th, tcp_options_t * to) } } +/** + * RFC1323: Check against wrapped sequence numbers (PAWS). If we have + * timestamp to echo and it's less than tsval_recent, drop segment + * but still send an ACK in order to retain TCP's mechanism for detecting + * and recovering from half-open connections + * + * Or at least that's what the theory says. It seems that this might not work + * very well with packet reordering and fast retransmit. XXX + */ always_inline int tcp_segment_check_paws (tcp_connection_t * tc) { @@ -215,6 +224,27 @@ tcp_segment_check_paws (tcp_connection_t * tc) && timestamp_lt (tc->opt.tsval, tc->tsval_recent); } +/** + * Update tsval recent + */ +always_inline void +tcp_update_timestamp (tcp_connection_t * tc, u32 seq, u32 seq_end) +{ + /* + * RFC1323: If Last.ACK.sent falls within the range of sequence numbers + * of an incoming segment: + * SEG.SEQ <= Last.ACK.sent < SEG.SEQ + SEG.LEN + * then the TSval from the segment is copied to TS.Recent; + * otherwise, the TSval is ignored. + */ + if (tcp_opts_tstamp (&tc->opt) && tc->tsval_recent + && seq_leq (seq, tc->rcv_las) && seq_leq (tc->rcv_las, seq_end)) + { + tc->tsval_recent = tc->opt.tsval; + tc->tsval_recent_age = tcp_time_now (); + } +} + /** * Validate incoming segment as per RFC793 p. 69 and RFC1323 p. 19 * @@ -228,21 +258,16 @@ static int tcp_segment_validate (vlib_main_t * vm, tcp_connection_t * tc0, vlib_buffer_t * b0, tcp_header_t * th0, u32 * next0) { - u8 paws_failed; - if (PREDICT_FALSE (!tcp_ack (th0) && !tcp_rst (th0) && !tcp_syn (th0))) return -1; tcp_options_parse (th0, &tc0->opt); - /* RFC1323: Check against wrapped sequence numbers (PAWS). If we have - * timestamp to echo and it's less than tsval_recent, drop segment - * but still send an ACK in order to retain TCP's mechanism for detecting - * and recovering from half-open connections */ - paws_failed = tcp_segment_check_paws (tc0); - if (paws_failed) + if (tcp_segment_check_paws (tc0)) { clib_warning ("paws failed"); + TCP_EVT_DBG (TCP_EVT_PAWS_FAIL, tc0, vnet_buffer (b0)->tcp.seq_number, + vnet_buffer (b0)->tcp.seq_end); /* If it just so happens that a segment updates tsval_recent for a * segment over 24 days old, invalidate tsval_recent. */ @@ -251,6 +276,7 @@ tcp_segment_validate (vlib_main_t * vm, tcp_connection_t * tc0, { /* Age isn't reset until we get a valid tsval (bsd inspired) */ tc0->tsval_recent = 0; + clib_warning ("paws failed - really old segment. REALLY?"); } else { @@ -305,12 +331,9 @@ tcp_segment_validate (vlib_main_t * vm, tcp_connection_t * tc0, return -1; } - /* If PAWS passed and segment in window, save timestamp */ - if (!paws_failed) - { - tc0->tsval_recent = tc0->opt.tsval; - tc0->tsval_recent_age = tcp_time_now (); - } + /* If segment in window, save timestamp */ + tcp_update_timestamp (tc0, vnet_buffer (b0)->tcp.seq_number, + vnet_buffer (b0)->tcp.seq_end); return 0; } @@ -835,7 +858,8 @@ tcp_rcv_ack (tcp_connection_t * tc, vlib_buffer_t * b, TCP_EVT_DBG (TCP_EVT_DUPACK_RCVD, tc); tcp_cc_rcv_dupack (tc, vnet_buffer (b)->tcp.ack_number); } - return -1; + /* Don't drop yet */ + return 0; } if (tcp_opts_sack_permitted (&tc->opt)) @@ -932,10 +956,6 @@ tcp_update_sack_list (tcp_connection_t * tc, u32 start, u32 end) { vec_add1 (new_list, tc->snd_sacks[i]); } - else - { - clib_warning ("dropped sack blocks"); - } } ASSERT (vec_len (new_list) <= TCP_MAX_SACK_BLOCKS); @@ -1011,7 +1031,6 @@ tcp_session_enqueue_ooo (tcp_connection_t * tc, vlib_buffer_t * b, u16 data_len) { stream_session_t *s0; - u32 offset; int rv; /* Pure ACK. Do nothing */ @@ -1021,12 +1040,11 @@ tcp_session_enqueue_ooo (tcp_connection_t * tc, vlib_buffer_t * b, } s0 = stream_session_get (tc->c_s_index, tc->c_thread_index); - offset = vnet_buffer (b)->tcp.seq_number - tc->irs; - clib_warning ("ooo: offset %d len %d", offset, data_len); - - rv = svm_fifo_enqueue_with_offset (s0->server_rx_fifo, offset, data_len, - vlib_buffer_get_current (b)); + /* Enqueue out-of-order data with absolute offset */ + rv = svm_fifo_enqueue_with_offset (s0->server_rx_fifo, + vnet_buffer (b)->tcp.seq_number, + data_len, vlib_buffer_get_current (b)); /* Nothing written */ if (rv) @@ -1542,6 +1560,9 @@ tcp46_syn_sent_inline (vlib_main_t * vm, vlib_node_runtime_t * node, /* Notify app that we have connection */ stream_session_connect_notify (&new_tc0->connection, sst, 0); + stream_session_init_fifos_pointers (&new_tc0->connection, + new_tc0->irs + 1, + new_tc0->iss + 1); /* Make sure after data segment processing ACK is sent */ new_tc0->flags |= TCP_CONN_SNDACK; } @@ -1552,7 +1573,9 @@ tcp46_syn_sent_inline (vlib_main_t * vm, vlib_node_runtime_t * node, /* Notify app that we have connection */ stream_session_connect_notify (&new_tc0->connection, sst, 0); - + stream_session_init_fifos_pointers (&new_tc0->connection, + new_tc0->irs + 1, + new_tc0->iss + 1); tcp_make_synack (new_tc0, b0); next0 = tcp_next_output (is_ip4); @@ -2139,6 +2162,10 @@ tcp46_listen_inline (vlib_main_t * vm, vlib_node_runtime_t * node, tcp_make_synack (child0, b0); next0 = tcp_next_output (is_ip4); + /* Init fifo pointers after we have iss */ + stream_session_init_fifos_pointers (&child0->connection, + child0->irs + 1, + child0->iss + 1); drop: if (PREDICT_FALSE (b0->flags & VLIB_BUFFER_IS_TRACED)) { @@ -2474,6 +2501,7 @@ do { \ _(LISTEN, TCP_FLAG_SYN, TCP_INPUT_NEXT_LISTEN, TCP_ERROR_NONE); /* ACK for for a SYN-ACK -> tcp-rcv-process. */ _(SYN_RCVD, TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); + _(SYN_RCVD, TCP_FLAG_RST, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); /* SYN-ACK for a SYN */ _(SYN_SENT, TCP_FLAG_SYN | TCP_FLAG_ACK, TCP_INPUT_NEXT_SYN_SENT, TCP_ERROR_NONE); @@ -2499,6 +2527,7 @@ do { \ _(FIN_WAIT_2, TCP_FLAG_FIN | TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); _(LAST_ACK, TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); + _(CLOSED, TCP_FLAG_ACK, TCP_INPUT_NEXT_DROP, TCP_ERROR_CONNECTION_CLOSED); #undef _ } diff --git a/src/vnet/tcp/tcp_output.c b/src/vnet/tcp/tcp_output.c index 4e1a7aa5ef4..a85d30daddb 100644 --- a/src/vnet/tcp/tcp_output.c +++ b/src/vnet/tcp/tcp_output.c @@ -359,7 +359,8 @@ tcp_make_established_options (tcp_connection_t * tc, tcp_options_t * opts) { opts->flags |= TCP_OPTS_FLAG_SACK; opts->sacks = tc->snd_sacks; - opts->n_sack_blocks = vec_len (tc->snd_sacks); + opts->n_sack_blocks = clib_min (vec_len (tc->snd_sacks), + TCP_OPTS_MAX_SACK_BLOCKS); len += 2 + TCP_OPTION_LEN_SACK_BLOCK * opts->n_sack_blocks; } } @@ -917,6 +918,7 @@ tcp_push_hdr_i (tcp_connection_t * tc, vlib_buffer_t * b, vnet_buffer (b)->tcp.connection_index = tc->c_c_index; tc->snd_nxt += data_len; + tc->rcv_las = tc->rcv_nxt; /* TODO this is updated in output as well ... */ if (tc->snd_nxt > tc->snd_una_max) diff --git a/src/vnet/tcp/tcp_test.c b/src/vnet/tcp/tcp_test.c index ed0322062bd..a457ac8f7fa 100644 --- a/src/vnet/tcp/tcp_test.c +++ b/src/vnet/tcp/tcp_test.c @@ -231,7 +231,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input) tcp_update_sack_list (tc, 300, 300); if (verbose) vlib_cli_output (vm, "overlap first 2 segments:\n%U", - format_tcp_sacks, tc->snd_sacks); + format_tcp_sacks, tc); TCP_TEST ((vec_len (tc->snd_sacks) == 3), "sack blocks %d expected %d", vec_len (tc->snd_sacks), 3); TCP_TEST ((tc->snd_sacks[0].start == 900), @@ -244,7 +244,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input) tcp_update_sack_list (tc, 1100, 1200); if (verbose) vlib_cli_output (vm, "add new segment [1100, 1200]\n%U", - format_tcp_sacks, tc->snd_sacks); + format_tcp_sacks, tc); TCP_TEST ((vec_len (tc->snd_sacks) == 4), "sack blocks %d expected %d", vec_len (tc->snd_sacks), 4); TCP_TEST ((tc->snd_sacks[0].start == 1100), @@ -257,7 +257,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input) tcp_update_sack_list (tc, 800, 900); if (verbose) vlib_cli_output (vm, "join middle segments [800, 900]\n%U", - format_tcp_sacks, tc->snd_sacks); + format_tcp_sacks, tc); TCP_TEST ((vec_len (tc->snd_sacks) == 3), "sack blocks %d expected %d", vec_len (tc->snd_sacks), 3); @@ -271,8 +271,7 @@ tcp_test_sack_tx (vlib_main_t * vm, unformat_input_t * input) tc->rcv_nxt = 1200; tcp_update_sack_list (tc, 1200, 1200); if (verbose) - vlib_cli_output (vm, "advance rcv_nxt to 1200\n%U", - format_tcp_sacks, tc->snd_sacks); + vlib_cli_output (vm, "advance rcv_nxt to 1200\n%U", format_tcp_sacks, tc); TCP_TEST ((vec_len (tc->snd_sacks) == 0), "sack blocks %d expected %d", vec_len (tc->snd_sacks), 0); return 0; @@ -502,7 +501,13 @@ tcp_test_fifo1 (vlib_main_t * vm, unformat_input_t * input) { offset = (2 * i + 1) * sizeof (u32); data = (u8 *) (test_data + (2 * i + 1)); - rv = svm_fifo_enqueue_with_offset (f, offset, sizeof (u32), data); + if (i == 0) + { + rv = svm_fifo_enqueue_nowait (f, sizeof (u32), data); + rv = rv > 0 ? 0 : rv; + } + else + rv = svm_fifo_enqueue_with_offset (f, offset, sizeof (u32), data); if (verbose) vlib_cli_output (vm, "add [%d] [%d, %d]", 2 * i + 1, offset, offset + sizeof (u32)); @@ -517,6 +522,26 @@ tcp_test_fifo1 (vlib_main_t * vm, unformat_input_t * input) vlib_cli_output (vm, "fifo after odd segs: %U", format_svm_fifo, f, 1); TCP_TEST ((f->tail == 8), "fifo tail %u", f->tail); + TCP_TEST ((svm_fifo_number_ooo_segments (f) == 2), + "number of ooo segments %u", svm_fifo_number_ooo_segments (f)); + + /* + * Try adding a completely overlapped segment + */ + offset = 3 * sizeof (u32); + data = (u8 *) (test_data + 3); + rv = svm_fifo_enqueue_with_offset (f, offset, sizeof (u32), data); + if (rv) + { + clib_warning ("enqueue returned %d", rv); + goto err; + } + + if (verbose) + vlib_cli_output (vm, "fifo after overlap seg: %U", format_svm_fifo, f, 1); + + TCP_TEST ((svm_fifo_number_ooo_segments (f) == 2), + "number of ooo segments %u", svm_fifo_number_ooo_segments (f)); /* * Make sure format functions are not buggy @@ -887,7 +912,7 @@ tcp_test_fifo3 (vlib_main_t * vm, unformat_input_t * input) f->head = fifo_initial_offset; f->tail = fifo_initial_offset; - for (i = 0; i < vec_len (generate); i++) + for (i = !randomize; i < vec_len (generate); i++) { tp = generate + i; svm_fifo_enqueue_with_offset (f, fifo_initial_offset + tp->offset, @@ -895,6 +920,10 @@ tcp_test_fifo3 (vlib_main_t * vm, unformat_input_t * input) (u8 *) data_pattern + tp->offset); } + /* Add the first segment in order for non random data */ + if (!randomize) + svm_fifo_enqueue_nowait (f, generate[0].len, (u8 *) data_pattern); + /* * Expected result: one big fat chunk at offset 1 if randomize == 1 */ @@ -964,6 +993,73 @@ tcp_test_fifo3 (vlib_main_t * vm, unformat_input_t * input) return 0; } +static int +tcp_test_fifo4 (vlib_main_t * vm, unformat_input_t * input) +{ + svm_fifo_t *f; + u32 fifo_size = 6 << 10; + u32 fifo_initial_offset = 1000000000; + u32 test_n_bytes = 5000, j; + u8 *test_data = 0, *data_buf = 0; + int i, rv, verbose = 0; + + while (unformat_check_input (input) != UNFORMAT_END_OF_INPUT) + { + if (unformat (input, "verbose")) + verbose = 1; + else + { + clib_error_t *e = clib_error_return + (0, "unknown input `%U'", format_unformat_error, input); + clib_error_report (e); + return -1; + } + } + + /* + * Create a fifo and add segments + */ + f = fifo_prepare (fifo_size); + + /* Set head and tail pointers */ + fifo_initial_offset = fifo_initial_offset % fifo_size; + svm_fifo_init_pointers (f, fifo_initial_offset); + + vec_validate (test_data, test_n_bytes - 1); + for (i = 0; i < vec_len (test_data); i++) + test_data[i] = i; + + for (i = test_n_bytes - 1; i > 0; i--) + { + rv = svm_fifo_enqueue_with_offset (f, fifo_initial_offset + i, + sizeof (u8), &test_data[i]); + if (verbose) + vlib_cli_output (vm, "add [%d] [%d, %d]", i, i, i + sizeof (u8)); + if (rv) + { + clib_warning ("enqueue returned %d", rv); + svm_fifo_free (f); + vec_free (test_data); + return -1; + } + } + + svm_fifo_enqueue_nowait (f, sizeof (u8), &test_data[0]); + + vec_validate (data_buf, vec_len (test_data)); + + svm_fifo_dequeue_nowait (f, vec_len (test_data), data_buf); + rv = compare_data (data_buf, test_data, 0, vec_len (test_data), &j); + if (rv) + vlib_cli_output (vm, "[%d] dequeued %u expected %u", j, data_buf[j], + test_data[j]); + TCP_TEST ((rv == 0), "dequeued compared to original returned %d", rv); + + svm_fifo_free (f); + vec_free (test_data); + return 0; +} + static int tcp_test_fifo (vlib_main_t * vm, unformat_input_t * input) { @@ -1028,6 +1124,10 @@ tcp_test_fifo (vlib_main_t * vm, unformat_input_t * input) { res = tcp_test_fifo1 (vm, input); } + else if (unformat (input, "fifo4")) + { + res = tcp_test_fifo4 (vm, input); + } } return res; -- cgit 1.2.3-korg