tcp: do not delete session on establish pop

Also:
- force reset if wait close pops in fin-wait-1 with unsent data
- adds more event logging.

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

diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c
index f637d62..9dc3523 100644 (file)
--- a/src/vnet/session/session.c
+++ b/src/vnet/session/session.c
@@ -174,9 +174,15 @@ session_alloc (u32 thread_index)
 void
 session_free (session_t * s)
 {
-  pool_put (session_main.wrk[s->thread_index].sessions, s);
   if (CLIB_DEBUG)
-    clib_memset (s, 0xFA, sizeof (*s));
+    {
+      u8 thread_index = s->thread_index;
+      clib_memset (s, 0xFA, sizeof (*s));
+      pool_put (session_main.wrk[thread_index].sessions, s);
+      return;
+    }
+  SESSION_EVT_DBG (SESSION_EVT_FREE, s);
+  pool_put (session_main.wrk[s->thread_index].sessions, s);
 }
 
 void

diff --git a/src/vnet/session/session_debug.h b/src/vnet/session/session_debug.h
index 2912ae3..a26cd7f 100644 (file)
--- a/src/vnet/session/session_debug.h
+++ b/src/vnet/session/session_debug.h
@@ -25,6 +25,7 @@
   _(POLL_GAP_TRACK, "poll gap track")  \
   _(POLL_DISPATCH_TIME, "dispatch time")\
   _(DISPATCH_END, "dispatch end")      \
+  _(FREE, "session free")              \
 
 typedef enum _session_evt_dbg
 {
@@ -36,6 +37,7 @@ typedef enum _session_evt_dbg
 #define SESSION_DEBUG 0 * (TRANSPORT_DEBUG > 0)
 #define SESSION_DEQ_NODE_EVTS (0)
 #define SESSION_EVT_POLL_DBG (0)
+#define SESSION_SM (0)
 
 #if SESSION_DEBUG
 
@@ -57,6 +59,21 @@ typedef enum _session_evt_dbg
   } * ed;                                                              \
   ed = ELOG_DATA (&vlib_global_main.elog_main, _e)
 
+#if SESSION_SM
+#define SESSION_EVT_FREE_HANDLER(_s)                                   \
+{                                                                      \
+  ELOG_TYPE_DECLARE (_e) =                                             \
+  {                                                                    \
+    .format = "free: idx %u",                                          \
+    .format_args = "i4",                                               \
+  };                                                                   \
+  DEC_SESSION_ETD(_s, _e, 1);                                          \
+  ed->data[0] =        _s->session_index;                                      \
+}
+#else
+#define SESSION_EVT_FREE_HANDLER(_s)
+#endif
+
 #if SESSION_DEQ_NODE_EVTS && SESSION_DEBUG > 1
 #define SESSION_EVT_DEQ_HANDLER(_s, _body)                             \
 {                                                                      \

diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c
index 78ada2b..b6884f5 100644 (file)
--- a/src/vnet/tcp/tcp.c
+++ b/src/vnet/tcp/tcp.c
@@ -266,9 +266,14 @@ void
 tcp_connection_free (tcp_connection_t * tc)
 {
   tcp_main_t *tm = &tcp_main;
+  if (CLIB_DEBUG)
+    {
+      u8 thread_index = tc->c_thread_index;
+      clib_memset (tc, 0xFA, sizeof (*tc));
+      pool_put (tm->connections[thread_index], tc);
+      return;
+    }
   pool_put (tm->connections[tc->c_thread_index], tc);
-  if (CLIB_DEBUG > 0)
-    clib_memset (tc, 0xFA, sizeof (*tc));
 }
 
 /** Notify session that connection has been reset.
@@ -1255,10 +1260,10 @@ tcp_timer_establish_handler (u32 conn_index)
   ASSERT (tc->state == TCP_STATE_SYN_RCVD);
   tc->timers[TCP_TIMER_ESTABLISH] = TCP_TIMER_HANDLE_INVALID;
   tcp_connection_set_state (tc, TCP_STATE_CLOSED);
-  /* Start cleanup. App wasn't notified yet so use delete notify as
-   * opposed to delete to cleanup session layer state. */
   tcp_connection_timers_reset (tc);
-  session_transport_delete_notify (&tc->connection);
+  /* Start cleanup. Do NOT delete the session until we do the connection
+   * cleanup. Otherwise, we end up with a dangling session index in the
+   * tcp connection. */
   tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
 }
 
@@ -1283,7 +1288,7 @@ tcp_timer_establish_ao_handler (u32 conn_index)
 static void
 tcp_timer_waitclose_handler (u32 conn_index)
 {
-  u32 thread_index = vlib_get_thread_index (), rto;
+  u32 thread_index = vlib_get_thread_index ();
   tcp_connection_t *tc;
 
   tc = tcp_connection_get (conn_index, thread_index);
@@ -1321,15 +1326,12 @@ tcp_timer_waitclose_handler (u32 conn_index)
       tcp_connection_timers_reset (tc);
       if (tc->flags & TCP_CONN_FINPNDG)
        {
-         /* If FIN pending send it before closing and wait as long as
-          * the rto timeout would wait. Notify session layer that transport
-          * 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) * TCP_TO_TIMER_TICK, 1);
-         tcp_timer_set (tc, TCP_TIMER_WAITCLOSE,
-                        clib_min (rto, TCP_2MSL_TIME));
+         /* If FIN pending, we haven't sent everything, but we did try.
+          * Notify session layer that transport is closed. */
+         tcp_connection_set_state (tc, TCP_STATE_CLOSED);
          session_transport_closed_notify (&tc->connection);
+         tcp_send_reset (tc);
+         tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
        }
       else
        {

diff --git a/src/vnet/tcp/tcp_debug.h b/src/vnet/tcp/tcp_debug.h
index 23ad391..262d3fa 100755 (executable)
--- a/src/vnet/tcp/tcp_debug.h
+++ b/src/vnet/tcp/tcp_debug.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 Cisco and/or its affiliates.
+ * Copyright (c) 2017-2019 Cisco and/or its affiliates.
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at:
@@ -177,7 +177,7 @@ typedef enum _tcp_dbg_evt
 {                                                                      \
   ELOG_TYPE_DECLARE (_e) =                                             \
   {                                                                    \
-    .format = "close: %d",                                             \
+    .format = "close: cidx %d",                                                \
     .format_args = "i4",                                               \
   };                                                                   \
   DECLARE_ETD(_tc, _e, 1);                                             \
@@ -201,12 +201,13 @@ typedef enum _tcp_dbg_evt
     TCP_EVT_INIT_HANDLER(_tc, 0);                                      \
   ELOG_TYPE_DECLARE (_e) =                                             \
   {                                                                    \
-    .format = "syn-rx: idx %u irs %u",                                 \
-    .format_args = "i4i4",                                             \
+    .format = "syn-rx: cidx %u sidx %u irs %u",                                \
+    .format_args = "i4i4i4",                                           \
   };                                                                   \
-  DECLARE_ETD(_tc, _e, 2);                                             \
+  DECLARE_ETD(_tc, _e, 3);                                             \
   ed->data[0] = _tc->c_c_index;                                                \
-  ed->data[1] = _tc->irs;                                              \
+  ed->data[1] = _tc->c_s_index;                                                \
+  ed->data[2] = _tc->irs;                                              \
   TCP_EVT_STATE_CHANGE_HANDLER(_tc);                                   \
 }
 
@@ -226,11 +227,12 @@ typedef enum _tcp_dbg_evt
 {                                                                      \
   ELOG_TYPE_DECLARE (_e) =                                             \
   {                                                                    \
-    .format = "delete: %d",                                            \
-    .format_args = "i4",                                               \
+    .format = "delete: cidx %d sidx %d",                               \
+    .format_args = "i4i4",                                             \
   };                                                                   \
-  DECLARE_ETD(_tc, _e, 1);                                             \
+  DECLARE_ETD(_tc, _e, 2);                                             \
   ed->data[0] = _tc->c_c_index;                                                \
+  ed->data[1] = _tc->c_s_index;                                                \
   TCP_EVT_DEALLOC_HANDLER(_tc);                                                \
 }
 
@@ -379,7 +381,7 @@ if (_tc)                                                            \
     .n_enum_strings = 2,                                               \
     .enum_strings = {                                                  \
        "syn",                                                          \
-        "syn-ack",                                                     \
+        "synack",                                                      \
     },                                                                 \
   };                                                                   \
   DECLARE_ETD(_tc, _e, 5);                                             \
@@ -405,8 +407,8 @@ if (_tc)                                                            \
     }                                                                  \
   ELOG_TYPE_DECLARE (_e) =                                             \
   {                                                                    \
-    .format = "timer-pop: %s (%d)",                                    \
-    .format_args = "t4i4",                                             \
+    .format = "timer-pop: %s cidx %u sidx %u",                         \
+    .format_args = "t4i4i4",                                           \
     .n_enum_strings = 8,                                               \
     .enum_strings = {                                                  \
       "retransmit",                                                    \
@@ -421,9 +423,10 @@ if (_tc)                                                           \
   };                                                                   \
   if (_tc)                                                             \
     {                                                                  \
-      DECLARE_ETD(_tc, _e, 2);                                         \
+      DECLARE_ETD(_tc, _e, 3);                                         \
       ed->data[0] = _timer_id;                                         \
-      ed->data[1] = _timer_id;                                         \
+      ed->data[1] = _tc->c_c_index;                                            \
+      ed->data[2] = _tc->c_s_index;                                            \
     }                                                                  \
   else                                                                 \
     {                                                                  \

diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c
index e0139a4..f838bc5 100644 (file)
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -3185,7 +3185,6 @@ tcp46_listen_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
       tcp_connection_init_vars (child0);
       child0->rto = TCP_RTO_MIN;
-      TCP_EVT_DBG (TCP_EVT_SYN_RCVD, child0, 1);
 
       if (session_stream_accept (&child0->connection, lc0->c_s_index,
                                 0 /* notify */ ))
@@ -3195,6 +3194,7 @@ tcp46_listen_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
          goto drop;
        }
 
+      TCP_EVT_DBG (TCP_EVT_SYN_RCVD, child0, 1);
       child0->tx_fifo_size = transport_tx_fifo_size (&child0->connection);
       tcp_send_synack (child0);
       tcp_timer_set (child0, TCP_TIMER_ESTABLISH, TCP_SYN_RCVD_TIME);