Skip to content

Commit

Permalink
avf: fix race between avf process node and avf_delete_if(...)
Browse files Browse the repository at this point in the history
It may happen that process node is suspended while it waits for response
from adminq and during that time CLI or API process can call
avf_delete_if. When avf process node resumes, it may happen that device
is not there anymeore.

This patch delegates interface deletion to process node, so CLI/API
process just sends signal instead of deleting device instance itself.

Type: fix

Change-Id: I7f12e12df3071650f6e60ad7eb5af23b7acfe335
Signed-off-by: Damjan Marion <[email protected]>
  • Loading branch information
dmarion authored and florincoras committed Sep 11, 2020
1 parent 78681de commit 66bb7dd
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 18 deletions.
4 changes: 2 additions & 2 deletions src/plugins/avf/avf.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ typedef struct
typedef enum
{
AVF_PROCESS_EVENT_START = 1,
AVF_PROCESS_EVENT_STOP = 2,
AVF_PROCESS_EVENT_DELETE_IF = 2,
AVF_PROCESS_EVENT_AQ_INT = 3,
} avf_process_event_t;

Expand Down Expand Up @@ -246,9 +246,9 @@ typedef struct
} avf_create_if_args_t;

void avf_create_if (vlib_main_t * vm, avf_create_if_args_t * args);
void avf_delete_if (vlib_main_t * vm, avf_device_t * ad);

extern vlib_node_registration_t avf_input_node;
extern vlib_node_registration_t avf_process_node;
extern vnet_device_class_t avf_device_class;

/* format.c */
Expand Down
9 changes: 5 additions & 4 deletions src/plugins/avf/avf_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ vl_api_avf_delete_t_handler (vl_api_avf_delete_t * mp)
vnet_main_t *vnm = vnet_get_main ();
avf_main_t *am = &avf_main;
vl_api_avf_delete_reply_t *rmp;
avf_device_t *ad;
vnet_hw_interface_t *hw;
int rv = 0;

Expand All @@ -79,9 +78,8 @@ vl_api_avf_delete_t_handler (vl_api_avf_delete_t * mp)
goto reply;
}

ad = pool_elt_at_index (am->devices, hw->dev_instance);

avf_delete_if (vm, ad);
vlib_process_signal_event (vm, avf_process_node.index,
AVF_PROCESS_EVENT_DELETE_IF, hw->dev_instance);

reply:
REPLY_MACRO (VL_API_AVF_DELETE_REPLY + am->msg_id_base);
Expand All @@ -93,6 +91,9 @@ static clib_error_t *
avf_plugin_api_hookup (vlib_main_t * vm)
{
avf_main_t *avm = &avf_main;
api_main_t *am = vlibapi_get_main ();

am->is_mp_safe[VL_API_AVF_DELETE] = 1;

/* ask for a correctly-sized block of API message decode slots */
avm->msg_id_base = setup_message_id_table ();
Expand Down
8 changes: 3 additions & 5 deletions src/plugins/avf/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ avf_delete_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 ();

/* Get a line of input. */
Expand Down Expand Up @@ -113,9 +111,8 @@ avf_delete_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 an AVF interface");

ad = pool_elt_at_index (am->devices, hw->dev_instance);

avf_delete_if (vm, ad);
vlib_process_signal_event (vm, avf_process_node.index,
AVF_PROCESS_EVENT_DELETE_IF, hw->dev_instance);

return 0;
}
Expand All @@ -126,6 +123,7 @@ VLIB_CLI_COMMAND (avf_delete_command, static) = {
.short_help = "delete interface avf "
"{<interface> | sw_if_index <sw_idx>}",
.function = avf_delete_command_fn,
.is_mp_safe = 1,
};
/* *INDENT-ON* */

Expand Down
31 changes: 24 additions & 7 deletions src/plugins/avf/device.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#define PCI_DEVICE_ID_INTEL_X722_VF 0x37cd

avf_main_t avf_main;
void avf_delete_if (vlib_main_t * vm, avf_device_t * ad, int with_barrier);

static pci_device_id_t avf_pci_device_ids[] = {
{.vendor_id = PCI_VENDOR_ID_INTEL,.device_id = PCI_DEVICE_ID_INTEL_AVF},
Expand Down Expand Up @@ -1202,7 +1203,6 @@ avf_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f)
vlib_process_wait_for_event (vm);

event_type = vlib_process_get_events (vm, &event_data);
vec_reset_length (event_data);
irq = 0;

switch (event_type)
Expand All @@ -1213,16 +1213,27 @@ avf_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f)
case AVF_PROCESS_EVENT_START:
enabled = 1;
break;
case AVF_PROCESS_EVENT_STOP:
enabled = 0;
continue;
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_delete_if (vm, ad, /* with_barrier */ 1);
}
if (pool_elts (am->devices) < 1)
enabled = 0;
break;
case AVF_PROCESS_EVENT_AQ_INT:
irq = 1;
break;
default:
ASSERT (0);
}

vec_reset_length (event_data);

if (enabled == 0)
continue;

/* *INDENT-OFF* */
pool_foreach (ad, am->devices,
{
Expand All @@ -1235,7 +1246,7 @@ avf_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f)
}

/* *INDENT-OFF* */
VLIB_REGISTER_NODE (avf_process_node, static) = {
VLIB_REGISTER_NODE (avf_process_node) = {
.function = avf_process,
.type = VLIB_NODE_TYPE_PROCESS,
.name = "avf-process",
Expand Down Expand Up @@ -1316,17 +1327,23 @@ avf_irq_n_handler (vlib_main_t * vm, vlib_pci_dev_handle_t h, u16 line)
}

void
avf_delete_if (vlib_main_t * vm, avf_device_t * ad)
avf_delete_if (vlib_main_t * vm, avf_device_t * ad, int with_barrier)
{
vnet_main_t *vnm = vnet_get_main ();
avf_main_t *am = &avf_main;
int i;

ad->flags &= ~AVF_DEVICE_F_ADMIN_UP;

if (ad->hw_if_index)
{
if (with_barrier)
vlib_worker_thread_barrier_sync (vm);
vnet_hw_interface_set_flags (vnm, ad->hw_if_index, 0);
vnet_hw_interface_unassign_rx_thread (vnm, ad->hw_if_index, 0);
ethernet_delete_interface (vnm, ad->hw_if_index);
if (with_barrier)
vlib_worker_thread_barrier_release (vm);
}

vlib_pci_device_close (vm, ad->pci_dev_handle);
Expand Down Expand Up @@ -1530,7 +1547,7 @@ avf_create_if (vlib_main_t * vm, avf_create_if_args_t * args)
return;

error:
avf_delete_if (vm, ad);
avf_delete_if (vm, ad, /* with_barrier */ 0);
args->rv = VNET_API_ERROR_INVALID_INTERFACE;
args->error = clib_error_return (error, "pci-addr %U",
format_vlib_pci_addr, &args->addr);
Expand Down

0 comments on commit 66bb7dd

Please sign in to comment.