From: Florin Coras Date: Thu, 27 Dec 2018 19:53:11 +0000 (-0800) Subject: session: free session after transport and app confirm X-Git-Tag: v19.04-rc0~71 X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=5a2ec8fc41d14ffc5275ab88761c1fb7e0420a33 session: free session after transport and app confirm In addition to that, a bit of refactoring. Change-Id: Iea1eabc2167bcdef185ec53bc09bae087c5398e6 Signed-off-by: Florin Coras --- 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; @@ -764,23 +802,6 @@ stream_session_disconnect_notify (transport_connection_t * tc) app->cb_fns.session_disconnect_callback (s); } -/** - * 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 * @@ -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); } }