openstack/neutron
Code Issues Proposed changes

Remove deprecated use_veth_interconnection option

Browse Source 
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
This commit is contained in:
Slawek Kaplonski
2020-10-27 21:55:39 +01:00
parent 6397a03ed8
commit d60febb2d3
6 changed files with 29 additions and 243 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
doc/source/contributor/internals/openvswitch_agent.rst
View File

@ -466,8 +466,7 @@ Implementation Trunk Bridge (Option C)
This implementation is based on this `etherpad <https://etherpad.openstack.org/p/trunk-bridge-tagged-patch-experiment>`_.
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
++++++++++++++

6
neutron/cmd/sanity_check.py
View File

@ -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)

9
neutron/conf/plugins/ml2/drivers/ovs_conf.py
View File

@ -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],

84
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
View File

@ -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

40
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
View File

@ -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"),\

128
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py
View File

@ -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