memif: fix crash caused by zero pkt len in memif and clear dirty cache while interfac... 21/10421/8
authorChun Li <chunl2@cisco.com>
Tue, 6 Feb 2018 07:17:20 +0000 (15:17 +0800)
committerDamjan Marion <dmarion.lists@gmail.com>
Fri, 9 Feb 2018 09:08:01 +0000 (09:08 +0000)
Change-Id: Ifc7eb2494a22c334d8899422545fca1a4bba4d05
Signed-off-by: Chun Li <chunl2@cisco.com>
src/plugins/memif/cli.c
src/plugins/memif/memif.c
src/plugins/memif/node.c
src/plugins/memif/private.h
src/plugins/memif/socket.c

index 3f0e281..6a149f9 100644 (file)
@@ -354,14 +354,13 @@ format_memif_if_mode (u8 * s, va_list * args)
 static u8 *
 format_memif_queue (u8 * s, va_list * args)
 {
-  memif_if_t *mif = va_arg (*args, memif_if_t *);
   memif_queue_t *mq = va_arg (*args, memif_queue_t *);
   uword i = va_arg (*args, uword);
   u32 indent = format_get_indent (s);
 
   s = format (s, "%U%s ring %u:\n",
              format_white_space, indent,
-             (mif->flags & MEMIF_IF_FLAG_IS_SLAVE) ?
+             (mq->type == MEMIF_RING_S2M) ?
              "slave-to-master" : "master-to-slave", i);
   s = format (s, "%Uregion %u offset %u ring-size %u int-fd %d\n",
              format_white_space, indent + 4,
@@ -514,14 +513,14 @@ memif_show_command_fn (vlib_main_t * vm, unformat_input_t * input,
       vec_foreach_index (i, mif->tx_queues)
       {
        mq = vec_elt_at_index (mif->tx_queues, i);
-       vlib_cli_output (vm, "  %U", format_memif_queue, mif, mq, i);
+       vlib_cli_output (vm, "  %U", format_memif_queue, mq, i);
        if (show_descr)
          vlib_cli_output (vm, "  %U", format_memif_descriptor, mif, mq);
       }
       vec_foreach_index (i, mif->rx_queues)
       {
        mq = vec_elt_at_index (mif->rx_queues, i);
-       vlib_cli_output (vm, "  %U", format_memif_queue, mif, mq, i);
+       vlib_cli_output (vm, "  %U", format_memif_queue, mq, i);
        if (show_descr)
          vlib_cli_output (vm, "  %U", format_memif_descriptor, mif, mq);
       }
index 7267ef2..d630f2a 100644 (file)
@@ -341,6 +341,7 @@ memif_init_regions_and_queues (memif_if_t * mif)
     mq->region = 0;
     mq->offset = (void *) mq->ring - (void *) mif->regions[mq->region].shm;
     mq->last_head = 0;
+    mq->type = MEMIF_RING_S2M;
   }
 
   ASSERT (mif->rx_queues == 0);
@@ -357,6 +358,7 @@ memif_init_regions_and_queues (memif_if_t * mif)
     mq->region = 0;
     mq->offset = (void *) mq->ring - (void *) mif->regions[mq->region].shm;
     mq->last_head = 0;
+    mq->type = MEMIF_RING_M2S;
   }
 
   return 0;
@@ -592,12 +594,15 @@ memif_delete_if (vlib_main_t * vm, memif_if_t * mif)
   memif_disconnect (mif, err);
   clib_error_free (err);
 
-  /* remove the interface */
-  if (mif->mode == MEMIF_INTERFACE_MODE_IP)
-    vnet_delete_hw_interface (vnm, mif->hw_if_index);
-  else
-    ethernet_delete_interface (vnm, mif->hw_if_index);
-  mif->hw_if_index = ~0;
+  if (mif->hw_if_index != ~0)
+    {
+      /* remove the interface */
+      if (mif->mode == MEMIF_INTERFACE_MODE_IP)
+       vnet_delete_hw_interface (vnm, mif->hw_if_index);
+      else
+       ethernet_delete_interface (vnm, mif->hw_if_index);
+      mif->hw_if_index = ~0;
+    }
 
   /* free interface data structures */
   clib_spinlock_free (&mif->lockp);
index a5187ad..83f1277 100644 (file)
@@ -140,6 +140,9 @@ memif_copy_buffer_from_rx_ring (vlib_main_t * vm, memif_if_t * mif,
          prev_bi = *bi;
          *bi = nm->rx_buffers[thread_index][last_buf];
          b = vlib_get_buffer (vm, *bi);
+         /* Clear the error first to ensure following node forget setting it */
+         /* It will cause null-node error counter increasement instead of potential crash */
+         b->error = 0x0;
          _vec_len (nm->rx_buffers[thread_index]) = last_buf;
          (*n_free_bufs)--;
          if (PREDICT_FALSE (*n_free_bufs == 0))
@@ -266,6 +269,7 @@ memif_device_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
     }
 
   head = ring->head;
+  mq->last_head = ring->tail;
   if (head == mq->last_head)
     return 0;
 
@@ -309,6 +313,10 @@ memif_device_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
                                                     &first_bi1, &bi1,
                                                     &num_slots);
 
+         if (PREDICT_FALSE (!first_bi0 || !first_bi1))
+           {
+             goto _invalid_pkt01;
+           }
          /* enqueue buffer */
          to_next[0] = first_bi0;
          to_next[1] = first_bi1;
@@ -376,6 +384,66 @@ memif_device_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
          /* next packet */
          n_rx_packets += 2;
          n_rx_bytes += b0_total + b1_total;
+
+         continue;
+       _invalid_pkt01:
+         if (!first_bi0 && !first_bi1)
+           {
+             continue;
+           }
+         if (first_bi1)
+           {
+             first_bi0 = first_bi1;
+             first_b0 = first_b1;
+             bi0 = bi1;
+             b0_total = b1_total;
+           }
+
+         if (mode == MEMIF_INTERFACE_MODE_IP)
+           {
+             next0 = memif_next_from_ip_hdr (node, first_b0);
+           }
+         else if (mode == MEMIF_INTERFACE_MODE_ETHERNET)
+           {
+             if (PREDICT_FALSE (mif->per_interface_next_index != ~0))
+               next0 = mif->per_interface_next_index;
+             else
+               /* redirect if feature path
+                * enabled */
+               vnet_feature_start_device_input_x1 (mif->sw_if_index, &next0,
+                                                   first_b0);
+           }
+
+         /* trace */
+         VLIB_BUFFER_TRACE_TRAJECTORY_INIT (first_b0);
+
+         if (PREDICT_FALSE (n_trace > 0))
+           {
+             if (PREDICT_TRUE (first_b0 != 0))
+               {
+                 memif_input_trace_t *tr;
+                 vlib_trace_buffer (vm, node, next0, first_b0,
+                                    /* follow_chain */ 0);
+                 vlib_set_trace_count (vm, node, --n_trace);
+                 tr = vlib_add_trace (vm, node, first_b0, sizeof (*tr));
+                 tr->next_index = next0;
+                 tr->hw_if_index = mif->hw_if_index;
+                 tr->ring = qid;
+               }
+           }
+
+         /* enqueue buffer */
+         to_next[0] = first_bi0;
+         to_next += 1;
+         n_left_to_next--;
+
+         /* enqueue */
+         vlib_validate_buffer_enqueue_x1 (vm, node, next_index, to_next,
+                                          n_left_to_next, first_bi0, next0);
+
+         /* next packet */
+         n_rx_packets++;
+         n_rx_bytes += b0_total;
        }
       while (num_slots && n_left_to_next)
        {
@@ -387,6 +455,10 @@ memif_device_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
                                                     &n_free_bufs, &first_b0,
                                                     &first_bi0, &bi0,
                                                     &num_slots);
+         if (PREDICT_FALSE (!first_bi0))
+           {
+             goto _invalid_pkt0;
+           }
 
          if (mode == MEMIF_INTERFACE_MODE_IP)
            {
@@ -433,11 +505,17 @@ memif_device_input_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
          /* next packet */
          n_rx_packets++;
          n_rx_bytes += b0_total;
+         continue;
+       _invalid_pkt0:
+         ;
+       }
+      if (PREDICT_TRUE (n_rx_packets != 0))
+       {
+         vlib_put_next_frame (vm, node, next_index, n_left_to_next);
        }
-      vlib_put_next_frame (vm, node, next_index, n_left_to_next);
     }
   CLIB_MEMORY_STORE_BARRIER ();
-  ring->tail = head;
+  ring->tail = mq->last_head;
 
   vlib_increment_combined_counter (vnm->interface_main.combined_sw_if_counters
                                   + VNET_INTERFACE_COUNTER_RX, thread_index,
index 5904fc0..b7c18c9 100644 (file)
@@ -106,6 +106,9 @@ typedef struct
   int int_fd;
   uword int_clib_file_index;
   u64 int_count;
+
+  /* queue type */
+  memif_ring_type_t type;
 } memif_queue_t;
 
 #define foreach_memif_if_flag \
index 5e14f08..384a7b1 100644 (file)
@@ -351,11 +351,16 @@ memif_msg_receive_add_ring (memif_if_t * mif, memif_msg_t * msg, int fd)
       mif->run.num_m2s_rings = vec_len (mif->tx_queues);
     }
 
+  // clear previous cache data if interface reconncected
+  memset (mq, 0, sizeof (memif_queue_t));
   mq->int_fd = fd;
   mq->int_clib_file_index = ~0;
   mq->log2_ring_size = ar->log2_ring_size;
   mq->region = ar->region;
   mq->offset = ar->offset;
+  mq->type =
+    (ar->flags & MEMIF_MSG_ADD_RING_FLAG_S2M) ? MEMIF_RING_S2M :
+    MEMIF_RING_M2S;
 
   return 0;
 }