From c4c37fe587070bd911b9ba9aae934ed2c6c3beab Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Fri, 2 May 2025 03:20:53 -0400 Subject: [PATCH] session: add support for postponing transport cleanups 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 --- src/vnet/session/application_worker.c | 4 +- src/vnet/session/session.c | 94 +++++++++++++++++++++++++++++++++++ src/vnet/session/session.h | 10 ++++ src/vnet/session/session_input.c | 19 +++++-- src/vnet/tcp/tcp.c | 7 ++- src/vnet/udp/udp.c | 3 +- 6 files changed, 126 insertions(+), 11 deletions(-) diff --git a/src/vnet/session/application_worker.c b/src/vnet/session/application_worker.c index a5b1e1f4ea4..c919cc10f78 100644 --- a/src/vnet/session/application_worker.c +++ b/src/vnet/session/application_worker.c @@ -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); diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c index 7eb6181adb9..382b0a9f3db 100644 --- a/src/vnet/session/session.c +++ b/src/vnet/session/session.c @@ -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 * diff --git a/src/vnet/session/session.h b/src/vnet/session/session.h index d5402b3571e..744955deec3 100644 --- a/src/vnet/session/session.h +++ b/src/vnet/session/session.h @@ -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 diff --git a/src/vnet/session/session_input.c b/src/vnet/session/session_input.c index dd3bde77058..ffd27802ab2 100644 --- a/src/vnet/session/session_input.c +++ b/src/vnet/session/session_input.c @@ -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: diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c index 0fb1c1cf616..ecbe0037076 100644 --- a/src/vnet/tcp/tcp.c +++ b/src/vnet/tcp/tcp.c @@ -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); } } diff --git a/src/vnet/udp/udp.c b/src/vnet/udp/udp.c index 4ed5a68fa02..5cee206bbf0 100644 --- a/src/vnet/udp/udp.c +++ b/src/vnet/udp/udp.c @@ -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 -- 2.16.6