From e96bf63bd0dc483179dc595b65ebd8bf2b310b8b Mon Sep 17 00:00:00 2001 From: Florin Coras Date: Tue, 18 Dec 2018 22:44:27 -0800 Subject: [PATCH] tcp/session: notify transport of close when tx fifo is not empty Disconnect transport even if tx fifo is not empty and have transport deal with the problem. In case of tcp, add timer to fin_wait_1. If it expires and we're still in established state, cleanup but only after waiting for session tx events to cleanup. Change-Id: I45759a3c43dd096bb2c03daf5372416c30678d62 Signed-off-by: Florin Coras --- src/vnet/session/session_node.c | 18 ++++++++---------- src/vnet/tcp/tcp.c | 23 ++++++++++++++--------- src/vnet/tcp/tcp.h | 5 +++-- src/vnet/tcp/tcp_input.c | 5 ++++- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/vnet/session/session_node.c b/src/vnet/session/session_node.c index 58d31ccb87c..e8ed1cf9b95 100644 --- a/src/vnet/session/session_node.c +++ b/src/vnet/session/session_node.c @@ -874,21 +874,19 @@ session_queue_node_fn (vlib_main_t * vm, vlib_node_runtime_t * node, } break; case FIFO_EVENT_DISCONNECT: - /* Make sure stream disconnects run after the pending list is - * drained */ s = session_get_from_handle_if_valid (e->session_handle); if (PREDICT_FALSE (!s)) break; - if (!e->postponed) - { - e->postponed = 1; - vec_add1 (wrk->pending_disconnects, *e); - continue; - } - /* If tx queue is still not empty, wait */ - if (svm_fifo_max_dequeue (s->server_tx_fifo)) + /* Make sure session disconnects run after the pending list is + * drained, i.e., postpone if the first time. If not the first + * and the tx queue is still not empty, try to wait for some + * dispatch cycles */ + if (!e->postponed + || (e->postponed < 200 + && svm_fifo_max_dequeue (s->server_tx_fifo))) { + e->postponed += 1; vec_add1 (wrk->pending_disconnects, *e); continue; } diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c index 285ed624d56..9a3c90425b2 100644 --- a/src/vnet/tcp/tcp.c +++ b/src/vnet/tcp/tcp.c @@ -290,7 +290,7 @@ tcp_connection_reset (tcp_connection_t * tc) tcp_connection_timers_reset (tc); /* Set the cleanup timer, in case the session layer/app don't * cleanly close the connection */ - tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); + tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLOSEWAIT_TIME); stream_session_reset_notify (&tc->connection); break; case TCP_STATE_CLOSE_WAIT: @@ -300,7 +300,7 @@ tcp_connection_reset (tcp_connection_t * tc) tc->state = TCP_STATE_CLOSED; TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc); tcp_connection_timers_reset (tc); - tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); + tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLOSEWAIT_TIME); break; case TCP_STATE_CLOSED: return; @@ -336,7 +336,7 @@ tcp_connection_close (tcp_connection_t * tc) tcp_connection_timers_reset (tc); tcp_send_fin (tc); tc->state = TCP_STATE_FIN_WAIT_1; - tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); + tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_FINWAIT1_TIME); break; case TCP_STATE_ESTABLISHED: if (!session_tx_fifo_max_dequeue (&tc->connection)) @@ -344,6 +344,9 @@ tcp_connection_close (tcp_connection_t * tc) else tc->flags |= TCP_CONN_FINPNDG; tc->state = TCP_STATE_FIN_WAIT_1; + /* Set a timer in case the peer stops responding. Otherwise the + * connection will be stuck here forever. */ + tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_FINWAIT1_TIME); break; case TCP_STATE_CLOSE_WAIT: if (!session_tx_fifo_max_dequeue (&tc->connection)) @@ -373,7 +376,7 @@ tcp_connection_close (tcp_connection_t * tc) * the session layer a chance to clear unhandled events */ if (!tcp_timer_is_active (tc, TCP_TIMER_WAITCLOSE) && tc->state == TCP_STATE_CLOSED) - tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, 1); + tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); } static void @@ -1251,11 +1254,6 @@ tcp_timer_waitclose_handler (u32 conn_index) * and switch to LAST_ACK. */ if (tc->state == TCP_STATE_CLOSE_WAIT) { - if (tc->flags & TCP_CONN_FINSNT) - { - clib_warning ("FIN was sent and still in CLOSE WAIT. Weird!"); - } - /* Make sure we don't try to send unsent data */ tcp_connection_timers_reset (tc); tcp_cong_recovery_off (tc); @@ -1269,6 +1267,13 @@ tcp_timer_waitclose_handler (u32 conn_index) /* Don't delete the connection yet */ return; } + else if (tc->state == TCP_STATE_FIN_WAIT_1) + { + /* Wait for session layer to clean up tx events */ + tc->state = TCP_STATE_CLOSED; + tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); + return; + } tcp_connection_del (tc); } diff --git a/src/vnet/tcp/tcp.h b/src/vnet/tcp/tcp.h index a62e01d70e1..2327b9a6ad5 100644 --- a/src/vnet/tcp/tcp.h +++ b/src/vnet/tcp/tcp.h @@ -102,7 +102,8 @@ extern timer_expiration_handler tcp_timer_retransmit_syn_handler; #define TCP_2MSL_TIME 300 /* 30s */ #define TCP_CLOSEWAIT_TIME 20 /* 2s */ #define TCP_TIMEWAIT_TIME 100 /* 10s */ -#define TCP_CLEANUP_TIME 10 /* 1s Time to wait before cleanup */ +#define TCP_FINWAIT1_TIME 600 /* 60s */ +#define TCP_CLEANUP_TIME 1 /* 0.1s */ #define TCP_TIMER_PERSIST_MIN 2 /* 0.2s */ #define TCP_RTO_MAX 60 * THZ /* Min max RTO (60s) as per RFC6298 */ @@ -124,7 +125,7 @@ extern timer_expiration_handler tcp_timer_retransmit_syn_handler; _(FRXT_PENDING, "Fast-retransmit pending") \ _(FRXT_FIRST, "Fast-retransmit first again") \ _(DEQ_PENDING, "Pending dequeue acked") \ - _(PSH_PENDING, "Pending psh packet") \ + _(PSH_PENDING, "PSH pending") \ typedef enum _tcp_connection_flag_bits { diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c index 8b16275359c..4e005265889 100644 --- a/src/vnet/tcp/tcp_input.c +++ b/src/vnet/tcp/tcp_input.c @@ -2861,7 +2861,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node, * we can't ensure that we have no packets already enqueued * to output. Rely instead on the waitclose timer */ tcp_connection_timers_reset (tc0); - tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, 1); + tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME); goto drop; @@ -2918,6 +2918,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node, tcp_send_fin (tc0); stream_session_disconnect_notify (&tc0->connection); tc0->state = TCP_STATE_CLOSE_WAIT; + tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_CLOSEWAIT_TIME); TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc0); break; case TCP_STATE_CLOSE_WAIT: @@ -3639,6 +3640,8 @@ do { \ /* FIN in reply to our FIN from the other side */ _(FIN_WAIT_1, TCP_FLAG_FIN, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); _(FIN_WAIT_1, TCP_FLAG_RST, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); + _(FIN_WAIT_1, TCP_FLAG_RST | TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS, + TCP_ERROR_NONE); _(CLOSING, TCP_FLAG_ACK, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); /* FIN confirming that the peer (app) has closed */ _(FIN_WAIT_2, TCP_FLAG_FIN, TCP_INPUT_NEXT_RCV_PROCESS, TCP_ERROR_NONE); -- 2.16.6