openstack
/
neutron
Code Issues Proposed changes

Cleanup stale OVS flows for physical bridges 

Perform deletion of the stale flows in physical bridges consistently with
br-int and br-tun, respecting drop_flows_on_start configuration option.
Added tests for auxiliary bridge and functional tests for the physical
bridge using VLAN/flat external network. Fixes part of the bug 1514056;
together with [1] and [2], the bug should be considered fixed.

The commit also fixes inconsistency between netmask of allocated IP
addresses assigned in _create_test_port_dict and ip_len in _plug_ports
of base.py.

Further, this commit sets agent UUID to physical bridges similarly to
tun and int bridges. This is necessary for stale flows cleanup to work
correctly. In upstream, it is treated using OVSBridgeCookieMixin.

[1] https://review.openstack.org/#/c/297211/
[2] https://review.openstack.org/#/c/297818/

Conflicts:
	neutron/tests/functional/agent/l2/base.py
	neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py

Co-Authored-By: Jian Wen <wenjianhn@gmail.com>
Co-Authored-By: Clayton O'Neill <clayton@oneill.net>
Partial-Bug: 1514056
Change-Id: I9801b76829021c9a0e6358982e1136637634a521
(cherry picked from commit cacde308ee)
This commit is contained in:
Hynek Mlnarik 2016-02-25 11:34:15 +01:00 committed by Ihar Hrachyshka
parent 260cbe1d2f
commit 8d29f38356
9 changed files with 135 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py
View File

@ -30,7 +30,6 @@ class OVSPhysicalBridge(ovs_bridge.OVSAgentBridge,
dvr_process_next_table_id = constants.LOCAL_VLAN_TRANSLATION
def setup_default_table(self):
self.delete_flows()
self.install_normal()
@staticmethod

1
neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_phys.py
View File

@ -30,7 +30,6 @@ class OVSPhysicalBridge(ovs_bridge.OVSAgentBridge,
dvr_process_next_table_id = constants.LOCAL_VLAN_TRANSLATION
def setup_default_table(self):
self.delete_flows()
self.install_normal()
def provision_local_vlan(self, port, lvid, segmentation_id, distributed):

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

@ -1132,7 +1132,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
'bridge': bridge})
sys.exit(1)
br = self.br_phys_cls(bridge)
br.set_agent_uuid_stamp(self.agent_uuid_stamp)
br.setup_controllers(self.conf)
if cfg.CONF.AGENT.drop_flows_on_start:
br.delete_flows()
br.setup_default_table()
self.phys_brs[physical_network] = br
@ -1694,6 +1697,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
def cleanup_stale_flows(self):
bridges = [self.int_br]
bridges.extend(self.phys_brs.values())
if self.enable_tunneling:
bridges.append(self.tun_br)
for bridge in bridges:

45
neutron/tests/functional/agent/l2/base.py
View File

@ -61,6 +61,8 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
prefix='br-int')
self.br_tun = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN,
prefix='br-tun')
self.br_phys = base.get_rand_name(n_const.DEVICE_NAME_MAX_LEN,
prefix='br-phys')
patch_name_len = n_const.DEVICE_NAME_MAX_LEN - len("-patch-tun")
self.patch_tun = "%s-patch-tun" % self.br_int[patch_name_len:]
self.patch_int = "%s-patch-int" % self.br_tun[patch_name_len:]
@ -106,7 +108,9 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
else:
tunnel_types = None
local_ip = '192.168.10.1'
bridge_mappings = {'physnet': self.br_int}
bridge_mappings = {'physnet': self.br_phys}
# Physical bridges should be created prior to running
self._bridge_classes()['br_phys'](self.br_phys).create()
agent = ovs_agent.OVSNeutronAgent(self._bridge_classes(),
self.br_int, self.br_tun,
local_ip, bridge_mappings,
@ -155,16 +159,20 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
return {'id': uuidutils.generate_uuid(),
'tenant_id': uuidutils.generate_uuid()}
def _plug_ports(self, network, ports, agent, ip_len=24):
def _plug_ports(self, network, ports, agent,
bridge=None, namespace=None):
if namespace is None:
namespace = self.namespace
for port in ports:
bridge = bridge or agent.int_br
self.driver.plug(
network.get('id'), port.get('id'), port.get('vif_name'),
port.get('mac_address'),
agent.int_br.br_name, namespace=self.namespace)
ip_cidrs = ["%s/%s" % (port.get('fixed_ips')[0][
'ip_address'], ip_len)]
bridge.br_name, namespace=namespace)
ip_cidrs = ["%s/8" % (port.get('fixed_ips')[0][
'ip_address'])]
self.driver.init_l3(port.get('vif_name'), ip_cidrs,
namespace=self.namespace)
namespace=namespace)
def _unplug_ports(self, ports, agent):
for port in ports:
@ -175,9 +183,9 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
dev = {'device': port['id'],
'port_id': port['id'],
'network_id': network['id'],
'network_type': 'vlan',
'physical_network': 'physnet',
'segmentation_id': 1,
'network_type': network.get('network_type', 'vlan'),
'physical_network': network.get('physical_network', 'physnet'),
'segmentation_id': network.get('segmentation_id', 1),
'fixed_ips': port['fixed_ips'],
'device_owner': 'compute',
'port_security_enabled': True,
@ -279,11 +287,26 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
timeout=timeout)
def setup_agent_and_ports(self, port_dicts, create_tunnels=True,
trigger_resync=False):
trigger_resync=False, network=None):
self.agent = self.create_agent(create_tunnels=create_tunnels)
self.start_agent(self.agent)
self.network = self._create_test_network_dict()
self.network = network or self._create_test_network_dict()
self.ports = port_dicts
if trigger_resync:
self._prepare_resync_trigger(self.agent)
self._plug_ports(self.network, self.ports, self.agent)
def plug_ports_to_phys_br(self, network, ports, namespace=None):
physical_network = network.get('physical_network', 'physnet')
phys_segmentation_id = network.get('segmentation_id', None)
network_type = network.get('network_type', 'flat')
phys_br = self.agent.phys_brs[physical_network]
self._plug_ports(network, ports, self.agent, bridge=phys_br,
namespace=namespace)
if phys_segmentation_id and network_type == 'vlan':
for port in ports:
phys_br.set_db_attribute(
"Port", port['vif_name'], "tag", phys_segmentation_id)

47
neutron/tests/functional/agent/test_l2_ovs_agent.py
View File

@ -130,7 +130,54 @@ class TestOVSAgent(base.OVSAgentTestFramework):
self.assertEqual(patch_phys_ofport_before,
self.agent.phys_ofports['physnet'])
def test_assert_pings_during_br_phys_setup_not_lost_in_vlan_to_flat(self):
provider_net = self._create_test_network_dict()
provider_net['network_type'] = 'flat'
self._test_assert_pings_during_br_phys_setup_not_lost(provider_net)
def test_assert_pings_during_br_phys_setup_not_lost_in_vlan_to_vlan(self):
provider_net = self._create_test_network_dict()
provider_net['network_type'] = 'vlan'
provider_net['segmentation_id'] = 876
self._test_assert_pings_during_br_phys_setup_not_lost(provider_net)
def _test_assert_pings_during_br_phys_setup_not_lost(self, provider_net):
# Separate namespace is needed when pinging from one port to another,
# otherwise Linux ping uses loopback instead for sending and receiving
# ping, hence ignoring flow setup.
ns_phys = self.useFixture(net_helpers.NamespaceFixture()).name
ports = self.create_test_ports(amount=2)
port_int = ports[0]
port_phys = ports[1]
ip_int = port_int['fixed_ips'][0]['ip_address']
ip_phys = port_phys['fixed_ips'][0]['ip_address']
self.setup_agent_and_ports(port_dicts=[port_int], create_tunnels=False,
network=provider_net)
self.plug_ports_to_phys_br(provider_net, [port_phys],
namespace=ns_phys)
# The OVS agent doesn't monitor the physical bridges, no notification
# is sent when a port is up on a physical bridge, hence waiting only
# for the ports connected to br-int
self.wait_until_ports_state([port_int], up=True)
with net_helpers.async_ping(ns_phys, [ip_int]) as done:
while not done():
self.agent.setup_physical_bridges(self.agent.bridge_mappings)
time.sleep(0.25)
with net_helpers.async_ping(self.namespace, [ip_phys]) as done:
while not done():
self.agent.setup_physical_bridges(self.agent.bridge_mappings)
time.sleep(0.25)
def test_noresync_after_port_gone(self):
'''This will test the scenario where a port is removed after listing
it but before getting vif info about it.
'''

1
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_phys.py
View File

@ -38,7 +38,6 @@ class OVSPhysicalBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase,
self.br.setup_default_table()
(dp, ofp, ofpp) = self._get_dp()
expected = [
call.delete_flows(),
call._send_msg(ofpp.OFPFlowMod(dp,
cookie=0,
instructions=[

1
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_phys.py
View File

@ -37,7 +37,6 @@ class OVSPhysicalBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase,
def test_setup_default_table(self):
self.br.setup_default_table()
expected = [
call.delete_flows(),
call.add_flow(priority=0, table=0, actions='normal'),
]
self.assertEqual(expected, self.mock.mock_calls)

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

@ -880,6 +880,7 @@ class TestOvsNeutronAgent(object):
self.agent.setup_physical_bridges({"physnet1": "br-eth"})
expected_calls = [
mock.call.phys_br_cls('br-eth'),
mock.call.phys_br.set_agent_uuid_stamp(mock.ANY),
mock.call.phys_br.setup_controllers(mock.ANY),
mock.call.phys_br.setup_default_table(),
mock.call.int_br.db_get_val('Interface', 'int-br-eth',
@ -964,6 +965,7 @@ class TestOvsNeutronAgent(object):
self.agent.setup_physical_bridges({"physnet1": "br-eth"})
expected_calls = [
mock.call.phys_br_cls('br-eth'),
mock.call.phys_br.set_agent_uuid_stamp(mock.ANY),
mock.call.phys_br.setup_controllers(mock.ANY),
mock.call.phys_br.setup_default_table(),
mock.call.int_br.delete_port('int-br-eth'),

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

@ -81,6 +81,7 @@ class TunnelTest(object):
self.INT_BRIDGE = 'integration_bridge'
self.TUN_BRIDGE = 'tunnel_bridge'
self.MAP_TUN_BRIDGE = 'tun_br_map'
self.AUX_BRIDGE = 'ancillary_bridge'
self.NET_MAPPING = {'net1': self.MAP_TUN_BRIDGE}
self.INT_OFPORT = 11111
self.TUN_OFPORT = 22222
@ -104,6 +105,8 @@ class TunnelTest(object):
self.br_tun_cls('br-tun')),
self.MAP_TUN_BRIDGE: mock.create_autospec(
self.br_phys_cls('br-phys')),
self.AUX_BRIDGE: mock.create_autospec(
ovs_lib.OVSBridge('br-aux')),
}
self.ovs_int_ofports = {
'patch-tun': self.TUN_OFPORT,
@ -122,8 +125,13 @@ class TunnelTest(object):
self.mock_tun_bridge_cls = mock.patch(self._BR_TUN_CLASS,
autospec=True).start()
self.mock_tun_bridge_cls.side_effect = lookup_br
self.mock_aux_bridge_cls = mock.patch(
'neutron.agent.common.ovs_lib.OVSBridge',
autospec=True).start()
self.mock_aux_bridge_cls.side_effect = lookup_br
self.mock_int_bridge = self.ovs_bridges[self.INT_BRIDGE]
self.mock_int_bridge.br_name = self.INT_BRIDGE
self.mock_int_bridge.add_port.return_value = self.MAP_TUN_INT_OFPORT
self.mock_int_bridge.add_patch_port.side_effect = (
lambda tap, peer: self.ovs_int_ofports[tap])
@ -143,6 +151,7 @@ class TunnelTest(object):
ovs_lib.INVALID_OFPORT)
self.mock_tun_bridge = self.ovs_bridges[self.TUN_BRIDGE]
self.mock_tun_bridge.br_name = self.TUN_BRIDGE
self.mock_tun_bridge.add_port.return_value = self.INT_OFPORT
self.mock_tun_bridge.add_patch_port.return_value = self.INT_OFPORT
@ -159,7 +168,13 @@ class TunnelTest(object):
'get_bridges').start()
self.get_bridges.return_value = [self.INT_BRIDGE,
self.TUN_BRIDGE,
self.MAP_TUN_BRIDGE]
self.MAP_TUN_BRIDGE,
self.AUX_BRIDGE]
self.get_bridge_external_bridge_id = mock.patch.object(
ovs_lib.BaseOVS,
'get_bridge_external_bridge_id').start()
self.get_bridge_external_bridge_id.side_effect = (
lambda bridge: bridge if bridge in self.ovs_bridges else None)
self.execute = mock.patch('neutron.agent.common.utils.execute').start()
@ -189,6 +204,7 @@ class TunnelTest(object):
]
self.mock_map_tun_bridge_expected = [
mock.call.set_agent_uuid_stamp(mock.ANY),
mock.call.setup_controllers(mock.ANY),
mock.call.setup_default_table(),
mock.call.get_port_ofport('phy-%s' % self.MAP_TUN_BRIDGE),
@ -216,6 +232,10 @@ class TunnelTest(object):
'options', {'peer': 'int-%s' % self.MAP_TUN_BRIDGE}),
]
self.mock_aux_bridge = self.ovs_bridges[self.AUX_BRIDGE]
self.mock_aux_bridge_expected = [
]
self.mock_tun_bridge_expected = [
mock.call.set_agent_uuid_stamp(mock.ANY),
mock.call.bridge_exists(mock.ANY),
@ -286,6 +306,8 @@ class TunnelTest(object):
self.mock_map_tun_bridge_expected)
self._verify_mock_call(self.mock_tun_bridge,
self.mock_tun_bridge_expected)
self._verify_mock_call(self.mock_aux_bridge,
self.mock_aux_bridge_expected)
self._verify_mock_call(self.device_exists, self.device_exists_expected)
self._verify_mock_call(self.ipdevice, self.ipdevice_expected)
self._verify_mock_call(self.ipwrapper, self.ipwrapper_expected)
@ -517,8 +539,16 @@ class TunnelTest(object):
self.mock_int_bridge_expected += [
mock.call.check_canary_table(),
mock.call.cleanup_flows(),
mock.call.check_canary_table()
]
self.mock_tun_bridge_expected += [
mock.call.cleanup_flows()
]
self.mock_map_tun_bridge_expected += [
mock.call.cleanup_flows()
]
# No cleanup is expected on ancillary bridge
self.ovs_bridges[self.INT_BRIDGE].check_canary_table.return_value = \
constants.OVS_NORMAL
@ -526,24 +556,31 @@ class TunnelTest(object):
'exception') as log_exception,\
mock.patch.object(self.mod_agent.OVSNeutronAgent,
'scan_ports') as scan_ports,\
mock.patch.object(
self.mod_agent.OVSNeutronAgent,
'scan_ancillary_ports') as scan_ancillary_ports,\
mock.patch.object(
self.mod_agent.OVSNeutronAgent,
'process_network_ports') as process_network_ports,\
mock.patch.object(
self.mod_agent.OVSNeutronAgent,
'process_ancillary_network_ports') as process_anc_ports,\
mock.patch.object(self.mod_agent.OVSNeutronAgent,
'tunnel_sync'),\
mock.patch.object(time, 'sleep'),\
mock.patch.object(
self.mod_agent.OVSNeutronAgent,
'update_stale_ofport_rules') as update_stale,\
mock.patch.object(
self.mod_agent.OVSNeutronAgent,
'cleanup_stale_flows') as cleanup:
'update_stale_ofport_rules') as update_stale:
log_exception.side_effect = Exception(
'Fake exception to get out of the loop')
scan_ports.side_effect = [reply2, reply3]
scan_ancillary_ports.return_value = {
'current': set([]), 'added': set([]), 'removed': set([]),
}
update_stale.return_value = []
process_network_ports.side_effect = [
False, Exception('Fake exception to get out of the loop')]
process_anc_ports.return_value = False
n_agent = self._build_agent()
@ -572,7 +609,6 @@ class TunnelTest(object):
'added': set([])}, False)
])
cleanup.assert_called_once_with()
self.assertTrue(update_stale.called)
self._verify_mock_calls()
@ -607,10 +643,12 @@ class TunnelTestUseVethInterco(TunnelTest):
mock.call.create(),
mock.call.set_secure_mode(),
mock.call.setup_controllers(mock.ANY),
mock.call.setup_default_table(),
]
self.mock_map_tun_bridge_expected = [
mock.call.set_agent_uuid_stamp(mock.ANY),
mock.call.setup_controllers(mock.ANY),
mock.call.setup_default_table(),
mock.call.add_port(self.intb),
@ -628,6 +666,10 @@ class TunnelTestUseVethInterco(TunnelTest):
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.set_agent_uuid_stamp(mock.ANY),
mock.call.bridge_exists(mock.ANY),