diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index bf7793ec4b8..3c4894c41cf 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -25,6 +25,7 @@ from neutron_lib import exceptions from oslo_config import cfg from oslo_log import log as logging from oslo_utils import excutils +from pyroute2.netlink.rtnl import ifinfmsg from pyroute2 import netns import six @@ -503,33 +504,41 @@ class IpLinkCommand(IpDeviceCommandBase): COMMAND = 'link' def set_address(self, mac_address): - self._as_root([], ('set', self.name, 'address', mac_address)) + privileged.set_link_attribute( + self.name, self._parent.namespace, address=mac_address) def set_allmulticast_on(self): - self._as_root([], ('set', self.name, 'allmulticast', 'on')) + privileged.set_link_flags( + self.name, self._parent.namespace, ifinfmsg.IFF_ALLMULTI) def set_mtu(self, mtu_size): - self._as_root([], ('set', self.name, 'mtu', mtu_size)) + privileged.set_link_attribute( + self.name, self._parent.namespace, mtu=mtu_size) def set_up(self): - return self._as_root([], ('set', self.name, 'up')) + privileged.set_link_attribute( + self.name, self._parent.namespace, state='up') def set_down(self): - return self._as_root([], ('set', self.name, 'down')) + privileged.set_link_attribute( + self.name, self._parent.namespace, state='down') def set_netns(self, namespace): - self._as_root([], ('set', self.name, 'netns', namespace)) + privileged.set_link_attribute( + self.name, self._parent.namespace, net_ns_fd=namespace) self._parent.namespace = namespace def set_name(self, name): - self._as_root([], ('set', self.name, 'name', name)) + privileged.set_link_attribute( + self.name, self._parent.namespace, ifname=name) self._parent.name = name def set_alias(self, alias_name): - self._as_root([], ('set', self.name, 'alias', alias_name)) + privileged.set_link_attribute( + self.name, self._parent.namespace, ifalias=alias_name) def delete(self): - self._as_root([], ('delete', self.name)) + privileged.delete_interface(self.name, self._parent.namespace) @property def address(self): @@ -539,6 +548,10 @@ class IpLinkCommand(IpDeviceCommandBase): def state(self): return self.attributes.get('state') + @property + def allmulticast(self): + return self.attributes.get('allmulticast') + @property def mtu(self): return self.attributes.get('mtu') @@ -557,19 +570,8 @@ class IpLinkCommand(IpDeviceCommandBase): @property def attributes(self): - return self._parse_line(self._run(['o'], ('show', self.name))) - - def _parse_line(self, value): - if not value: - return {} - - device_name, settings = value.replace("\\", '').split('>', 1) - tokens = settings.split() - keys = tokens[::2] - values = [int(v) if v.isdigit() else v for v in tokens[1::2]] - - retval = dict(zip(keys, values)) - return retval + return privileged.get_link_attributes(self.name, + self._parent.namespace) class IpAddrCommand(IpDeviceCommandBase): @@ -1103,7 +1105,6 @@ def network_namespace_exists(namespace, **kwargs): def ensure_device_is_ready(device_name, namespace=None): dev = IPDevice(device_name, namespace=namespace) - dev.set_log_fail_as_error(False) try: # Ensure the device has a MAC address and is up, even if it is already # up. If the device doesn't exist, a RuntimeError will be raised. diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 18dd32effd5..8c9e10b4e6f 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -16,6 +16,7 @@ import socket from neutron_lib import constants import pyroute2 from pyroute2.netlink import rtnl +from pyroute2.netlink.rtnl import ifinfmsg from pyroute2.netlink.rtnl import ndmsg from pyroute2 import NetlinkError from pyroute2 import netns @@ -115,7 +116,7 @@ def _get_link_id(device, namespace): raise NetworkInterfaceNotFound(msg) -def _run_iproute_link(command, device, namespace, **kwargs): +def _run_iproute_link(command, device, namespace=None, **kwargs): try: with _get_iproute(namespace) as ip: idx = _get_link_id(device, namespace) @@ -234,6 +235,33 @@ def interface_exists(ifname, namespace): raise +@privileged.default.entrypoint +def set_link_flags(device, namespace, flags): + link = _run_iproute_link("get", device, namespace)[0] + new_flags = flags | link['flags'] + return _run_iproute_link("set", device, namespace, flags=new_flags) + + +@privileged.default.entrypoint +def set_link_attribute(device, namespace, **attributes): + return _run_iproute_link("set", device, namespace, **attributes) + + +@privileged.default.entrypoint +def get_link_attributes(device, namespace): + link = _run_iproute_link("get", device, namespace)[0] + return { + 'mtu': link.get_attr('IFLA_MTU'), + 'qlen': link.get_attr('IFLA_TXQLEN'), + 'state': link.get_attr('IFLA_OPERSTATE'), + 'qdisc': link.get_attr('IFLA_QDISC'), + 'brd': link.get_attr('IFLA_BROADCAST'), + 'link/ether': link.get_attr('IFLA_ADDRESS'), + 'alias': link.get_attr('IFLA_IFALIAS'), + 'allmulticast': bool(link['flags'] & ifinfmsg.IFF_ALLMULTI) + } + + @privileged.default.entrypoint def add_neigh_entry(ip_version, ip_address, mac_address, device, namespace, **kwargs): diff --git a/neutron/tests/functional/agent/linux/test_ip_lib.py b/neutron/tests/functional/agent/linux/test_ip_lib.py index 30b1d8c0404..c9882b86d79 100644 --- a/neutron/tests/functional/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/agent/linux/test_ip_lib.py @@ -480,6 +480,14 @@ class IpLibTestCase(IpLibTestFramework): self.assertEqual(1450, device.link.mtu) + def test_set_link_allmulticast_on(self): + attr = self.generate_device_details() + device = self.manage_device(attr) + + self.assertFalse(device.link.allmulticast) + device.link.set_allmulticast_on() + self.assertTrue(device.link.allmulticast) + def test_set_link_netns(self): attr = self.generate_device_details() device = self.manage_device(attr) diff --git a/neutron/tests/unit/agent/l3/test_dvr_snat_ns.py b/neutron/tests/unit/agent/l3/test_dvr_snat_ns.py index fa7270973d0..69575d6a314 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_snat_ns.py +++ b/neutron/tests/unit/agent/l3/test_dvr_snat_ns.py @@ -37,10 +37,11 @@ class TestDvrSnatNs(base.BaseTestCase): self.driver, use_ipv6=False) + @mock.patch('neutron.privileged.agent.linux.ip_lib.set_link_attribute') @mock.patch.object(utils, 'execute') @mock.patch.object(ip_lib, 'create_network_namespace') @mock.patch.object(ip_lib, 'network_namespace_exists') - def test_create(self, exists, create, execute): + def test_create(self, exists, create, execute, set_link_attr): exists.return_value = False self.snat_ns.create() diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index a86ae3b341c..15fb8ea2c56 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -20,6 +20,7 @@ import mock import netaddr from neutron_lib import exceptions import pyroute2 +from pyroute2.netlink.rtnl import ifinfmsg from pyroute2.netlink.rtnl import ndmsg from pyroute2 import NetlinkError import testtools @@ -36,53 +37,6 @@ NETNS_SAMPLE = [ 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', 'cccccccc-cccc-cccc-cccc-cccccccccccc'] -LINK_SAMPLE = [ - '1: lo: mtu 16436 qdisc noqueue state UNKNOWN \\' - 'link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 promiscuity 0', - '2: eth0: mtu 1500 qdisc mq state UP ' - 'qlen 1000\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff' - '\ alias openvswitch', - '3: br-int: mtu 1500 qdisc noop state DOWN ' - '\ link/ether aa:bb:cc:dd:ee:ff brd ff:ff:ff:ff:ff:ff promiscuity 0', - '4: gw-ddc717df-49: mtu 1500 qdisc noop ' - 'state DOWN \ link/ether fe:dc:ba:fe:dc:ba brd ff:ff:ff:ff:ff:ff ' - 'promiscuity 0', - '5: foo:foo: mtu 1500 qdisc mq state ' - 'UP qlen 1000\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff ' - 'promiscuity 0', - '6: foo@foo: mtu 1500 qdisc mq state ' - 'UP qlen 1000\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff ' - 'promiscuity 0', - '7: foo:foo@foo: mtu 1500 qdisc mq ' - 'state UP qlen 1000' - '\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0', - '8: foo@foo:foo: mtu 1500 qdisc mq ' - 'state UP qlen 1000' - '\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0', - '9: bar.9@eth0: mtu 1500 qdisc ' - ' noqueue master brq0b24798c-07 state UP mode DEFAULT' - '\ link/ether ab:04:49:b6:ab:a0 brd ff:ff:ff:ff:ff:ff promiscuity 0' - '\ vlan protocol 802.1q id 9 ', - '10: bar@eth0: mtu 1500 qdisc ' - ' noqueue master brq0b24798c-07 state UP mode DEFAULT' - '\ link/ether ab:04:49:b6:ab:a0 brd ff:ff:ff:ff:ff:ff promiscuity 0' - '\ vlan protocol 802.1Q id 10 ', - '11: bar:bar@eth0: mtu 1500 qdisc mq ' - 'state UP qlen 1000' - '\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0' - '\ vlan id 11 ', - '12: bar@bar@eth0: mtu 1500 qdisc mq ' - 'state UP qlen 1000' - '\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0' - '\ vlan id 12 ', - '13: bar:bar@bar@eth0: mtu 1500 ' - 'qdisc mq state UP qlen 1000' - '\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0' - '\ vlan protocol 802.1q id 13 ', - '14: bar@bar:bar@eth0: mtu 1500 ' - 'qdisc mq state UP qlen 1000' - '\ link/ether cc:dd:ee:ff:ab:cd brd ff:ff:ff:ff:ff:ff promiscuity 0' - '\ vlan protocol 802.1Q id 14 '] ADDR_SAMPLE = (""" 2: eth0: mtu 1500 qdisc mq state UP qlen 1000 @@ -784,85 +738,72 @@ class TestIpRuleCommand(TestIPCmdBase): class TestIpLinkCommand(TestIPCmdBase): def setUp(self): super(TestIpLinkCommand, self).setUp() - self.parent._run.return_value = LINK_SAMPLE[1] self.command = 'link' self.link_cmd = ip_lib.IpLinkCommand(self.parent) - def test_set_address(self): + @mock.patch.object(priv_lib, 'set_link_attribute') + def test_set_address(self, set_link_attribute): self.link_cmd.set_address('aa:bb:cc:dd:ee:ff') - self._assert_sudo([], ('set', 'eth0', 'address', 'aa:bb:cc:dd:ee:ff')) + set_link_attribute.assert_called_once_with( + self.parent.name, self.parent.namespace, + address='aa:bb:cc:dd:ee:ff') - def test_set_allmulticast_on(self): + @mock.patch.object(priv_lib, 'set_link_flags') + def test_set_allmulticast_on(self, set_link_flags): self.link_cmd.set_allmulticast_on() - self._assert_sudo([], ('set', 'eth0', 'allmulticast', 'on')) + set_link_flags.assert_called_once_with( + self.parent.name, self.parent.namespace, ifinfmsg.IFF_ALLMULTI) - def test_set_mtu(self): + @mock.patch.object(priv_lib, 'set_link_attribute') + def test_set_mtu(self, set_link_attribute): self.link_cmd.set_mtu(1500) - self._assert_sudo([], ('set', 'eth0', 'mtu', 1500)) + set_link_attribute.assert_called_once_with( + self.parent.name, self.parent.namespace, mtu=1500) - def test_set_up(self): - observed = self.link_cmd.set_up() - self.assertEqual(self.parent._as_root.return_value, observed) - self._assert_sudo([], ('set', 'eth0', 'up')) + @mock.patch.object(priv_lib, 'set_link_attribute') + def test_set_up(self, set_link_attribute): + self.link_cmd.set_up() + set_link_attribute.assert_called_once_with( + self.parent.name, self.parent.namespace, state='up') - def test_set_down(self): - observed = self.link_cmd.set_down() - self.assertEqual(self.parent._as_root.return_value, observed) - self._assert_sudo([], ('set', 'eth0', 'down')) + @mock.patch.object(priv_lib, 'set_link_attribute') + def test_set_down(self, set_link_attribute): + self.link_cmd.set_down() + set_link_attribute.assert_called_once_with( + self.parent.name, self.parent.namespace, state='down') - def test_set_netns(self): + @mock.patch.object(priv_lib, 'set_link_attribute') + def test_set_netns(self, set_link_attribute): + original_namespace = self.parent.namespace self.link_cmd.set_netns('foo') - self._assert_sudo([], ('set', 'eth0', 'netns', 'foo')) + set_link_attribute.assert_called_once_with( + 'eth0', original_namespace, net_ns_fd='foo') self.assertEqual(self.parent.namespace, 'foo') - def test_set_name(self): + @mock.patch.object(priv_lib, 'set_link_attribute') + def test_set_name(self, set_link_attribute): + original_name = self.parent.name self.link_cmd.set_name('tap1') - self._assert_sudo([], ('set', 'eth0', 'name', 'tap1')) + set_link_attribute.assert_called_once_with( + original_name, self.parent.namespace, ifname='tap1') self.assertEqual(self.parent.name, 'tap1') - def test_set_alias(self): + @mock.patch.object(priv_lib, 'set_link_attribute') + def test_set_alias(self, set_link_attribute): self.link_cmd.set_alias('openvswitch') - self._assert_sudo([], ('set', 'eth0', 'alias', 'openvswitch')) + set_link_attribute.assert_called_once_with( + self.parent.name, self.parent.namespace, ifalias='openvswitch') - def test_delete(self): + @mock.patch.object(priv_lib, 'delete_interface') + def test_delete(self, delete): self.link_cmd.delete() - self._assert_sudo([], ('delete', 'eth0')) + delete.assert_called_once_with(self.parent.name, self.parent.namespace) - def test_address_property(self): - self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1]) - self.assertEqual(self.link_cmd.address, 'cc:dd:ee:ff:ab:cd') - - def test_mtu_property(self): - self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1]) - self.assertEqual(self.link_cmd.mtu, 1500) - - def test_qdisc_property(self): - self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1]) - self.assertEqual(self.link_cmd.qdisc, 'mq') - - def test_qlen_property(self): - self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1]) - self.assertEqual(self.link_cmd.qlen, 1000) - - def test_alias_property(self): - self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1]) - self.assertEqual(self.link_cmd.alias, 'openvswitch') - - def test_state_property(self): - self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1]) - self.assertEqual(self.link_cmd.state, 'UP') - - def test_settings_property(self): - expected = {'mtu': 1500, - 'qlen': 1000, - 'state': 'UP', - 'qdisc': 'mq', - 'brd': 'ff:ff:ff:ff:ff:ff', - 'link/ether': 'cc:dd:ee:ff:ab:cd', - 'alias': 'openvswitch'} - self.parent._execute = mock.Mock(return_value=LINK_SAMPLE[1]) - self.assertEqual(self.link_cmd.attributes, expected) - self._assert_call(['o'], ('show', 'eth0')) + @mock.patch.object(priv_lib, 'get_link_attributes') + def test_settings_property(self, get_link_attributes): + self.link_cmd.attributes + get_link_attributes.assert_called_once_with( + self.parent.name, self.parent.namespace) class TestIpAddrCommand(TestIPCmdBase): @@ -1394,10 +1335,15 @@ class TestDeviceExists(base.BaseTestCase): self.assertFalse(ip_lib.ensure_device_is_ready("eth0")) def test_ensure_device_is_ready_no_link_address(self): - with mock.patch.object(ip_lib.IPDevice, '_execute') as _execute: - # Use lo, it has no MAC address - _execute.return_value = LINK_SAMPLE[0] + with mock.patch.object( + priv_lib, 'get_link_attributes' + ) as get_link_attributes, mock.patch.object( + priv_lib, 'set_link_attribute' + ) as set_link_attribute: + get_link_attributes.return_value = {} self.assertFalse(ip_lib.ensure_device_is_ready("lo")) + get_link_attributes.assert_called_once_with("lo", None) + set_link_attribute.assert_not_called() class TestGetRoutingTable(base.BaseTestCase):