From c717a6365c1aecb0e3957f0857a06f2334f99d5d Mon Sep 17 00:00:00 2001 From: Matthew Thode Date: Mon, 9 Feb 2015 11:02:58 -0600 Subject: [PATCH] replaces enumeration method used to get a list of interfaces 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 | 1 + etc/neutron/rootwrap.d/dhcp.filters | 1 + etc/neutron/rootwrap.d/l3.filters | 1 + .../rootwrap.d/linuxbridge-plugin.filters | 1 + .../rootwrap.d/openvswitch-plugin.filters | 1 + neutron/agent/linux/ip_lib.py | 41 +++++++------ neutron/tests/unit/test_linux_ip_lib.py | 59 ++++++------------- 7 files changed, 45 insertions(+), 60 deletions(-) diff --git a/etc/neutron/rootwrap.d/cisco-apic.filters b/etc/neutron/rootwrap.d/cisco-apic.filters index 69e4afcc891..a74a3602d0b 100644 --- a/etc/neutron/rootwrap.d/cisco-apic.filters +++ b/etc/neutron/rootwrap.d/cisco-apic.filters @@ -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 diff --git a/etc/neutron/rootwrap.d/dhcp.filters b/etc/neutron/rootwrap.d/dhcp.filters index 989384d4885..20d2800116c 100644 --- a/etc/neutron/rootwrap.d/dhcp.filters +++ b/etc/neutron/rootwrap.d/dhcp.filters @@ -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 diff --git a/etc/neutron/rootwrap.d/l3.filters b/etc/neutron/rootwrap.d/l3.filters index a3818a2dcf7..e826f13cf9d 100644 --- a/etc/neutron/rootwrap.d/l3.filters +++ b/etc/neutron/rootwrap.d/l3.filters @@ -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 diff --git a/etc/neutron/rootwrap.d/linuxbridge-plugin.filters b/etc/neutron/rootwrap.d/linuxbridge-plugin.filters index 03df39592c7..1e0b891b973 100644 --- a/etc/neutron/rootwrap.d/linuxbridge-plugin.filters +++ b/etc/neutron/rootwrap.d/linuxbridge-plugin.filters @@ -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 diff --git a/etc/neutron/rootwrap.d/openvswitch-plugin.filters b/etc/neutron/rootwrap.d/openvswitch-plugin.filters index b63a83b9438..ed7f1ce78c2 100644 --- a/etc/neutron/rootwrap.d/openvswitch-plugin.filters +++ b/etc/neutron/rootwrap.d/openvswitch-plugin.filters @@ -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 diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index e37d6a536a5..1c4aa49b543 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -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: + 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)) + ) + + for name in output: + if exclude_loopback and name == LOOPBACK_DEVNAME: 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() + retval.append(IPDevice(name, namespace=self.namespace)) - if exclude_loopback and name == LOOPBACK_DEVNAME: - continue - - retval.append(IPDevice(name, namespace=self.namespace)) return retval def add_tuntap(self, name, mode='tap'): diff --git a/neutron/tests/unit/test_linux_ip_lib.py b/neutron/tests/unit/test_linux_ip_lib.py index 7af31632c72..2400dc887e8 100644 --- a/neutron/tests/unit/test_linux_ip_lib.py +++ b/neutron/tests/unit/test_linux_ip_lib.py @@ -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')]) + mocked_islink.assert_called_once_with('/sys/class/net/lo') + self.assertEqual(retval, [ip_lib.IPDevice('lo')]) - 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) + @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)