From d60febb2d3e141d03f033ac81136ba71922613a0 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Tue, 27 Oct 2020 21:55:39 +0100 Subject: [PATCH] Remove deprecated use_veth_interconnection option Using veth to interconnect openvswitch bridges was deprecated in Victoria cycle. Now it's time to remove it from the code. In neutron-ovs-agent code, there is still kept piece of code which migrates from the veth to the patch ports for bridges interconnection. We will be able to remove that piece of code in X release. Change-Id: I94545c3c3d9be46ac2062691f69663e5e59cd648 Closes-Bug: #1587296 --- .../internals/openvswitch_agent.rst | 5 +- neutron/cmd/sanity_check.py | 6 +- neutron/conf/plugins/ml2/drivers/ovs_conf.py | 9 -- .../openvswitch/agent/ovs_neutron_agent.py | 84 ++++-------- .../agent/test_ovs_neutron_agent.py | 40 ------ .../openvswitch/agent/test_ovs_tunnel.py | 128 ------------------ 6 files changed, 29 insertions(+), 243 deletions(-) diff --git a/doc/source/contributor/internals/openvswitch_agent.rst b/doc/source/contributor/internals/openvswitch_agent.rst index fcfd540cc12..5aa0f8a96ab 100644 --- a/doc/source/contributor/internals/openvswitch_agent.rst +++ b/doc/source/contributor/internals/openvswitch_agent.rst @@ -466,8 +466,7 @@ Implementation Trunk Bridge (Option C) This implementation is based on this `etherpad `_. Credits to Bence Romsics. -The option use_veth_interconnection=true won't be supported, it is deprecated since Victoria, -see [1]. The IDs used for bridge and port names are truncated. +The IDs used for bridge and port names are truncated. :: @@ -499,8 +498,6 @@ tpi-parent-id: int bridge side of the patch port that implements a trunk. spt-subport-id: trunk bridge side of the patch port that implements a subport. spi-subport-id: int bridge side of the patch port that implements a subport. -[1] https://bugs.launchpad.net/neutron/+bug/1587296 - Trunk creation ++++++++++++++ diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 86888a17187..7e201cd6de3 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -296,7 +296,7 @@ OPTS = [ help=_('Check for OVS Geneve support')), BoolOptCallback('iproute2_vxlan', check_iproute2_vxlan, default=False, help=_('Check for iproute2 vxlan support')), - BoolOptCallback('ovs_patch', check_ovs_patch, default=False, + BoolOptCallback('ovs_patch', check_ovs_patch, default=True, help=_('Check for patch port support')), BoolOptCallback('nova_notify', check_nova_notify, help=_('Check for nova notification support')), @@ -366,10 +366,6 @@ def enable_tests_from_config(): if ('vxlan' in cfg.CONF.ml2.type_drivers or cfg.CONF.VXLAN.enable_vxlan): cfg.CONF.set_default('iproute2_vxlan', True) - if cfg.CONF.AGENT.tunnel_types: - cfg.CONF.set_default('ovs_patch', True) - if not cfg.CONF.OVS.use_veth_interconnection: - cfg.CONF.set_default('ovs_patch', True) if (cfg.CONF.notify_nova_on_port_status_changes or cfg.CONF.notify_nova_on_port_data_changes): cfg.CONF.set_default('nova_notify', True) diff --git a/neutron/conf/plugins/ml2/drivers/ovs_conf.py b/neutron/conf/plugins/ml2/drivers/ovs_conf.py index ea060a7c0b3..2347aedd69e 100644 --- a/neutron/conf/plugins/ml2/drivers/ovs_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovs_conf.py @@ -100,15 +100,6 @@ ovs_opts = [ "See also: " "https://docs.openstack.org/api-ref/placement/" "#update-resource-provider-inventories")), - cfg.BoolOpt('use_veth_interconnection', default=False, - deprecated_for_removal=True, - deprecated_since="Victoria", - deprecated_reason="Patch ports should be used to provide " - "bridges interconnection.", - help=_("Use veths instead of patch ports to interconnect the " - "integration bridge to physical networks. " - "Support kernel without Open vSwitch patch port " - "support so long as it is set to True.")), cfg.StrOpt('datapath_type', default=constants.OVS_DATAPATH_SYSTEM, choices=[constants.OVS_DATAPATH_SYSTEM, constants.OVS_DATAPATH_NETDEV], diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 5be505931ed..8be0e21ff10 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -170,12 +170,6 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, datapath_type=ovs_conf.datapath_type) for b in ('br_int', 'br_phys', 'br_tun')) - self.use_veth_interconnection = ovs_conf.use_veth_interconnection - if self.use_veth_interconnection: - LOG.warning("Usage of veth instead of patch ports for bridges " - "interconnection is deprecated in Victoria and will " - "be removed in W release. Please use patch ports " - "instead.") self.veth_mtu = agent_conf.veth_mtu self.available_local_vlans = set(range(n_const.MIN_VLAN_TAG, n_const.MAX_VLAN_TAG + 1)) @@ -1488,7 +1482,6 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, self.int_ofports = {} self.phys_ofports = {} datapath_ids_set = set() - ip_wrapper = ip_lib.IPWrapper() ovs = ovs_lib.BaseOVS() ovs_bridges = ovs.get_bridges() for physical_network, bridge in bridge_mappings.items(): @@ -1526,45 +1519,31 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, # be same, so check only one of them. # Not logging error here, as the interface may not exist yet. # Type check is done to cleanup wrong interface if any. + + # TODO(slaweq) In X release we can remove code which is here just + # to move from old "veth" interconnection between bridges to the + # patch ports (L1527 - L1547) int_type = self.int_br.db_get_val("Interface", int_if_name, "type", log_errors=False) - if self.use_veth_interconnection: - # Drop ports if the interface types doesn't match the - # configuration value. - if int_type == 'patch': - self.int_br.delete_port(int_if_name) - br.delete_port(phys_if_name) - device = ip_lib.IPDevice(int_if_name) - if device.exists(): - device.link.delete() - # Give udev a chance to process its rules here, to avoid - # race conditions between commands launched by udev rules - # and the subsequent call to ip_wrapper.add_veth - utils.execute(['udevadm', 'settle', '--timeout=10']) - int_veth, phys_veth = ip_wrapper.add_veth(int_if_name, - phys_if_name) - int_ofport = self.int_br.add_port(int_if_name) - phys_ofport = br.add_port(phys_if_name) - else: - # Drop ports if the interface type doesn't match the - # configuration value - if int_type == 'veth': - self.int_br.delete_port(int_if_name) - br.delete_port(phys_if_name) + # Drop ports if the interface type doesn't match the + # configuration value + if int_type == 'veth': + self.int_br.delete_port(int_if_name) + br.delete_port(phys_if_name) - # Setup int_br to physical bridge patches. If they already - # exist we leave them alone, otherwise we create them but don't - # connect them until after the drop rules are in place. - if self.int_br.port_exists(int_if_name): - int_ofport = self.int_br.get_port_ofport(int_if_name) - else: - int_ofport = self.int_br.add_patch_port( - int_if_name, constants.NONEXISTENT_PEER) - if br.port_exists(phys_if_name): - phys_ofport = br.get_port_ofport(phys_if_name) - else: - phys_ofport = br.add_patch_port( - phys_if_name, constants.NONEXISTENT_PEER) + # Setup int_br to physical bridge patches. If they already + # exist we leave them alone, otherwise we create them but don't + # connect them until after the drop rules are in place. + if self.int_br.port_exists(int_if_name): + int_ofport = self.int_br.get_port_ofport(int_if_name) + else: + int_ofport = self.int_br.add_patch_port( + int_if_name, constants.NONEXISTENT_PEER) + if br.port_exists(phys_if_name): + phys_ofport = br.get_port_ofport(phys_if_name) + else: + phys_ofport = br.add_patch_port( + phys_if_name, constants.NONEXISTENT_PEER) self.int_ofports[physical_network] = int_ofport self.phys_ofports[physical_network] = phys_ofport @@ -1578,20 +1557,11 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, if not self.enable_distributed_routing: br.drop_port(in_port=phys_ofport) - if self.use_veth_interconnection: - # enable veth to pass traffic - int_veth.link.set_up() - phys_veth.link.set_up() - if self.veth_mtu: - # set up mtu size for veth interfaces - int_veth.link.set_mtu(self.veth_mtu) - phys_veth.link.set_mtu(self.veth_mtu) - else: - # associate patch ports to pass traffic - self.int_br.set_db_attribute('Interface', int_if_name, - 'options', {'peer': phys_if_name}) - br.set_db_attribute('Interface', phys_if_name, - 'options', {'peer': int_if_name}) + # associate patch ports to pass traffic + self.int_br.set_db_attribute('Interface', int_if_name, + 'options', {'peer': phys_if_name}) + br.set_db_attribute('Interface', phys_if_name, + 'options', {'peer': int_if_name}) def update_stale_ofport_rules(self): # ARP spoofing rules and drop-flow upon port-delete diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 7dd462ea24b..83bc269d5a0 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -1609,46 +1609,6 @@ class TestOvsNeutronAgent(object): def test_setup_physical_bridges_dvr_enabled(self): self._test_setup_physical_bridges(dvr_enabled=True) - def test_setup_physical_bridges_using_veth_interconnection(self): - self.agent.use_veth_interconnection = True - with mock.patch.object(ip_lib.IPDevice, "exists") as devex_fn,\ - mock.patch.object(sys, "exit"),\ - mock.patch.object(utils, "execute") as utilsexec_fn,\ - mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\ - mock.patch.object(self.agent, 'int_br') as int_br,\ - mock.patch.object(self.agent, '_check_bridge_datapath_id'),\ - mock.patch.object(ip_lib.IPWrapper, "add_veth") as addveth_fn,\ - mock.patch.object(ip_lib.IpLinkCommand, - "delete") as linkdel_fn,\ - mock.patch.object(ip_lib.IpLinkCommand, "set_up"),\ - mock.patch.object(ip_lib.IpLinkCommand, "set_mtu"),\ - mock.patch.object(ovs_lib.BaseOVS, "get_bridges") as get_br_fn: - devex_fn.return_value = True - parent = mock.MagicMock() - parent.attach_mock(utilsexec_fn, 'utils_execute') - parent.attach_mock(linkdel_fn, 'link_delete') - parent.attach_mock(addveth_fn, 'add_veth') - addveth_fn.return_value = (ip_lib.IPDevice("int-br-eth1"), - ip_lib.IPDevice("phy-br-eth1")) - phys_br = phys_br_cls() - phys_br.add_port.return_value = "phys_veth_ofport" - int_br.add_port.return_value = "int_veth_ofport" - get_br_fn.return_value = ["br-eth"] - self.agent.setup_physical_bridges({"physnet1": "br-eth"}) - expected_calls = [mock.call.link_delete(), - mock.call.utils_execute(['udevadm', - 'settle', - '--timeout=10']), - mock.call.add_veth('int-br-eth', - 'phy-br-eth')] - parent.assert_has_calls(expected_calls, any_order=False) - self.assertEqual("int_veth_ofport", - self.agent.int_ofports["physnet1"]) - self.assertEqual("phys_veth_ofport", - self.agent.phys_ofports["physnet1"]) - int_br.add_port.assert_called_with("int-br-eth") - phys_br.add_port.assert_called_with("phy-br-eth") - def _test_setup_physical_bridges_change_from_veth_to_patch_conf( self, port_exists=False): with mock.patch.object(sys, "exit"),\ diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index edccc6beb97..24cc555fd46 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -66,7 +66,6 @@ class DummyVlanBinding(object): class TunnelTest(object): - USE_VETH_INTERCONNECTION = False VETH_MTU = None def setUp(self): @@ -312,8 +311,6 @@ class TunnelTest(object): cfg.CONF.set_override('tunnel_types', ['gre'], 'AGENT') cfg.CONF.set_override('veth_mtu', self.VETH_MTU, 'AGENT') cfg.CONF.set_override('minimize_polling', False, 'AGENT') - cfg.CONF.set_override('use_veth_interconnection', - self.USE_VETH_INTERCONNECTION, 'OVS') for k, v in config_opts_agent.items(): cfg.CONF.set_override(k, v, 'AGENT') @@ -344,7 +341,6 @@ class TunnelTest(object): self._verify_mock_call(self.mock_aux_bridge, self.mock_aux_bridge_expected) self._verify_mock_call(self.ipdevice, self.ipdevice_expected) - self._verify_mock_call(self.ipwrapper, self.ipwrapper_expected) self._verify_mock_call(self.get_bridges, self.get_bridges_expected) self._verify_mock_call(self.inta, self.inta_expected) self._verify_mock_call(self.intb, self.intb_expected) @@ -673,127 +669,3 @@ class TunnelTest(object): class TunnelTestOSKen(TunnelTest, ovs_test_base.OVSOSKenTestBase): pass - - -class TunnelTestUseVethInterco(TunnelTest): - USE_VETH_INTERCONNECTION = True - - def _define_expected_calls(self, arp_responder=False, igmp_snooping=False): - self.mock_int_bridge_cls_expected = [ - mock.call(self.INT_BRIDGE, - datapath_type=mock.ANY), - ] - self.mock_phys_bridge_cls_expected = [ - mock.call(self.MAP_TUN_BRIDGE, - datapath_type=mock.ANY), - ] - self.mock_tun_bridge_cls_expected = [ - mock.call(self.TUN_BRIDGE, - datapath_type=mock.ANY), - ] - - self.mock_int_bridge_expected = [ - mock.call.create(), - mock.call.set_secure_mode(), - mock.call.setup_controllers(mock.ANY), - mock.call.set_igmp_snooping_state(igmp_snooping), - mock.call.setup_default_table(), - ] - - self.mock_map_tun_bridge_expected = [ - mock.call.create(), - mock.call.set_secure_mode(), - mock.call.setup_controllers(mock.ANY), - mock.call.setup_default_table(), - mock.call.add_port('phy-%s' % self.MAP_TUN_BRIDGE), - ] - self.mock_int_bridge_expected += [ - mock.call.db_get_val('Interface', 'int-%s' % self.MAP_TUN_BRIDGE, - 'type', log_errors=False), - mock.call.add_port('int-%s' % self.MAP_TUN_BRIDGE) - ] - - self.mock_int_bridge_expected += [ - mock.call.drop_port(in_port=self.MAP_TUN_INT_OFPORT), - ] - self.mock_map_tun_bridge_expected += [ - mock.call.drop_port(in_port=self.MAP_TUN_PHY_OFPORT), - ] - - self.mock_aux_bridge = self.ovs_bridges[self.AUX_BRIDGE] - self.mock_aux_bridge_expected = [ - ] - - self.mock_tun_bridge_expected = [ - mock.call.create(secure_mode=True), - mock.call.setup_controllers(mock.ANY), - mock.call.port_exists('patch-int'), - mock.ANY, - mock.call.add_patch_port('patch-int', 'patch-tun'), - ] - self.mock_int_bridge_expected += [ - mock.call.port_exists('patch-tun'), - mock.call.add_patch_port('patch-tun', 'patch-int') - ] - self.mock_int_bridge_expected += [ - mock.call.get_vif_ports((ovs_lib.INVALID_OFPORT, - ovs_lib.UNASSIGNED_OFPORT)), - mock.call.get_ports_attributes( - 'Port', columns=['name', 'other_config', 'tag'], ports=[]) - ] - self.mock_tun_bridge_expected += [ - mock.call.setup_default_table(self.INT_OFPORT, arp_responder), - ] - - self.ipdevice_expected = [ - mock.call('int-%s' % self.MAP_TUN_BRIDGE), - mock.call().exists(), - mock.ANY, - mock.call().link.delete() - ] - self.ipwrapper_expected = [ - mock.call(), - mock.call().add_veth('int-%s' % self.MAP_TUN_BRIDGE, - 'phy-%s' % self.MAP_TUN_BRIDGE) - ] - - self.get_bridges_expected = [mock.call(), mock.call()] - - self.inta_expected = [mock.call.link.set_up()] - self.intb_expected = [mock.call.link.set_up()] - self.execute_expected = [mock.call(['udevadm', 'settle', - '--timeout=10'])] - - self.mock_int_bridge_expected += [ - mock.call.install_goto( - dest_table_id=constants.LOCAL_MAC_DIRECT, - in_port=self.MAP_TUN_INT_OFPORT, - priority=4, table_id=constants.TRANSIENT_TABLE), - mock.call.install_goto( - dest_table_id=constants.LOCAL_MAC_DIRECT, - in_port=self.TUN_OFPORT, - priority=4, table_id=constants.TRANSIENT_TABLE), - mock.call.install_goto( - dest_table_id=constants.TRANSIENT_EGRESS_TABLE, - table_id=constants.LOCAL_MAC_DIRECT), - ] - - -class TunnelTestUseVethIntercoOSKen(TunnelTestUseVethInterco, - ovs_test_base.OVSOSKenTestBase): - pass - - -class TunnelTestWithMTU(TunnelTestUseVethInterco): - VETH_MTU = 1500 - - def _define_expected_calls(self, arp_responder=False, igmp_snooping=False): - super(TunnelTestWithMTU, self)._define_expected_calls( - arp_responder, igmp_snooping) - self.inta_expected.append(mock.call.link.set_mtu(self.VETH_MTU)) - self.intb_expected.append(mock.call.link.set_mtu(self.VETH_MTU)) - - -class TunnelTestWithMTUOSKen(TunnelTestWithMTU, - ovs_test_base.OVSOSKenTestBase): - pass