bond: packet drops on VPP bond interface [VPP-1544]

We register callback for VNET_HW_INTERFACE_LINK_UP_DOWN_FUNCTION and
VNET_SW_INTERFACE_ADMIN_UP_DOWN_FUNCTION to add and remove the slave
interface from the bond interface accordingly. For static bonding without
lacp, one would think that it is good enough to put the slave interface into
the ective slave set as soon as it is configured. Wrong, sometimes the slave
interface is configured to be part of the bonding without ever bringing up the
hardware carrier or setting the admin state to up. In that case, we send
traffic to the "dead" slave interface.

The fix is to make sure both the carrier and admin state are up before we put
the slave into the active set for forwarding traffic.

Change-Id: I93b1c36d5481ca76cc8b87e8ca1b375ca3bd453b
Signed-off-by: Steven <sluong@cisco.com>

2 files changed, 21 insertions, 20 deletions

diff --git a/src/vnet/bonding/cli.c b/src/vnet/bonding/cli.c
index 2ccf0d4b4ce..bccbb2c036e 100644
--- a/src/vnet/bonding/cli.c
+++ b/src/vnet/bonding/cli.c
@@ -531,11 +531,13 @@ bond_enslave (vlib_main_t * vm, bond_enslave_args_t * args)
       ethernet_set_rx_redirect (vnm, sif_hw, 1);
     }
 
-  if ((bif->mode == BOND_MODE_LACP) && bm->lacp_enable_disable)
+  if (bif->mode == BOND_MODE_LACP)
     {
-      (*bm->lacp_enable_disable) (vm, bif, sif, 1);
+      if (bm->lacp_enable_disable)
+	(*bm->lacp_enable_disable) (vm, bif, sif, 1);
     }
-  else
+  else if (sif->port_enabled &&
+	   (sif_hw->flags & VNET_HW_INTERFACE_FLAG_LINK_UP))
     {
       bond_enable_collecting_distributing (vm, sif);
     }
diff --git a/src/vnet/bonding/node.c b/src/vnet/bonding/node.c
index d9450692138..80013428101 100644
--- a/src/vnet/bonding/node.c
+++ b/src/vnet/bonding/node.c
@@ -396,19 +396,21 @@ bond_sw_interface_up_down (vnet_main_t * vnm, u32 sw_if_index, u32 flags)
   if (sif)
     {
       sif->port_enabled = flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP;
+      if (sif->lacp_enabled)
+	return 0;
+
       if (sif->port_enabled == 0)
 	{
-	  if (sif->lacp_enabled == 0)
-	    {
-	      bond_disable_collecting_distributing (vm, sif);
-	    }
+	  bond_disable_collecting_distributing (vm, sif);
 	}
       else
 	{
-	  if (sif->lacp_enabled == 0)
-	    {
-	      bond_enable_collecting_distributing (vm, sif);
-	    }
+	  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);
 	}
     }
 
@@ -429,19 +431,16 @@ bond_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 (sif->lacp_enabled)
+	return 0;
+
       if (!(flags & VNET_HW_INTERFACE_FLAG_LINK_UP))
 	{
-	  if (sif->lacp_enabled == 0)
-	    {
-	      bond_disable_collecting_distributing (vm, sif);
-	    }
+	  bond_disable_collecting_distributing (vm, sif);
 	}
-      else
+      else if (sif->port_enabled)
 	{
-	  if (sif->lacp_enabled == 0)
-	    {
-	      bond_enable_collecting_distributing (vm, sif);
-	    }
+	  bond_enable_collecting_distributing (vm, sif);
 	}
     }