avf: fix race between avf and cli/api process 24/28724/4
authorDamjan Marion <damarion@cisco.com>
Wed, 9 Sep 2020 15:40:02 +0000 (17:40 +0200)
committerAndrew Yourtchenko <ayourtch@gmail.com>
Fri, 11 Sep 2020 11:11:02 +0000 (11:11 +0000)
device pool my grow during suspemd which will cause crash in avf process
after it exits from suspend.

Type: fix

Change-Id: I51fec90088c909cfbaaca6c245272a28c0827ca0
Signed-off-by: Damjan Marion <damarion@cisco.com>
src/plugins/avf/avf.h
src/plugins/avf/cli.c
src/plugins/avf/device.c
src/plugins/avf/format.c
src/plugins/avf/input.c
src/plugins/avf/output.c

index 4b35899..c4c0c13 100644 (file)
@@ -223,7 +223,7 @@ typedef struct
 {
   u16 msg_id_base;
 
-  avf_device_t *devices;
+  avf_device_t **devices;
   avf_per_thread_data_t *per_thread_data;
 
   vlib_log_class_t log_class;
@@ -256,6 +256,12 @@ format_function_t format_avf_device;
 format_function_t format_avf_device_name;
 format_function_t format_avf_input_trace;
 
+static_always_inline avf_device_t *
+avf_get_device (u32 dev_instance)
+{
+  return pool_elt_at_index (avf_main.devices, dev_instance)[0];
+}
+
 static inline u32
 avf_get_u32 (void *start, int offset)
 {
index 29c2a6b..32c19f4 100644 (file)
@@ -134,7 +134,6 @@ avf_test_command_fn (vlib_main_t * vm, unformat_input_t * input,
   unformat_input_t _line_input, *line_input = &_line_input;
   u32 sw_if_index = ~0;
   vnet_hw_interface_t *hw;
-  avf_main_t *am = &avf_main;
   avf_device_t *ad;
   vnet_main_t *vnm = vnet_get_main ();
   int test_irq = 0, enable_elog = 0, disable_elog = 0;
@@ -170,7 +169,7 @@ avf_test_command_fn (vlib_main_t * vm, unformat_input_t * input,
   if (hw == NULL || avf_device_class.index != hw->dev_class_index)
     return clib_error_return (0, "not a AVF interface");
 
-  ad = pool_elt_at_index (am->devices, hw->dev_instance);
+  ad = avf_get_device (hw->dev_instance);
 
   if (enable_elog)
     ad->flags |= AVF_DEVICE_F_ELOG;
index 62a18cc..b0cf486 100644 (file)
@@ -1157,8 +1157,7 @@ static u32
 avf_flag_change (vnet_main_t * vnm, vnet_hw_interface_t * hw, u32 flags)
 {
   vlib_main_t *vm = vlib_get_main ();
-  avf_main_t *am = &avf_main;
-  avf_device_t *ad = vec_elt_at_index (am->devices, hw->dev_instance);
+  avf_device_t *ad = avf_get_device (hw->dev_instance);
   clib_error_t *error;
   u8 promisc_enabled;
 
@@ -1189,11 +1188,12 @@ static uword
 avf_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f)
 {
   avf_main_t *am = &avf_main;
-  avf_device_t *ad;
   uword *event_data = 0, event_type;
   int enabled = 0, irq;
   f64 last_run_duration = 0;
   f64 last_periodic_time = 0;
+  avf_device_t **dev_pointers = 0;
+  u32 i;
 
   while (1)
     {
@@ -1216,7 +1216,7 @@ avf_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f)
        case AVF_PROCESS_EVENT_DELETE_IF:
          for (int i = 0; i < vec_len (event_data); i++)
            {
-             ad = pool_elt_at_index (am->devices, event_data[i]);
+             avf_device_t *ad = avf_get_device (event_data[i]);
              avf_delete_if (vm, ad, /* with_barrier */ 1);
            }
          if (pool_elts (am->devices) < 1)
@@ -1234,11 +1234,19 @@ avf_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f)
       if (enabled == 0)
        continue;
 
+      /* create local list of device pointers as device pool may grow
+       * during suspend */
+      vec_reset_length (dev_pointers);
       /* *INDENT-OFF* */
-      pool_foreach (ad, am->devices,
+      pool_foreach_index (i, am->devices,
+        {
+         vec_add1 (dev_pointers, avf_get_device (i));
+       });
+
+      vec_foreach_index (i, dev_pointers)
         {
-         avf_process_one_device (vm, ad, irq);
-        });
+         avf_process_one_device (vm, dev_pointers[i], irq);
+        };
       /* *INDENT-ON* */
       last_run_duration = vlib_time_now (vm) - last_periodic_time;
     }
@@ -1256,9 +1264,8 @@ VLIB_REGISTER_NODE (avf_process_node)  = {
 static void
 avf_irq_0_handler (vlib_main_t * vm, vlib_pci_dev_handle_t h, u16 line)
 {
-  avf_main_t *am = &avf_main;
   uword pd = vlib_pci_get_private_data (vm, h);
-  avf_device_t *ad = pool_elt_at_index (am->devices, pd);
+  avf_device_t *ad = avf_get_device (pd);
   u32 icr0;
 
   icr0 = avf_reg_read (ad, AVFINT_ICR0);
@@ -1295,9 +1302,8 @@ static void
 avf_irq_n_handler (vlib_main_t * vm, vlib_pci_dev_handle_t h, u16 line)
 {
   vnet_main_t *vnm = vnet_get_main ();
-  avf_main_t *am = &avf_main;
   uword pd = vlib_pci_get_private_data (vm, h);
-  avf_device_t *ad = pool_elt_at_index (am->devices, pd);
+  avf_device_t *ad = avf_get_device (pd);
 
   if (ad->flags & AVF_DEVICE_F_ELOG)
     {
@@ -1386,7 +1392,8 @@ avf_delete_if (vlib_main_t * vm, avf_device_t * ad, int with_barrier)
 
   clib_error_free (ad->error);
   clib_memset (ad, 0, sizeof (*ad));
-  pool_put (am->devices, ad);
+  pool_put_index (am->devices, ad->dev_instance);
+  clib_mem_free (ad);
 }
 
 void
@@ -1394,7 +1401,7 @@ avf_create_if (vlib_main_t * vm, avf_create_if_args_t * args)
 {
   vnet_main_t *vnm = vnet_get_main ();
   avf_main_t *am = &avf_main;
-  avf_device_t *ad;
+  avf_device_t *ad, **adp;
   vlib_pci_dev_handle_t h;
   clib_error_t *error = 0;
   int i;
@@ -1412,8 +1419,11 @@ avf_create_if (vlib_main_t * vm, avf_create_if_args_t * args)
       return;
     }
 
-  pool_get (am->devices, ad);
-  ad->dev_instance = ad - am->devices;
+  pool_get (am->devices, adp);
+  adp[0] = ad = clib_mem_alloc_aligned (sizeof (avf_device_t),
+                                       CLIB_CACHE_LINE_BYTES);
+  clib_memset (ad, 0, sizeof (avf_device_t));
+  ad->dev_instance = adp - am->devices;
   ad->per_interface_next_index = ~0;
   ad->name = vec_dup (args->name);
 
@@ -1423,7 +1433,8 @@ avf_create_if (vlib_main_t * vm, avf_create_if_args_t * args)
   if ((error = vlib_pci_device_open (vm, &args->addr, avf_pci_device_ids,
                                     &h)))
     {
-      pool_put (am->devices, ad);
+      pool_put (am->devices, adp);
+      clib_mem_free (ad);
       args->rv = VNET_API_ERROR_INVALID_INTERFACE;
       args->error =
        clib_error_return (error, "pci-addr %U", format_vlib_pci_addr,
@@ -1558,8 +1569,7 @@ static clib_error_t *
 avf_interface_admin_up_down (vnet_main_t * vnm, u32 hw_if_index, u32 flags)
 {
   vnet_hw_interface_t *hi = vnet_get_hw_interface (vnm, hw_if_index);
-  avf_main_t *am = &avf_main;
-  avf_device_t *ad = vec_elt_at_index (am->devices, hi->dev_instance);
+  avf_device_t *ad = avf_get_device (hi->dev_instance);
   uword is_up = (flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP) != 0;
 
   if (ad->flags & AVF_DEVICE_F_ERROR)
@@ -1583,9 +1593,8 @@ static clib_error_t *
 avf_interface_rx_mode_change (vnet_main_t * vnm, u32 hw_if_index, u32 qid,
                              vnet_hw_interface_rx_mode mode)
 {
-  avf_main_t *am = &avf_main;
   vnet_hw_interface_t *hw = vnet_get_hw_interface (vnm, hw_if_index);
-  avf_device_t *ad = pool_elt_at_index (am->devices, hw->dev_instance);
+  avf_device_t *ad = avf_get_device (hw->dev_instance);
   avf_rxq_t *rxq = vec_elt_at_index (ad->rxqs, qid);
 
   if (mode == VNET_HW_INTERFACE_RX_MODE_POLLING)
@@ -1615,9 +1624,8 @@ static void
 avf_set_interface_next_node (vnet_main_t * vnm, u32 hw_if_index,
                             u32 node_index)
 {
-  avf_main_t *am = &avf_main;
   vnet_hw_interface_t *hw = vnet_get_hw_interface (vnm, hw_if_index);
-  avf_device_t *ad = pool_elt_at_index (am->devices, hw->dev_instance);
+  avf_device_t *ad = avf_get_device (hw->dev_instance);
 
   /* Shut off redirection */
   if (node_index == ~0)
@@ -1639,8 +1647,7 @@ static char *avf_tx_func_error_strings[] = {
 static void
 avf_clear_hw_interface_counters (u32 instance)
 {
-  avf_main_t *am = &avf_main;
-  avf_device_t *ad = vec_elt_at_index (am->devices, instance);
+  avf_device_t *ad = avf_get_device (instance);
   clib_memcpy_fast (&ad->last_cleared_eth_stats,
                    &ad->eth_stats, sizeof (ad->eth_stats));
 }
index bc2b94e..e5da0e2 100644 (file)
@@ -27,8 +27,7 @@ format_avf_device_name (u8 * s, va_list * args)
 {
   vlib_main_t *vm = vlib_get_main ();
   u32 i = va_arg (*args, u32);
-  avf_main_t *am = &avf_main;
-  avf_device_t *ad = vec_elt_at_index (am->devices, i);
+  avf_device_t *ad = avf_get_device (i);
   vlib_pci_addr_t *addr = vlib_pci_get_addr (vm, ad->pci_dev_handle);
 
   if (ad->name)
@@ -88,8 +87,7 @@ u8 *
 format_avf_device (u8 * s, va_list * args)
 {
   u32 i = va_arg (*args, u32);
-  avf_main_t *am = &avf_main;
-  avf_device_t *ad = vec_elt_at_index (am->devices, i);
+  avf_device_t *ad = avf_get_device (i);
   u32 indent = format_get_indent (s);
   u8 *a = 0;
 
index da5556a..0ccf772 100644 (file)
@@ -444,14 +444,13 @@ VLIB_NODE_FN (avf_input_node) (vlib_main_t * vm, vlib_node_runtime_t * node,
                               vlib_frame_t * frame)
 {
   u32 n_rx = 0;
-  avf_main_t *am = &avf_main;
   vnet_device_input_runtime_t *rt = (void *) node->runtime_data;
   vnet_device_and_queue_t *dq;
 
   foreach_device_and_queue (dq, rt->devices_and_queues)
   {
     avf_device_t *ad;
-    ad = vec_elt_at_index (am->devices, dq->dev_instance);
+    ad = avf_get_device (dq->dev_instance);
     if ((ad->flags & AVF_DEVICE_F_ADMIN_UP) == 0)
       continue;
     n_rx += avf_device_input_inline (vm, node, frame, ad, dq->queue_id);
index 6c43885..e5b53ba 100644 (file)
@@ -267,9 +267,8 @@ VNET_DEVICE_CLASS_TX_FN (avf_device_class) (vlib_main_t * vm,
                                            vlib_node_runtime_t * node,
                                            vlib_frame_t * frame)
 {
-  avf_main_t *am = &avf_main;
   vnet_interface_output_runtime_t *rd = (void *) node->runtime_data;
-  avf_device_t *ad = pool_elt_at_index (am->devices, rd->dev_instance);
+  avf_device_t *ad = avf_get_device (rd->dev_instance);
   u32 thread_index = vm->thread_index;
   u8 qid = thread_index;
   avf_txq_t *txq = vec_elt_at_index (ad->txqs, qid % ad->num_queue_pairs);