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.

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

Co-Authored-By: Jian Wen <wenjianhn@gmail.com>
Partial-Bug: 1514056
Change-Id: I9801b76829021c9a0e6358982e1136637634a521
This commit is contained in:
Hynek Mlnarik 2016-02-25 11:34:15 +01:00
parent f3bb7efb0a
commit cacde308ee
8 changed files with 112 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):

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

@ -1090,6 +1090,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
# handle things like changing the datapath_type
br.create()
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
@ -1739,6 +1741,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:

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

@ -184,18 +184,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,
bridge=None):
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'),
bridge.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:
@ -206,9 +208,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,
@ -367,12 +369,13 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
ancillary_bridge=None,
trigger_resync=False,
failed_dev_up=False,
failed_dev_down=False):
failed_dev_down=False,
network=None):
self.ports = port_dicts
self.agent = self.create_agent(create_tunnels=create_tunnels,
ancillary_bridge=ancillary_bridge)
self.polling_manager = self.start_agent(self.agent, ports=self.ports)
self.network = self._create_test_network_dict()
self.network = network or self._create_test_network_dict()
if trigger_resync:
self._prepare_resync_trigger(self.agent)
elif failed_dev_up:
@ -382,3 +385,18 @@ class OVSAgentTestFramework(base.BaseOVSLinuxTestCase):
self._plug_ports(self.network, self.ports, self.agent,
bridge=ancillary_bridge)
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

@ -238,7 +238,54 @@ class TestOVSAgent(base.OVSAgentTestFramework):
self.assertEqual(patch_int_ofport_before, self.agent.patch_int_ofport)
self.assertEqual(patch_tun_ofport_before, self.agent.patch_tun_ofport)
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

@ -39,7 +39,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=self.stamp,
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)

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

@ -82,6 +82,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:%s' % self.MAP_TUN_BRIDGE]
self.INT_OFPORT = 11111
self.TUN_OFPORT = 22222
@ -109,6 +110,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,
@ -127,6 +130,10 @@ 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.add_port.return_value = self.MAP_TUN_INT_OFPORT
@ -157,7 +164,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()
@ -211,6 +224,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.create(secure_mode=True),
mock.call.setup_controllers(mock.ANY),
@ -286,6 +303,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.ipdevice, self.ipdevice_expected)
self._verify_mock_call(self.ipwrapper, self.ipwrapper_expected)
self._verify_mock_call(self.get_bridges, self.get_bridges_expected)
@ -530,8 +549,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
@ -547,10 +574,7 @@ class TunnelTest(object):
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')
update_stale.return_value = []
@ -593,7 +617,6 @@ class TunnelTest(object):
'added': set(['tap0'])}, False),
])
cleanup.assert_called_once_with()
self.assertTrue(update_stale.called)
self._verify_mock_calls()
@ -649,6 +672,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.create(secure_mode=True),
mock.call.setup_controllers(mock.ANY),