SCTP: fix corrupted buffers seen in output node

The issue observed in the output-node was actually
caused by one of the input-node pushing buffers to
the output node when not required. That is the case
with the parsing/handling of incoming packets like
the COOKIE_ACK, HEARTBEAT_ACK, DATA, SACK which do
not require a response to be sent to the other peer.
In all the mentioned cases the packets (buffers) need
to be consumed and dropped instead of heading to the
output-node.

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

4 files changed, 62 insertions, 44 deletions

diff --git a/src/vnet/sctp/sctp.h b/src/vnet/sctp/sctp.h
index bc974d948de..25fae37b6f7 100644
--- a/src/vnet/sctp/sctp.h
+++ b/src/vnet/sctp/sctp.h
@@ -251,7 +251,8 @@ void sctp_send_init (sctp_connection_t * sctp_conn);
 void sctp_send_shutdown (sctp_connection_t * sctp_conn);
 void sctp_send_shutdown_ack (sctp_connection_t * sctp_conn,
 			     vlib_buffer_t * b);
-void sctp_send_shutdown_complete (sctp_connection_t * sctp_conn);
+void sctp_send_shutdown_complete (sctp_connection_t * sctp_conn,
+				  vlib_buffer_t * b0);
 void sctp_send_heartbeat (sctp_connection_t * sctp_conn);
 void sctp_flush_frame_to_output (vlib_main_t * vm, u8 thread_index,
 				 u8 is_ip4);
@@ -718,6 +719,9 @@ sctp_pick_conn_idx_on_chunk (sctp_chunk_type chunk_type)
     case CWR:
     case SHUTDOWN_COMPLETE:
       idx = MAIN_SCTP_SUB_CONN_IDX;
+      break;
+    default:
+      idx = 0;
     }
   return idx;
 }
diff --git a/src/vnet/sctp/sctp_input.c b/src/vnet/sctp/sctp_input.c
index 3f7ab4b8f8a..8f4b043f297 100644
--- a/src/vnet/sctp/sctp_input.c
+++ b/src/vnet/sctp/sctp_input.c
@@ -27,7 +27,8 @@ static char *sctp_error_strings[] = {
 
 /* All SCTP nodes have the same outgoing arcs */
 #define foreach_sctp_state_next                  \
-  _ (DROP, "error-drop")                        \
+  _ (DROP4, "ip4-drop")                         \
+  _ (DROP6, "ip6-drop")                         \
   _ (SCTP4_OUTPUT, "sctp4-output")                \
   _ (SCTP6_OUTPUT, "sctp6-output")
 
@@ -233,6 +234,8 @@ typedef struct
 #define sctp_next_output(is_ip4) (is_ip4 ? SCTP_NEXT_SCTP4_OUTPUT          \
                                         : SCTP_NEXT_SCTP6_OUTPUT)
 
+#define sctp_next_drop(is_ip4) (is_ip4 ? SCTP_NEXT_DROP4                  \
+                                      : SCTP_NEXT_DROP6)
 
 void
 sctp_set_rx_trace_data (sctp_rx_trace_t * rx_trace,
@@ -744,7 +747,7 @@ sctp_handle_data (sctp_payload_data_chunk_t * sctp_data_chunk,
     }
   sctp_conn->last_rcvd_tsn = tsn;
 
-  *next0 = sctp_next_output (sctp_conn->sub_conn[idx].c_is_ip4);
+  *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4);
 
   SCTP_ADV_DBG ("POINTER_WITH_DATA = %p", b->data);
 
@@ -816,7 +819,7 @@ sctp_handle_cookie_ack (sctp_header_t * sctp_hdr,
   sctp_timer_reset (sctp_conn, idx, SCTP_TIMER_T1_COOKIE);
   /* Change state */
   sctp_conn->state = SCTP_STATE_ESTABLISHED;
-  *next0 = sctp_next_output (sctp_conn->sub_conn[idx].c_is_ip4);
+  *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4);
 
   sctp_timer_set (sctp_conn, idx, SCTP_TIMER_T4_HEARTBEAT,
 		  sctp_conn->sub_conn[idx].RTO);
@@ -977,7 +980,7 @@ sctp46_rcv_phase_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	       */
 	    default:
 	      error0 = SCTP_ERROR_UNKOWN_CHUNK;
-	      next0 = SCTP_NEXT_DROP;
+	      next0 = sctp_next_drop (is_ip4);
 	      goto drop;
 	    }
 
@@ -985,7 +988,7 @@ sctp46_rcv_phase_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	    {
 	      clib_warning ("error while parsing chunk");
 	      sctp_connection_cleanup (sctp_conn);
-	      next0 = SCTP_NEXT_DROP;
+	      next0 = sctp_next_drop (is_ip4);
 	      goto drop;
 	    }
 
@@ -1153,7 +1156,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);
+  sctp_send_shutdown_complete (sctp_conn, b0);
 
   *next0 = sctp_next_output (sctp_conn->sub_conn[idx].c_is_ip4);
 
@@ -1190,7 +1193,7 @@ sctp_handle_shutdown_complete (sctp_header_t * sctp_hdr,
   stream_session_disconnect_notify (&sctp_conn->sub_conn
 				    [MAIN_SCTP_SUB_CONN_IDX].connection);
 
-  *next0 = sctp_next_output (sctp_conn->sub_conn[idx].c_is_ip4);
+  *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4);
 
   return SCTP_ERROR_NONE;
 }
@@ -1307,7 +1310,7 @@ sctp46_shutdown_phase_inline (vlib_main_t * vm,
 	       */
 	    default:
 	      error0 = SCTP_ERROR_UNKOWN_CHUNK;
-	      next0 = SCTP_NEXT_DROP;
+	      next0 = sctp_next_drop (is_ip4);
 	      goto drop;
 	    }
 
@@ -1315,7 +1318,7 @@ sctp46_shutdown_phase_inline (vlib_main_t * vm,
 	    {
 	      clib_warning ("error while parsing chunk");
 	      sctp_connection_cleanup (sctp_conn);
-	      next0 = SCTP_NEXT_DROP;
+	      next0 = sctp_next_drop (is_ip4);
 	      goto drop;
 	    }
 
@@ -1431,7 +1434,7 @@ sctp_handle_sack (sctp_selective_ack_chunk_t * sack_chunk,
 
   sctp_conn->sub_conn[idx].RTO_pending = 0;
 
-  *next0 = sctp_next_output (sctp_conn->sub_conn[idx].connection.is_ip4);
+  *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4);
 
   return SCTP_ERROR_NONE;
 }
@@ -1464,7 +1467,7 @@ sctp_handle_heartbeat_ack (sctp_hb_ack_chunk_t * sctp_hb_ack_chunk,
   sctp_timer_update (sctp_conn, idx, SCTP_TIMER_T4_HEARTBEAT,
 		     sctp_conn->sub_conn[idx].RTO);
 
-  *next0 = sctp_next_output (sctp_conn->sub_conn[idx].connection.is_ip4);
+  *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4);
 
   return SCTP_ERROR_NONE;
 }
@@ -1591,7 +1594,7 @@ sctp46_listen_process_inline (vlib_main_t * vm,
 		 connection.c_index, sctp_chunk_to_string (chunk_type));
 
 	      error0 = SCTP_ERROR_UNKOWN_CHUNK;
-	      next0 = SCTP_NEXT_DROP;
+	      next0 = sctp_next_drop (is_ip4);
 	      goto drop;
 	    }
 
@@ -1699,7 +1702,8 @@ sctp46_established_phase_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	  ip4_header_t *ip4_hdr = 0;
 	  ip6_header_t *ip6_hdr = 0;
 	  sctp_connection_t *sctp_conn;
-	  u16 error0 = SCTP_ERROR_NONE, next0 = SCTP_ESTABLISHED_PHASE_N_NEXT;
+	  u16 error0 = SCTP_ERROR_ENQUEUED, next0 =
+	    SCTP_ESTABLISHED_PHASE_N_NEXT;
 	  u8 idx;
 
 	  bi0 = from[0];
@@ -1807,7 +1811,7 @@ sctp46_established_phase_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 	       */
 	    default:
 	      error0 = SCTP_ERROR_UNKOWN_CHUNK;
-	      next0 = SCTP_NEXT_DROP;
+	      next0 = sctp_next_drop (is_ip4);
 	      goto done;
 	    }
 
@@ -2068,7 +2072,16 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node,
 	  sctp_conn = sctp_get_connection_from_transport (trans_conn);
 	  vnet_sctp_common_hdr_params_net_to_host (sctp_chunk_hdr);
 
-	  u8 type = vnet_sctp_get_chunk_type (sctp_chunk_hdr);
+	  u8 chunk_type = vnet_sctp_get_chunk_type (sctp_chunk_hdr);
+	  if (chunk_type >= UNKNOWN)
+	    {
+	      clib_warning
+		("Received an unrecognized chunk... something is really bad.");
+	      error0 = SCTP_ERROR_UNKOWN_CHUNK;
+	      next0 = SCTP_INPUT_NEXT_DROP;
+	      goto done;
+	    }
+
 #if SCTP_DEBUG_STATE_MACHINE
 	  u8 idx = sctp_pick_conn_idx_on_state (sctp_conn->state);
 #endif
@@ -2083,8 +2096,8 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node,
 	      vnet_buffer (b0)->sctp.data_offset = n_advance_bytes0;
 	      vnet_buffer (b0)->sctp.data_len = n_data_bytes0;
 
-	      next0 = tm->dispatch_table[sctp_conn->state][type].next;
-	      error0 = tm->dispatch_table[sctp_conn->state][type].error;
+	      next0 = tm->dispatch_table[sctp_conn->state][chunk_type].next;
+	      error0 = tm->dispatch_table[sctp_conn->state][chunk_type].error;
 
 	      SCTP_DBG_STATE_MACHINE ("CONNECTION_INDEX = %u: "
 				      "CURRENT_CONNECTION_STATE = %s,"
@@ -2096,7 +2109,7 @@ sctp46_input_dispatcher (vlib_main_t * vm, vlib_node_runtime_t * node,
 				      sctp_chunk_to_string (type),
 				      phase_to_string (next0));
 
-	      if (type == DATA)
+	      if (chunk_type == DATA)
 		SCTP_ADV_DBG ("n_advance_bytes0 = %u, n_data_bytes0 = %u",
 			      n_advance_bytes0, n_data_bytes0);
 
diff --git a/src/vnet/sctp/sctp_output.c b/src/vnet/sctp/sctp_output.c
index bb8332f5732..6012e50b908 100644
--- a/src/vnet/sctp/sctp_output.c
+++ b/src/vnet/sctp/sctp_output.c
@@ -352,13 +352,6 @@ sctp_enqueue_to_output_i (vlib_main_t * vm, vlib_buffer_t * b, u32 bi,
 }
 
 always_inline void
-sctp_enqueue_to_output (vlib_main_t * vm, vlib_buffer_t * b, u32 bi,
-			u8 is_ip4)
-{
-  sctp_enqueue_to_output_i (vm, b, bi, is_ip4, 0);
-}
-
-always_inline void
 sctp_enqueue_to_output_now (vlib_main_t * vm, vlib_buffer_t * b, u32 bi,
 			    u8 is_ip4)
 {
@@ -1007,23 +1000,19 @@ sctp_prepare_shutdown_complete_chunk (sctp_connection_t * sctp_conn,
 }
 
 void
-sctp_send_shutdown_complete (sctp_connection_t * sctp_conn)
+sctp_send_shutdown_complete (sctp_connection_t * sctp_conn,
+			     vlib_buffer_t * b0)
 {
-  vlib_buffer_t *b;
-  u32 bi;
-  sctp_main_t *tm = vnet_get_sctp_main ();
   vlib_main_t *vm = vlib_get_main ();
 
-  if (PREDICT_FALSE (sctp_get_free_buffer_index (tm, &bi)))
+  if (sctp_check_outstanding_data_chunks (sctp_conn) > 0)
     return;
 
-  b = vlib_get_buffer (vm, bi);
-  sctp_init_buffer (vm, b);
-  sctp_prepare_shutdown_complete_chunk (sctp_conn, b);
+  sctp_reuse_buffer (vm, b0);
 
-  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_prepare_shutdown_complete_chunk (sctp_conn, b0);
+
+  sctp_conn->state = SCTP_STATE_CLOSED;
 }
 
 
@@ -1143,6 +1132,7 @@ sctp_push_header (transport_connection_t * trans_conn, vlib_buffer_t * b)
 
 }
 
+#if SCTP_DEBUG_STATE_MACHINE
 always_inline u8
 sctp_validate_output_state_machine (sctp_connection_t * sctp_conn,
 				    u8 chunk_type)
@@ -1175,6 +1165,7 @@ sctp_validate_output_state_machine (sctp_connection_t * sctp_conn,
     }
   return result;
 }
+#endif
 
 always_inline u8
 sctp_is_retransmitting (sctp_connection_t * sctp_conn, u8 idx)
@@ -1293,6 +1284,18 @@ sctp46_output_inline (vlib_main_t * vm,
 #endif
 	    }
 
+	  sctp_full_hdr_t *full_hdr = (sctp_full_hdr_t *) sctp_hdr;
+	  u8 chunk_type = vnet_sctp_get_chunk_type (&full_hdr->common_hdr);
+	  if (chunk_type >= UNKNOWN)
+	    {
+	      clib_warning
+		("Trying to send an unrecognized chunk... something is really bad.");
+	      error0 = SCTP_ERROR_UNKOWN_CHUNK;
+	      next0 = SCTP_OUTPUT_NEXT_DROP;
+	      goto done;
+	    }
+
+#if SCTP_DEBUG_STATE_MACHINE
 	  u8 is_valid =
 	    (sctp_conn->sub_conn[idx].connection.lcl_port ==
 	     sctp_hdr->src_port
@@ -1303,9 +1306,6 @@ sctp46_output_inline (vlib_main_t * vm,
 		|| sctp_conn->sub_conn[idx].connection.rmt_port ==
 		sctp_hdr->src_port);
 
-	  sctp_full_hdr_t *full_hdr = (sctp_full_hdr_t *) sctp_hdr;
-	  u8 chunk_type = vnet_sctp_get_chunk_type (&full_hdr->common_hdr);
-
 	  if (!is_valid)
 	    {
 	      SCTP_DBG_STATE_MACHINE ("BUFFER IS INCORRECT: conn_index = %u, "
@@ -1313,8 +1313,8 @@ sctp46_output_inline (vlib_main_t * vm,
 				      "chunk_type = %u [%s], "
 				      "connection.lcl_port = %u, sctp_hdr->src_port = %u, "
 				      "connection.rmt_port = %u, sctp_hdr->dst_port = %u",
-				      sctp_conn->sub_conn
-				      [idx].connection.c_index, packet_length,
+				      sctp_conn->sub_conn[idx].
+				      connection.c_index, packet_length,
 				      chunk_type,
 				      sctp_chunk_to_string (chunk_type),
 				      sctp_conn->sub_conn[idx].
@@ -1327,7 +1327,7 @@ sctp46_output_inline (vlib_main_t * vm,
 	      next0 = SCTP_OUTPUT_NEXT_DROP;
 	      goto done;
 	    }
-
+#endif
 	  SCTP_DBG_STATE_MACHINE
 	    ("CONN_INDEX = %u, CURR_CONN_STATE = %u (%s), "
 	     "CHUNK_TYPE = %s, " "SRC_PORT = %u, DST_PORT = %u",
diff --git a/src/vnet/sctp/sctp_packet.h b/src/vnet/sctp/sctp_packet.h
index b831d249f76..3ca05b5a11e 100644
--- a/src/vnet/sctp/sctp_packet.h
+++ b/src/vnet/sctp/sctp_packet.h
@@ -163,7 +163,8 @@ typedef enum
   COOKIE_ACK,
   ECNE,
   CWR,
-  SHUTDOWN_COMPLETE
+  SHUTDOWN_COMPLETE,
+  UNKNOWN
 } sctp_chunk_type;
 
 /*