Merge "Do not skip ports with ofport unset or invalid"

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",

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,

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,

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)

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',

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'}]