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>

4 files changed, 97 insertions, 129 deletions

diff --git a/src/vnet/sctp/sctp.c b/src/vnet/sctp/sctp.c
index 61c0252b1ba..8fc5d9b55b2 100644
--- 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 60a195f47c1..bc974d948de 100644
--- 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 8df95642ce4..3f7ab4b8f8a 100644
--- 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 7657d0cd1e9..bb8332f5732 100644
--- 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;