From 85a3ddd4c854c6217272d8be031cfcb04d2e7685 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Mon, 24 Dec 2018 16:54:34 -0800 Subject: [PATCH] tcp: separate active and passive establish timers Change-Id: Ia2241e963cf45765d8d17c65eea781edbf74d4f9 Signed-off-by: Florin Coras --- src/vnet/tcp/tcp.c | 80 ++++++++++++++++++++++++-------------------- src/vnet/tcp/tcp.h | 3 +- src/vnet/tcp/tcp_debug.h | 85 ++++++++++++++++++++++++----------------------- src/vnet/tcp/tcp_input.c | 13 ++++---- src/vnet/tcp/tcp_output.c | 20 +++++------ 5 files changed, 107 insertions(+), 94 deletions(-) diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c index 8c8df00f098..4a63f3bc00c 100644 --- a/src/vnet/tcp/tcp.c +++ b/src/vnet/tcp/tcp.c @@ -172,7 +172,7 @@ tcp_half_open_connection_cleanup (tcp_connection_t * tc) /* Make sure this is the owning thread */ if (tc->c_thread_index != vlib_get_thread_index ()) return 1; - tcp_timer_reset (tc, TCP_TIMER_ESTABLISH); + tcp_timer_reset (tc, TCP_TIMER_ESTABLISH_AO); tcp_timer_reset (tc, TCP_TIMER_RETRANSMIT_SYN); tcp_half_open_connection_del (tc); return 0; @@ -308,6 +308,8 @@ tcp_connection_reset (tcp_connection_t * tc) break; case TCP_STATE_CLOSED: break; + default: + TCP_DBG ("reset state: %u", tc->state); } } @@ -333,7 +335,9 @@ tcp_connection_close (tcp_connection_t * tc) switch (tc->state) { case TCP_STATE_SYN_SENT: - /* Do nothing. Establish timer will pop and cleanup the connection */ + /* Try to cleanup. If not on the right thread, mark as half-open done. + * Connection will be cleaned up when establish timer pops */ + tcp_connection_cleanup (tc); break; case TCP_STATE_SYN_RCVD: tcp_connection_timers_reset (tc); @@ -349,7 +353,8 @@ tcp_connection_close (tcp_connection_t * tc) tcp_connection_set_state (tc, TCP_STATE_FIN_WAIT_1); /* Set a timer in case the peer stops responding. Otherwise the * connection will be stuck here forever. */ - tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_FINWAIT1_TIME); + ASSERT (tc->timers[TCP_TIMER_WAITCLOSE] == TCP_TIMER_HANDLE_INVALID); + tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_FINWAIT1_TIME); break; case TCP_STATE_CLOSE_WAIT: if (!session_tx_fifo_max_dequeue (&tc->connection)) @@ -367,17 +372,14 @@ tcp_connection_close (tcp_connection_t * tc) break; case TCP_STATE_CLOSED: tcp_connection_timers_reset (tc); + /* Delete connection but instead of doing it now wait until next + * dispatch cycle to give the session layer a chance to clear + * unhandled events */ + tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); break; default: TCP_DBG ("state: %u", tc->state); } - - /* If in CLOSED and WAITCLOSE timer is not set, delete connection. - * But instead of doing it now wait until next dispatch cycle to give - * the session layer a chance to clear unhandled events */ - if (!tcp_timer_is_active (tc, TCP_TIMER_WAITCLOSE) - && tc->state == TCP_STATE_CLOSED) - tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); } static void @@ -393,9 +395,7 @@ tcp_session_cleanup (u32 conn_index, u32 thread_index) { tcp_connection_t *tc; tc = tcp_connection_get (conn_index, thread_index); - tcp_connection_timers_reset (tc); - tc->state = TCP_STATE_CLOSED; - TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc); + tcp_connection_set_state (tc, TCP_STATE_CLOSED); tcp_connection_cleanup (tc); } @@ -1217,39 +1217,45 @@ tcp_timer_establish_handler (u32 conn_index) { tcp_connection_t *tc; - tc = tcp_half_open_connection_get (conn_index); - if (tc) - { - ASSERT (tc->state == TCP_STATE_SYN_SENT); - /* Notify app if we haven't tried to clean this up already */ - if (!(tc->flags & TCP_CONN_HALF_OPEN_DONE)) - session_stream_connect_notify (&tc->connection, 1 /* fail */ ); - } - else - { - tc = tcp_connection_get (conn_index, vlib_get_thread_index ()); - /* note: the connection may have already disappeared */ - if (PREDICT_FALSE (tc == 0)) - return; - ASSERT (tc->state == TCP_STATE_SYN_RCVD); - /* Start cleanup. App wasn't notified yet so use delete notify as - * opposed to delete to cleanup session layer state. */ - stream_session_delete_notify (&tc->connection); - } + tc = tcp_connection_get (conn_index, vlib_get_thread_index ()); + /* note: the connection may have already disappeared */ + if (PREDICT_FALSE (tc == 0)) + return; + ASSERT (tc->state == TCP_STATE_SYN_RCVD); + /* Start cleanup. App wasn't notified yet so use delete notify as + * opposed to delete to cleanup session layer state. */ + stream_session_delete_notify (&tc->connection); tc->timers[TCP_TIMER_ESTABLISH] = TCP_TIMER_HANDLE_INVALID; tcp_connection_cleanup (tc); } +static void +tcp_timer_establish_ao_handler (u32 conn_index) +{ + tcp_connection_t *tc; + + tc = tcp_half_open_connection_get (conn_index); + if (!tc) + return; + + ASSERT (tc->state == TCP_STATE_SYN_SENT); + /* Notify app if we haven't tried to clean this up already */ + if (!(tc->flags & TCP_CONN_HALF_OPEN_DONE)) + session_stream_connect_notify (&tc->connection, 1 /* fail */ ); + + tc->timers[TCP_TIMER_ESTABLISH_AO] = TCP_TIMER_HANDLE_INVALID; + tcp_connection_cleanup (tc); +} + static void tcp_timer_waitclose_handler (u32 conn_index) { - u32 thread_index = vlib_get_thread_index (); + u32 thread_index = vlib_get_thread_index (), rto; tcp_connection_t *tc; tc = tcp_connection_get (conn_index, thread_index); if (!tc) return; - tc->timers[TCP_TIMER_WAITCLOSE] = TCP_TIMER_HANDLE_INVALID; switch (tc->state) @@ -1287,8 +1293,9 @@ tcp_timer_waitclose_handler (u32 conn_index) * is closed. We haven't sent everything but we did try. */ tcp_cong_recovery_off (tc); tcp_send_fin (tc); + rto = clib_max (tc->rto >> tc->rto_boff, 1); tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, - clib_max (tc->rto * TCP_TO_TIMER_TICK, 1)); + clib_min (rto * TCP_TO_TIMER_TICK, TCP_2MSL_TIME)); session_stream_close_notify (&tc->connection); } else @@ -1321,7 +1328,8 @@ static timer_expiration_handler *timer_expiration_handlers[TCP_N_TIMERS] = tcp_timer_keep_handler, tcp_timer_waitclose_handler, tcp_timer_retransmit_syn_handler, - tcp_timer_establish_handler + tcp_timer_establish_handler, + tcp_timer_establish_ao_handler, }; /* *INDENT-ON* */ diff --git a/src/vnet/tcp/tcp.h b/src/vnet/tcp/tcp.h index bd0fa9a7204..3848f03f1dc 100644 --- a/src/vnet/tcp/tcp.h +++ b/src/vnet/tcp/tcp.h @@ -74,7 +74,8 @@ format_function_t format_tcp_rcv_sacks; _(KEEP, "KEEP") \ _(WAITCLOSE, "WAIT CLOSE") \ _(RETRANSMIT_SYN, "RETRANSMIT SYN") \ - _(ESTABLISH, "ESTABLISH") + _(ESTABLISH, "ESTABLISH") \ + _(ESTABLISH_AO, "ESTABLISH_AO") \ typedef enum _tcp_timers { diff --git a/src/vnet/tcp/tcp_debug.h b/src/vnet/tcp/tcp_debug.h index 5032588ca1e..b848d341dd7 100755 --- a/src/vnet/tcp/tcp_debug.h +++ b/src/vnet/tcp/tcp_debug.h @@ -388,6 +388,49 @@ if (_tc) \ ed->data[3] = _tc->snd_nxt - _tc->iss; \ ed->data[4] = _tc->rcv_nxt - _tc->irs; \ } + +#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...) \ +{ \ + tcp_connection_t *_tc; \ + if (_timer_id == TCP_TIMER_RETRANSMIT_SYN \ + || _timer_id == TCP_TIMER_ESTABLISH_AO) \ + { \ + _tc = tcp_half_open_connection_get (_tc_index); \ + } \ + else \ + { \ + u32 _thread_index = vlib_get_thread_index (); \ + _tc = tcp_connection_get (_tc_index, _thread_index); \ + } \ + ELOG_TYPE_DECLARE (_e) = \ + { \ + .format = "timer-pop: %s (%d)", \ + .format_args = "t4i4", \ + .n_enum_strings = 8, \ + .enum_strings = { \ + "retransmit", \ + "delack", \ + "persist", \ + "keep", \ + "waitclose", \ + "retransmit syn", \ + "establish", \ + "establish-ao", \ + }, \ + }; \ + if (_tc) \ + { \ + DECLARE_ETD(_tc, _e, 2); \ + ed->data[0] = _timer_id; \ + ed->data[1] = _timer_id; \ + } \ + else \ + { \ + clib_warning ("pop %d for unexisting connection %d", _timer_id, \ + _tc_index); \ + } \ +} + #else #define TCP_EVT_SYN_SENT_HANDLER(_tc, ...) #define TCP_EVT_SYNACK_SENT_HANDLER(_tc, ...) @@ -398,6 +441,7 @@ if (_tc) \ #define TCP_EVT_FIN_RCVD_HANDLER(_tc, ...) #define TCP_EVT_RST_RCVD_HANDLER(_tc, ...) #define TCP_EVT_STATE_CHANGE_HANDLER(_tc, ...) +#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...) #endif #if TCP_DEBUG_SM > 1 @@ -542,52 +586,11 @@ if (_av > 0) \ ed->data[4] = _tc->rcv_wnd - (_tc->rcv_nxt - _tc->rcv_las); \ } -#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...) \ -{ \ - tcp_connection_t *_tc; \ - if (_timer_id == TCP_TIMER_RETRANSMIT_SYN \ - || _timer_id == TCP_TIMER_ESTABLISH) \ - { \ - _tc = tcp_half_open_connection_get (_tc_index); \ - } \ - else \ - { \ - u32 _thread_index = vlib_get_thread_index (); \ - _tc = tcp_connection_get (_tc_index, _thread_index); \ - } \ - ELOG_TYPE_DECLARE (_e) = \ - { \ - .format = "timer-pop: %s (%d)", \ - .format_args = "t4i4", \ - .n_enum_strings = 7, \ - .enum_strings = { \ - "retransmit", \ - "delack", \ - "persist", \ - "keep", \ - "waitclose", \ - "retransmit syn", \ - "establish", \ - }, \ - }; \ - if (_tc) \ - { \ - DECLARE_ETD(_tc, _e, 2); \ - ed->data[0] = _timer_id; \ - ed->data[1] = _timer_id; \ - } \ - else \ - { \ - clib_warning ("pop %d for unexisting connection %d", _timer_id, \ - _tc_index); \ - } \ -} #else #define TCP_EVT_ACK_SENT_HANDLER(_tc, ...) #define TCP_EVT_ACK_RCVD_HANDLER(_tc, ...) #define TCP_EVT_PKTIZE_HANDLER(_tc, ...) #define TCP_EVT_INPUT_HANDLER(_tc, _type, _len, _written, ...) -#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...) #endif /* diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c index fe1476a2d1c..3da02ac6b10 100644 --- a/src/vnet/tcp/tcp_input.c +++ b/src/vnet/tcp/tcp_input.c @@ -2452,7 +2452,7 @@ tcp46_syn_sent_inline (vlib_main_t * vm, vlib_node_runtime_t * node, new_tc0->c_thread_index = my_thread_index; new_tc0->rcv_nxt = vnet_buffer (b0)->tcp.seq_end; new_tc0->irs = seq0; - new_tc0->timers[TCP_TIMER_ESTABLISH] = TCP_TIMER_HANDLE_INVALID; + new_tc0->timers[TCP_TIMER_ESTABLISH_AO] = TCP_TIMER_HANDLE_INVALID; new_tc0->timers[TCP_TIMER_RETRANSMIT_SYN] = TCP_TIMER_HANDLE_INVALID; new_tc0->sw_if_index = vnet_buffer (b0)->sw_if_index[VLIB_RX]; @@ -2775,7 +2775,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node, * to send. Enable waitclose though because we're willing to * wait for peer's FIN but not indefinitely. */ tcp_connection_timers_reset (tc0); - tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME); + tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME); /* Don't try to deq the FIN acked */ if (tc0->burst_acked > 1) @@ -2805,7 +2805,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node, tcp_send_fin (tc0); tcp_connection_timers_reset (tc0); tcp_connection_set_state (tc0, TCP_STATE_LAST_ACK); - tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME); + tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME); } } break; @@ -2816,8 +2816,9 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node, if (tcp_rcv_ack (wrk, tc0, b0, tcp0, &error0)) goto drop; + tcp_connection_timers_reset (tc0); tcp_connection_set_state (tc0, TCP_STATE_TIME_WAIT); - tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME); + tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME); goto drop; break; @@ -2846,7 +2847,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node, * we can't ensure that we have no packets already enqueued * to output. Rely instead on the waitclose timer */ tcp_connection_timers_reset (tc0); - tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); + tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); goto drop; @@ -2931,7 +2932,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node, tc0->rcv_nxt += 1; tcp_connection_set_state (tc0, TCP_STATE_TIME_WAIT); tcp_connection_timers_reset (tc0); - tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME); + tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME); tcp_program_ack (wrk, tc0); break; case TCP_STATE_TIME_WAIT: diff --git a/src/vnet/tcp/tcp_output.c b/src/vnet/tcp/tcp_output.c index 5f48d410126..97f5b81f9e8 100644 --- a/src/vnet/tcp/tcp_output.c +++ b/src/vnet/tcp/tcp_output.c @@ -994,7 +994,7 @@ tcp_send_syn (tcp_connection_t * tc) * Setup retransmit and establish timers before requesting buffer * such that we can return if we've ran out. */ - tcp_timer_set (tc, TCP_TIMER_ESTABLISH, TCP_ESTABLISH_TIME); + tcp_timer_set (tc, TCP_TIMER_ESTABLISH_AO, TCP_ESTABLISH_TIME); tcp_timer_update (tc, TCP_TIMER_RETRANSMIT_SYN, tc->rto * TCP_TO_TIMER_TICK); @@ -1096,7 +1096,9 @@ tcp_send_fin (tcp_connection_t * tc) { /* Out of buffers so program fin retransmit ASAP */ tcp_timer_update (tc, TCP_TIMER_RETRANSMIT, 1); - goto post_enqueue; + if (fin_snt) + tc->snd_nxt = tc->snd_una_max; + return; } tcp_retransmit_timer_force_update (tc); @@ -1106,7 +1108,6 @@ tcp_send_fin (tcp_connection_t * tc) tcp_enqueue_to_output_now (wrk, b, bi, tc->c_is_ip4); TCP_EVT_DBG (TCP_EVT_FIN_SENT, tc); -post_enqueue: if (!fin_snt) { tc->flags |= TCP_CONN_FINSNT; @@ -1538,9 +1539,12 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn) { tc = tcp_connection_get (index, thread_index); /* Note: the connection may have been closed and pool_put */ - if (PREDICT_FALSE (tc == 0 || tc->state < TCP_STATE_SYN_RCVD)) + if (PREDICT_FALSE (tc == 0 || tc->state == TCP_STATE_SYN_SENT)) return; tc->timers[TCP_TIMER_RETRANSMIT] = TCP_TIMER_HANDLE_INVALID; + /* Wait-close and retransmit could pop at the same time */ + if (tc->state == TCP_STATE_CLOSED) + return; } if (tc->state >= TCP_STATE_ESTABLISHED) @@ -1595,8 +1599,7 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn) /* Send one segment. Note that n_bytes may be zero due to buffer * shortfall */ n_bytes = tcp_prepare_retransmit_segment (wrk, tc, 0, tc->snd_mss, &b); - - if (n_bytes == 0) + if (!n_bytes) { tcp_retransmit_timer_force_update (tc); return; @@ -1620,10 +1623,7 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn) if (tc->flags & TCP_CONN_HALF_OPEN_DONE) { if (tcp_half_open_connection_cleanup (tc)) - { - clib_warning ("could not remove half-open connection"); - ASSERT (0); - } + TCP_DBG ("could not remove half-open connection"); return; } -- 2.16.6