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
This commit is contained in:
Rodolfo Alonso Hernandez 2021-02-24 14:46:10 +00:00 committed by Rodolfo Alonso
parent 482d0fe2bf
commit a6cdf273c0
3 changed files with 105 additions and 56 deletions

View File

@ -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

View File

@ -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))

View File

@ -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 = {