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>

diff --git a/src/vnet/sctp/sctp.h b/src/vnet/sctp/sctp.h
index bc974d9..25fae37 100644 (file)
--- 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 3f7ab4b..8f4b043 100644 (file)
--- 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 bb8332f..6012e50 100644 (file)
--- a/src/vnet/sctp/sctp_output.c
+++ b/src/vnet/sctp/sctp_output.c
@@ -351,13 +351,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 b831d24..3ca05b5 100644 (file)
--- 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;
 
 /*