tcp: invalidate expired timer handles before dispatching

Type: fix

Change-Id: Icc8cd05cb9c1844a9f8b0399797ad554f4ccf1a1
Signed-off-by: Florin Coras <fcoras@cisco.com>

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))