tcp: separate active and passive establish timers 15/16615/7
authorFlorin Coras <fcoras@cisco.com>
Tue, 25 Dec 2018 00:54:34 +0000 (16:54 -0800)
committerDamjan Marion <dmarion@me.com>
Wed, 26 Dec 2018 15:45:27 +0000 (15:45 +0000)
Change-Id: Ia2241e963cf45765d8d17c65eea781edbf74d4f9
Signed-off-by: Florin Coras <fcoras@cisco.com>
src/vnet/tcp/tcp.c
src/vnet/tcp/tcp.h
src/vnet/tcp/tcp_debug.h
src/vnet/tcp/tcp_input.c
src/vnet/tcp/tcp_output.c

index 8c8df00..4a63f3b 100644 (file)
@@ -172,7 +172,7 @@ tcp_half_open_connection_cleanup (tcp_connection_t * tc)
   /* Make sure this is the owning thread */
   if (tc->c_thread_index != vlib_get_thread_index ())
     return 1;
-  tcp_timer_reset (tc, TCP_TIMER_ESTABLISH);
+  tcp_timer_reset (tc, TCP_TIMER_ESTABLISH_AO);
   tcp_timer_reset (tc, TCP_TIMER_RETRANSMIT_SYN);
   tcp_half_open_connection_del (tc);
   return 0;
@@ -308,6 +308,8 @@ tcp_connection_reset (tcp_connection_t * tc)
       break;
     case TCP_STATE_CLOSED:
       break;
+    default:
+      TCP_DBG ("reset state: %u", tc->state);
     }
 }
 
@@ -333,7 +335,9 @@ tcp_connection_close (tcp_connection_t * tc)
   switch (tc->state)
     {
     case TCP_STATE_SYN_SENT:
-      /* Do nothing. Establish timer will pop and cleanup the connection */
+      /* Try to cleanup. If not on the right thread, mark as half-open done.
+       * Connection will be cleaned up when establish timer pops */
+      tcp_connection_cleanup (tc);
       break;
     case TCP_STATE_SYN_RCVD:
       tcp_connection_timers_reset (tc);
@@ -349,7 +353,8 @@ tcp_connection_close (tcp_connection_t * tc)
       tcp_connection_set_state (tc, 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);
+      ASSERT (tc->timers[TCP_TIMER_WAITCLOSE] == TCP_TIMER_HANDLE_INVALID);
+      tcp_timer_set (tc, TCP_TIMER_WAITCLOSE, TCP_FINWAIT1_TIME);
       break;
     case TCP_STATE_CLOSE_WAIT:
       if (!session_tx_fifo_max_dequeue (&tc->connection))
@@ -367,17 +372,14 @@ tcp_connection_close (tcp_connection_t * tc)
       break;
     case TCP_STATE_CLOSED:
       tcp_connection_timers_reset (tc);
+      /* Delete connection but instead of doing it now wait until next
+       * dispatch cycle to give the session layer a chance to clear
+       * unhandled events */
+      tcp_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
       break;
     default:
       TCP_DBG ("state: %u", tc->state);
     }
-
-  /* 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_timer_update (tc, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
 }
 
 static void
@@ -393,9 +395,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);
-  tc->state = TCP_STATE_CLOSED;
-  TCP_EVT_DBG (TCP_EVT_STATE_CHANGE, tc);
+  tcp_connection_set_state (tc, TCP_STATE_CLOSED);
   tcp_connection_cleanup (tc);
 }
 
@@ -1217,39 +1217,45 @@ tcp_timer_establish_handler (u32 conn_index)
 {
   tcp_connection_t *tc;
 
-  tc = tcp_half_open_connection_get (conn_index);
-  if (tc)
-    {
-      ASSERT (tc->state == TCP_STATE_SYN_SENT);
-      /* Notify app if we haven't tried to clean this up already */
-      if (!(tc->flags & TCP_CONN_HALF_OPEN_DONE))
-       session_stream_connect_notify (&tc->connection, 1 /* fail */ );
-    }
-  else
-    {
-      tc = tcp_connection_get (conn_index, vlib_get_thread_index ());
-      /* note: the connection may have already disappeared */
-      if (PREDICT_FALSE (tc == 0))
-       return;
-      ASSERT (tc->state == TCP_STATE_SYN_RCVD);
-      /* Start cleanup. App wasn't notified yet so use delete notify as
-       * opposed to delete to cleanup session layer state. */
-      stream_session_delete_notify (&tc->connection);
-    }
+  tc = tcp_connection_get (conn_index, vlib_get_thread_index ());
+  /* note: the connection may have already disappeared */
+  if (PREDICT_FALSE (tc == 0))
+    return;
+  ASSERT (tc->state == TCP_STATE_SYN_RCVD);
+  /* Start cleanup. App wasn't notified yet so use delete notify as
+   * opposed to delete to cleanup session layer state. */
+  stream_session_delete_notify (&tc->connection);
   tc->timers[TCP_TIMER_ESTABLISH] = TCP_TIMER_HANDLE_INVALID;
   tcp_connection_cleanup (tc);
 }
 
+static void
+tcp_timer_establish_ao_handler (u32 conn_index)
+{
+  tcp_connection_t *tc;
+
+  tc = tcp_half_open_connection_get (conn_index);
+  if (!tc)
+    return;
+
+  ASSERT (tc->state == TCP_STATE_SYN_SENT);
+  /* Notify app if we haven't tried to clean this up already */
+  if (!(tc->flags & TCP_CONN_HALF_OPEN_DONE))
+    session_stream_connect_notify (&tc->connection, 1 /* fail */ );
+
+  tc->timers[TCP_TIMER_ESTABLISH_AO] = TCP_TIMER_HANDLE_INVALID;
+  tcp_connection_cleanup (tc);
+}
+
 static void
 tcp_timer_waitclose_handler (u32 conn_index)
 {
-  u32 thread_index = vlib_get_thread_index ();
+  u32 thread_index = vlib_get_thread_index (), rto;
   tcp_connection_t *tc;
 
   tc = tcp_connection_get (conn_index, thread_index);
   if (!tc)
     return;
-
   tc->timers[TCP_TIMER_WAITCLOSE] = TCP_TIMER_HANDLE_INVALID;
 
   switch (tc->state)
@@ -1287,8 +1293,9 @@ tcp_timer_waitclose_handler (u32 conn_index)
           * 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, 1);
          tcp_timer_set (tc, TCP_TIMER_WAITCLOSE,
-                        clib_max (tc->rto * TCP_TO_TIMER_TICK, 1));
+                        clib_min (rto * TCP_TO_TIMER_TICK, TCP_2MSL_TIME));
          session_stream_close_notify (&tc->connection);
        }
       else
@@ -1321,7 +1328,8 @@ static timer_expiration_handler *timer_expiration_handlers[TCP_N_TIMERS] =
     tcp_timer_keep_handler,
     tcp_timer_waitclose_handler,
     tcp_timer_retransmit_syn_handler,
-    tcp_timer_establish_handler
+    tcp_timer_establish_handler,
+    tcp_timer_establish_ao_handler,
 };
 /* *INDENT-ON* */
 
index bd0fa9a..3848f03 100644 (file)
@@ -74,7 +74,8 @@ format_function_t format_tcp_rcv_sacks;
   _(KEEP, "KEEP")                       \
   _(WAITCLOSE, "WAIT CLOSE")            \
   _(RETRANSMIT_SYN, "RETRANSMIT SYN")   \
-  _(ESTABLISH, "ESTABLISH")
+  _(ESTABLISH, "ESTABLISH")            \
+  _(ESTABLISH_AO, "ESTABLISH_AO")      \
 
 typedef enum _tcp_timers
 {
index 5032588..b848d34 100755 (executable)
@@ -388,6 +388,49 @@ if (_tc)                                                           \
   ed->data[3] = _tc->snd_nxt - _tc->iss;                               \
   ed->data[4] = _tc->rcv_nxt - _tc->irs;                               \
 }
+
+#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...)            \
+{                                                                      \
+  tcp_connection_t *_tc;                                               \
+  if (_timer_id == TCP_TIMER_RETRANSMIT_SYN                            \
+      || _timer_id == TCP_TIMER_ESTABLISH_AO)                          \
+    {                                                                  \
+      _tc = tcp_half_open_connection_get (_tc_index);                  \
+    }                                                                  \
+  else                                                                 \
+    {                                                                  \
+      u32 _thread_index = vlib_get_thread_index ();                    \
+      _tc = tcp_connection_get (_tc_index, _thread_index);             \
+    }                                                                  \
+  ELOG_TYPE_DECLARE (_e) =                                             \
+  {                                                                    \
+    .format = "timer-pop: %s (%d)",                                    \
+    .format_args = "t4i4",                                             \
+    .n_enum_strings = 8,                                               \
+    .enum_strings = {                                                  \
+      "retransmit",                                                    \
+      "delack",                                                        \
+      "persist",                                                       \
+      "keep",                                                          \
+      "waitclose",                                                     \
+      "retransmit syn",                                                \
+      "establish",                                                     \
+      "establish-ao",                                                  \
+    },                                                                 \
+  };                                                                   \
+  if (_tc)                                                             \
+    {                                                                  \
+      DECLARE_ETD(_tc, _e, 2);                                         \
+      ed->data[0] = _timer_id;                                         \
+      ed->data[1] = _timer_id;                                         \
+    }                                                                  \
+  else                                                                 \
+    {                                                                  \
+      clib_warning ("pop %d for unexisting connection %d", _timer_id,  \
+                   _tc_index);                                         \
+    }                                                                  \
+}
+
 #else
 #define TCP_EVT_SYN_SENT_HANDLER(_tc, ...)
 #define TCP_EVT_SYNACK_SENT_HANDLER(_tc, ...)
@@ -398,6 +441,7 @@ if (_tc)                                                            \
 #define TCP_EVT_FIN_RCVD_HANDLER(_tc, ...)
 #define TCP_EVT_RST_RCVD_HANDLER(_tc, ...)
 #define TCP_EVT_STATE_CHANGE_HANDLER(_tc, ...)
+#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...)
 #endif
 
 #if TCP_DEBUG_SM > 1
@@ -542,52 +586,11 @@ if (_av > 0)                                                              \
   ed->data[4] = _tc->rcv_wnd - (_tc->rcv_nxt - _tc->rcv_las);          \
 }
 
-#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...)            \
-{                                                                      \
-  tcp_connection_t *_tc;                                               \
-  if (_timer_id == TCP_TIMER_RETRANSMIT_SYN                            \
-    || _timer_id == TCP_TIMER_ESTABLISH)                               \
-    {                                                                  \
-      _tc = tcp_half_open_connection_get (_tc_index);                  \
-    }                                                                  \
-  else                                                                 \
-    {                                                                  \
-      u32 _thread_index = vlib_get_thread_index ();                    \
-      _tc = tcp_connection_get (_tc_index, _thread_index);             \
-    }                                                                  \
-  ELOG_TYPE_DECLARE (_e) =                                             \
-  {                                                                    \
-    .format = "timer-pop: %s (%d)",                                    \
-    .format_args = "t4i4",                                             \
-    .n_enum_strings = 7,                                               \
-    .enum_strings = {                                                  \
-      "retransmit",                                                    \
-      "delack",                                                        \
-      "persist",                                                       \
-      "keep",                                                          \
-      "waitclose",                                                     \
-      "retransmit syn",                                                \
-      "establish",                                                     \
-    },                                                                 \
-  };                                                                   \
-  if (_tc)                                                             \
-    {                                                                  \
-      DECLARE_ETD(_tc, _e, 2);                                         \
-      ed->data[0] = _timer_id;                                         \
-      ed->data[1] = _timer_id;                                         \
-    }                                                                  \
-  else                                                                 \
-    {                                                                  \
-      clib_warning ("pop %d for unexisting connection %d", _timer_id,  \
-                   _tc_index);                                         \
-    }                                                                  \
-}
 #else
 #define TCP_EVT_ACK_SENT_HANDLER(_tc, ...)
 #define TCP_EVT_ACK_RCVD_HANDLER(_tc, ...)
 #define TCP_EVT_PKTIZE_HANDLER(_tc, ...)
 #define TCP_EVT_INPUT_HANDLER(_tc, _type, _len, _written, ...)
-#define TCP_EVT_TIMER_POP_HANDLER(_tc_index, _timer_id, ...)
 #endif
 
 /*
index fe1476a..3da02ac 100644 (file)
@@ -2452,7 +2452,7 @@ tcp46_syn_sent_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
       new_tc0->c_thread_index = my_thread_index;
       new_tc0->rcv_nxt = vnet_buffer (b0)->tcp.seq_end;
       new_tc0->irs = seq0;
-      new_tc0->timers[TCP_TIMER_ESTABLISH] = TCP_TIMER_HANDLE_INVALID;
+      new_tc0->timers[TCP_TIMER_ESTABLISH_AO] = TCP_TIMER_HANDLE_INVALID;
       new_tc0->timers[TCP_TIMER_RETRANSMIT_SYN] = TCP_TIMER_HANDLE_INVALID;
       new_tc0->sw_if_index = vnet_buffer (b0)->sw_if_index[VLIB_RX];
 
@@ -2775,7 +2775,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
               * to send. Enable waitclose though because we're willing to
               * wait for peer's FIN but not indefinitely. */
              tcp_connection_timers_reset (tc0);
-             tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
+             tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
 
              /* Don't try to deq the FIN acked */
              if (tc0->burst_acked > 1)
@@ -2805,7 +2805,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
                  tcp_send_fin (tc0);
                  tcp_connection_timers_reset (tc0);
                  tcp_connection_set_state (tc0, TCP_STATE_LAST_ACK);
-                 tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
+                 tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_2MSL_TIME);
                }
            }
          break;
@@ -2816,8 +2816,9 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
          if (tcp_rcv_ack (wrk, tc0, b0, tcp0, &error0))
            goto drop;
 
+         tcp_connection_timers_reset (tc0);
          tcp_connection_set_state (tc0, TCP_STATE_TIME_WAIT);
-         tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME);
+         tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME);
          goto drop;
 
          break;
@@ -2846,7 +2847,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, TCP_CLEANUP_TIME);
+         tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_CLEANUP_TIME);
 
          goto drop;
 
@@ -2931,7 +2932,7 @@ tcp46_rcv_process_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
          tc0->rcv_nxt += 1;
          tcp_connection_set_state (tc0, TCP_STATE_TIME_WAIT);
          tcp_connection_timers_reset (tc0);
-         tcp_timer_update (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME);
+         tcp_timer_set (tc0, TCP_TIMER_WAITCLOSE, TCP_TIMEWAIT_TIME);
          tcp_program_ack (wrk, tc0);
          break;
        case TCP_STATE_TIME_WAIT:
index 5f48d41..97f5b81 100644 (file)
@@ -994,7 +994,7 @@ tcp_send_syn (tcp_connection_t * tc)
    * Setup retransmit and establish timers before requesting buffer
    * such that we can return if we've ran out.
    */
-  tcp_timer_set (tc, TCP_TIMER_ESTABLISH, TCP_ESTABLISH_TIME);
+  tcp_timer_set (tc, TCP_TIMER_ESTABLISH_AO, TCP_ESTABLISH_TIME);
   tcp_timer_update (tc, TCP_TIMER_RETRANSMIT_SYN,
                    tc->rto * TCP_TO_TIMER_TICK);
 
@@ -1096,7 +1096,9 @@ tcp_send_fin (tcp_connection_t * tc)
     {
       /* Out of buffers so program fin retransmit ASAP */
       tcp_timer_update (tc, TCP_TIMER_RETRANSMIT, 1);
-      goto post_enqueue;
+      if (fin_snt)
+       tc->snd_nxt = tc->snd_una_max;
+      return;
     }
 
   tcp_retransmit_timer_force_update (tc);
@@ -1106,7 +1108,6 @@ tcp_send_fin (tcp_connection_t * tc)
   tcp_enqueue_to_output_now (wrk, b, bi, tc->c_is_ip4);
   TCP_EVT_DBG (TCP_EVT_FIN_SENT, tc);
 
-post_enqueue:
   if (!fin_snt)
     {
       tc->flags |= TCP_CONN_FINSNT;
@@ -1538,9 +1539,12 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn)
     {
       tc = tcp_connection_get (index, thread_index);
       /* Note: the connection may have been closed and pool_put */
-      if (PREDICT_FALSE (tc == 0 || tc->state < TCP_STATE_SYN_RCVD))
+      if (PREDICT_FALSE (tc == 0 || tc->state == TCP_STATE_SYN_SENT))
        return;
       tc->timers[TCP_TIMER_RETRANSMIT] = TCP_TIMER_HANDLE_INVALID;
+      /* Wait-close and retransmit could pop at the same time */
+      if (tc->state == TCP_STATE_CLOSED)
+       return;
     }
 
   if (tc->state >= TCP_STATE_ESTABLISHED)
@@ -1595,8 +1599,7 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn)
       /* Send one segment. Note that n_bytes may be zero due to buffer
        * shortfall */
       n_bytes = tcp_prepare_retransmit_segment (wrk, tc, 0, tc->snd_mss, &b);
-
-      if (n_bytes == 0)
+      if (!n_bytes)
        {
          tcp_retransmit_timer_force_update (tc);
          return;
@@ -1620,10 +1623,7 @@ tcp_timer_retransmit_handler_i (u32 index, u8 is_syn)
       if (tc->flags & TCP_CONN_HALF_OPEN_DONE)
        {
          if (tcp_half_open_connection_cleanup (tc))
-           {
-             clib_warning ("could not remove half-open connection");
-             ASSERT (0);
-           }
+           TCP_DBG ("could not remove half-open connection");
          return;
        }