From 3c7d4f9e1f54ec6627795b64525f182e2cda7490 Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Fri, 14 Dec 2018 11:28:43 -0800 Subject: [PATCH] vcl/session: handle reset/disconnect before app accept Also further improves reset handling. Change-Id: I6e517632f700f181761726b965134e0c217eb06d Signed-off-by: Florin Coras --- src/vcl/vcl_private.h | 11 +- src/vcl/vppcom.c | 202 +++++++++++++++++++++---------- src/vnet/session/application_interface.h | 1 - src/vnet/session/session.c | 6 +- src/vnet/session/session_node.c | 22 ++-- src/vnet/tcp/tcp.c | 6 +- 6 files changed, 167 insertions(+), 81 deletions(-) diff --git a/src/vcl/vcl_private.h b/src/vcl/vcl_private.h index 44c6520703e..3d6c85ea6f7 100644 --- a/src/vcl/vcl_private.h +++ b/src/vcl/vcl_private.h @@ -67,13 +67,13 @@ typedef enum STATE_CONNECT = 0x02, STATE_LISTEN = 0x04, STATE_ACCEPT = 0x08, - STATE_CLOSE_ON_EMPTY = 0x10, + STATE_VPP_CLOSING = 0x10, STATE_DISCONNECT = 0x20, STATE_FAILED = 0x40 } session_state_t; -#define SERVER_STATE_OPEN (STATE_ACCEPT|STATE_CLOSE_ON_EMPTY) -#define CLIENT_STATE_OPEN (STATE_CONNECT|STATE_CLOSE_ON_EMPTY) +#define SERVER_STATE_OPEN (STATE_ACCEPT|STATE_VPP_CLOSING) +#define CLIENT_STATE_OPEN (STATE_CONNECT|STATE_VPP_CLOSING) #define STATE_OPEN (SERVER_STATE_OPEN | CLIENT_STATE_OPEN) typedef struct epoll_event vppcom_epoll_event_t; @@ -95,6 +95,9 @@ typedef struct ip46_address_t ip46; } vppcom_ip46_t; +#define VCL_ACCEPTED_F_CLOSED (1 << 0) +#define VCL_ACCEPTED_F_RESET (1 << 1) + typedef struct vcl_session_msg { u32 next; @@ -102,6 +105,7 @@ typedef struct vcl_session_msg { session_accepted_msg_t accepted_msg; }; + u32 flags; } vcl_session_msg_t; enum @@ -155,6 +159,7 @@ typedef struct u32 sm_seg_index; u32 client_context; u64 vpp_handle; + u32 vpp_thread_index; /* Socket configuration state */ u8 is_vep; diff --git a/src/vcl/vppcom.c b/src/vcl/vppcom.c index 88a67ddab30..773350ae0c9 100644 --- a/src/vcl/vppcom.c +++ b/src/vcl/vppcom.c @@ -67,8 +67,8 @@ vppcom_session_state_str (session_state_t state) st = "STATE_ACCEPT"; break; - case STATE_CLOSE_ON_EMPTY: - st = "STATE_CLOSE_ON_EMPTY"; + case STATE_VPP_CLOSING: + st = "STATE_VPP_CLOSING"; break; case STATE_DISCONNECT: @@ -181,7 +181,7 @@ vcl_session_vpp_evt_q (vcl_worker_t * wrk, vcl_session_t * s) if (vcl_session_is_ct (s)) return wrk->vpp_event_queues[0]; else - return wrk->vpp_event_queues[s->tx_fifo->master_thread_index]; + return wrk->vpp_event_queues[s->vpp_thread_index]; } static void @@ -289,6 +289,7 @@ vcl_session_accepted_handler (vcl_worker_t * wrk, session_accepted_msg_t * mp) } session->vpp_handle = mp->handle; + session->vpp_thread_index = rx_fifo->master_thread_index; session->client_context = mp->context; session->rx_fifo = rx_fifo; session->tx_fifo = tx_fifo; @@ -380,6 +381,7 @@ vcl_session_connected_handler (vcl_worker_t * wrk, session->rx_fifo = rx_fifo; session->tx_fifo = tx_fifo; session->vpp_handle = mp->handle; + session->vpp_thread_index = rx_fifo->master_thread_index; session->transport.is_ip4 = mp->is_ip4; clib_memcpy_fast (&session->transport.lcl_ip, mp->lcl_ip, sizeof (session->transport.lcl_ip)); @@ -387,7 +389,7 @@ vcl_session_connected_handler (vcl_worker_t * wrk, session->session_state = STATE_CONNECT; /* Add it to lookup table */ - hash_set (wrk->session_index_by_vpp_handles, mp->handle, session_index); + vcl_session_table_add_vpp_handle (wrk, mp->handle, session_index); VDBG (1, "VCL<%d>: vpp handle 0x%llx, sid %u: connect succeeded! " "session_rx_fifo %p, refcnt %d, session_tx_fifo %p, refcnt %d", @@ -397,6 +399,24 @@ vcl_session_connected_handler (vcl_worker_t * wrk, return session_index; } +static int +vcl_flag_accepted_session (vcl_session_t * session, u64 handle, u32 flags) +{ + vcl_session_msg_t *accepted_msg; + int i; + + for (i = 0; i < vec_len (session->accept_evts_fifo); i++) + { + accepted_msg = &session->accept_evts_fifo[i]; + if (accepted_msg->accepted_msg.handle == handle) + { + accepted_msg->flags = flags; + return 1; + } + } + return 0; +} + static u32 vcl_session_reset_handler (vcl_worker_t * wrk, session_reset_msg_t * reset_msg) @@ -411,8 +431,21 @@ vcl_session_reset_handler (vcl_worker_t * wrk, VDBG (0, "request to reset unknown handle 0x%llx", reset_msg->handle); return VCL_INVALID_SESSION_INDEX; } - session->session_state = STATE_CLOSE_ON_EMPTY; - VDBG (0, "reset handle 0x%llx, sid %u ", reset_msg->handle, sid); + if (session->session_state >= STATE_VPP_CLOSING) + return sid; + + /* Caught a reset before actually accepting the session */ + if (session->session_state == STATE_LISTEN) + { + + if (!vcl_flag_accepted_session (session, reset_msg->handle, + VCL_ACCEPTED_F_RESET)) + VDBG (0, "session was not accepted!"); + return VCL_INVALID_SESSION_INDEX; + } + + session->session_state = STATE_DISCONNECT; + VDBG (0, "reset session %u [0x%llx]", sid, reset_msg->handle); vcl_send_session_reset_reply (vcl_session_vpp_evt_q (wrk, session), wrk->my_client_index, reset_msg->handle, 0); return sid; @@ -468,15 +501,64 @@ vcl_session_bound_handler (vcl_worker_t * wrk, session_bound_msg_t * mp) return sid; } +static vcl_session_t * +vcl_session_accepted (vcl_worker_t * wrk, session_accepted_msg_t * msg) +{ + vcl_session_msg_t *vcl_msg; + vcl_session_t *session; + + session = vcl_session_get_w_vpp_handle (wrk, msg->handle); + if (PREDICT_FALSE (session != 0)) + VWRN ("session handle overlap %lu!", msg->handle); + + session = vcl_session_table_lookup_listener (wrk, msg->listener_handle); + if (!session) + { + VERR ("couldn't find listen session: listener handle %llx", + msg->listener_handle); + return 0; + } + + clib_fifo_add2 (session->accept_evts_fifo, vcl_msg); + vcl_msg->accepted_msg = *msg; + /* Session handle points to listener until fully accepted by app */ + vcl_session_table_add_vpp_handle (wrk, msg->handle, session->session_index); + + return session; +} + +static vcl_session_t * +vcl_session_disconnected_handler (vcl_worker_t * wrk, + session_disconnected_msg_t * msg) +{ + vcl_session_t *session; + + session = vcl_session_get_w_vpp_handle (wrk, msg->handle); + if (!session) + { + VDBG (0, "request to disconnect unknown handle 0x%llx", msg->handle); + return 0; + } + + /* Caught a disconnect before actually accepting the session */ + if (session->session_state == STATE_LISTEN) + { + + if (!vcl_flag_accepted_session (session, msg->handle, + VCL_ACCEPTED_F_CLOSED)) + VDBG (0, "session was not accepted!"); + return 0; + } + + session->session_state = STATE_VPP_CLOSING; + return session; +} + static int vcl_handle_mq_event (vcl_worker_t * wrk, session_event_t * e) { - session_accepted_msg_t *accepted_msg; session_disconnected_msg_t *disconnected_msg; - vcl_session_msg_t *vcl_msg; vcl_session_t *session; - u64 handle; - u32 sid; switch (e->event_type) { @@ -487,18 +569,7 @@ vcl_handle_mq_event (vcl_worker_t * wrk, session_event_t * e) vec_add1 (wrk->unhandled_evts_vector, *e); break; case SESSION_CTRL_EVT_ACCEPTED: - accepted_msg = (session_accepted_msg_t *) e->data; - handle = accepted_msg->listener_handle; - session = vcl_session_table_lookup_listener (wrk, handle); - if (!session) - { - clib_warning ("VCL<%d>: ERROR: couldn't find listen session:" - "listener handle %llx", getpid (), handle); - break; - } - - clib_fifo_add2 (session->accept_evts_fifo, vcl_msg); - vcl_msg->accepted_msg = *accepted_msg; + vcl_session_accepted (wrk, (session_accepted_msg_t *) e->data); break; case SESSION_CTRL_EVT_CONNECTED: vcl_session_connected_handler (wrk, @@ -506,17 +577,12 @@ vcl_handle_mq_event (vcl_worker_t * wrk, session_event_t * e) break; case SESSION_CTRL_EVT_DISCONNECTED: disconnected_msg = (session_disconnected_msg_t *) e->data; - sid = vcl_session_index_from_vpp_handle (wrk, disconnected_msg->handle); - session = vcl_session_get (wrk, sid); + session = vcl_session_disconnected_handler (wrk, disconnected_msg); if (!session) - { - VDBG (0, "request to disconnect unknown handle 0x%llx", - disconnected_msg->handle); - break; - } + break; session->session_state = STATE_DISCONNECT; - VDBG (0, "disconnected handle 0x%llx, sid %u", disconnected_msg->handle, - sid); + VDBG (0, "disconnected session %u [0x%llx]", session->session_index, + session->vpp_handle); break; case SESSION_CTRL_EVT_RESET: vcl_session_reset_handler (wrk, (session_reset_msg_t *) e->data); @@ -663,7 +729,7 @@ vppcom_session_disconnect (u32 session_handle) return VPPCOM_EBADFD; } - if (state & STATE_CLOSE_ON_EMPTY) + if (state & STATE_VPP_CLOSING) { vpp_evt_q = vcl_session_vpp_evt_q (wrk, session); vcl_send_session_disconnected_reply (vpp_evt_q, wrk->my_client_index, @@ -1202,7 +1268,7 @@ int vppcom_session_accept (uint32_t listen_session_handle, vppcom_endpt_t * ep, uint32_t flags) { - u32 client_session_index = ~0, listen_session_index; + u32 client_session_index = ~0, listen_session_index, accept_flags = 0; vcl_worker_t *wrk = vcl_worker_get_current (); session_accepted_msg_t accepted_msg; vcl_session_t *listen_session = 0; @@ -1226,6 +1292,7 @@ vppcom_session_accept (uint32_t listen_session_handle, vppcom_endpt_t * ep, if (clib_fifo_elts (listen_session->accept_evts_fifo)) { clib_fifo_sub2 (listen_session->accept_evts_fifo, evt); + accept_flags = evt->flags; accepted_msg = evt->accepted_msg; goto handle; } @@ -1302,6 +1369,25 @@ handle: vcl_evt (VCL_EVT_ACCEPT, client_session, listen_session, client_session_index); + /* + * Session might have been closed already + */ + if (accept_flags) + { + svm_msg_q_t *mq = vcl_session_vpp_evt_q (wrk, client_session); + if (accept_flags & VCL_ACCEPTED_F_CLOSED) + { + client_session->session_state = STATE_DISCONNECT; + vcl_send_session_disconnected_reply (mq, wrk->my_client_index, + client_session->vpp_handle, 0); + } + else if (accept_flags & VCL_ACCEPTED_F_RESET) + { + client_session->session_state = STATE_DISCONNECT; + vcl_send_session_reset_reply (mq, wrk->my_client_index, + client_session->vpp_handle, 0); + } + } return vcl_session_handle (client_session); } @@ -1472,7 +1558,7 @@ vppcom_session_read_internal (uint32_t session_handle, void *buf, int n, } svm_msg_q_free_msg (mq, &msg); - if (PREDICT_FALSE (s->session_state == STATE_CLOSE_ON_EMPTY)) + if (PREDICT_FALSE (s->session_state == STATE_VPP_CLOSING)) return 0; } } @@ -1565,7 +1651,7 @@ vppcom_session_read_segments (uint32_t session_handle, } svm_msg_q_free_msg (mq, &msg); - if (PREDICT_FALSE (s->session_state == STATE_CLOSE_ON_EMPTY)) + if (PREDICT_FALSE (s->session_state == STATE_VPP_CLOSING)) return 0; } } @@ -1719,6 +1805,9 @@ vppcom_session_write_inline (uint32_t session_handle, void *buf, size_t n, } } + if (PREDICT_FALSE (!(s->session_state & STATE_OPEN))) + return VPPCOM_ECONNRESET; + ASSERT (FIFO_EVENT_APP_TX + 1 == SESSION_IO_EVT_CT_TX); et = FIFO_EVENT_APP_TX + vcl_session_is_ct (s); if (is_flush && !vcl_session_is_ct (s)) @@ -1845,10 +1934,7 @@ vcl_select_handle_mq_event (vcl_worker_t * wrk, session_event_t * e, { session_disconnected_msg_t *disconnected_msg; session_connected_msg_t *connected_msg; - session_accepted_msg_t *accepted_msg; - vcl_session_msg_t *vcl_msg; vcl_session_t *session; - u64 handle; u32 sid; switch (e->event_type) @@ -1900,18 +1986,10 @@ vcl_select_handle_mq_event (vcl_worker_t * wrk, session_event_t * e, } break; case SESSION_CTRL_EVT_ACCEPTED: - accepted_msg = (session_accepted_msg_t *) e->data; - handle = accepted_msg->listener_handle; - session = vcl_session_table_lookup_listener (wrk, handle); + session = vcl_session_accepted (wrk, + (session_accepted_msg_t *) e->data); if (!session) - { - clib_warning ("VCL<%d>: ERROR: couldn't find listen session:" - "listener handle %llx", getpid (), handle); - break; - } - - clib_fifo_add2 (session->accept_evts_fifo, vcl_msg); - vcl_msg->accepted_msg = *accepted_msg; + break; sid = session->session_index; if (sid < n_bits && read_map) { @@ -1925,7 +2003,10 @@ vcl_select_handle_mq_event (vcl_worker_t * wrk, session_event_t * e, break; case SESSION_CTRL_EVT_DISCONNECTED: disconnected_msg = (session_disconnected_msg_t *) e->data; - sid = vcl_session_index_from_vpp_handle (wrk, disconnected_msg->handle); + session = vcl_session_disconnected_handler (wrk, disconnected_msg); + if (!session) + break; + sid = session->session_index; if (sid < n_bits && except_map) { clib_bitmap_set_no_check (except_map, sid, 1); @@ -2454,10 +2535,8 @@ vcl_epoll_wait_handle_mq_event (vcl_worker_t * wrk, session_event_t * e, { session_disconnected_msg_t *disconnected_msg; session_connected_msg_t *connected_msg; - session_accepted_msg_t *accepted_msg; - u64 session_evt_data = ~0, handle; u32 sid = ~0, session_events; - vcl_session_msg_t *vcl_msg; + u64 session_evt_data = ~0; vcl_session_t *session; u8 add_event = 0; @@ -2509,18 +2588,11 @@ vcl_epoll_wait_handle_mq_event (vcl_worker_t * wrk, session_event_t * e, session_evt_data = session->vep.ev.data.u64; break; case SESSION_CTRL_EVT_ACCEPTED: - accepted_msg = (session_accepted_msg_t *) e->data; - handle = accepted_msg->listener_handle; - session = vcl_session_table_lookup_listener (wrk, handle); + session = vcl_session_accepted (wrk, + (session_accepted_msg_t *) e->data); if (!session) - { - clib_warning ("VCL<%d>: ERROR: couldn't find listen session:" - "listener handle %llx", getpid (), handle); - break; - } + break; - clib_fifo_add2 (session->accept_evts_fifo, vcl_msg); - vcl_msg->accepted_msg = *accepted_msg; session_events = session->vep.ev.events; if (!(EPOLLIN & session_events)) break; @@ -2545,8 +2617,8 @@ vcl_epoll_wait_handle_mq_event (vcl_worker_t * wrk, session_event_t * e, break; case SESSION_CTRL_EVT_DISCONNECTED: disconnected_msg = (session_disconnected_msg_t *) e->data; - sid = vcl_session_index_from_vpp_handle (wrk, disconnected_msg->handle); - if (!(session = vcl_session_get (wrk, sid))) + session = vcl_session_disconnected_handler (wrk, disconnected_msg); + if (!session) break; add_event = 1; events[*num_ev].events |= EPOLLHUP | EPOLLRDHUP; diff --git a/src/vnet/session/application_interface.h b/src/vnet/session/application_interface.h index dfb45a78fc9..a156c82a745 100644 --- a/src/vnet/session/application_interface.h +++ b/src/vnet/session/application_interface.h @@ -297,7 +297,6 @@ typedef struct session_reset_msg_ typedef struct session_reset_reply_msg_ { - u32 client_index; u32 context; i32 retval; u64 handle; diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c index 6533303d0c6..7f9a32f46de 100644 --- a/src/vnet/session/session.c +++ b/src/vnet/session/session.c @@ -836,6 +836,9 @@ stream_session_reset_notify (transport_connection_t * tc) application_t *app; s = session_get (tc->s_index, tc->thread_index); svm_fifo_dequeue_drop_all (s->server_tx_fifo); + if (s->session_state >= SESSION_STATE_TRANSPORT_CLOSING) + return; + s->session_state = SESSION_STATE_TRANSPORT_CLOSING; app_wrk = app_worker_get (s->app_wrk_index); app = application_get (app_wrk->app_index); app->cb_fns.session_reset_callback (s); @@ -1204,11 +1207,10 @@ session_vpp_event_queues_allocate (session_manager_main_t * smm) for (i = 0; i < vec_len (smm->wrk); i++) { svm_msg_q_cfg_t _cfg, *cfg = &_cfg; - u32 notif_q_size = clib_max (16, evt_q_length >> 4); svm_msg_q_ring_cfg_t rc[SESSION_MQ_N_RINGS] = { {evt_q_length, evt_size, 0} , - {notif_q_size, 256, 0} + {evt_q_length << 1, 256, 0} }; cfg->consumer_pid = 0; cfg->n_rings = 2; diff --git a/src/vnet/session/session_node.c b/src/vnet/session/session_node.c index f4e0eaa993e..eb9026c6457 100644 --- a/src/vnet/session/session_node.c +++ b/src/vnet/session/session_node.c @@ -66,7 +66,7 @@ session_mq_accepted_reply_handler (void *data) s = session_get_from_handle_if_valid (mp->handle); if (!s) { - clib_warning ("session doesn't exist"); + clib_warning ("session 0x%llx doesn't exist", mp->handle); return; } app_wrk = app_worker_get (s->app_wrk_index); @@ -92,17 +92,17 @@ session_mq_reset_reply_handler (void *data) u32 index, thread_index; mp = (session_reset_reply_msg_t *) data; - app = application_lookup (mp->client_index); + app = application_lookup (mp->context); if (!app) return; session_parse_handle (mp->handle, &index, &thread_index); s = session_get_if_valid (index, thread_index); - if (!s) - { - SESSION_DBG ("Invalid session!"); - return; - } + + /* Session was already closed or already cleaned up */ + if (!s || s->session_state != SESSION_STATE_TRANSPORT_CLOSING) + return; + app_wrk = app_worker_get (s->app_wrk_index); if (!app_wrk || app_wrk->app_index != app->app_index) { @@ -811,6 +811,9 @@ session_queue_node_fn (vlib_main_t * vm, vlib_node_runtime_t * node, { vec_add2 (fifo_events, e, 1); svm_msg_q_sub_w_lock (mq, msg); + /* Works because reply messages are smaller than a session evt. + * If we ever need to support bigger messages this needs to be + * fixed */ clib_memcpy_fast (e, svm_msg_q_msg_data (mq, msg), sizeof (*e)); svm_msg_q_free_msg (mq, msg); } @@ -875,7 +878,10 @@ session_queue_node_fn (vlib_main_t * vm, vlib_node_runtime_t * node, case FIFO_EVENT_DISCONNECT: /* Make sure stream disconnects run after the pending list is * drained */ - s = session_get_from_handle (e->session_handle); + s = session_get_from_handle_if_valid (e->session_handle); + if (PREDICT_FALSE (!s)) + break; + if (!e->postponed) { e->postponed = 1; diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c index 6d6a880eda8..a3cd1f3e1bb 100644 --- a/src/vnet/tcp/tcp.c +++ b/src/vnet/tcp/tcp.c @@ -359,10 +359,12 @@ tcp_connection_close (tcp_connection_t * tc) TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc); - /* If in CLOSED and WAITCLOSE timer is not set, delete connection now */ + /* If in CLOSED and WAITCLOSE timer is not set, delete connection. + * But instead of doing it now wait until next dispatch cycle to give + * the session layer a chance to clear unhandled events */ if (!tcp_timer_is_active (tc, TCP_TIMER_WAITCLOSE) && tc->state == TCP_STATE_CLOSED) - tcp_connection_del (tc); + tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, 1); } static void -- 2.16.6