diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py index a3aad0fc7ed..9b5d5754760 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py @@ -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 diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_phys.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_phys.py index e76b7ddaf7f..ff9d87e5405 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_phys.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/br_phys.py @@ -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): 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 2a1f3498a38..c1a8917a31a 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -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: diff --git a/neutron/tests/functional/agent/l2/base.py b/neutron/tests/functional/agent/l2/base.py index 39e511b11df..672b3122ab9 100644 --- a/neutron/tests/functional/agent/l2/base.py +++ b/neutron/tests/functional/agent/l2/base.py @@ -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) diff --git a/neutron/tests/functional/agent/test_l2_ovs_agent.py b/neutron/tests/functional/agent/test_l2_ovs_agent.py index bf8682bd08b..92e85247d37 100644 --- a/neutron/tests/functional/agent/test_l2_ovs_agent.py +++ b/neutron/tests/functional/agent/test_l2_ovs_agent.py @@ -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. ''' diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_phys.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_phys.py index a478f9a8b61..9ddb72987e6 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_phys.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_phys.py @@ -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=[ diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_phys.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_phys.py index 47ca3177dc9..ab372977bff 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_phys.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/test_br_phys.py @@ -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) 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 2076c41fd52..4e881f56e18 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 @@ -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'), 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 5c6f749e34a..cd1d03527a5 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 @@ -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),