From a6cdf273c06a3bba713d7b8d0ef3a9b71b0ffc57 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 24 Feb 2021 14:46:10 +0000 Subject: [PATCH] Improve "get_devices_with_ip" performance As reported in LP#1896734, there is a limit in the size of information that can be transmitted in one single message between an application and the privsep daemon. The read socket buffer is limited in size; a message exceeding this size will generate an exception. In order to limit the amount of information to be sent, this patch improves the performance of "get_devices_with_ip". In the previous implementation, the whole list of network devices from a namespace was retrieved. In some environments, the list of devices could be so big that the list returned by "privileged.get_link_devices" can exceed the read buffer size (as reported in the LP bug when the OVS agent tries to retrieve the list of IP addresses in the system). Now the function calls "privileged.get_ip_addresses", that returns a much smaller list. This patch is also reducing the number of system calls to just one; the previous implementation was retrieving first the devices link information list (that method was returning a much bigger blob) and then, per device, retrieving the IP address information. Change-Id: I97ada62484023b9833ed12afd68eb4c8d337fd1f Related-Bug: #1896734 --- neutron/agent/linux/ip_lib.py | 46 ++++++----- .../functional/agent/linux/test_ip_lib.py | 81 +++++++++++++++++++ neutron/tests/unit/agent/linux/test_ip_lib.py | 34 -------- 3 files changed, 105 insertions(+), 56 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 7854c2e462c..89f0919bad5 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -1361,35 +1361,37 @@ def _parse_ip_address(pyroute2_address, device_name): 'event': event} -def _parse_link_device(namespace, device, **kwargs): - """Parse pytoute2 link device information - - For each link device, the IP address information is retrieved and returned - in a dictionary. - IP address scope: http://linux-ip.net/html/tools-ip-address.html - """ - retval = [] - name = get_attr(device, 'IFLA_IFNAME') - ip_addresses = privileged.get_ip_addresses(namespace, - index=device['index'], - **kwargs) - for ip_address in ip_addresses: - retval.append(_parse_ip_address(ip_address, name)) - return retval - - def get_devices_with_ip(namespace, name=None, **kwargs): + retval = [] link_args = {} if name: link_args['ifname'] = name scope = kwargs.pop('scope', None) if scope: kwargs['scope'] = IP_ADDRESS_SCOPE_NAME[scope] - devices = privileged.get_link_devices(namespace, **link_args) - retval = [] - for parsed_ips in (_parse_link_device(namespace, device, **kwargs) - for device in devices): - retval += parsed_ips + + if not link_args: + ip_addresses = privileged.get_ip_addresses(namespace, **kwargs) + else: + device = get_devices_info(namespace, **link_args) + if not device: + return retval + ip_addresses = privileged.get_ip_addresses( + namespace, index=device[0]['index'], **kwargs) + + devices = {} # {device index: name} + for ip_address in ip_addresses: + index = ip_address['index'] + name = get_attr(ip_address, 'IFA_LABEL') or devices.get(index) + if not name: + device = get_devices_info(namespace, index=index) + if not device: + continue + name = device[0]['name'] + + retval.append(_parse_ip_address(ip_address, name)) + devices[index] = name + return retval diff --git a/neutron/tests/functional/agent/linux/test_ip_lib.py b/neutron/tests/functional/agent/linux/test_ip_lib.py index fc511850156..6bf4d3dea8d 100644 --- a/neutron/tests/functional/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/agent/linux/test_ip_lib.py @@ -14,6 +14,7 @@ # under the License. import collections +import copy import itertools import signal @@ -1055,3 +1056,83 @@ class IpAddrCommandTestCase(functional_base.BaseSudoTestCase): devices_filtered = self.device.addr.list(scope=scope) devices_cidr = {device['cidr'] for device in devices_filtered} self.assertIn(_ip, devices_cidr) + + +class GetDevicesWithIpTestCase(functional_base.BaseSudoTestCase): + + def setUp(self): + super().setUp() + self.namespace = self.useFixture(net_helpers.NamespaceFixture()).name + self.devices = [] + self.num_devices = 5 + self.num_devices_with_ip = 3 + for idx in range(self.num_devices): + dev_name = 'test_device_%s' % idx + ip_lib.IPWrapper(self.namespace).add_dummy(dev_name) + device = ip_lib.IPDevice(dev_name, namespace=self.namespace) + device.link.set_up() + self.devices.append(device) + + self.cidrs = [netaddr.IPNetwork('10.10.0.0/24'), + netaddr.IPNetwork('10.20.0.0/24'), + netaddr.IPNetwork('2001:db8:1234:1111::/64'), + netaddr.IPNetwork('2001:db8:1234:2222::/64')] + for idx in range(self.num_devices_with_ip): + for cidr in self.cidrs: + self.devices[idx].addr.add(str(cidr.ip + idx) + '/' + + str(cidr.netmask.netmask_bits())) + + @staticmethod + def _remove_loopback_interface(ip_addresses): + return [ipa for ipa in ip_addresses if + ipa['name'] != ip_lib.LOOPBACK_DEVNAME] + + @staticmethod + def _remove_ipv6_scope_link(ip_addresses): + # Remove all IPv6 addresses with scope link (fe80::...). + return [ipa for ipa in ip_addresses if not ( + ipa['scope'] == 'link' and utils.get_ip_version(ipa['cidr']))] + + @staticmethod + def _pop_ip_address(ip_addresses, cidr): + for idx, ip_address in enumerate(copy.deepcopy(ip_addresses)): + if cidr == ip_address['cidr']: + ip_addresses.pop(idx) + return + + def test_get_devices_with_ip(self): + ip_addresses = ip_lib.get_devices_with_ip(self.namespace) + ip_addresses = self._remove_loopback_interface(ip_addresses) + ip_addresses = self._remove_ipv6_scope_link(ip_addresses) + self.assertEqual(self.num_devices_with_ip * len(self.cidrs), + len(ip_addresses)) + for idx in range(self.num_devices_with_ip): + for cidr in self.cidrs: + cidr = (str(cidr.ip + idx) + '/' + + str(cidr.netmask.netmask_bits())) + self._pop_ip_address(ip_addresses, cidr) + + self.assertEqual(0, len(ip_addresses)) + + def test_get_devices_with_ip_name(self): + for idx in range(self.num_devices_with_ip): + dev_name = 'test_device_%s' % idx + ip_addresses = ip_lib.get_devices_with_ip(self.namespace, + name=dev_name) + ip_addresses = self._remove_loopback_interface(ip_addresses) + ip_addresses = self._remove_ipv6_scope_link(ip_addresses) + + for cidr in self.cidrs: + cidr = (str(cidr.ip + idx) + '/' + + str(cidr.netmask.netmask_bits())) + self._pop_ip_address(ip_addresses, cidr) + + self.assertEqual(0, len(ip_addresses)) + + for idx in range(self.num_devices_with_ip, self.num_devices): + dev_name = 'test_device_%s' % idx + ip_addresses = ip_lib.get_devices_with_ip(self.namespace, + name=dev_name) + ip_addresses = self._remove_loopback_interface(ip_addresses) + ip_addresses = self._remove_ipv6_scope_link(ip_addresses) + self.assertEqual(0, len(ip_addresses)) diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 64d28182d50..1324be5cc56 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -24,7 +24,6 @@ from neutron_lib import exceptions from oslo_utils import netutils from oslo_utils import uuidutils import pyroute2 -from pyroute2.netlink.rtnl import ifaddrmsg from pyroute2.netlink.rtnl import ifinfmsg from pyroute2.netlink.rtnl import ndmsg from pyroute2 import NetlinkError @@ -1622,39 +1621,6 @@ class ListIpRulesTestCase(base.BaseTestCase): self.assertEqual(reference, retval) -class ParseLinkDeviceTestCase(base.BaseTestCase): - - def setUp(self): - super(ParseLinkDeviceTestCase, self).setUp() - self._mock_get_ip_addresses = mock.patch.object(priv_lib, - 'get_ip_addresses') - self.mock_get_ip_addresses = self._mock_get_ip_addresses.start() - self.addCleanup(self._stop_mock) - - def _stop_mock(self): - self._mock_get_ip_addresses.stop() - - def test_parse_link_devices(self): - device = ({'index': 1, 'attrs': [['IFLA_IFNAME', 'int_name']]}) - self.mock_get_ip_addresses.return_value = [ - {'prefixlen': 24, 'scope': 200, 'event': 'RTM_NEWADDR', 'attrs': [ - ['IFA_ADDRESS', '192.168.10.20'], - ['IFA_FLAGS', ifaddrmsg.IFA_F_PERMANENT]]}, - {'prefixlen': 64, 'scope': 200, 'event': 'RTM_DELADDR', 'attrs': [ - ['IFA_ADDRESS', '2001:db8::1'], - ['IFA_FLAGS', ifaddrmsg.IFA_F_PERMANENT]]}] - - retval = ip_lib._parse_link_device('namespace', device) - expected = [{'scope': 'site', 'cidr': '192.168.10.20/24', - 'dynamic': False, 'dadfailed': False, 'name': 'int_name', - 'broadcast': None, 'tentative': False, 'event': 'added'}, - {'scope': 'site', 'cidr': '2001:db8::1/64', - 'dynamic': False, 'dadfailed': False, 'name': 'int_name', - 'broadcast': None, 'tentative': False, - 'event': 'removed'}] - self.assertEqual(expected, retval) - - class GetDevicesInfoTestCase(base.BaseTestCase): DEVICE_LO = {