session: add support for postponing transport cleanups 18/42918/14
authorFlorin Coras <[email protected]>
Fri, 2 May 2025 07:20:53 +0000 (03:20 -0400)
committerDave Barach <[email protected]>
Mon, 5 May 2025 18:29:56 +0000 (18:29 +0000)
Add new session layer api that allows transports to request postponed
cleanups in session_input. Eventually this should become default
especially if transport connections are to be scrapable before cleanup.

Update tcp and udp for now.

Type: improvement

Change-Id: I74beef41d5deed68efa664b78c1cf95e980b3bde
Signed-off-by: Florin Coras <[email protected]>
src/vnet/session/application_worker.c
src/vnet/session/session.c
src/vnet/session/session.h
src/vnet/session/session_input.c
src/vnet/tcp/tcp.c
src/vnet/udp/udp.c

index a5b1e1f..c919cc1 100644 (file)
@@ -524,7 +524,9 @@ app_worker_cleanup_notify (app_worker_t * app_wrk, session_t * s,
 {
   session_event_t evt = { .event_type = SESSION_CTRL_EVT_CLEANUP,
                          .as_u64[0] = (u64) ntf << 32 | s->session_index,
-                         .as_u64[1] = pointer_to_uword (session_cleanup) };
+                         .as_u64[1] = ntf == SESSION_CLEANUP_SESSION ?
+                                        pointer_to_uword (session_cleanup) :
+                                        0 };
 
   app_worker_add_event_custom (app_wrk, s->thread_index, &evt);
 
index 7eb6181..382b0a9 100644 (file)
@@ -327,6 +327,27 @@ session_cleanup_notify (session_t * s, session_cleanup_ntf_t ntf)
   app_worker_cleanup_notify (app_wrk, s, ntf);
 }
 
+static void
+session_cleanup_notify_custom (session_t *s, session_cleanup_ntf_t ntf,
+                              transport_cleanup_cb_fn cb_fn)
+{
+  app_worker_t *app_wrk;
+
+  app_wrk = app_worker_get_if_valid (s->app_wrk_index);
+  if (PREDICT_FALSE (!app_wrk))
+    {
+      if (ntf == SESSION_CLEANUP_TRANSPORT)
+       {
+         transport_cleanup_cb (cb_fn, session_get_transport (s));
+         return;
+       }
+
+      session_cleanup (s);
+      return;
+    }
+  app_worker_cleanup_notify_custom (app_wrk, s, ntf, cb_fn);
+}
+
 void
 session_program_cleanup (session_t *s)
 {
@@ -974,6 +995,79 @@ session_transport_delete_notify (transport_connection_t * tc)
     }
 }
 
+/**
+ * Request from transport to program connection deletion
+ *
+ * Similar to session_transport_delete_notify just that transport
+ * is asking session layer to delete the transport connection after
+ * it delievers notifications to app. Must be used if transport
+ * stats are to be collected.
+ */
+void
+session_transport_delete_request (transport_connection_t *tc,
+                                 transport_cleanup_cb_fn cb_fn)
+{
+  session_t *s;
+
+  /* App might've been removed already */
+  if (!(s = session_get_if_valid (tc->s_index, tc->thread_index)))
+    {
+      transport_cleanup_cb (cb_fn, tc);
+      return;
+    }
+
+  switch (s->session_state)
+    {
+    case SESSION_STATE_CREATED:
+      /* Session was created but accept notification was not yet sent to the
+       * app. Cleanup everything. */
+      session_lookup_del_session (s);
+      segment_manager_dealloc_fifos (s->rx_fifo, s->tx_fifo);
+      transport_cleanup_cb (cb_fn, tc);
+      session_free (s);
+      break;
+    case SESSION_STATE_ACCEPTING:
+    case SESSION_STATE_TRANSPORT_CLOSING:
+    case SESSION_STATE_CLOSING:
+    case SESSION_STATE_TRANSPORT_CLOSED:
+      /* If transport finishes or times out before we get a reply
+       * 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);
+      session_set_state (s, SESSION_STATE_TRANSPORT_DELETED);
+      session_cleanup_notify_custom (s, SESSION_CLEANUP_TRANSPORT, cb_fn);
+      svm_fifo_dequeue_drop_all (s->tx_fifo);
+      break;
+    case SESSION_STATE_APP_CLOSED:
+      /* 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);
+      session_set_state (s, SESSION_STATE_TRANSPORT_DELETED);
+      session_cleanup_notify_custom (s, SESSION_CLEANUP_TRANSPORT, cb_fn);
+      svm_fifo_dequeue_drop_all (s->tx_fifo);
+      session_program_transport_ctrl_evt (s, SESSION_CTRL_EVT_CLOSE);
+      break;
+    case SESSION_STATE_TRANSPORT_DELETED:
+      transport_cleanup_cb (cb_fn, tc);
+      break;
+    case SESSION_STATE_CLOSED:
+      session_cleanup_notify_custom (s, SESSION_CLEANUP_TRANSPORT, cb_fn);
+      session_set_state (s, SESSION_STATE_TRANSPORT_DELETED);
+      session_delete (s);
+      break;
+    default:
+      clib_warning ("session state %u", s->session_state);
+      session_cleanup_notify_custom (s, SESSION_CLEANUP_TRANSPORT, cb_fn);
+      session_delete (s);
+      break;
+    }
+}
+
 /**
  * Notification from transport that it is closed
  *
index d5402b3..744955d 100644 (file)
@@ -581,6 +581,14 @@ uword unformat_transport_connection (unformat_input_t * input,
  * Interface to transport protos
  */
 
+typedef void *transport_cleanup_cb_fn;
+
+static inline void
+transport_cleanup_cb (void *cb_fn, transport_connection_t *tc)
+{
+  ((void (*) (transport_connection_t *)) cb_fn) (tc);
+}
+
 int session_stream_connect_notify (transport_connection_t * tc,
                                   session_error_t err);
 int session_dgram_connect_notify (transport_connection_t * tc,
@@ -598,6 +606,8 @@ int session_stream_accept (transport_connection_t *tc, u32 listener_index,
                           clib_thread_index_t thread_index, u8 notify);
 int session_dgram_accept (transport_connection_t *tc, u32 listener_index,
                          clib_thread_index_t thread_index);
+void session_transport_delete_request (transport_connection_t *tc,
+                                      transport_cleanup_cb_fn cb_fn);
 
 /**
  * Initialize session layer for given transport proto and ip version
index dd3bde7..ffd2780 100644 (file)
@@ -54,8 +54,13 @@ app_worker_del_all_events (app_worker_t *app_wrk)
              break;
            case SESSION_CTRL_EVT_CLEANUP:
              s = session_get (evt->as_u64[0] & 0xffffffff, thread_index);
-             if (evt->as_u64[0] >> 32 != SESSION_CLEANUP_SESSION)
-               break;
+             if (evt->as_u64[0] >> 32 == SESSION_CLEANUP_TRANSPORT)
+               {
+                 if (evt->as_u64[1])
+                   transport_cleanup_cb ((void *) evt->as_u64[1],
+                                         session_get_transport (s));
+                 break;
+               }
              uword_to_pointer (evt->as_u64[1], void (*) (session_t * s)) (s);
              break;
            case SESSION_CTRL_EVT_HALF_CLEANUP:
@@ -244,8 +249,14 @@ app_worker_flush_events_inline (app_worker_t *app_wrk,
              if (app->cb_fns.session_cleanup_callback)
                app->cb_fns.session_cleanup_callback (s, evt->as_u64[0] >> 32);
            }
-         if (evt->as_u64[0] >> 32 != SESSION_CLEANUP_SESSION)
-           break;
+         if (evt->as_u64[0] >> 32 == SESSION_CLEANUP_TRANSPORT)
+           {
+             /* postponed cleanup requested */
+             if (evt->as_u64[1])
+               transport_cleanup_cb ((void *) evt->as_u64[1],
+                                     session_get_transport (s));
+             break;
+           }
          uword_to_pointer (evt->as_u64[1], void (*) (session_t * s)) (s);
          break;
        case SESSION_CTRL_EVT_HALF_CLEANUP:
index 0fb1c1c..ecbe003 100644 (file)
@@ -285,8 +285,8 @@ tcp_connection_cleanup (tcp_connection_t * tc)
 void
 tcp_connection_cleanup_and_notify (tcp_connection_t *tc)
 {
-  session_transport_delete_notify (&tc->connection);
-  tcp_connection_cleanup (tc);
+  tcp_connection_timers_reset (tc);
+  session_transport_delete_request (&tc->connection, tcp_connection_cleanup);
 }
 
 tcp_connection_t *
@@ -1291,8 +1291,7 @@ tcp_handle_cleanups (tcp_worker_ctx_t * wrk, clib_time_type_t now)
       tc = tcp_connection_get (req->connection_index, thread_index);
       if (PREDICT_FALSE (!tc))
        continue;
-      session_transport_delete_notify (&tc->connection);
-      tcp_connection_cleanup (tc);
+      tcp_connection_cleanup_and_notify (tc);
     }
 }
 
index 4ed5a68..5cee206 100644 (file)
@@ -108,8 +108,7 @@ udp_connection_cleanup (udp_connection_t * uc)
 void
 udp_connection_delete (udp_connection_t * uc)
 {
-  session_transport_delete_notify (&uc->connection);
-  udp_connection_cleanup (uc);
+  session_transport_delete_request (&uc->connection, udp_connection_cleanup);
 }
 
 static void