]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
replaces enumeration method used to get a list of interfaces
authorMatthew Thode <mthode@mthode.org>
Mon, 9 Feb 2015 17:02:58 +0000 (11:02 -0600)
committerMatthew Thode <mthode@mthode.org>
Sun, 15 Mar 2015 02:15:53 +0000 (21:15 -0500)
ip_lib was parsing tunnel links incorrectly. We can create interface
names with any character the filesystem supports (not '..', '/', ':').
Given this we do not know what to delimit on so parsing iproute2 output
is probably not a good idea.

I asked the iproute2 devs what the proper way we should get interface
names is and was told NOT to parse iproute2 output but to use something
like sysfs instead.  http://www.spinics.net/lists/netdev/msg316577.html

This patch pulls interfaces from sysfs (/sys/class/net) and verifies them
via checking if they are links (bonding creates files for instance and
needs to be skipped).

Currently it is not possible without jumping through a ton of hoops to
access a network namespace without iproute2 or cython, so we use ip to
run find to find the correct sysfs directory.  We also only call out to
iproute2 _ONLY_ if needed.

Change-Id: I07d1d297f07857d216649cccf717896574aac301
Closes-Bug: 1374663

etc/neutron/rootwrap.d/cisco-apic.filters
etc/neutron/rootwrap.d/dhcp.filters
etc/neutron/rootwrap.d/l3.filters
etc/neutron/rootwrap.d/linuxbridge-plugin.filters
etc/neutron/rootwrap.d/openvswitch-plugin.filters
neutron/agent/linux/ip_lib.py
neutron/tests/unit/test_linux_ip_lib.py

index 69e4afcc8910a04c842908d190dd3a965291d4ce..a74a3602d0b004f6249dad0dee5e9d5213b5f922 100644 (file)
@@ -13,4 +13,5 @@ lldpctl: CommandFilter, lldpctl, root
 
 # ip_lib filters
 ip: IpFilter, ip, root
+find: RegExpFilter, find, root, find, /sys/class/net, -maxdepth, 1, -type, l, -printf, %.*
 ip_exec: IpNetnsExecFilter, ip, root
index 989384d4885ef17aa88d8b822226fdc2b9302ee4..20d2800116c49e80212aff1975085ad46755e1ff 100644 (file)
@@ -32,4 +32,5 @@ kill_metadata7: KillFilter, root, python2.7, -9
 
 # ip_lib
 ip: IpFilter, ip, root
+find: RegExpFilter, find, root, find, /sys/class/net, -maxdepth, 1, -type, l, -printf, %.*
 ip_exec: IpNetnsExecFilter, ip, root
index a3818a2dcf780b0698a20b1cc7887e1638d4a106..e826f13cf9df5f9415dc17598c818ddf8c31d652 100644 (file)
@@ -29,6 +29,7 @@ kill_radvd: KillFilter, root, /sbin/radvd, -9, -HUP
 
 # ip_lib
 ip: IpFilter, ip, root
+find: RegExpFilter, find, root, find, /sys/class/net, -maxdepth, 1, -type, l, -printf, %.*
 ip_exec: IpNetnsExecFilter, ip, root
 
 # For ip monitor
index 03df39592c7711e6ba1b953f4054b38eb62c901a..1e0b891b973ea686812d9d05b0fd745165d3e034 100644 (file)
@@ -16,4 +16,5 @@ bridge: CommandFilter, bridge, root
 
 # ip_lib
 ip: IpFilter, ip, root
+find: RegExpFilter, find, root, find, /sys/class/net, -maxdepth, 1, -type, l, -printf, %.*
 ip_exec: IpNetnsExecFilter, ip, root
index b63a83b9438a71a9b848ac0cbf21fcb176228968..ed7f1ce78c2cea432e4b316bc0f35e1f2feb7e7b 100644 (file)
@@ -19,4 +19,5 @@ xe: CommandFilter, xe, root
 
 # ip_lib
 ip: IpFilter, ip, root
+find: RegExpFilter, find, root, find, /sys/class/net, -maxdepth, 1, -type, l, -printf, %.*
 ip_exec: IpNetnsExecFilter, ip, root
index e37d6a536a5003e3d3ed63ab1b9001c1bb44233c..1c4aa49b54346a99f2e75e366d65a02354ae6854 100644 (file)
@@ -15,6 +15,7 @@
 
 import eventlet
 import netaddr
+import os
 from oslo_config import cfg
 from oslo_log import log as logging
 
@@ -32,11 +33,8 @@ OPTS = [
 
 
 LOOPBACK_DEVNAME = 'lo'
-# NOTE(ethuleau): depend of the version of iproute2, the vlan
-# interface details vary.
-VLAN_INTERFACE_DETAIL = ['vlan protocol 802.1q',
-                         'vlan protocol 802.1Q',
-                         'vlan id']
+
+SYS_NET_PATH = '/sys/class/net'
 
 
 class SubProcessBase(object):
@@ -93,22 +91,27 @@ class IPWrapper(SubProcessBase):
 
     def get_devices(self, exclude_loopback=False):
         retval = []
-        output = self._run(['o', 'd'], 'link', ('list',))
-        for line in output.split('\n'):
-            if '<' not in line:
-                continue
-            tokens = line.split(' ', 2)
-            if len(tokens) == 3:
-                if any(v in tokens[2] for v in VLAN_INTERFACE_DETAIL):
-                    delimiter = '@'
-                else:
-                    delimiter = ':'
-                name = tokens[1].rpartition(delimiter)[0].strip()
+        if self.namespace:
+            # we call out manually because in order to avoid screen scraping
+            # iproute2 we use find to see what is in the sysfs directory, as
+            # suggested by Stephen Hemminger (iproute2 dev).
+            output = utils.execute(['ip', 'netns', 'exec', self.namespace,
+                                    'find', SYS_NET_PATH, '-maxdepth', '1',
+                                    '-type', 'l', '-printf', '%f '],
+                                   run_as_root=True,
+                                   log_fail_as_error=self.log_fail_as_error
+                                   ).split()
+        else:
+            output = (
+                i for i in os.listdir(SYS_NET_PATH)
+                if os.path.islink(os.path.join(SYS_NET_PATH, i))
+            )
 
-                if exclude_loopback and name == LOOPBACK_DEVNAME:
-                    continue
+        for name in output:
+            if exclude_loopback and name == LOOPBACK_DEVNAME:
+                continue
+            retval.append(IPDevice(name, namespace=self.namespace))
 
-                retval.append(IPDevice(name, namespace=self.namespace))
         return retval
 
     def add_tuntap(self, name, mode='tap'):
index 7af31632c72d454fddd8c9b51893063ad969a827..2400dc887e82ec61db52c62f44e95d758114237f 100644 (file)
@@ -17,6 +17,7 @@ import mock
 import netaddr
 
 from neutron.agent.linux import ip_lib
+from neutron.agent.linux import utils  # noqa
 from neutron.common import exceptions
 from neutron.tests import base
 
@@ -235,49 +236,25 @@ class TestIpWrapper(base.BaseTestCase):
         self.execute_p = mock.patch.object(ip_lib.IPWrapper, '_execute')
         self.execute = self.execute_p.start()
 
-    def test_get_devices(self):
-        self.execute.return_value = '\n'.join(LINK_SAMPLE)
+    @mock.patch('os.path.islink')
+    @mock.patch('os.listdir', return_value=['lo'])
+    def test_get_devices(self, mocked_listdir, mocked_islink):
         retval = ip_lib.IPWrapper().get_devices()
-        self.assertEqual(retval,
-                         [ip_lib.IPDevice('lo'),
-                          ip_lib.IPDevice('eth0'),
-                          ip_lib.IPDevice('br-int'),
-                          ip_lib.IPDevice('gw-ddc717df-49'),
-                          ip_lib.IPDevice('foo:foo'),
-                          ip_lib.IPDevice('foo@foo'),
-                          ip_lib.IPDevice('foo:foo@foo'),
-                          ip_lib.IPDevice('foo@foo:foo'),
-                          ip_lib.IPDevice('bar.9'),
-                          ip_lib.IPDevice('bar'),
-                          ip_lib.IPDevice('bar:bar'),
-                          ip_lib.IPDevice('bar@bar'),
-                          ip_lib.IPDevice('bar:bar@bar'),
-                          ip_lib.IPDevice('bar@bar:bar')])
-
-        self.execute.assert_called_once_with(['o', 'd'], 'link', ('list',),
-                                             log_fail_as_error=True)
-
-    def test_get_devices_malformed_line(self):
-        self.execute.return_value = '\n'.join(LINK_SAMPLE + ['gibberish'])
-        retval = ip_lib.IPWrapper().get_devices()
-        self.assertEqual(retval,
-                         [ip_lib.IPDevice('lo'),
-                          ip_lib.IPDevice('eth0'),
-                          ip_lib.IPDevice('br-int'),
-                          ip_lib.IPDevice('gw-ddc717df-49'),
-                          ip_lib.IPDevice('foo:foo'),
-                          ip_lib.IPDevice('foo@foo'),
-                          ip_lib.IPDevice('foo:foo@foo'),
-                          ip_lib.IPDevice('foo@foo:foo'),
-                          ip_lib.IPDevice('bar.9'),
-                          ip_lib.IPDevice('bar'),
-                          ip_lib.IPDevice('bar:bar'),
-                          ip_lib.IPDevice('bar@bar'),
-                          ip_lib.IPDevice('bar:bar@bar'),
-                          ip_lib.IPDevice('bar@bar:bar')])
-
-        self.execute.assert_called_once_with(['o', 'd'], 'link', ('list',),
-                                             log_fail_as_error=True)
+        mocked_islink.assert_called_once_with('/sys/class/net/lo')
+        self.assertEqual(retval, [ip_lib.IPDevice('lo')])
+
+    @mock.patch('neutron.agent.linux.utils.execute')
+    def test_get_devices_namespaces(self, mocked_execute):
+        fake_str = mock.Mock()
+        fake_str.split.return_value = ['lo']
+        mocked_execute.return_value = fake_str
+        retval = ip_lib.IPWrapper(namespace='foo').get_devices()
+        mocked_execute.assert_called_once_with(
+                ['ip', 'netns', 'exec', 'foo', 'find', '/sys/class/net',
+                 '-maxdepth', '1', '-type', 'l', '-printf', '%f '],
+                run_as_root=True, log_fail_as_error=True)
+        self.assertTrue(fake_str.split.called)
+        self.assertEqual(retval, [ip_lib.IPDevice('lo', namespace='foo')])
 
     def test_get_namespaces(self):
         self.execute.return_value = '\n'.join(NETNS_SAMPLE)