bonding: toggle bond admin state may cause hw link state to down

For bond interface, we set hardware link state to up initially when
admin state is set to up. Thereafter, if we toggle the admin state to
down and up, we require at least one active member in the bond prior
to bringing up the hardware state which is inconsistent. The fix is to
remove the unnecessary condition.

This is a rework of the original proposed patch which is more complex
and could be tricky to get it right
https://gerrit.fd.io/r/c/vpp/+/27141

Type: fix

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

diff --git a/src/vnet/bonding/device.c b/src/vnet/bonding/device.c
index 38759ee..bc18586 100644 (file)
--- a/src/vnet/bonding/device.c
+++ b/src/vnet/bonding/device.c
@@ -132,7 +132,7 @@ bond_interface_admin_up_down (vnet_main_t * vnm, u32 hw_if_index, u32 flags)
   bond_if_t *bif = pool_elt_at_index (bm->interfaces, hif->dev_instance);
 
   bif->admin_up = is_up;
-  if (is_up && vec_len (bif->active_members))
+  if (is_up)
     vnet_hw_interface_set_flags (vnm, bif->hw_if_index,
                                 VNET_HW_INTERFACE_FLAG_LINK_UP);
   return 0;

diff --git a/test/test_bond.py b/test/test_bond.py
index 911b1fb..c5d786f 100644 (file)
--- a/test/test_bond.py
+++ b/test/test_bond.py
@@ -3,12 +3,13 @@
 import socket
 import unittest
 
-from framework import VppTestCase, VppTestRunner
 from scapy.packet import Raw
 from scapy.layers.l2 import Ether
 from scapy.layers.inet import IP, UDP
+
+from framework import VppTestCase, VppTestRunner
 from vpp_bond_interface import VppBondInterface
-from vpp_papi import MACAddress
+from vpp_papi import MACAddress, VppEnum
 
 
 class TestBondInterface(VppTestCase):
@@ -275,6 +276,40 @@ class TestBondInterface(VppTestCase):
             sw_if_index=bond0.sw_if_index)
         self.assertFalse(bond0.is_interface_config_in_dump(if_dump))
 
+    def test_bond_link(self):
+        """ Bond hw interface link state test """
+
+        # for convenience
+        bond_modes = VppEnum.vl_api_bond_mode_t
+        intf_flags = VppEnum.vl_api_if_status_flags_t
+
+        # create interface 1 (BondEthernet0)
+        self.logger.info("Create bond interface")
+        # use round-robin mode to avoid negotiation required by LACP
+        bond0 = VppBondInterface(self,
+                                 mode=bond_modes.BOND_API_MODE_ROUND_ROBIN)
+        bond0.add_vpp_config()
+
+        # set bond admin up.
+        self.logger.info("set interface BondEthernet0 admin up")
+        bond0.admin_up()
+        # confirm link up
+        bond0.assert_interface_state(intf_flags.IF_STATUS_API_FLAG_ADMIN_UP,
+                                     intf_flags.IF_STATUS_API_FLAG_LINK_UP)
+
+        # toggle bond admin state
+        self.logger.info("toggle interface BondEthernet0")
+        bond0.admin_down()
+        bond0.admin_up()
+
+        # confirm link is still up
+        bond0.assert_interface_state(intf_flags.IF_STATUS_API_FLAG_ADMIN_UP,
+                                     intf_flags.IF_STATUS_API_FLAG_LINK_UP)
+
+        # delete BondEthernet0
+        self.logger.info("Deleting BondEthernet0")
+        bond0.remove_vpp_config()
+
 
 if __name__ == '__main__':
     unittest.main(testRunner=VppTestRunner)