From cc9b849bf39c894355b533f60d8a5a0dc8f04b8f 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. Conflicts: neutron/tests/functional/agent/linux/test_ip_lib.py Change-Id: I97ada62484023b9833ed12afd68eb4c8d337fd1f Related-Bug: #1896734 (cherry picked from commit a6cdf273c06a3bba713d7b8d0ef3a9b71b0ffc57) --- 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 d649706b717..289065a620f 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -1340,35 +1340,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 dca760b77b0..f8dcc8b44b5 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 @@ -1013,3 +1014,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(GetDevicesWithIpTestCase, self).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 6d2a4004303..a324658a25e 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -23,7 +23,6 @@ from neutron_lib import constants from neutron_lib import exceptions 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 @@ -1605,39 +1604,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 = {