From b72a0ff73514ec7ea79ed3fad49b77d56b248642 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Fri, 22 Nov 2019 17:38:25 -0800 Subject: [PATCH] tcp: invalidate expired timer handles before dispatching Type: fix Change-Id: Icc8cd05cb9c1844a9f8b0399797ad554f4ccf1a1 Signed-off-by: Florin Coras --- src/vnet/tcp/tcp.c | 32 ++++++++++++++++++++++++-------- src/vnet/tcp/tcp.h | 2 +- src/vnet/tcp/tcp_output.c | 20 ++++---------------- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c index baa5c0daa61..3b1cfe112ed 100644 --- a/src/vnet/tcp/tcp.c +++ b/src/vnet/tcp/tcp.c @@ -1417,17 +1417,14 @@ tcp_connection_tx_pacer_reset (tcp_connection_t * tc, u32 window, } static void -tcp_timer_waitclose_handler (u32 conn_index) +tcp_timer_waitclose_handler (u32 conn_index, u32 thread_index) { - u32 thread_index = vlib_get_thread_index (); 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) { case TCP_STATE_CLOSE_WAIT: @@ -1500,19 +1497,38 @@ static timer_expiration_handler *timer_expiration_handlers[TCP_N_TIMERS] = static void tcp_expired_timers_dispatch (u32 * expired_timers) { - int i; + u32 thread_index = vlib_get_thread_index (); u32 connection_index, timer_id; + tcp_connection_t *tc; + int i; + /* + * Invalidate all timer handles before dispatching. This avoids dangling + * index references to timer wheel pool entries that have been freed. + */ for (i = 0; i < vec_len (expired_timers); i++) { - /* Get session index and timer id */ connection_index = expired_timers[i] & 0x0FFFFFFF; timer_id = expired_timers[i] >> 28; + if (timer_id != TCP_TIMER_RETRANSMIT_SYN) + tc = tcp_connection_get (connection_index, thread_index); + else + tc = tcp_half_open_connection_get (connection_index); + TCP_EVT (TCP_EVT_TIMER_POP, connection_index, timer_id); - /* Handle expiration */ - (*timer_expiration_handlers[timer_id]) (connection_index); + tc->timers[timer_id] = TCP_TIMER_HANDLE_INVALID; + } + + /* + * Dispatch expired timers + */ + for (i = 0; i < vec_len (expired_timers); i++) + { + connection_index = expired_timers[i] & 0x0FFFFFFF; + timer_id = expired_timers[i] >> 28; + (*timer_expiration_handlers[timer_id]) (connection_index, thread_index); } } diff --git a/src/vnet/tcp/tcp.h b/src/vnet/tcp/tcp.h index 299e76de5a7..2de25277b60 100644 --- a/src/vnet/tcp/tcp.h +++ b/src/vnet/tcp/tcp.h @@ -82,7 +82,7 @@ typedef enum _tcp_timers TCP_N_TIMERS } tcp_timers_e; -typedef void (timer_expiration_handler) (u32 index); +typedef void (timer_expiration_handler) (u32 index, u32 thread_index); extern timer_expiration_handler tcp_timer_delack_handler; extern timer_expiration_handler tcp_timer_retransmit_handler; diff --git a/src/vnet/tcp/tcp_output.c b/src/vnet/tcp/tcp_output.c index d6e1adc9600..f63ffa64022 100644 --- a/src/vnet/tcp/tcp_output.c +++ b/src/vnet/tcp/tcp_output.c @@ -1210,13 +1210,11 @@ tcp_program_retransmit (tcp_connection_t * tc) * Sends delayed ACK when timer expires */ void -tcp_timer_delack_handler (u32 index) +tcp_timer_delack_handler (u32 index, u32 thread_index) { - u32 thread_index = vlib_get_thread_index (); tcp_connection_t *tc; tc = tcp_connection_get (index, thread_index); - tc->timers[TCP_TIMER_DELACK] = TCP_TIMER_HANDLE_INVALID; tcp_send_ack (tc); } @@ -1446,9 +1444,8 @@ tcp_cc_init_rxt_timeout (tcp_connection_t * tc) } void -tcp_timer_retransmit_handler (u32 tc_index) +tcp_timer_retransmit_handler (u32 tc_index, u32 thread_index) { - u32 thread_index = vlib_get_thread_index (); tcp_worker_ctx_t *wrk = tcp_get_worker (thread_index); vlib_main_t *vm = wrk->vm; tcp_connection_t *tc; @@ -1461,8 +1458,6 @@ tcp_timer_retransmit_handler (u32 tc_index) 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; @@ -1595,9 +1590,8 @@ tcp_timer_retransmit_handler (u32 tc_index) * SYN retransmit timer handler. Active open only. */ void -tcp_timer_retransmit_syn_handler (u32 tc_index) +tcp_timer_retransmit_syn_handler (u32 tc_index, u32 thread_index) { - u32 thread_index = vlib_get_thread_index (); tcp_worker_ctx_t *wrk = tcp_get_worker (thread_index); vlib_main_t *vm = wrk->vm; tcp_connection_t *tc; @@ -1610,8 +1604,6 @@ tcp_timer_retransmit_syn_handler (u32 tc_index) if (PREDICT_FALSE (tc == 0 || tc->state != TCP_STATE_SYN_SENT)) return; - tc->timers[TCP_TIMER_RETRANSMIT_SYN] = TCP_TIMER_HANDLE_INVALID; - /* Half-open connection actually moved to established but we were * waiting for syn retransmit to pop to call cleanup from the right * thread. */ @@ -1664,9 +1656,8 @@ tcp_timer_retransmit_syn_handler (u32 tc_index) * */ void -tcp_timer_persist_handler (u32 index) +tcp_timer_persist_handler (u32 index, u32 thread_index) { - u32 thread_index = vlib_get_thread_index (); tcp_worker_ctx_t *wrk = tcp_get_worker (thread_index); u32 bi, max_snd_bytes, available_bytes, offset; tcp_main_t *tm = vnet_get_tcp_main (); @@ -1680,9 +1671,6 @@ tcp_timer_persist_handler (u32 index) if (!tc) return; - /* Make sure timer handle is set to invalid */ - tc->timers[TCP_TIMER_PERSIST] = TCP_TIMER_HANDLE_INVALID; - /* Problem already solved or worse */ if (tc->state == TCP_STATE_CLOSED || tc->snd_wnd > tc->snd_mss || (tc->flags & TCP_CONN_FINSNT)) -- 2.16.6