Don't unnecessarily loop through all ports/interfaces

The ovs-vsctl 'list' command can take a list of records as an
argument, so there is no need to manually loop through all records
discarding the ones with names that don't match the bridge's
port name list.

Also, since the data is returned as json, the ofport returned isn't
a string, so we don't have to convert it to int for testing.

Closes-Bug: #1404935
Change-Id: Ica20f8f8315196aa5a04657d28dde41a75f0368d

diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py
index 81c72b745c67e9f7876e44c7fcab1300db923283..90c76db97f4bb1ce6d792ab2870af00168687c0d 100644 (file)
--- a/neutron/agent/linux/ovs_lib.py
+++ b/neutron/agent/linux/ovs_lib.py
@@ -14,6 +14,7 @@
 #    under the License.
 
 import itertools
+import numbers
 import operator
 
 from oslo.config import cfg
@@ -327,41 +328,32 @@ class OVSBridge(BaseOVS):
         return edge_ports
 
     def get_vif_port_set(self):
-        port_names = self.get_port_name_list()
         edge_ports = set()
-        args = ['--format=json', '--', '--columns=name,external_ids,ofport',
-                'list', 'Interface']
+        args = ['--format=json', '--', '--columns=external_ids,ofport',
+                'list', 'Interface'] + self.get_port_name_list()
         result = self.run_vsctl(args, check_error=True)
         if not result:
             return edge_ports
         for row in jsonutils.loads(result)['data']:
-            name = row[0]
-            if name not in port_names:
-                continue
-            external_ids = dict(row[1][1])
+            external_ids = dict(row[0][1])
             # Do not consider VIFs which aren't yet ready
             # This can happen when ofport values are either [] or ["set", []]
             # We will therefore consider only integer values for ofport
-            ofport = row[2]
-            try:
-                int_ofport = int(ofport)
-            except (ValueError, TypeError):
+            ofport = row[1]
+            if not isinstance(ofport, numbers.Integral):
                 LOG.warn(_LW("Found not yet ready openvswitch port: %s"), row)
-            else:
-                if int_ofport > 0:
-                    if ("iface-id" in external_ids and
-                        "attached-mac" in external_ids):
-                        edge_ports.add(external_ids['iface-id'])
-                    elif ("xs-vif-uuid" in external_ids and
-                          "attached-mac" in external_ids):
-                        # if this is a xenserver and iface-id is not
-                        # automatically synced to OVS from XAPI, we grab it
-                        # from XAPI directly
-                        iface_id = self.get_xapi_iface_id(
-                            external_ids["xs-vif-uuid"])
-                        edge_ports.add(iface_id)
-                else:
-                    LOG.warn(_LW("Found failed openvswitch port: %s"), row)
+            elif ofport < 1:
+                LOG.warn(_LW("Found failed openvswitch port: %s"), row)
+            elif 'attached-mac' in external_ids:
+                if "iface-id" in external_ids:
+                    edge_ports.add(external_ids['iface-id'])
+                elif "xs-vif-uuid" in external_ids:
+                    # if this is a xenserver and iface-id is not
+                    # automatically synced to OVS from XAPI, we grab it
+                    # from XAPI directly
+                    iface_id = self.get_xapi_iface_id(
+                        external_ids["xs-vif-uuid"])
+                    edge_ports.add(iface_id)
         return edge_ports
 
     def get_port_tag_dict(self):
@@ -379,15 +371,13 @@ class OVSBridge(BaseOVS):
         in the "Interface" table queried by the get_vif_port_set() method.
 
         """
-        port_names = self.get_port_name_list()
         args = ['--format=json', '--', '--columns=name,tag', 'list', 'Port']
+        args += self.get_port_name_list()
         result = self.run_vsctl(args, check_error=True)
         port_tag_dict = {}
         if not result:
             return port_tag_dict
         for name, tag in jsonutils.loads(result)['data']:
-            if name not in port_names:
-                continue
             # 'tag' can be [u'set', []] or an integer
             if isinstance(tag, list):
                 tag = tag[1]

diff --git a/neutron/tests/unit/agent/linux/test_ovs_lib.py b/neutron/tests/unit/agent/linux/test_ovs_lib.py
index b6780a968aa344996a0cb397da1f02e228cc98d6..4b12156ad2bea1af3d4522d09cb65e4c349799b4 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_ovs_lib.py
+++ b/neutron/tests/unit/agent/linux/test_ovs_lib.py
@@ -625,20 +625,18 @@ class OVS_Lib_Test(base.BaseTestCase):
         else:
             id_key = 'iface-id'
 
-        headings = ['name', 'external_ids']
+        headings = ['external_ids', 'ofport']
         data = [
             # A vif port on this bridge:
-            ['tap99', {id_key: 'tap99id', 'attached-mac': 'tap99mac'}, 1],
+            [{id_key: 'tap99id', 'attached-mac': 'tap99mac'}, 1],
             # A vif port on this bridge not yet configured
-            ['tap98', {id_key: 'tap98id', 'attached-mac': 'tap98mac'}, []],
+            [{id_key: 'tap98id', 'attached-mac': 'tap98mac'}, []],
             # Another vif port on this bridge not yet configured
-            ['tap97', {id_key: 'tap97id', 'attached-mac': 'tap97mac'},
+            [{id_key: 'tap97id', 'attached-mac': 'tap97mac'},
              ['set', []]],
 
-            # A vif port on another bridge:
-            ['tap88', {id_key: 'tap88id', 'attached-mac': 'tap88id'}, 1],
             # Non-vif port on this bridge:
-            ['tun22', {}, 2],
+            [{}, 2],
         ]
 
         # Each element is a tuple of (expected mock call, return_value)
@@ -647,8 +645,8 @@ class OVS_Lib_Test(base.BaseTestCase):
                        root_helper=self.root_helper),
              'tap99\ntun22'),
             (mock.call(["ovs-vsctl", self.TO, "--format=json",
-                        "--", "--columns=name,external_ids,ofport",
-                        "list", "Interface"],
+                        "--", "--columns=external_ids,ofport",
+                        "list", "Interface", 'tap99', 'tun22'],
                        root_helper=self.root_helper),
              self._encode_ovs_json(headings, data)),
         ]
@@ -703,8 +701,8 @@ class OVS_Lib_Test(base.BaseTestCase):
                        root_helper=self.root_helper),
              'tap99\n'),
             (mock.call(["ovs-vsctl", self.TO, "--format=json",
-                        "--", "--columns=name,external_ids,ofport",
-                        "list", "Interface"],
+                        "--", "--columns=external_ids,ofport",
+                        "list", "Interface", "tap99"],
                        root_helper=self.root_helper),
              RuntimeError()),
         ]