tcp: updates to connection closing procedure (VPP-996)

- add separate TIME_WAIT time constant
- fix output node for TIME_WAIT acks
- ensure snd_nxt is snd_una_max after retransmitting fin
- debugging improvements

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

4 files changed, 63 insertions, 17 deletions

diff --git a/src/vnet/tcp/tcp.c b/src/vnet/tcp/tcp.c
index 38a21dbbdd6..2a705d01b4c 100644
--- a/src/vnet/tcp/tcp.c
+++ b/src/vnet/tcp/tcp.c
@@ -314,19 +314,22 @@ tcp_connection_close (tcp_connection_t * tc)
       tc->state = TCP_STATE_FIN_WAIT_1;
       break;
     case TCP_STATE_CLOSE_WAIT:
+      tcp_connection_timers_reset (tc);
       tcp_send_fin (tc);
       tc->state = TCP_STATE_LAST_ACK;
+      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
       break;
     case TCP_STATE_FIN_WAIT_1:
+      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
       break;
     default:
-      clib_warning ("state: %u", tc->state);
+      TCP_DBG ("state: %u", tc->state);
     }
 
   TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc);
 
   /* If in CLOSED and WAITCLOSE timer is not set, delete connection now */
-  if (tc->timers[TCP_TIMER_WAITCLOSE] == TCP_TIMER_HANDLE_INVALID
+  if (!tcp_timer_is_active (tc, TCP_TIMER_WAITCLOSE)
       && tc->state == TCP_STATE_CLOSED)
     tcp_connection_del (tc);
 }
@@ -344,6 +347,7 @@ 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;
@@ -748,6 +752,31 @@ format_tcp_state (u8 * s, va_list * args)
   return s;
 }
 
+const char *tcp_connection_flags_str[] = {
+#define _(sym, str) str,
+  foreach_tcp_connection_flag
+#undef _
+};
+
+u8 *
+format_tcp_connection_flags (u8 * s, va_list * args)
+{
+  tcp_connection_t *tc = va_arg (*args, tcp_connection_t *);
+  int i, last = -1;
+
+  for (i = 0; i < TCP_CONN_N_FLAG_BITS; i++)
+    if (tc->flags & (1 << i))
+      last = i;
+  for (i = 0; i < last; i++)
+    {
+      if (tc->flags & (1 << i))
+	s = format (s, "%s, ", tcp_connection_flags_str[i]);
+    }
+  if (last >= 0)
+    s = format (s, "%s", tcp_connection_flags_str[last]);
+  return s;
+}
+
 const char *tcp_conn_timers[] = {
 #define _(sym, str) str,
   foreach_tcp_timer
@@ -796,6 +825,8 @@ u8 *
 format_tcp_vars (u8 * s, va_list * args)
 {
   tcp_connection_t *tc = va_arg (*args, tcp_connection_t *);
+  s = format (s, " flags: %U timers: %U\n", format_tcp_connection_flags, tc,
+	      format_tcp_timers, tc);
   s = format (s, " snd_una %u snd_nxt %u snd_una_max %u",
 	      tc->snd_una - tc->iss, tc->snd_nxt - tc->iss,
 	      tc->snd_una_max - tc->iss);
@@ -866,7 +897,7 @@ format_tcp_connection (u8 * s, va_list * args)
     {
       s = format (s, "%-15U", format_tcp_state, tc->state);
       if (verbose > 1)
-	s = format (s, " %U\n%U", format_tcp_timers, tc, format_tcp_vars, tc);
+	s = format (s, "\n%U", format_tcp_vars, tc);
     }
 
   return s;
diff --git a/src/vnet/tcp/tcp.h b/src/vnet/tcp/tcp.h
index 259dbca1519..2a65dfae784 100644
--- a/src/vnet/tcp/tcp.h
+++ b/src/vnet/tcp/tcp.h
@@ -100,7 +100,7 @@ extern timer_expiration_handler tcp_timer_retransmit_syn_handler;
 #define TCP_SYN_RCVD_TIME	600	/* 60s */
 #define TCP_2MSL_TIME           300	/* 30s */
 #define TCP_CLOSEWAIT_TIME	20	/* 2s */
-#define TCP_TIMEWAIT_TIME	20	/* 2s */
+#define TCP_TIMEWAIT_TIME	100	/* 10s */
 #define TCP_CLEANUP_TIME	10	/* 1s Time to wait before cleanup */
 #define TCP_TIMER_PERSIST_MIN	2	/* 0.2s */
 
@@ -114,9 +114,9 @@ extern timer_expiration_handler tcp_timer_retransmit_syn_handler;
 #define foreach_tcp_connection_flag             \
   _(SNDACK, "Send ACK")                         \
   _(FINSNT, "FIN sent")				\
-  _(SENT_RCV_WND0, "Sent 0 receive window")     \
-  _(RECOVERY, "Recovery on")                    \
-  _(FAST_RECOVERY, "Fast Recovery on")		\
+  _(SENT_RCV_WND0, "Sent 0 rcv_wnd")     	\
+  _(RECOVERY, "Recovery")                    	\
+  _(FAST_RECOVERY, "Fast Recovery")		\
   _(FR_1_SMSS, "Sent 1 SMSS")			\
   _(HALF_OPEN_DONE, "Half-open completed")	\
   _(FINPNDG, "FIN pending")
diff --git a/src/vnet/tcp/tcp_input.c b/src/vnet/tcp/tcp_input.c
index 63d6fd8785b..252b0014c8c 100644
--- a/src/vnet/tcp/tcp_input.c
+++ b/src/vnet/tcp/tcp_input.c
@@ -2352,7 +2352,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	       */
 	      if (!tcp_rcv_ack_is_acceptable (tc0, b0))
 		{
-		  clib_warning ("connection not accepted");
+		  TCP_DBG ("connection not accepted");
 		  tcp_send_reset_w_pkt (tc0, b0, is_ip4);
 		  goto drop;
 		}
@@ -2431,7 +2431,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
 	      tc0->state = TCP_STATE_TIME_WAIT;
 	      TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc0);
-	      tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
+	      tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME);
 	      goto drop;
 
 	      break;
@@ -2441,11 +2441,14 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	       * delete the TCB, enter the CLOSED state, and return. */
 
 	      if (!tcp_rcv_ack_is_acceptable (tc0, b0))
-		goto drop;
+		{
+		  error0 = TCP_ERROR_ACK_INVALID;
+		  goto drop;
+		}
 
 	      tc0->snd_una = vnet_buffer (b0)->tcp.ack_number;
-	      /* Apparently our FIN was lost */
-	      if (is_fin0)
+	      /* Apparently our ACK for the peer's FIN was lost */
+	      if (is_fin0 && tc0->snd_una != tc0->snd_una_max)
 		{
 		  tcp_send_fin (tc0);
 		  goto drop;
@@ -2453,13 +2456,13 @@ 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_update (tc0, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
-	      tcp_retransmit_timer_reset (tc0);
+	      tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
 
 	      goto drop;
 
@@ -2473,7 +2476,8 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 		goto drop;
 
 	      tcp_make_ack (tc0, b0);
-	      tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
+	      next0 = tcp_next_output (is_ip4);
+	      tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME);
 
 	      goto drop;
 
diff --git a/src/vnet/tcp/tcp_output.c b/src/vnet/tcp/tcp_output.c
index a954bfa7b4e..6482e894348 100644
--- a/src/vnet/tcp/tcp_output.c
+++ b/src/vnet/tcp/tcp_output.c
@@ -1055,7 +1055,6 @@ tcp_send_fin (tcp_connection_t * tc)
   u32 bi;
   u8 fin_snt = 0;
 
-
   if (PREDICT_FALSE (tcp_get_free_buffer_index (tm, &bi)))
     return;
   b = vlib_get_buffer (vm, bi);
@@ -1072,6 +1071,10 @@ tcp_send_fin (tcp_connection_t * tc)
       tc->snd_una_max += 1;
       tc->snd_nxt = tc->snd_una_max;
     }
+  else
+    {
+      tc->snd_nxt = tc->snd_una_max;
+    }
   tcp_retransmit_timer_force_update (tc);
   TCP_EVT_DBG (TCP_EVT_FIN_SENT, tc);
 }
@@ -1381,6 +1384,13 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn)
 	  return;
 	}
 
+      /* Shouldn't be here */
+      if (tc->snd_una == tc->snd_una_max)
+	{
+	  tcp_recovery_off (tc);
+	  return;
+	}
+
       /* We're not in recovery so make sure rto_boff is 0 */
       if (!tcp_in_recovery (tc) && tc->rto_boff > 0)
 	{
@@ -1485,7 +1495,8 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn)
   else
     {
       ASSERT (tc->state == TCP_STATE_CLOSED);
-      TCP_DBG ("connection state: %d", tc->state);
+      if (CLIB_DEBUG)
+	TCP_DBG ("connection state: %U", format_tcp_connection, tc, 2);
       return;
     }
 }