summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSteven Luong <sluong@cisco.com>2019-08-05 09:47:58 -0700
committerAndrew Yourtchenko <ayourtch@gmail.com>2019-09-20 05:48:27 +0000
commitc5dd20250b74aa0d84b0280df034fc69b273709f (patch)
tree09935fa9fa69ca7b0fbb40c51d9c724a9acc63d7
parent69dd1f5bceae81b2b422a410db0f72d12043c97d (diff)
bonding lacp: deleting virtual interface which was enslaved may cause crash
Virtual interfaces may be part of the bonding like physical interfaces. The difference is virtual interfaces may disappear dynamically. As an example, the following CLI sequence may crash the debug image create vhost-user socket /tmp/sock1 create bond mode lacp bond add BondEthernet0 VirtualEthernet0/0/0 delete vhost-user VirtualEhernet0/0/0 Notice the virtual interface is deleted without first doing bond delete. The proper order is to first remove the slave interface from the bond prior to deleting the virtual interface as shown below. But we should handle it anyway. create vhost-user socket /tmp/sock1 create bond mode lacp bond add BondEthernet0 VirtualEthernet0/0/0 bond del VirtualEthernet0/0/0 <----- delete vhost-user VirtualEhernet0/0/0 The fix is to register for VNET_SW_INTERFACE_ADD_DEL_FUNCTION and remove the slave interface from the bond if the to-be-deleted interface is part of the bond. We check the interface that it is actually up before we send the lacp pdu. Up means both hw and sw admin up. Type: fix Signed-off-by: Steven Luong <sluong@cisco.com> Change-Id: If4d2da074338b16aab0df54e00d719e55c45221a (cherry picked from commit bac326cb7c5f8856786ca046df8cfa3be9f53926)
-rw-r--r--src/plugins/lacp/cli.c7
-rw-r--r--src/plugins/lacp/lacp.c40
-rw-r--r--src/vnet/bonding/cli.c6
-rw-r--r--src/vnet/bonding/device.c26
-rw-r--r--src/vnet/bonding/node.c33
5 files changed, 66 insertions, 46 deletions
diff --git a/src/plugins/lacp/cli.c b/src/plugins/lacp/cli.c
index b8ff199c2f4..92a890d2e37 100644
--- a/src/plugins/lacp/cli.c
+++ b/src/plugins/lacp/cli.c
@@ -30,7 +30,7 @@ lacp_dump_ifs (lacp_interface_details_t ** out_lacpifs)
/* *INDENT-OFF* */
pool_foreach (sif, bm->neighbors,
- if ((sif->port_enabled == 0) || (sif->lacp_enabled == 0))
+ if (sif->lacp_enabled == 0)
continue;
vec_add2(r_lacpifs, lacpif, 1);
clib_memset (lacpif, 0, sizeof (*lacpif));
@@ -88,7 +88,7 @@ show_lacp (vlib_main_t * vm, u32 * sw_if_indices)
for (i = 0; i < vec_len (sw_if_indices); i++)
{
sif = bond_get_slave_by_sw_if_index (sw_if_indices[i]);
- if (!sif || (sif->port_enabled == 0) || (sif->lacp_enabled == 0))
+ if (!sif || (sif->lacp_enabled == 0))
continue;
bif = bond_get_master_by_dev_instance (sif->bif_dev_instance);
vlib_cli_output (vm,
@@ -157,7 +157,7 @@ show_lacp_details (vlib_main_t * vm, u32 * sw_if_indices)
for (i = 0; i < vec_len (sw_if_indices); i++)
{
sif = bond_get_slave_by_sw_if_index (sw_if_indices[i]);
- if (!sif || (sif->port_enabled == 0) || (sif->lacp_enabled == 0))
+ if (!sif || (sif->lacp_enabled == 0))
continue;
vlib_cli_output (vm, " %U", format_vnet_sw_if_index_name,
vnet_get_main (), sif->sw_if_index);
@@ -186,6 +186,7 @@ show_lacp_details (vlib_main_t * vm, u32 * sw_if_indices)
now - sif->last_marker_pdu_sent_time);
vlib_cli_output (vm, " debug: %d", sif->debug);
vlib_cli_output (vm, " loopback port: %d", sif->loopback_port);
+ vlib_cli_output (vm, " port_enabled: %d", sif->port_enabled);
vlib_cli_output (vm, " port moved: %d", sif->port_moved);
vlib_cli_output (vm, " ready_n: %d", sif->ready_n);
vlib_cli_output (vm, " ready: %d", sif->ready);
diff --git a/src/plugins/lacp/lacp.c b/src/plugins/lacp/lacp.c
index 57d8bb0edb3..dba6cb17f63 100644
--- a/src/plugins/lacp/lacp.c
+++ b/src/plugins/lacp/lacp.c
@@ -107,7 +107,7 @@ lacp_pick_packet_template (slave_if_t * sif)
void
lacp_send_lacp_pdu (vlib_main_t * vm, slave_if_t * sif)
{
- if (sif->mode != BOND_MODE_LACP)
+ if ((sif->mode != BOND_MODE_LACP) || (sif->port_enabled == 0))
{
lacp_stop_timer (&sif->periodic_timer);
return;
@@ -374,16 +374,18 @@ lacp_sw_interface_up_down (vnet_main_t * vnm, u32 sw_if_index, u32 flags)
sif = bond_get_slave_by_sw_if_index (sw_if_index);
if (sif)
{
- sif->port_enabled = flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP;
+ if (sif->lacp_enabled == 0)
+ return 0;
+
+ /* port_enabled is both admin up and hw link up */
+ sif->port_enabled = ((flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP) &&
+ vnet_sw_interface_is_link_up (vnm, sw_if_index));
if (sif->port_enabled == 0)
{
- if (sif->lacp_enabled)
- {
- lacp_init_neighbor (sif, sif->actor_admin.system,
- ntohs (sif->actor_admin.port_number),
- ntohs (sif->actor_admin.key));
- lacp_init_state_machines (vm, sif);
- }
+ lacp_init_neighbor (sif, sif->actor_admin.system,
+ ntohs (sif->actor_admin.port_number),
+ ntohs (sif->actor_admin.key));
+ lacp_init_state_machines (vm, sif);
}
}
@@ -404,15 +406,19 @@ lacp_hw_interface_up_down (vnet_main_t * vnm, u32 hw_if_index, u32 flags)
sif = bond_get_slave_by_sw_if_index (sw->sw_if_index);
if (sif)
{
- if (!(flags & VNET_HW_INTERFACE_FLAG_LINK_UP))
+ if (sif->lacp_enabled == 0)
+ return 0;
+
+ /* port_enabled is both admin up and hw link up */
+ sif->port_enabled = ((flags & VNET_HW_INTERFACE_FLAG_LINK_UP) &&
+ vnet_sw_interface_is_admin_up (vnm,
+ sw->sw_if_index));
+ if (sif->port_enabled == 0)
{
- if (sif->lacp_enabled)
- {
- lacp_init_neighbor (sif, sif->actor_admin.system,
- ntohs (sif->actor_admin.port_number),
- ntohs (sif->actor_admin.key));
- lacp_init_state_machines (vm, sif);
- }
+ lacp_init_neighbor (sif, sif->actor_admin.system,
+ ntohs (sif->actor_admin.port_number),
+ ntohs (sif->actor_admin.key));
+ lacp_init_state_machines (vm, sif);
}
}
diff --git a/src/vnet/bonding/cli.c b/src/vnet/bonding/cli.c
index 9d3b9429ba7..f45a31ce99a 100644
--- a/src/vnet/bonding/cli.c
+++ b/src/vnet/bonding/cli.c
@@ -576,7 +576,8 @@ bond_enslave (vlib_main_t * vm, bond_enslave_args_t * args)
pool_get (bm->neighbors, sif);
clib_memset (sif, 0, sizeof (*sif));
sw = pool_elt_at_index (im->sw_interfaces, args->slave);
- sif->port_enabled = sw->flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP;
+ /* port_enabled is both admin up and hw link up */
+ sif->port_enabled = vnet_sw_interface_is_up (vnm, sw->sw_if_index);
sif->sw_if_index = sw->sw_if_index;
sif->hw_if_index = sw->hw_if_index;
sif->packet_template_index = (u8) ~ 0;
@@ -642,8 +643,7 @@ bond_enslave (vlib_main_t * vm, bond_enslave_args_t * args)
if (bm->lacp_enable_disable)
(*bm->lacp_enable_disable) (vm, bif, sif, 1);
}
- else if (sif->port_enabled &&
- (sif_hw->flags & VNET_HW_INTERFACE_FLAG_LINK_UP))
+ else if (sif->port_enabled)
{
bond_enable_collecting_distributing (vm, sif);
}
diff --git a/src/vnet/bonding/device.c b/src/vnet/bonding/device.c
index e9771959437..77a53b6a17a 100644
--- a/src/vnet/bonding/device.c
+++ b/src/vnet/bonding/device.c
@@ -777,9 +777,10 @@ bond_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f)
for (i = 0; i < vec_len (event_data); i++)
{
hw_if_index = event_data[i];
- /* walk hw interface to process all subinterfaces */
- vnet_hw_interface_walk_sw (vnm, hw_if_index,
- bond_active_interface_switch_cb, 0);
+ if (vnet_get_hw_interface_or_null (vnm, hw_if_index))
+ /* walk hw interface to process all subinterfaces */
+ vnet_hw_interface_walk_sw (vnm, hw_if_index,
+ bond_active_interface_switch_cb, 0);
}
vec_reset_length (event_data);
}
@@ -808,6 +809,25 @@ VNET_DEVICE_CLASS (bond_dev_class) = {
/* *INDENT-ON* */
+static clib_error_t *
+bond_slave_interface_add_del (vnet_main_t * vnm, u32 sw_if_index, u32 is_add)
+{
+ bond_main_t *bm = &bond_main;
+ slave_if_t *sif;
+ bond_detach_slave_args_t args = { 0 };
+
+ if (is_add)
+ return 0;
+ sif = bond_get_slave_by_sw_if_index (sw_if_index);
+ if (!sif)
+ return 0;
+ args.slave = sw_if_index;
+ bond_detach_slave (bm->vlib_main, &args);
+ return args.error;
+}
+
+VNET_SW_INTERFACE_ADD_DEL_FUNCTION (bond_slave_interface_add_del);
+
/*
* fd.io coding-style-patch-verification: ON
*
diff --git a/src/vnet/bonding/node.c b/src/vnet/bonding/node.c
index 6fc74710fda..ce5aefab2fd 100644
--- a/src/vnet/bonding/node.c
+++ b/src/vnet/bonding/node.c
@@ -394,23 +394,16 @@ bond_sw_interface_up_down (vnet_main_t * vnm, u32 sw_if_index, u32 flags)
sif = bond_get_slave_by_sw_if_index (sw_if_index);
if (sif)
{
- sif->port_enabled = flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP;
if (sif->lacp_enabled)
return 0;
+ /* port_enabled is both admin up and hw link up */
+ sif->port_enabled = ((flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP) &&
+ vnet_sw_interface_is_link_up (vnm, sw_if_index));
if (sif->port_enabled == 0)
- {
- bond_disable_collecting_distributing (vm, sif);
- }
+ bond_disable_collecting_distributing (vm, sif);
else
- {
- vnet_main_t *vnm = vnet_get_main ();
- vnet_hw_interface_t *hw =
- vnet_get_sup_hw_interface (vnm, sw_if_index);
-
- if (hw->flags & VNET_HW_INTERFACE_FLAG_LINK_UP)
- bond_enable_collecting_distributing (vm, sif);
- }
+ bond_enable_collecting_distributing (vm, sif);
}
return 0;
@@ -433,14 +426,14 @@ bond_hw_interface_up_down (vnet_main_t * vnm, u32 hw_if_index, u32 flags)
if (sif->lacp_enabled)
return 0;
- if (!(flags & VNET_HW_INTERFACE_FLAG_LINK_UP))
- {
- bond_disable_collecting_distributing (vm, sif);
- }
- else if (sif->port_enabled)
- {
- bond_enable_collecting_distributing (vm, sif);
- }
+ /* port_enabled is both admin up and hw link up */
+ sif->port_enabled = ((flags & VNET_HW_INTERFACE_FLAG_LINK_UP) &&
+ vnet_sw_interface_is_admin_up (vnm,
+ sw->sw_if_index));
+ if (sif->port_enabled == 0)
+ bond_disable_collecting_distributing (vm, sif);
+ else
+ bond_enable_collecting_distributing (vm, sif);
}
return 0;