SCTP: refactoring

This patch takes care of some refactoring, including the initialization
of the timestamp to calculate the RTO, the output state-machine
validation which can be enabled (disabled by default) when debugging and
some clean-up of unused fields.
It also addresses the requirement of Karn's algorithm when computing the
RTO.

Change-Id: I6b875152369bff23cad085708cec1f7e1151cfa8
Signed-off-by: Marco Varlese <marco.varlese@suse.com>

diff --git a/src/vnet/sctp/sctp.c b/src/vnet/sctp/sctp.c
index 61c0252..8fc5d9b 100644 (file)
--- a/src/vnet/sctp/sctp.c
+++ b/src/vnet/sctp/sctp.c
@@ -470,10 +470,10 @@ void
 sctp_session_close (u32 conn_index, u32 thread_index)
 {
   ASSERT (thread_index == 0);
-
   sctp_connection_t *sctp_conn;
   sctp_conn = sctp_connection_get (conn_index, thread_index);
-  sctp_connection_close (sctp_conn);
+  if (sctp_conn != NULL)
+    sctp_connection_close (sctp_conn);
 }
 
 void
@@ -481,10 +481,13 @@ sctp_session_cleanup (u32 conn_index, u32 thread_index)
 {
   sctp_connection_t *sctp_conn;
   sctp_conn = sctp_connection_get (conn_index, thread_index);
-  sctp_connection_timers_reset (sctp_conn);
 
-  /* Wait for the session tx events to clear */
-  sctp_conn->state = SCTP_STATE_CLOSED;
+  if (sctp_conn != NULL)
+    {
+      sctp_connection_timers_reset (sctp_conn);
+      /* Wait for the session tx events to clear */
+      sctp_conn->state = SCTP_STATE_CLOSED;
+    }
 }
 
 /**

diff --git a/src/vnet/sctp/sctp.h b/src/vnet/sctp/sctp.h
index 60a195f..bc974d9 100644 (file)
--- a/src/vnet/sctp/sctp.h
+++ b/src/vnet/sctp/sctp.h
@@ -128,6 +128,8 @@ typedef struct _sctp_sub_connection
   u8 unacknowledged_hb;        /**< Used to track how many unacknowledged heartbeats we had;
                                  If more than Max.Retransmit then connetion is considered unreachable. */
 
+  u8 is_retransmitting;        /**< A flag (0 = no, 1 = yes) indicating whether the connection is retransmitting a previous packet */
+
 } sctp_sub_connection_t;
 
 typedef struct
@@ -284,20 +286,6 @@ void sctp_prepare_heartbeat_ack_chunk (sctp_connection_t * sctp_conn,
 
 u16 sctp_check_outstanding_data_chunks (sctp_connection_t * tc);
 
-#define SCTP_TICK 0.001                        /**< SCTP tick period (s) */
-#define STHZ (u32) (1/SCTP_TICK)               /**< SCTP tick frequency */
-#define SCTP_TSTAMP_RESOLUTION SCTP_TICK       /**< Time stamp resolution */
-#define SCTP_PAWS_IDLE 24 * 24 * 60 * 60 * THZ /**< 24 days */
-#define SCTP_FIB_RECHECK_PERIOD        1 * THZ /**< Recheck every 1s */
-#define SCTP_MAX_OPTION_SPACE 40
-
-#define SCTP_DUPACK_THRESHOLD  3
-#define SCTP_MAX_RX_FIFO_SIZE  4 << 20
-#define SCTP_MIN_RX_FIFO_SIZE  4 << 10
-#define SCTP_IW_N_SEGMENTS     10
-#define SCTP_ALWAYS_ACK                1       /**< On/off delayed acks */
-#define SCTP_USE_SACKS         1       /**< Disable only for testing */
-
 #define IP_PROTOCOL_SCTP       132
 
 /** SSCTP FSM state definitions as per RFC4960. */
@@ -408,6 +396,7 @@ sctp_optparam_type_to_string (u8 type)
 
 #define SCTP_TICK 0.001                        /**< SCTP tick period (s) */
 #define SHZ (u32) (1/SCTP_TICK)                /**< SCTP tick frequency */
+#define SCTP_TSTAMP_RESOLUTION SCTP_TICK       /**< Time stamp resolution */
 
 /* As per RFC4960, page 83 */
 #define SCTP_RTO_INIT 3 * SHZ  /* 3 seconds */

diff --git a/src/vnet/sctp/sctp_input.c b/src/vnet/sctp/sctp_input.c
index 8df9564..3f7ab4b 100644 (file)
--- a/src/vnet/sctp/sctp_input.c
+++ b/src/vnet/sctp/sctp_input.c
@@ -645,14 +645,14 @@ sctp_session_enqueue_data (sctp_connection_t * sctp_conn, vlib_buffer_t * b,
 }
 
 always_inline u8
-sctp_is_sack_delayable (sctp_connection_t * sctp_conn, u8 gapping)
+sctp_is_sack_delayable (sctp_connection_t * sctp_conn, u8 is_gapping)
 {
-  if (gapping != 0)
+  if (is_gapping != 0)
     {
       SCTP_CONN_TRACKING_DBG
        ("gapping != 0: CONN_INDEX = %u, sctp_conn->ack_state = %u",
         sctp_conn->sub_conn[idx].connection.c_index, sctp_conn->ack_state);
-      return 1;
+      return 0;
     }
 
   if (sctp_conn->ack_state >= MAX_ENQUEABLE_SACKS)
@@ -660,12 +660,12 @@ sctp_is_sack_delayable (sctp_connection_t * sctp_conn, u8 gapping)
       SCTP_CONN_TRACKING_DBG
        ("sctp_conn->ack_state >= MAX_ENQUEABLE_SACKS: CONN_INDEX = %u, sctp_conn->ack_state = %u",
         sctp_conn->sub_conn[idx].connection.c_index, sctp_conn->ack_state);
-      return 1;
+      return 0;
     }
 
   sctp_conn->ack_state += 1;
 
-  return 0;
+  return 1;
 }
 
 always_inline void
@@ -748,7 +748,7 @@ sctp_handle_data (sctp_payload_data_chunk_t * sctp_data_chunk,
 
   SCTP_ADV_DBG ("POINTER_WITH_DATA = %p", b->data);
 
-  if (sctp_is_sack_delayable (sctp_conn, is_gapping) != 0)
+  if (!sctp_is_sack_delayable (sctp_conn, is_gapping))
     sctp_prepare_sack_chunk (sctp_conn, b);
 
   return error;
@@ -1152,6 +1152,7 @@ sctp_handle_shutdown_ack (sctp_header_t * sctp_hdr,
    */
   sctp_timer_reset (sctp_conn, MAIN_SCTP_SUB_CONN_IDX,
                    SCTP_TIMER_T2_SHUTDOWN);
+
   sctp_send_shutdown_complete (sctp_conn);
 
   *next0 = sctp_next_output (sctp_conn->sub_conn[idx].c_is_ip4);

diff --git a/src/vnet/sctp/sctp_output.c b/src/vnet/sctp/sctp_output.c
index 7657d0c..bb8332f 100644 (file)
--- a/src/vnet/sctp/sctp_output.c
+++ b/src/vnet/sctp/sctp_output.c
@@ -554,9 +554,6 @@ sctp_prepare_cookie_ack_chunk (sctp_connection_t * sctp_conn,
   vnet_sctp_set_chunk_type (&cookie_ack_chunk->chunk_hdr, COOKIE_ACK);
   vnet_sctp_set_chunk_length (&cookie_ack_chunk->chunk_hdr, chunk_len);
 
-  /* Measure RTT with this */
-  sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
-
   vnet_buffer (b)->sctp.connection_index =
     sctp_conn->sub_conn[idx].connection.c_index;
 }
@@ -591,9 +588,6 @@ sctp_prepare_cookie_echo_chunk (sctp_connection_t * sctp_conn,
   clib_memcpy (&(cookie_echo_chunk->cookie), sc,
               sizeof (sctp_state_cookie_param_t));
 
-  /* Measure RTT with this */
-  sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
-
   vnet_buffer (b)->sctp.connection_index =
     sctp_conn->sub_conn[idx].connection.c_index;
 }
@@ -736,9 +730,6 @@ sctp_prepare_initack_chunk (sctp_connection_t * sctp_conn, vlib_buffer_t * b,
 
   sctp_conn->local_tag = init_ack_chunk->initiate_tag;
 
-  /* Measure RTT with this */
-  sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
-
   vnet_buffer (b)->sctp.connection_index =
     sctp_conn->sub_conn[idx].connection.c_index;
 }
@@ -802,9 +793,6 @@ sctp_send_shutdown (sctp_connection_t * sctp_conn)
   u8 idx = sctp_pick_conn_idx_on_chunk (SHUTDOWN);
   sctp_enqueue_to_output_now (vm, b, bi,
                              sctp_conn->sub_conn[idx].connection.is_ip4);
-
-  /* Measure RTT with this */
-  sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
 }
 
 /**
@@ -852,11 +840,6 @@ sctp_send_shutdown_ack (sctp_connection_t * sctp_conn, vlib_buffer_t * b)
   sctp_reuse_buffer (vm, b);
 
   sctp_prepare_shutdown_ack_chunk (sctp_conn, b);
-
-  u8 idx = sctp_pick_conn_idx_on_chunk (SHUTDOWN_ACK);
-
-  /* Measure RTT with this */
-  sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
 }
 
 /**
@@ -1041,8 +1024,6 @@ sctp_send_shutdown_complete (sctp_connection_t * sctp_conn)
   u8 idx = sctp_pick_conn_idx_on_chunk (SHUTDOWN_COMPLETE);
   sctp_enqueue_to_output (vm, b, bi,
                          sctp_conn->sub_conn[idx].connection.is_ip4);
-
-  sctp_conn->state = SCTP_STATE_CLOSED;
 }
 
 
@@ -1069,15 +1050,15 @@ sctp_send_init (sctp_connection_t * sctp_conn)
   sctp_push_ip_hdr (tm, &sctp_conn->sub_conn[idx], b);
   sctp_enqueue_to_ip_lookup (vm, b, bi, sctp_conn->sub_conn[idx].c_is_ip4);
 
-  /* Measure RTT with this */
-  sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
-
   /* Start the T1_INIT timer */
   sctp_timer_set (sctp_conn, idx, SCTP_TIMER_T1_INIT,
                  sctp_conn->sub_conn[idx].RTO);
 
   /* Change state to COOKIE_WAIT */
   sctp_conn->state = SCTP_STATE_COOKIE_WAIT;
+
+  /* Measure RTT with this */
+  sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
 }
 
 /**
@@ -1156,18 +1137,51 @@ sctp_push_header (transport_connection_t * trans_conn, vlib_buffer_t * b)
 
   sctp_push_hdr_i (sctp_conn, idx, b, SCTP_STATE_ESTABLISHED);
 
-  if (sctp_conn->sub_conn[idx].RTO_pending == 0)
-    {
-      sctp_conn->sub_conn[idx].RTO_pending = 1;
-      sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
-    }
-
   sctp_trajectory_add_start (b0, 3);
 
   return 0;
 
 }
 
+always_inline u8
+sctp_validate_output_state_machine (sctp_connection_t * sctp_conn,
+                                   u8 chunk_type)
+{
+  u8 result = 0;
+  switch (sctp_conn->state)
+    {
+    case SCTP_STATE_CLOSED:
+      if (chunk_type != INIT && chunk_type != INIT_ACK)
+       result = 1;
+      break;
+    case SCTP_STATE_ESTABLISHED:
+      if (chunk_type != DATA && chunk_type != HEARTBEAT &&
+         chunk_type != HEARTBEAT_ACK && chunk_type != SACK &&
+         chunk_type != COOKIE_ACK && chunk_type != SHUTDOWN)
+       result = 1;
+      break;
+    case SCTP_STATE_COOKIE_WAIT:
+      if (chunk_type != COOKIE_ECHO)
+       result = 1;
+      break;
+    case SCTP_STATE_SHUTDOWN_SENT:
+      if (chunk_type != SHUTDOWN_COMPLETE)
+       result = 1;
+      break;
+    case SCTP_STATE_SHUTDOWN_RECEIVED:
+      if (chunk_type != SHUTDOWN_ACK)
+       result = 1;
+      break;
+    }
+  return result;
+}
+
+always_inline u8
+sctp_is_retransmitting (sctp_connection_t * sctp_conn, u8 idx)
+{
+  return sctp_conn->sub_conn[idx].is_retransmitting;
+}
+
 always_inline uword
 sctp46_output_inline (vlib_main_t * vm,
                      vlib_node_runtime_t * node,
@@ -1322,93 +1336,49 @@ sctp46_output_inline (vlib_main_t * vm,
             sctp_chunk_to_string (chunk_type), full_hdr->hdr.src_port,
             full_hdr->hdr.dst_port);
 
-         if (chunk_type == DATA)
-           SCTP_ADV_DBG_OUTPUT ("PACKET_LENGTH = %u", packet_length);
-
          /* Let's make sure the state-machine does not send anything crazy */
-         switch (sctp_conn->state)
+#if SCTP_DEBUG_STATE_MACHINE
+         if (sctp_validate_output_state_machine (sctp_conn, chunk_type) != 0)
            {
-           case SCTP_STATE_CLOSED:
-             {
-               if (chunk_type != INIT && chunk_type != INIT_ACK)
-                 {
-                   SCTP_DBG_STATE_MACHINE
-                     ("Sending the wrong chunk (%s) based on state-machine status (%s)",
-                      sctp_chunk_to_string (chunk_type),
-                      sctp_state_to_string (sctp_conn->state));
-
-                   error0 = SCTP_ERROR_UNKOWN_CHUNK;
-                   next0 = SCTP_OUTPUT_NEXT_DROP;
-                   goto done;
-                 }
-               break;
-             }
-           case SCTP_STATE_ESTABLISHED:
-             if (chunk_type != DATA && chunk_type != HEARTBEAT &&
-                 chunk_type != HEARTBEAT_ACK && chunk_type != SACK &&
-                 chunk_type != COOKIE_ACK && chunk_type != SHUTDOWN)
-               {
-                 SCTP_DBG_STATE_MACHINE
-                   ("Sending the wrong chunk (%s) based on state-machine status (%s)",
-                    sctp_chunk_to_string (chunk_type),
-                    sctp_state_to_string (sctp_conn->state));
-
-                 error0 = SCTP_ERROR_UNKOWN_CHUNK;
-                 next0 = SCTP_OUTPUT_NEXT_DROP;
-                 goto done;
-               }
-             break;
-           case SCTP_STATE_COOKIE_WAIT:
-             if (chunk_type != COOKIE_ECHO)
-               {
-                 SCTP_DBG_STATE_MACHINE
-                   ("Sending the wrong chunk (%s) based on state-machine status (%s)",
-                    sctp_chunk_to_string (chunk_type),
-                    sctp_state_to_string (sctp_conn->state));
-
-                 error0 = SCTP_ERROR_UNKOWN_CHUNK;
-                 next0 = SCTP_OUTPUT_NEXT_DROP;
-                 goto done;
-               }
-             /* Change state */
-             sctp_conn->state = SCTP_STATE_COOKIE_ECHOED;
-             break;
-           case SCTP_STATE_SHUTDOWN_SENT:
-             if (chunk_type != SHUTDOWN_COMPLETE)
-               {
-                 SCTP_DBG_STATE_MACHINE
-                   ("Sending the wrong chunk (%s) based on state-machine status (%s)",
-                    sctp_chunk_to_string (chunk_type),
-                    sctp_state_to_string (sctp_conn->state));
-
-                 error0 = SCTP_ERROR_UNKOWN_CHUNK;
-                 next0 = SCTP_OUTPUT_NEXT_DROP;
-                 goto done;
-               }
-           case SCTP_STATE_SHUTDOWN_RECEIVED:
-             if (chunk_type != SHUTDOWN_ACK)
-               {
-                 SCTP_DBG_STATE_MACHINE
-                   ("Sending the wrong chunk (%s) based on state-machine status (%s)",
-                    sctp_chunk_to_string (chunk_type),
-                    sctp_state_to_string (sctp_conn->state));
-
-                 error0 = SCTP_ERROR_UNKOWN_CHUNK;
-                 next0 = SCTP_OUTPUT_NEXT_DROP;
-                 goto done;
-               }
-           default:
              SCTP_DBG_STATE_MACHINE
-               ("Sending chunk (%s) based on state-machine status (%s)",
+               ("Sending the wrong chunk (%s) based on state-machine status (%s)",
                 sctp_chunk_to_string (chunk_type),
                 sctp_state_to_string (sctp_conn->state));
-             break;
+
+             error0 = SCTP_ERROR_UNKOWN_CHUNK;
+             next0 = SCTP_OUTPUT_NEXT_DROP;
+             goto done;
+           }
+#endif
+
+         /* Karn's algorithm: RTT measurements MUST NOT be made using
+          * packets that were retransmitted
+          */
+         if (!sctp_is_retransmitting (sctp_conn, idx))
+           {
+             /* Measure RTT with this */
+             if (chunk_type == DATA
+                 && sctp_conn->sub_conn[idx].RTO_pending == 0)
+               {
+                 sctp_conn->sub_conn[idx].RTO_pending = 1;
+                 sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
+               }
+             else
+               sctp_conn->sub_conn[idx].rtt_ts = sctp_time_now ();
            }
 
+         /* Let's take care of TIMERS */
          switch (chunk_type)
            {
+           case COOKIE_ECHO:
+             {
+               sctp_conn->state = SCTP_STATE_COOKIE_ECHOED;
+               break;
+             }
            case DATA:
              {
+               SCTP_ADV_DBG_OUTPUT ("PACKET_LENGTH = %u", packet_length);
+
                sctp_timer_update (sctp_conn, idx, SCTP_TIMER_T3_RXTX,
                                   sctp_conn->sub_conn[idx].RTO);
                break;
@@ -1429,6 +1399,11 @@ sctp46_output_inline (vlib_main_t * vm,
                sctp_conn->state = SCTP_STATE_SHUTDOWN_ACK_SENT;
                break;
              }
+           case SHUTDOWN_COMPLETE:
+             {
+               sctp_conn->state = SCTP_STATE_CLOSED;
+               break;
+             }
            }
 
          vnet_buffer (b0)->sw_if_index[VLIB_RX] = 0;