tcp: cleanup connection/session fixes

- Cleanup session state after last ack and avoid using a cleanup timer.
- Change session cleanup to free the session as opposed to waiting for
delete notify.
- When in close-wait, postpone sending the fin on close until all
outstanding data has been sent.
- Don't flush rx fifo unless in closed state

Change-Id: Ic2a4f0d5568b65c83f4b55b6c469a7b24b947f39
Signed-off-by: Florin Coras <fcoras@cisco.com>

4 files changed, 36 insertions, 24 deletions

diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c
index 15ac7d37edc..2a696f19d22 100644
--- a/src/vnet/tcp/tcp.c
+++ b/src/vnet/tcp/tcp.c
@@ -277,19 +277,19 @@ tcp_connection_reset (tcp_connection_t * tc)
       tcp_connection_cleanup (tc);
       break;
     case TCP_STATE_ESTABLISHED:
+      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);
       stream_session_reset_notify (&tc->connection);
-      /* fall through */
+      break;
     case TCP_STATE_CLOSE_WAIT:
     case TCP_STATE_FIN_WAIT_1:
     case TCP_STATE_FIN_WAIT_2:
     case TCP_STATE_CLOSING:
       tc->state = TCP_STATE_CLOSED;
       TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc);
-
-      /* Make sure all timers are cleared */
       tcp_connection_timers_reset (tc);
-
-      /* Wait for cleanup from session layer but not forever */
       tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
       break;
     case TCP_STATE_CLOSED:
@@ -326,17 +326,22 @@ tcp_connection_close (tcp_connection_t * tc)
       tc->state = TCP_STATE_FIN_WAIT_1;
       break;
     case TCP_STATE_ESTABLISHED:
-      if (!stream_session_tx_fifo_max_dequeue (&tc->connection))
+      if (!session_tx_fifo_max_dequeue (&tc->connection))
 	tcp_send_fin (tc);
       else
 	tc->flags |= TCP_CONN_FINPNDG;
       tc->state = TCP_STATE_FIN_WAIT_1;
       break;
     case TCP_STATE_CLOSE_WAIT:
-      tcp_send_fin (tc);
-      tcp_connection_timers_reset (tc);
-      tc->state = TCP_STATE_LAST_ACK;
-      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
+      if (!session_tx_fifo_max_dequeue (&tc->connection))
+	{
+	  tcp_send_fin (tc);
+	  tcp_connection_timers_reset (tc);
+	  tc->state = TCP_STATE_LAST_ACK;
+	  tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
+	}
+      else
+	tc->flags |= TCP_CONN_FINPNDG;
       break;
     case TCP_STATE_FIN_WAIT_1:
       tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
@@ -367,11 +372,9 @@ tcp_session_cleanup (u32 conn_index, u32 thread_index)
   tcp_connection_t *tc;
   tc = tcp_connection_get (conn_index, thread_index);
   tcp_connection_timers_reset (tc);
-
-  /* Wait for the session tx events to clear */
   tc->state = TCP_STATE_CLOSED;
   TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc);
-  tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
+  tcp_connection_cleanup (tc);
 }
 
 /**
diff --git a/src/vnet/tcp/tcp.h b/src/vnet/tcp/tcp.h
index 10aa721a4eb..3a31234876e 100644
--- a/src/vnet/tcp/tcp.h
+++ b/src/vnet/tcp/tcp.h
@@ -787,7 +787,7 @@ tcp_timer_is_active (tcp_connection_t * tc, tcp_timers_e timer)
 
 #define tcp_validate_txf_size(_tc, _a) 					\
   ASSERT(_tc->state != TCP_STATE_ESTABLISHED 				\
-	 || stream_session_tx_fifo_max_dequeue (&_tc->connection) >= _a)
+	 || session_tx_fifo_max_dequeue (&_tc->connection) >= _a)
 
 void
 scoreboard_remove_hole (sack_scoreboard_t * sb,
diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c
index 04612f885f2..f77d4845da2 100644
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -2474,7 +2474,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	      if (tc0->flags & TCP_CONN_FINPNDG)
 		{
 		  /* TX fifo finally drained */
-		  if (!stream_session_tx_fifo_max_dequeue (&tc0->connection))
+		  if (!session_tx_fifo_max_dequeue (&tc0->connection))
 		    tcp_send_fin (tc0);
 		}
 	      /* If FIN is ACKed */
@@ -2507,6 +2507,18 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 		  tcp_maybe_inc_counter (rcv_process, error0, 1);
 		  goto drop;
 		}
+	      if (tc0->flags & TCP_CONN_FINPNDG)
+		{
+		  /* TX fifo finally drained */
+		  if (!session_tx_fifo_max_dequeue (&tc0->connection))
+		    {
+		      tcp_send_fin (tc0);
+		      tcp_connection_timers_reset (tc0);
+		      tc0->state = TCP_STATE_LAST_ACK;
+		      tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE,
+					TCP_2MSL_TIME);
+		    }
+		}
 	      break;
 	    case TCP_STATE_CLOSING:
 	      /* In addition to the processing for the ESTABLISHED state, if
@@ -2545,13 +2557,9 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
 	      tc0->state = TCP_STATE_CLOSED;
 	      TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc0);
-	      tcp_connection_timers_reset (tc0);
-
-	      /* Don't delete the connection/session yet. Instead, wait a
-	       * reasonable amount of time until the pipes are cleared. In
-	       * particular, this makes sure that we won't have dead sessions
-	       * when processing events on the tx path */
-	      tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
+	      /* Delete the connection/session since the pipes should be
+	       * clear by now */
+	      tcp_connection_del (tc0);
 
 	      goto drop;
 
diff --git a/src/vnet/tcp/tcp_output.c b/src/vnet/tcp/tcp_output.c
index 91c0e90bb35..8a88bf80b13 100644
--- a/src/vnet/tcp/tcp_output.c
+++ b/src/vnet/tcp/tcp_output.c
@@ -390,6 +390,7 @@ tcp_make_options (tcp_connection_t * tc, tcp_options_t * opts,
     case TCP_STATE_ESTABLISHED:
     case TCP_STATE_FIN_WAIT_1:
     case TCP_STATE_CLOSED:
+    case TCP_STATE_CLOSE_WAIT:
       return tcp_make_established_options (tc, opts);
     case TCP_STATE_SYN_RCVD:
       return tcp_make_synack_options (tc, opts);
@@ -1213,7 +1214,7 @@ tcp_prepare_retransmit_segment (tcp_connection_t * tc, u32 offset,
   /*
    * Make sure we can retransmit something
    */
-  available_bytes = stream_session_tx_fifo_max_dequeue (&tc->connection);
+  available_bytes = session_tx_fifo_max_dequeue (&tc->connection);
   ASSERT (available_bytes >= offset);
   available_bytes -= offset;
   if (!available_bytes)
@@ -1554,7 +1555,7 @@ tcp_timer_persist_handler (u32 index)
       || tc->snd_wnd > tc->snd_mss || tcp_in_recovery (tc))
     return;
 
-  available_bytes = stream_session_tx_fifo_max_dequeue (&tc->connection);
+  available_bytes = session_tx_fifo_max_dequeue (&tc->connection);
   offset = tc->snd_una_max - tc->snd_una;
 
   /* Reprogram persist if no new bytes available to send. We may have data