From 171d6aceb039a7f0b0d67c837ff74359dae01ae4 Mon Sep 17 00:00:00 2001 From: Damjan Marion Date: Wed, 9 Sep 2020 17:40:02 +0200 Subject: avf: fix race between avf and cli/api process 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 --- src/plugins/avf/avf.h | 8 ++++++- src/plugins/avf/cli.c | 3 +-- src/plugins/avf/device.c | 55 +++++++++++++++++++++++++++--------------------- src/plugins/avf/format.c | 6 ++---- src/plugins/avf/input.c | 3 +-- src/plugins/avf/output.c | 3 +-- 6 files changed, 43 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/plugins/avf/avf.h b/src/plugins/avf/avf.h index 4b35899da38..c4c0c13ebb9 100644 --- a/src/plugins/avf/avf.h +++ b/src/plugins/avf/avf.h @@ -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) { diff --git a/src/plugins/avf/cli.c b/src/plugins/avf/cli.c index 29c2a6b1f6e..32c19f46590 100644 --- a/src/plugins/avf/cli.c +++ b/src/plugins/avf/cli.c @@ -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; diff --git a/src/plugins/avf/device.c b/src/plugins/avf/device.c index 62a18cc3c5c..b0cf4863a11 100644 --- a/src/plugins/avf/device.c +++ b/src/plugins/avf/device.c @@ -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)); } diff --git a/src/plugins/avf/format.c b/src/plugins/avf/format.c index bc2b94ecc46..e5da0e2bbf6 100644 --- a/src/plugins/avf/format.c +++ b/src/plugins/avf/format.c @@ -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; diff --git a/src/plugins/avf/input.c b/src/plugins/avf/input.c index da5556a391e..0ccf7721835 100644 --- a/src/plugins/avf/input.c +++ b/src/plugins/avf/input.c @@ -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); diff --git a/src/plugins/avf/output.c b/src/plugins/avf/output.c index 6c43885569e..e5b53ba5457 100644 --- a/src/plugins/avf/output.c +++ b/src/plugins/avf/output.c @@ -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); -- cgit 1.2.3-korg