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>

5 files changed, 59 insertions, 31 deletions

diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c
index f637d625616..9dc35238795 100644
--- 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 2912ae3828c..a26cd7f2636 100644
--- 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 78ada2b13c3..b6884f56fd5 100644
--- 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 23ad39187c2..262d3faae10 100755
--- 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 e0139a47e0c..f838bc562ff 100644
--- 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);