From: Terry Wilson Date: Sat, 20 Dec 2014 02:01:36 +0000 (-0700) Subject: Don't unnecessarily loop through all ports/interfaces X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3f0bf6cfac2e151d5a4a7f076062b3365bdbf457;p=openstack-build%2Fneutron-build.git 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 81c72b745..90c76db97 100644 --- 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 b6780a968..4b12156ad 100644 --- 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()), ]