session: free session after transport and app confirm

In addition to that, a bit of refactoring.

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

14 files changed, 98 insertions, 83 deletions

diff --git a/src/plugins/unittest/session_test.c b/src/plugins/unittest/session_test.c
index 5a3c9fbc35e..c504d8df15d 100644
--- a/src/plugins/unittest/session_test.c
+++ b/src/plugins/unittest/session_test.c
@@ -388,9 +388,9 @@ session_test_endpoint_cfg (vlib_main_t * vm, unformat_input_t * input)
    * main thread, even when we have workers and we avoid polling main thread,
    * i.e., we can't cleanup pending disconnects, so force cleanup for both
    */
-  stream_session_cleanup (s);
+  session_transport_cleanup (s);
   s = session_get (accepted_session_index, accepted_session_thread);
-  stream_session_cleanup (s);
+  session_transport_cleanup (s);
 
   vnet_app_detach_args_t detach_args = {
     .app_index = server_index,
diff --git a/src/plugins/unittest/tcp_test.c b/src/plugins/unittest/tcp_test.c
index bbb80404019..66260df2ff6 100644
--- a/src/plugins/unittest/tcp_test.c
+++ b/src/plugins/unittest/tcp_test.c
@@ -1795,7 +1795,7 @@ tcp_test_session (vlib_main_t * vm, unformat_input_t * input)
     {
       tc0 = tcp_connection_get (0 /* connection index */ , 0 /* thread */ );
       tc0->state = TCP_STATE_CLOSED;
-      stream_session_disconnect_notify (&tc0->connection);
+      session_transport_closing_notify (&tc0->connection);
     }
 
   return rv;
diff --git a/src/vnet/sctp/sctp.c b/src/vnet/sctp/sctp.c
index 85ca9b8048f..482f81a1e2a 100644
--- a/src/vnet/sctp/sctp.c
+++ b/src/vnet/sctp/sctp.c
@@ -740,8 +740,8 @@ sctp_expired_timers_cb (u32 conn_index, u32 timer_id)
 	{
 	  /* Start cleanup. App wasn't notified yet so use delete notify as
 	   * opposed to delete to cleanup session layer state. */
-	  stream_session_delete_notify (&sctp_conn->sub_conn
-					[SCTP_PRIMARY_PATH_IDX].connection);
+	  session_transport_delete_notify (&sctp_conn->sub_conn
+					   [SCTP_PRIMARY_PATH_IDX].connection);
 
 	  sctp_connection_timers_reset (sctp_conn);
 
diff --git a/src/vnet/sctp/sctp_input.c b/src/vnet/sctp/sctp_input.c
index c58da9ef9b5..4454f99e6bb 100644
--- a/src/vnet/sctp/sctp_input.c
+++ b/src/vnet/sctp/sctp_input.c
@@ -304,7 +304,7 @@ sctp_handle_operation_err (sctp_header_t * sctp_hdr,
 	{
 	  sctp_connection_cleanup (sctp_conn);
 
-	  stream_session_disconnect_notify (&sctp_conn->
+	  session_transport_closing_notify (&sctp_conn->
 					    sub_conn[idx].connection);
 	}
     }
@@ -1295,7 +1295,7 @@ sctp_handle_shutdown_complete (sctp_header_t * sctp_hdr,
 
   sctp_timer_reset (sctp_conn, idx, SCTP_TIMER_T2_SHUTDOWN);
 
-  stream_session_disconnect_notify (&sctp_conn->sub_conn[idx].connection);
+  session_transport_closing_notify (&sctp_conn->sub_conn[idx].connection);
 
   sctp_conn->state = SCTP_STATE_CLOSED;
 
diff --git a/src/vnet/session/application_interface.c b/src/vnet/session/application_interface.c
index d254b9bb55a..c3c84fd8da9 100644
--- a/src/vnet/session/application_interface.c
+++ b/src/vnet/session/application_interface.c
@@ -669,7 +669,7 @@ vnet_disconnect_session (vnet_disconnect_args_t * a)
       /* We're peeking into another's thread pool. Make sure */
       ASSERT (s->session_index == session_index_from_handle (a->handle));
 
-      stream_session_disconnect (s);
+      session_close (s);
     }
   return 0;
 }
diff --git a/src/vnet/session/segment_manager.c b/src/vnet/session/segment_manager.c
index 47704a7178b..2b8a7326ea7 100644
--- a/src/vnet/session/segment_manager.c
+++ b/src/vnet/session/segment_manager.c
@@ -406,7 +406,7 @@ segment_manager_del_sessions (segment_manager_t * sm)
 	  }
 	session = session_get (fifo->master_session_index,
 	                       fifo->master_thread_index);
-	stream_session_disconnect (session);
+	session_close (session);
 	fifo = fifo->next;
       }
 
diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c
index 6492ce7089e..cbe0dd76430 100644
--- a/src/vnet/session/session.c
+++ b/src/vnet/session/session.c
@@ -119,6 +119,27 @@ session_send_rpc_evt_to_thread (u32 thread_index, void *fp, void *rpc_args)
     }
 }
 
+static void
+session_program_transport_close (stream_session_t * s)
+{
+  u32 thread_index = vlib_get_thread_index ();
+  session_manager_worker_t *wrk;
+  session_event_t *evt;
+
+  /* If we are in the handler thread, or being called with the worker barrier
+   * held, just append a new event to pending disconnects vector. */
+  if (vlib_thread_is_main_w_barrier () || thread_index == s->thread_index)
+    {
+      wrk = session_manager_get_worker (s->thread_index);
+      vec_add2 (wrk->pending_disconnects, evt, 1);
+      clib_memset (evt, 0, sizeof (*evt));
+      evt->session_handle = session_handle (s);
+      evt->event_type = FIFO_EVENT_DISCONNECT;
+    }
+  else
+    session_send_ctrl_evt_to_thread (s, FIFO_EVENT_DISCONNECT);
+}
+
 stream_session_t *
 session_alloc (u32 thread_index)
 {
@@ -160,6 +181,23 @@ session_free_w_fifos (stream_session_t * s)
   session_free (s);
 }
 
+/**
+ * Cleans up session and lookup table.
+ *
+ * Transport connection must still be valid.
+ */
+static void
+session_delete (stream_session_t * s)
+{
+  int rv;
+
+  /* Delete from the main lookup table. */
+  if ((rv = session_lookup_del_session (s)))
+    clib_warning ("hash delete error, rv %d", rv);
+
+  session_free_w_fifos (s);
+}
+
 int
 session_alloc_fifos (segment_manager_t * sm, stream_session_t * s)
 {
@@ -646,7 +684,7 @@ session_stream_connect_notify (transport_connection_t * tc, u8 is_fail)
       if (!is_fail)
 	{
 	  new_s = session_get (new_si, new_ti);
-	  stream_session_disconnect_transport (new_s);
+	  session_transport_close (new_s);
 	}
     }
   else
@@ -747,7 +785,7 @@ stream_session_accept_notify (transport_connection_t * tc)
  * Ultimately this leads to close being called on transport (passive close).
  */
 void
-stream_session_disconnect_notify (transport_connection_t * tc)
+session_transport_closing_notify (transport_connection_t * tc)
 {
   app_worker_t *app_wrk;
   application_t *app;
@@ -765,23 +803,6 @@ stream_session_disconnect_notify (transport_connection_t * tc)
 }
 
 /**
- * Cleans up session and lookup table.
- *
- * Transport connection must still be valid.
- */
-void
-stream_session_delete (stream_session_t * s)
-{
-  int rv;
-
-  /* Delete from the main lookup table. */
-  if ((rv = session_lookup_del_session (s)))
-    clib_warning ("hash delete error, rv %d", rv);
-
-  session_free_w_fifos (s);
-}
-
-/**
  * Notification from transport that connection is being deleted
  *
  * This removes the session if it is still valid. It should be called only on
@@ -790,7 +811,7 @@ stream_session_delete (stream_session_t * s)
  * failed.
  */
 void
-stream_session_delete_notify (transport_connection_t * tc)
+session_transport_delete_notify (transport_connection_t * tc)
 {
   stream_session_t *s;
 
@@ -805,26 +826,30 @@ stream_session_delete_notify (transport_connection_t * tc)
     {
     case SESSION_STATE_TRANSPORT_CLOSING:
       /* If transport finishes or times out before we get a reply
-       * from the app, do the whole disconnect since we might still
-       * have lingering events. Cleanup session table in advance
+       * from the app, mark transport as closed and wait for reply
+       * before removing the session. Cleanup session table in advance
        * because transport will soon be closed and closed sessions
        * are assumed to have been removed from the lookup table */
       session_lookup_del_session (s);
-      stream_session_disconnect (s);
-      s->session_state = SESSION_STATE_CLOSED;
+      s->session_state = SESSION_STATE_TRANSPORT_CLOSED;
       break;
     case SESSION_STATE_CLOSING:
-      /* Cleanup lookup table. Transport needs to still be valid */
+    case SESSION_STATE_CLOSED_WAITING:
+      /* Cleanup lookup table as transport needs to still be valid.
+       * Program transport close to ensure that all session events
+       * have been cleaned up. Once transport close is called, the
+       * session is just removed because both transport and app have
+       * confirmed the close*/
       session_lookup_del_session (s);
-      s->session_state = SESSION_STATE_CLOSED;
+      s->session_state = SESSION_STATE_TRANSPORT_CLOSED;
+      session_program_transport_close (s);
       break;
     case SESSION_STATE_CLOSED:
     case SESSION_STATE_ACCEPTING:
-    case SESSION_STATE_CLOSED_WAITING:
-      stream_session_delete (s);
+      session_delete (s);
       break;
     default:
-      stream_session_delete (s);
+      session_delete (s);
       break;
     }
 }
@@ -838,7 +863,7 @@ stream_session_delete_notify (transport_connection_t * tc)
  * to cleanup any outstanding events.
  */
 void
-session_stream_close_notify (transport_connection_t * tc)
+session_transport_closed_notify (transport_connection_t * tc)
 {
   stream_session_t *s;
 
@@ -851,7 +876,7 @@ session_stream_close_notify (transport_connection_t * tc)
  * Notify application that connection has been reset.
  */
 void
-stream_session_reset_notify (transport_connection_t * tc)
+session_transport_reset_notify (transport_connection_t * tc)
 {
   stream_session_t *s;
   app_worker_t *app_wrk;
@@ -1077,23 +1102,24 @@ session_stop_listen (stream_session_t * s)
 }
 
 /**
- * Initialize session disconnect.
+ * Initialize session closing procedure.
  *
  * Request is always sent to session node to ensure that all outstanding
  * requests are served before transport is notified.
  */
 void
-stream_session_disconnect (stream_session_t * s)
+session_close (stream_session_t * s)
 {
-  u32 thread_index = vlib_get_thread_index ();
-  session_manager_worker_t *wrk;
-  session_event_t *evt;
-
   if (!s)
     return;
 
   if (s->session_state >= SESSION_STATE_CLOSING)
     {
+      /* Session will only be removed once both app and transport
+       * acknowledge the close */
+      if (s->session_state == SESSION_STATE_TRANSPORT_CLOSED)
+	session_program_transport_close (s);
+
       /* Session already closed. Clear the tx fifo */
       if (s->session_state == SESSION_STATE_CLOSED)
 	svm_fifo_dequeue_drop_all (s->server_tx_fifo);
@@ -1101,19 +1127,7 @@ stream_session_disconnect (stream_session_t * s)
     }
 
   s->session_state = SESSION_STATE_CLOSING;
-
-  /* If we are in the handler thread, or being called with the worker barrier
-   * held, just append a new event to pending disconnects vector. */
-  if (vlib_thread_is_main_w_barrier () || thread_index == s->thread_index)
-    {
-      wrk = session_manager_get_worker (s->thread_index);
-      vec_add2 (wrk->pending_disconnects, evt, 1);
-      clib_memset (evt, 0, sizeof (*evt));
-      evt->session_handle = session_handle (s);
-      evt->event_type = FIFO_EVENT_DISCONNECT;
-    }
-  else
-    session_send_ctrl_evt_to_thread (s, FIFO_EVENT_DISCONNECT);
+  session_program_transport_close (s);
 }
 
 /**
@@ -1124,10 +1138,10 @@ stream_session_disconnect (stream_session_t * s)
  * Must be called from the session's thread.
  */
 void
-stream_session_disconnect_transport (stream_session_t * s)
+session_transport_close (stream_session_t * s)
 {
   /* If transport is already closed, just free the session */
-  if (s->session_state == SESSION_STATE_CLOSED)
+  if (s->session_state == SESSION_STATE_TRANSPORT_CLOSED)
     {
       session_free_w_fifos (s);
       return;
@@ -1156,7 +1170,7 @@ stream_session_disconnect_transport (stream_session_t * s)
  * closed.
  */
 void
-stream_session_cleanup (stream_session_t * s)
+session_transport_cleanup (stream_session_t * s)
 {
   s->session_state = SESSION_STATE_CLOSED;
 
diff --git a/src/vnet/session/session.h b/src/vnet/session/session.h
index 3b729234967..e3c73000edf 100644
--- a/src/vnet/session/session.h
+++ b/src/vnet/session/session.h
@@ -611,18 +611,18 @@ void stream_session_init_fifos_pointers (transport_connection_t * tc,
 					 u32 rx_pointer, u32 tx_pointer);
 
 int stream_session_accept_notify (transport_connection_t * tc);
-void stream_session_disconnect_notify (transport_connection_t * tc);
-void stream_session_delete_notify (transport_connection_t * tc);
-void session_stream_close_notify (transport_connection_t * tc);
-void stream_session_reset_notify (transport_connection_t * tc);
+void session_transport_closing_notify (transport_connection_t * tc);
+void session_transport_delete_notify (transport_connection_t * tc);
+void session_transport_closed_notify (transport_connection_t * tc);
+void session_transport_reset_notify (transport_connection_t * tc);
 int stream_session_accept (transport_connection_t * tc, u32 listener_index,
 			   u8 notify);
 int session_open (u32 app_index, session_endpoint_t * tep, u32 opaque);
 int session_listen (stream_session_t * s, session_endpoint_cfg_t * sep);
 int session_stop_listen (stream_session_t * s);
-void stream_session_disconnect (stream_session_t * s);
-void stream_session_disconnect_transport (stream_session_t * s);
-void stream_session_cleanup (stream_session_t * s);
+void session_close (stream_session_t * s);
+void session_transport_close (stream_session_t * s);
+void session_transport_cleanup (stream_session_t * s);
 int session_send_io_evt_to_thread (svm_fifo_t * f,
 				   session_evt_type_t evt_type);
 int session_send_io_evt_to_thread_custom (void *data, u32 thread_index,
diff --git a/src/vnet/session/session_node.c b/src/vnet/session/session_node.c
index e8ed1cf9b95..4323ed83cf8 100644
--- a/src/vnet/session/session_node.c
+++ b/src/vnet/session/session_node.c
@@ -446,7 +446,7 @@ session_tx_not_ready (stream_session_t * s, u8 peek_data)
        * session is not ready or closed */
       if (s->session_state < SESSION_STATE_READY)
 	return 1;
-      if (s->session_state == SESSION_STATE_CLOSED)
+      if (s->session_state >= SESSION_STATE_TRANSPORT_CLOSED)
 	return 2;
     }
   return 0;
@@ -891,7 +891,7 @@ session_queue_node_fn (vlib_main_t * vm, vlib_node_runtime_t * node,
 	      continue;
 	    }
 
-	  stream_session_disconnect_transport (s);
+	  session_transport_close (s);
 	  break;
 	case FIFO_EVENT_BUILTIN_RX:
 	  s = session_event_get_session (e, thread_index);
diff --git a/src/vnet/session/stream_session.h b/src/vnet/session/stream_session.h
index 79a6839ad21..63adc2b909b 100644
--- a/src/vnet/session/stream_session.h
+++ b/src/vnet/session/stream_session.h
@@ -34,6 +34,7 @@ typedef enum
   SESSION_STATE_TRANSPORT_CLOSING,
   SESSION_STATE_CLOSING,
   SESSION_STATE_CLOSED_WAITING,
+  SESSION_STATE_TRANSPORT_CLOSED,
   SESSION_STATE_CLOSED,
   SESSION_STATE_N_STATES,
 } stream_session_state_t;
diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c
index 4a63f3bc00c..564f200bc00 100644
--- a/src/vnet/tcp/tcp.c
+++ b/src/vnet/tcp/tcp.c
@@ -241,7 +241,7 @@ void
 tcp_connection_del (tcp_connection_t * tc)
 {
   TCP_EVT_DBG (TCP_EVT_DELETE, tc);
-  stream_session_delete_notify (&tc->connection);
+  session_transport_delete_notify (&tc->connection);
   tcp_connection_cleanup (tc);
 }
 
@@ -279,7 +279,7 @@ tcp_connection_reset (tcp_connection_t * tc)
     {
     case TCP_STATE_SYN_RCVD:
       /* Cleanup everything. App wasn't notified yet */
-      stream_session_delete_notify (&tc->connection);
+      session_transport_delete_notify (&tc->connection);
       tcp_connection_cleanup (tc);
       break;
     case TCP_STATE_SYN_SENT:
@@ -291,7 +291,7 @@ tcp_connection_reset (tcp_connection_t * tc)
       /* Set the cleanup timer, in case the session layer/app don't
        * cleanly close the connection */
       tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLOSEWAIT_TIME);
-      stream_session_reset_notify (&tc->connection);
+      session_transport_reset_notify (&tc->connection);
       tcp_connection_set_state (tc, TCP_STATE_CLOSED);
       break;
     case TCP_STATE_CLOSE_WAIT:
@@ -303,7 +303,7 @@ tcp_connection_reset (tcp_connection_t * tc)
       tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLOSEWAIT_TIME);
       /* Make sure we mark the session as closed. In some states we may
        * be still trying to send data */
-      session_stream_close_notify (&tc->connection);
+      session_transport_closed_notify (&tc->connection);
       tcp_connection_set_state (tc, TCP_STATE_CLOSED);
       break;
     case TCP_STATE_CLOSED:
@@ -1224,7 +1224,7 @@ tcp_timer_establish_handler (u32 conn_index)
   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);
+  session_transport_delete_notify (&tc->connection);
   tc->timers[TCP_TIMER_ESTABLISH] = TCP_TIMER_HANDLE_INVALID;
   tcp_connection_cleanup (tc);
 }
@@ -1262,7 +1262,7 @@ tcp_timer_waitclose_handler (u32 conn_index)
     {
     case TCP_STATE_CLOSE_WAIT:
       tcp_connection_timers_reset (tc);
-      session_stream_close_notify (&tc->connection);
+      session_transport_closed_notify (&tc->connection);
 
       if (!(tc->flags & TCP_CONN_FINPNDG))
 	{
@@ -1296,7 +1296,7 @@ tcp_timer_waitclose_handler (u32 conn_index)
 	  rto = clib_max (tc->rto >> tc->rto_boff, 1);
 	  tcp_timer_set (tc, TCP_TIMER_WAITCLOSE,
 			 clib_min (rto * TCP_TO_TIMER_TICK, TCP_2MSL_TIME));
-	  session_stream_close_notify (&tc->connection);
+	  session_transport_closed_notify (&tc->connection);
 	}
       else
 	{
@@ -1311,7 +1311,7 @@ tcp_timer_waitclose_handler (u32 conn_index)
       tcp_connection_timers_reset (tc);
       tcp_connection_set_state (tc, TCP_STATE_CLOSED);
       tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
-      session_stream_close_notify (&tc->connection);
+      session_transport_closed_notify (&tc->connection);
       break;
     default:
       tcp_connection_del (tc);
diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c
index ee7f4ae72a9..c3ce2eb1ae1 100644
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -1639,7 +1639,7 @@ tcp_handle_disconnects (tcp_worker_ctx_t * wrk)
     {
       tc = tcp_connection_get (pending_disconnects[i], thread_index);
       tcp_disconnect_pending_off (tc);
-      stream_session_disconnect_notify (&tc->connection);
+      session_transport_closing_notify (&tc->connection);
     }
   _vec_len (wrk->pending_disconnects) = 0;
 }
diff --git a/src/vnet/tls/tls.c b/src/vnet/tls/tls.c
index 0fdd68c7d38..fdba08c5915 100644
--- a/src/vnet/tls/tls.c
+++ b/src/vnet/tls/tls.c
@@ -390,7 +390,7 @@ tls_session_disconnect_callback (stream_session_t * tls_session)
   ctx = tls_ctx_get (tls_session->opaque);
   if (!tls_ctx_handshake_is_over (ctx))
     {
-      stream_session_disconnect (tls_session);
+      session_close (tls_session);
       return;
     }
   ctx->is_passive_close = 1;
@@ -568,7 +568,7 @@ tls_disconnect (u32 ctx_handle, u32 thread_index)
 
   ctx = tls_ctx_get (ctx_handle);
   tls_disconnect_transport (ctx);
-  stream_session_delete_notify (&ctx->connection);
+  session_transport_delete_notify (&ctx->connection);
   tls_ctx_free (ctx);
 }
 
diff --git a/src/vnet/udp/udp.c b/src/vnet/udp/udp.c
index 8c2daaf66e6..7674ef2e5d6 100644
--- a/src/vnet/udp/udp.c
+++ b/src/vnet/udp/udp.c
@@ -168,7 +168,7 @@ udp_session_close (u32 connection_index, u32 thread_index)
     {
       udp_unregister_dst_port (vm, clib_net_to_host_u16 (uc->c_lcl_port),
 			       uc->c_is_ip4);
-      stream_session_delete_notify (&uc->connection);
+      session_transport_delete_notify (&uc->connection);
       udp_connection_free (uc);
     }
 }