diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 5c2162b6636..ca192a9f8d3 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -632,22 +632,11 @@ class OVSBridge(BaseOVS): {'port_id': port_id, 'br_name': self.br_name}) continue pinfo = by_id[port_id] - if not self._check_ofport(port_id, pinfo): - continue mac = pinfo['external_ids'].get('attached-mac') result[port_id] = VifPort(pinfo['name'], pinfo['ofport'], port_id, mac, self) return result - @staticmethod - def _check_ofport(port_id, port_info): - if port_info['ofport'] in [UNASSIGNED_OFPORT, INVALID_OFPORT]: - LOG.warning("ofport: %(ofport)s for VIF: %(vif)s " - "is not a positive integer", - {'ofport': port_info['ofport'], 'vif': port_id}) - return False - return True - def get_vif_port_by_id(self, port_id): ports = self.ovsdb.db_find( 'Interface', ('external_ids', '=', {'iface-id': port_id}), @@ -656,8 +645,6 @@ class OVSBridge(BaseOVS): for port in ports: if self.br_name != self.get_bridge_for_iface(port['name']): continue - if not self._check_ofport(port_id, port): - continue mac = port['external_ids'].get('attached-mac') return VifPort(port['name'], port['ofport'], port_id, mac, self) LOG.info("Port %(port_id)s not present in bridge %(br_name)s", diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py index 48bb7af123a..0091118d6bb 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py @@ -23,6 +23,7 @@ import oslo_messaging from oslo_utils import excutils from osprofiler import profiler +from neutron.agent.common import ovs_lib from neutron.agent.linux.openvswitch_firewall import firewall as ovs_firewall from neutron.common import utils as n_utils from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants @@ -424,6 +425,13 @@ class OVSDVRNeutronAgent(object): local_compute_ports) vif_by_id = self.int_br.get_vifs_by_ids( [local_port['id'] for local_port in local_compute_ports]) + + # A router port has an OVS interface with type internal. Once the + # interface is created, a valid ofport will be assigned. + vif_by_id = {k: v for k, v in vif_by_id.items() + if not v or v.ofport not in + (ovs_lib.INVALID_OFPORT, ovs_lib.UNASSIGNED_OFPORT)} + for local_port in local_compute_ports: vif = vif_by_id.get(local_port['id']) if not vif: @@ -605,6 +613,10 @@ class OVSDVRNeutronAgent(object): "%(ofport)s, rebinding.", {'vif': port.vif_id, 'ofport': port.ofport}) self.unbind_port_from_dvr(port, local_vlan_map) + if port.ofport in (ovs_lib.INVALID_OFPORT, + ovs_lib.UNASSIGNED_OFPORT): + return + if device_owner == n_const.DEVICE_OWNER_DVR_INTERFACE: self._bind_distributed_router_interface_port(port, local_vlan_map, 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 66016b73686..5aa231d47f5 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1911,6 +1911,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, devices = devices_details_list.get('devices') vif_by_id = self.int_br.get_vifs_by_ids( [vif['device'] for vif in devices]) + devices_not_in_datapath = set() for details in devices: device = details['device'] LOG.debug("Processing port: %s", device) @@ -1923,6 +1924,9 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, skipped_devices.append(device) continue + if not port.ofport or port.ofport == ovs_lib.INVALID_OFPORT: + devices_not_in_datapath.add(device) + if 'port_id' in details: LOG.info("Port %(device)s updated. Details: %(details)s", {'device': device, 'details': details}) @@ -1942,7 +1946,9 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, details['network_id']) if details['device'] in re_added: self.ext_manager.delete_port(self.context, details) - self.ext_manager.handle_port(self.context, details) + if device not in devices_not_in_datapath: + self.ext_manager.handle_port(self.context, details) + else: if n_const.NO_ACTIVE_BINDING in details: # Port was added to the bridge, but its binding in this @@ -1958,7 +1964,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, if (port and port.ofport != -1): self.port_dead(port) return (skipped_devices, binding_no_activated_devices, - need_binding_devices, failed_devices) + need_binding_devices, failed_devices, devices_not_in_datapath) def _update_port_network(self, port_id, network_id): self._clean_network_ports(port_id) @@ -2051,10 +2057,12 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, need_binding_devices = [] skipped_devices = set() binding_no_activated_devices = set() + devices_not_in_datapath = set() start = time.time() if devices_added_updated: (skipped_devices, binding_no_activated_devices, - need_binding_devices, failed_devices['added']) = ( + need_binding_devices, failed_devices['added'], + devices_not_in_datapath) = ( self.treat_devices_added_or_updated( devices_added_updated, provisioning_needed, re_added)) LOG.info("process_network_ports - iteration:%(iter_num)d - " @@ -2079,11 +2087,11 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, added_ports = (port_info.get('added', set()) - skipped_devices - binding_no_activated_devices) self._add_port_tag_info(need_binding_devices) - self.process_install_ports_egress_flows(need_binding_devices) - - self.sg_agent.setup_port_filters(added_ports, + added_to_datapath = added_ports - devices_not_in_datapath + self.sg_agent.setup_port_filters(added_to_datapath, port_info.get('updated', set())) + LOG.info("process_network_ports - iteration:%(iter_num)d - " "agent port security group processed in %(elapsed).3f", {'iter_num': self.iter_num, diff --git a/neutron/tests/unit/agent/common/test_ovs_lib.py b/neutron/tests/unit/agent/common/test_ovs_lib.py index 0521ce055da..3d889efadb6 100644 --- a/neutron/tests/unit/agent/common/test_ovs_lib.py +++ b/neutron/tests/unit/agent/common/test_ovs_lib.py @@ -498,9 +498,9 @@ class OVS_Lib_Test(base.BaseTestCase): self.br.ovsdb.list_ports.return_value.execute.return_value = [ 'qvo1', 'qvo2', 'qvo4'] by_id = self.br.get_vifs_by_ids(['pid1', 'pid2', 'pid3', 'pid4']) - # pid3 isn't on bridge and pid4 doesn't have a valid ofport + # pid3 isn't on bridge self.assertIsNone(by_id['pid3']) - self.assertIsNone(by_id['pid4']) + self.assertEqual(-1, by_id['pid4'].ofport) self.assertEqual('pid1', by_id['pid1'].vif_id) self.assertEqual('qvo1', by_id['pid1'].port_name) self.assertEqual(1, by_id['pid1'].ofport) 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 57a79097ec0..7dd462ea24b 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 @@ -882,7 +882,7 @@ class TestOvsNeutronAgent(object): 'get_port_tag_dict', return_value={}),\ mock.patch.object(self.agent, func_name) as func: - skip_devs, _, need_bound_devices, _ = ( + skip_devs, _, need_bound_devices, _, _ = ( self.agent.treat_devices_added_or_updated([], False, set())) # The function should not raise self.assertFalse(skip_devs) @@ -900,7 +900,7 @@ class TestOvsNeutronAgent(object): 'get_vifs_by_ids', return_value={details['device']: port}),\ mock.patch.object(self.agent, 'port_dead') as func: - skip_devs, binding_no_activated_devices, _, _ = ( + skip_devs, binding_no_activated_devices, _, _, _ = ( self.agent.treat_devices_added_or_updated([], False, set())) self.assertFalse(skip_devs) self.assertTrue(func.called) @@ -977,7 +977,7 @@ class TestOvsNeutronAgent(object): [], False, set()) # The function should return False for resync and no device # processed - self.assertEqual((['the_skipped_one'], set(), [], set()), + self.assertEqual((['the_skipped_one'], set(), [], set(), set()), skip_devs) ext_mgr_delete_port.assert_called_once_with( self.agent.context, {'port_id': 'the_skipped_one'}) @@ -995,7 +995,7 @@ class TestOvsNeutronAgent(object): mock.patch.object(self.agent, 'treat_vif_port') as treat_vif_port: failed_devices = {'added': set(), 'removed': set()} - (_, _, _, failed_devices['added']) = ( + (_, _, _, failed_devices['added'], _) = ( self.agent.treat_devices_added_or_updated([], False, set())) # The function should return False for resync and no device # processed @@ -1026,7 +1026,7 @@ class TestOvsNeutronAgent(object): return_value={}),\ mock.patch.object(self.agent, 'treat_vif_port') as treat_vif_port: - skip_devs, _, need_bound_devices, _ = ( + skip_devs, _, need_bound_devices, _, _ = ( self.agent.treat_devices_added_or_updated([], False, set())) # The function should return False for resync self.assertFalse(skip_devs) @@ -1135,7 +1135,8 @@ class TestOvsNeutronAgent(object): self.agent, "treat_devices_added_or_updated", return_value=( skipped_devices, binding_no_activated_devices, [], - failed_devices['added'])) as device_added_updated,\ + failed_devices['added'], + set())) as device_added_updated,\ mock.patch.object(self.agent.int_br, "get_ports_attributes", return_value=[]),\ mock.patch.object(self.agent, @@ -3157,7 +3158,7 @@ class TestOvsDvrNeutronAgent(object): ), ] + self._expected_port_bound(self._port, lvid, network_type=network_type) - int_br.assert_has_calls(expected_on_int_br) + int_br.assert_has_calls(expected_on_int_br, any_order=True) tun_br.assert_not_called() phys_br.assert_has_calls(expected_on_phys_br) int_br.reset_mock() @@ -3241,7 +3242,7 @@ class TestOvsDvrNeutronAgent(object): lvid=lvid, ip_version=ip_version, gateway_ip=gateway_ip) - int_br.assert_has_calls(expected_on_int_br) + int_br.assert_has_calls(expected_on_int_br, any_order=True) tun_br.assert_has_calls(expected_on_tun_br) phys_br.assert_not_called() int_br.reset_mock() @@ -3489,7 +3490,8 @@ class TestOvsDvrNeutronAgent(object): False) lvid = self.agent.vlan_manager.get(self._net_uuid).vlan int_br.assert_has_calls( - self._expected_port_bound(self._port, lvid)) + self._expected_port_bound(self._port, lvid), + any_order=True) expected_on_tun_br = [ mock.call.provision_local_vlan(network_type='vxlan', lvid=lvid, segmentation_id=None, distributed=True), @@ -3594,7 +3596,7 @@ class TestOvsDvrNeutronAgent(object): False) lvid = self.agent.vlan_manager.get(self._net_uuid).vlan int_br.assert_has_calls( - self._expected_port_bound(self._port, lvid)) + self._expected_port_bound(self._port, lvid), any_order=True) expected_on_tun_br = [ mock.call.provision_local_vlan( network_type='vxlan', 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 734f0e42fa1..914bf7d231d 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 @@ -14,6 +14,7 @@ # under the License. # +import collections import time from unittest import mock @@ -30,6 +31,8 @@ from neutron.tests.unit.plugins.ml2.drivers.openvswitch.agent \ import test_vlanmanager +Switch = collections.namedtuple('Switch', ['br_name']) + # Useful global dummy variables. NET_UUID = '3faeebfe-5d37-11e1-a64b-000c29d5f0a7' LS_ID = 420 @@ -38,8 +41,8 @@ LV_IDS = [42, 43] VIF_ID = '404deaec-5d37-11e1-a64b-000c29d5f0a8' VIF_MAC = '3c:09:24:1e:78:23' OFPORT_NUM = 1 -VIF_PORT = ovs_lib.VifPort('port', OFPORT_NUM, - VIF_ID, VIF_MAC, 'switch') +VIF_PORT = ovs_lib.VifPort('port', OFPORT_NUM, VIF_ID, VIF_MAC, + Switch(br_name='br_name')) VIF_PORTS = {VIF_ID: VIF_PORT} FIXED_IPS = [{'subnet_id': 'my-subnet-uuid', 'ip_address': '1.1.1.1'}]