From: Matthew Smith Date: Fri, 29 May 2020 17:29:45 +0000 (-0500) Subject: bonding: Revert adjust link state X-Git-Tag: v21.01-rc0~324 X-Git-Url: https://gerrit.fd.io/r/gitweb?a=commitdiff_plain;h=7285103a744dda9e4910677cc44a5eeb091a7a5a;p=vpp.git bonding: Revert adjust link state This reverts commit 9121c415a91904be50071ec55143d9c89b2f1b91. Reason for revert: causes deadlock with multiple worker threads Type: fix Change-Id: Icf3d6d343d99d887abacbaa03acbf04d4628514e Signed-off-by: Matthew Smith --- diff --git a/src/vnet/bonding/cli.c b/src/vnet/bonding/cli.c index c40d4f391f6..92a9ff067f6 100644 --- a/src/vnet/bonding/cli.c +++ b/src/vnet/bonding/cli.c @@ -26,7 +26,6 @@ void bond_disable_collecting_distributing (vlib_main_t * vm, slave_if_t * sif) { bond_main_t *bm = &bond_main; - vnet_main_t *vnm = vnet_get_main (); bond_if_t *bif; int i; uword p; @@ -55,15 +54,6 @@ bond_disable_collecting_distributing (vlib_main_t * vm, slave_if_t * sif) ASSERT (bif->n_numa_slaves >= 0); } } - /* If that was the last active slave, set bond link state down */ - if (!vec_len (bif->active_slaves)) - { - vnet_hw_interface_flags_t flags; - - flags = vnet_hw_interface_get_flags (vnm, bif->hw_if_index); - flags &= ~VNET_HW_INTERFACE_FLAG_LINK_UP; - vnet_hw_interface_set_flags (vnm, bif->hw_if_index, flags); - } break; } } @@ -168,16 +158,6 @@ bond_enable_collecting_distributing (vlib_main_t * vm, slave_if_t * sif) else vec_add1 (bif->active_slaves, sif->sw_if_index); - /* If this was the first active slave, set bond link state up */ - if (vec_len (bif->active_slaves) == 1) - { - vnet_hw_interface_flags_t flags; - - flags = vnet_hw_interface_get_flags (vnm, bif->hw_if_index); - flags |= VNET_HW_INTERFACE_FLAG_LINK_UP; - vnet_hw_interface_set_flags (vnm, bif->hw_if_index, flags); - } - sif->is_local_numa = (vm->numa_node == hw->numa_node) ? 1 : 0; if (bif->mode == BOND_MODE_ACTIVE_BACKUP) { @@ -489,6 +469,9 @@ bond_create_if (vlib_main_t * vm, bond_create_if_args_t * args) if (vlib_get_thread_main ()->n_vlib_mains > 1) clib_spinlock_init (&bif->lockp); + vnet_hw_interface_set_flags (vnm, bif->hw_if_index, + VNET_HW_INTERFACE_FLAG_LINK_UP); + hash_set (bm->bond_by_sw_if_index, bif->sw_if_index, bif->dev_instance); // for return diff --git a/test/test_bond.py b/test/test_bond.py index dab0dd1d26b..dd4a6453977 100644 --- a/test/test_bond.py +++ b/test/test_bond.py @@ -8,7 +8,7 @@ from scapy.packet import Raw from scapy.layers.l2 import Ether from scapy.layers.inet import IP, UDP from vpp_bond_interface import VppBondInterface -from vpp_papi import MACAddress, VppEnum +from vpp_papi import MACAddress class TestBondInterface(VppTestCase): @@ -272,63 +272,6 @@ class TestBondInterface(VppTestCase): if_dump = self.vapi.sw_interface_bond_dump() 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() - - # initially admin state is down and link is down - bond0.assert_interface_state(0, 0) - - # set bond admin up. confirm link down because no slaves are active - self.logger.info("set interface BondEthernet0 admin up") - bond0.admin_up() - bond0.assert_interface_state(intf_flags.IF_STATUS_API_FLAG_ADMIN_UP, 0) - - # make sure slaves are down. enslave them to bond. - self.logger.info("set interface pg0 admin down") - self.pg0.admin_down() - self.logger.info("bond enslave interface pg0 to BondEthernet0") - bond0.enslave_vpp_bond_interface(sw_if_index=self.pg0.sw_if_index, - is_passive=0, - is_long_timeout=0) - self.logger.info("set interface pg1 admin down") - self.pg1.admin_down() - self.logger.info("bond enslave interface pg1 to BondEthernet0") - bond0.enslave_vpp_bond_interface(sw_if_index=self.pg1.sw_if_index, - is_passive=0, - is_long_timeout=0) - - # bring slaves up, confirm bond link is up - self.logger.info("set interface pg0 admin up") - self.pg0.admin_up() - self.logger.info("set interface pg1 admin up") - self.pg1.admin_up() - bond0.assert_interface_state(intf_flags.IF_STATUS_API_FLAG_ADMIN_UP, - intf_flags.IF_STATUS_API_FLAG_LINK_UP) - - # detach pg0, pg1 - self.logger.info("detach interface pg0") - bond0.detach_vpp_bond_interface(sw_if_index=self.pg0.sw_if_index) - self.logger.info("detach interface pg1") - bond0.detach_vpp_bond_interface(sw_if_index=self.pg1.sw_if_index) - - # link should be down now - bond0.assert_interface_state(intf_flags.IF_STATUS_API_FLAG_ADMIN_UP, 0) - - # delete BondEthernet0 - self.logger.info("Deleting BondEthernet0") - bond0.remove_vpp_config() - if __name__ == '__main__': unittest.main(testRunner=VppTestRunner)