From: Florin Coras Date: Thu, 4 Apr 2019 00:52:43 +0000 (-0700) Subject: tcp: do not delete session on establish pop X-Git-Tag: v19.04-rc1~45 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F75%2F18675%2F5;p=vpp.git tcp: do not delete session on establish pop Also: - force reset if wait close pops in fin-wait-1 with unsent data - adds more event logging. Change-Id: I4ddada046214fa71e17514cdec57b3026f84a283 Signed-off-by: Florin Coras --- diff --git a/src/vnet/session/session.c b/src/vnet/session/session.c index f637d625616..9dc35238795 100644 --- a/src/vnet/session/session.c +++ b/src/vnet/session/session.c @@ -174,9 +174,15 @@ session_alloc (u32 thread_index) void session_free (session_t * s) { - pool_put (session_main.wrk[s->thread_index].sessions, s); if (CLIB_DEBUG) - clib_memset (s, 0xFA, sizeof (*s)); + { + u8 thread_index = s->thread_index; + clib_memset (s, 0xFA, sizeof (*s)); + pool_put (session_main.wrk[thread_index].sessions, s); + return; + } + SESSION_EVT_DBG (SESSION_EVT_FREE, s); + pool_put (session_main.wrk[s->thread_index].sessions, s); } void diff --git a/src/vnet/session/session_debug.h b/src/vnet/session/session_debug.h index 2912ae3828c..a26cd7f2636 100644 --- a/src/vnet/session/session_debug.h +++ b/src/vnet/session/session_debug.h @@ -25,6 +25,7 @@ _(POLL_GAP_TRACK, "poll gap track") \ _(POLL_DISPATCH_TIME, "dispatch time")\ _(DISPATCH_END, "dispatch end") \ + _(FREE, "session free") \ typedef enum _session_evt_dbg { @@ -36,6 +37,7 @@ typedef enum _session_evt_dbg #define SESSION_DEBUG 0 * (TRANSPORT_DEBUG > 0) #define SESSION_DEQ_NODE_EVTS (0) #define SESSION_EVT_POLL_DBG (0) +#define SESSION_SM (0) #if SESSION_DEBUG @@ -57,6 +59,21 @@ typedef enum _session_evt_dbg } * ed; \ ed = ELOG_DATA (&vlib_global_main.elog_main, _e) +#if SESSION_SM +#define SESSION_EVT_FREE_HANDLER(_s) \ +{ \ + ELOG_TYPE_DECLARE (_e) = \ + { \ + .format = "free: idx %u", \ + .format_args = "i4", \ + }; \ + DEC_SESSION_ETD(_s, _e, 1); \ + ed->data[0] = _s->session_index; \ +} +#else +#define SESSION_EVT_FREE_HANDLER(_s) +#endif + #if SESSION_DEQ_NODE_EVTS && SESSION_DEBUG > 1 #define SESSION_EVT_DEQ_HANDLER(_s, _body) \ { \ diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c index 78ada2b13c3..b6884f56fd5 100644 --- a/src/vnet/tcp/tcp.c +++ b/src/vnet/tcp/tcp.c @@ -266,9 +266,14 @@ void tcp_connection_free (tcp_connection_t * tc) { tcp_main_t *tm = &tcp_main; + if (CLIB_DEBUG) + { + u8 thread_index = tc->c_thread_index; + clib_memset (tc, 0xFA, sizeof (*tc)); + pool_put (tm->connections[thread_index], tc); + return; + } pool_put (tm->connections[tc->c_thread_index], tc); - if (CLIB_DEBUG > 0) - clib_memset (tc, 0xFA, sizeof (*tc)); } /** Notify session that connection has been reset. @@ -1255,10 +1260,10 @@ tcp_timer_establish_handler (u32 conn_index) ASSERT (tc->state == TCP_STATE_SYN_RCVD); tc->timers[TCP_TIMER_ESTABLISH] = TCP_TIMER_HANDLE_INVALID; tcp_connection_set_state (tc, TCP_STATE_CLOSED); - /* Start cleanup. App wasn't notified yet so use delete notify as - * opposed to delete to cleanup session layer state. */ tcp_connection_timers_reset (tc); - session_transport_delete_notify (&tc->connection); + /* Start cleanup. Do NOT delete the session until we do the connection + * cleanup. Otherwise, we end up with a dangling session index in the + * tcp connection. */ tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); } @@ -1283,7 +1288,7 @@ tcp_timer_establish_ao_handler (u32 conn_index) static void tcp_timer_waitclose_handler (u32 conn_index) { - u32 thread_index = vlib_get_thread_index (), rto; + u32 thread_index = vlib_get_thread_index (); tcp_connection_t *tc; tc = tcp_connection_get (conn_index, thread_index); @@ -1321,15 +1326,12 @@ tcp_timer_waitclose_handler (u32 conn_index) tcp_connection_timers_reset (tc); if (tc->flags & TCP_CONN_FINPNDG) { - /* If FIN pending send it before closing and wait as long as - * the rto timeout would wait. Notify session layer that transport - * is closed. We haven't sent everything but we did try. */ - tcp_cong_recovery_off (tc); - tcp_send_fin (tc); - rto = clib_max ((tc->rto >> tc->rto_boff) * TCP_TO_TIMER_TICK, 1); - tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, - clib_min (rto, TCP_2MSL_TIME)); + /* If FIN pending, we haven't sent everything, but we did try. + * Notify session layer that transport is closed. */ + tcp_connection_set_state (tc, TCP_STATE_CLOSED); session_transport_closed_notify (&tc->connection); + tcp_send_reset (tc); + tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); } else { diff --git a/src/vnet/tcp/tcp_debug.h b/src/vnet/tcp/tcp_debug.h index 23ad39187c2..262d3faae10 100755 --- a/src/vnet/tcp/tcp_debug.h +++ b/src/vnet/tcp/tcp_debug.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Cisco and/or its affiliates. + * Copyright (c) 2017-2019 Cisco and/or its affiliates. * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at: @@ -177,7 +177,7 @@ typedef enum _tcp_dbg_evt { \ ELOG_TYPE_DECLARE (_e) = \ { \ - .format = "close: %d", \ + .format = "close: cidx %d", \ .format_args = "i4", \ }; \ DECLARE_ETD(_tc, _e, 1); \ @@ -201,12 +201,13 @@ typedef enum _tcp_dbg_evt TCP_EVT_INIT_HANDLER(_tc, 0); \ ELOG_TYPE_DECLARE (_e) = \ { \ - .format = "syn-rx: idx %u irs %u", \ - .format_args = "i4i4", \ + .format = "syn-rx: cidx %u sidx %u irs %u", \ + .format_args = "i4i4i4", \ }; \ - DECLARE_ETD(_tc, _e, 2); \ + DECLARE_ETD(_tc, _e, 3); \ ed->data[0] = _tc->c_c_index; \ - ed->data[1] = _tc->irs; \ + ed->data[1] = _tc->c_s_index; \ + ed->data[2] = _tc->irs; \ TCP_EVT_STATE_CHANGE_HANDLER(_tc); \ } @@ -226,11 +227,12 @@ typedef enum _tcp_dbg_evt { \ ELOG_TYPE_DECLARE (_e) = \ { \ - .format = "delete: %d", \ - .format_args = "i4", \ + .format = "delete: cidx %d sidx %d", \ + .format_args = "i4i4", \ }; \ - DECLARE_ETD(_tc, _e, 1); \ + DECLARE_ETD(_tc, _e, 2); \ ed->data[0] = _tc->c_c_index; \ + ed->data[1] = _tc->c_s_index; \ TCP_EVT_DEALLOC_HANDLER(_tc); \ } @@ -379,7 +381,7 @@ if (_tc) \ .n_enum_strings = 2, \ .enum_strings = { \ "syn", \ - "syn-ack", \ + "synack", \ }, \ }; \ DECLARE_ETD(_tc, _e, 5); \ @@ -405,8 +407,8 @@ if (_tc) \ } \ ELOG_TYPE_DECLARE (_e) = \ { \ - .format = "timer-pop: %s (%d)", \ - .format_args = "t4i4", \ + .format = "timer-pop: %s cidx %u sidx %u", \ + .format_args = "t4i4i4", \ .n_enum_strings = 8, \ .enum_strings = { \ "retransmit", \ @@ -421,9 +423,10 @@ if (_tc) \ }; \ if (_tc) \ { \ - DECLARE_ETD(_tc, _e, 2); \ + DECLARE_ETD(_tc, _e, 3); \ ed->data[0] = _timer_id; \ - ed->data[1] = _timer_id; \ + ed->data[1] = _tc->c_c_index; \ + ed->data[2] = _tc->c_s_index; \ } \ else \ { \ diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c index e0139a47e0c..f838bc562ff 100644 --- a/src/vnet/tcp/tcp_input.c +++ b/src/vnet/tcp/tcp_input.c @@ -3185,7 +3185,6 @@ tcp46_listen_inline (vlib_main_t * vm, vlib_node_runtime_t * node, tcp_connection_init_vars (child0); child0->rto = TCP_RTO_MIN; - TCP_EVT_DBG (TCP_EVT_SYN_RCVD, child0, 1); if (session_stream_accept (&child0->connection, lc0->c_s_index, 0 /* notify */ )) @@ -3195,6 +3194,7 @@ tcp46_listen_inline (vlib_main_t * vm, vlib_node_runtime_t * node, goto drop; } + TCP_EVT_DBG (TCP_EVT_SYN_RCVD, child0, 1); child0->tx_fifo_size = transport_tx_fifo_size (&child0->connection); tcp_send_synack (child0); tcp_timer_set (child0, TCP_TIMER_ESTABLISH, TCP_SYN_RCVD_TIME);