Skip to content

Commit 1d9ce04

Browse files
committed
Improve agent provision performance for large networks
Before this patch, the metadata agent would provision network namespace for all subnets under a network(datapath) as soon as the first VM(vif port) was mounted on the chassis. This operation can take very long time for networks with lots of subnets. See the linked bug for more details. This patch changes this mechanism to "lazy load" where metadata agent provisions metadata namespace with only the subnets belonging to the active ports on the chassis. This results in virtually constant throughput not effected by the number of subnets. Merge Conflict: Using datapath_uuid :str in addition to net_name for teardown_datapath method to remain compatible with the method implementation in Yoga and before. Updated unit tests accordingly neutron/agent/ovn/metadata/agent.py neutron/tests/unit/agent/ovn/metadata/test_agent.py Closes-Bug: #1981113 Change-Id: Ia2a66cfd3fd1380c5204109742d44f09160548d2 (cherry picked from commit edf3b3f191c2eae229a754dcbfc448fa41bd8bc3)
1 parent 2bf242a commit 1d9ce04

File tree

2 files changed

+382
-137
lines changed

2 files changed

+382
-137
lines changed

neutron/agent/ovn/metadata/agent.py

Lines changed: 132 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import threading
1919
import uuid
2020

21+
import netaddr
2122
from neutron_lib import constants as n_const
2223
from oslo_concurrency import lockutils
2324
from oslo_log import log
@@ -46,7 +47,8 @@
4647
OVN_VIF_PORT_TYPES = ("", "external", )
4748

4849
MetadataPortInfo = collections.namedtuple('MetadataPortInfo', ['mac',
49-
'ip_addresses'])
50+
'ip_addresses',
51+
'logical_port'])
5052

5153
OVN_METADATA_UUID_NAMESPACE = uuid.UUID('d34bf9f6-da32-4871-9af8-15a4626b41ab')
5254

@@ -87,7 +89,7 @@ def run(self, event, row, old):
8789
net_name = ovn_utils.get_network_name_from_datapath(
8890
row.datapath)
8991
LOG.info(self.LOG_MSG, row.logical_port, net_name)
90-
self.agent.update_datapath(str(row.datapath.uuid), net_name)
92+
self.agent.provision_datapath(row.datapath)
9193
except ConfigException:
9294
# We're now in the reader lock mode, we need to exit the
9395
# context and then use writer lock
@@ -315,11 +317,12 @@ def _get_ovn_bridge(self):
315317
"br-int instead.")
316318
return 'br-int'
317319

318-
def get_networks(self):
320+
def get_networks_datapaths(self):
321+
"""Return a set of datapath objects of the VIF ports on the current
322+
chassis.
323+
"""
319324
ports = self.sb_idl.get_ports_on_chassis(self.chassis)
320-
return {(str(p.datapath.uuid),
321-
ovn_utils.get_network_name_from_datapath(p.datapath))
322-
for p in self._vif_ports(ports)}
325+
return set(p.datapath for p in self._vif_ports(ports))
323326

324327
@_sync_lock
325328
def sync(self):
@@ -334,10 +337,10 @@ def sync(self):
334337
system_namespaces = tuple(
335338
ns.decode('utf-8') if isinstance(ns, bytes) else ns
336339
for ns in ip_lib.list_network_namespaces())
337-
nets = self.get_networks()
340+
net_datapaths = self.get_networks_datapaths()
338341
metadata_namespaces = [
339-
self._get_namespace_name(net[1])
340-
for net in nets
342+
self._get_namespace_name(str(datapath.uuid))
343+
for datapath in net_datapaths
341344
]
342345
unused_namespaces = [ns for ns in system_namespaces if
343346
ns.startswith(NS_PREFIX) and
@@ -347,7 +350,8 @@ def sync(self):
347350

348351
# now that all obsolete namespaces are cleaned up, deploy required
349352
# networks
350-
self.ensure_all_networks_provisioned(nets)
353+
for datapath in net_datapaths:
354+
self.provision_datapath(datapath)
351355

352356
@staticmethod
353357
def _get_veth_name(datapath):
@@ -395,25 +399,6 @@ def teardown_datapath(self, datapath, net_name=None):
395399

396400
ip.garbage_collect_namespace()
397401

398-
def update_datapath(self, datapath, net_name):
399-
"""Update the metadata service for this datapath.
400-
401-
This function will:
402-
* Provision the namespace if it wasn't already in place.
403-
* Update the namespace if it was already serving metadata (for example,
404-
after binding/unbinding the first/last port of a subnet in our
405-
chassis).
406-
* Tear down the namespace if there are no more ports in our chassis
407-
for this datapath.
408-
"""
409-
ports = self.sb_idl.get_ports_on_chassis(self.chassis)
410-
datapath_ports = [p for p in self._vif_ports(ports) if
411-
str(p.datapath.uuid) == datapath]
412-
if datapath_ports:
413-
self.provision_datapath(datapath, net_name)
414-
else:
415-
self.teardown_datapath(datapath, net_name)
416-
417402
def _ensure_datapath_checksum(self, namespace):
418403
"""Ensure the correct checksum in the metadata packets in DPDK bridges
419404
@@ -433,45 +418,130 @@ def _ensure_datapath_checksum(self, namespace):
433418
iptables_mgr.ipv4['mangle'].add_rule('POSTROUTING', rule, wrap=False)
434419
iptables_mgr.apply()
435420

436-
def provision_datapath(self, datapath, net_name):
437-
"""Provision the datapath so that it can serve metadata.
421+
def _get_port_ips(self, port):
422+
# Retrieve IPs from the port mac column which is in form
423+
# ["<port_mac> <ip1> <ip2> ... <ipN>"]
424+
mac_field_attrs = port.mac[0].split()
425+
ips = mac_field_attrs[1:]
426+
if not ips:
427+
LOG.debug("Port %s IP addresses were not retrieved from the "
428+
"Port_Binding MAC column %s", port.uuid, mac_field_attrs)
429+
return ips
430+
431+
def _active_subnets_cidrs(self, datapath_ports_ips, metadata_port_cidrs):
432+
active_subnets_cidrs = set()
433+
# Prepopulate a dictionary where each metadata_port_cidr(string) maps
434+
# to its netaddr.IPNetwork object. This is so we dont have to
435+
# reconstruct IPNetwork objects repeatedly in the for loop
436+
metadata_cidrs_to_network_objects = {
437+
metadata_port_cidr: netaddr.IPNetwork(metadata_port_cidr)
438+
for metadata_port_cidr in metadata_port_cidrs
439+
}
440+
441+
for datapath_port_ip in datapath_ports_ips:
442+
ip_obj = netaddr.IPAddress(datapath_port_ip)
443+
for metadata_cidr, metadata_cidr_obj in \
444+
metadata_cidrs_to_network_objects.items():
445+
if ip_obj in metadata_cidr_obj:
446+
active_subnets_cidrs.add(metadata_cidr)
447+
break
448+
return active_subnets_cidrs
449+
450+
def _process_cidrs(self, current_namespace_cidrs,
451+
datapath_ports_ips, metadata_port_subnet_cidrs):
452+
active_subnets_cidrs = self._active_subnets_cidrs(
453+
datapath_ports_ips, metadata_port_subnet_cidrs)
454+
455+
cidrs_to_add = active_subnets_cidrs - current_namespace_cidrs
456+
457+
if n_const.METADATA_CIDR not in current_namespace_cidrs:
458+
cidrs_to_add.add(n_const.METADATA_CIDR)
459+
else:
460+
active_subnets_cidrs.add(n_const.METADATA_CIDR)
438461

439-
This function will create the namespace and VETH pair if needed
440-
and assign the IP addresses to the interface corresponding to the
441-
metadata port of the network. It will also remove existing IP
442-
addresses that are no longer needed.
462+
cidrs_to_delete = current_namespace_cidrs - active_subnets_cidrs
463+
464+
return cidrs_to_add, cidrs_to_delete
465+
466+
def _get_provision_params(self, datapath):
467+
"""Performs datapath preprovision checks and returns paremeters
468+
needed to provision namespace.
469+
470+
Function will confirm that:
471+
1. Datapath metadata port has valid MAC and subnet CIDRs
472+
2. There are datapath port IPs
473+
474+
If any of those rules are not valid the nemaspace for the
475+
provided datapath will be tore down.
476+
If successful, returns datapath's network name, ports IPs
477+
and meta port info
443478
"""
444-
LOG.debug("Provisioning metadata for network %s", net_name)
445-
port = self.sb_idl.get_metadata_port_network(datapath)
479+
net_name = ovn_utils.get_network_name_from_datapath(datapath)
480+
datapath_uuid = str(datapath.uuid)
481+
482+
metadata_port = self.sb_idl.get_metadata_port_network(datapath_uuid)
446483
# If there's no metadata port or it doesn't have a MAC or IP
447484
# addresses, then tear the namespace down if needed. This might happen
448485
# when there are no subnets yet created so metadata port doesn't have
449486
# an IP address.
450-
if not (port and port.mac and
451-
port.external_ids.get(ovn_const.OVN_CIDRS_EXT_ID_KEY, None)):
487+
if not (metadata_port and metadata_port.mac and
488+
metadata_port.external_ids.get(
489+
ovn_const.OVN_CIDRS_EXT_ID_KEY, None)):
452490
LOG.debug("There is no metadata port for network %s or it has no "
453491
"MAC or IP addresses configured, tearing the namespace "
454492
"down if needed", net_name)
455-
self.teardown_datapath(datapath, net_name)
493+
self.teardown_datapath(datapath_uuid, net_name)
456494
return
457495

458496
# First entry of the mac field must be the MAC address.
459-
match = MAC_PATTERN.match(port.mac[0].split(' ')[0])
460-
# If it is not, we can't provision the namespace. Tear it down if
461-
# needed and log the error.
497+
match = MAC_PATTERN.match(metadata_port.mac[0].split(' ')[0])
462498
if not match:
463499
LOG.error("Metadata port for network %s doesn't have a MAC "
464500
"address, tearing the namespace down if needed",
465501
net_name)
466-
self.teardown_datapath(datapath)
502+
self.teardown_datapath(datapath_uuid, net_name)
467503
return
468504

469505
mac = match.group()
470506
ip_addresses = set(
471-
port.external_ids[ovn_const.OVN_CIDRS_EXT_ID_KEY].split(' '))
472-
ip_addresses.add(n_const.METADATA_CIDR)
473-
metadata_port = MetadataPortInfo(mac, ip_addresses)
507+
metadata_port.external_ids[
508+
ovn_const.OVN_CIDRS_EXT_ID_KEY].split(' '))
509+
metadata_port_info = MetadataPortInfo(mac, ip_addresses,
510+
metadata_port.logical_port)
511+
512+
chassis_ports = self.sb_idl.get_ports_on_chassis(self.chassis)
513+
datapath_ports_ips = []
514+
for chassis_port in self._vif_ports(chassis_ports):
515+
if str(chassis_port.datapath.uuid) == datapath_uuid:
516+
datapath_ports_ips.extend(self._get_port_ips(chassis_port))
517+
518+
if not datapath_ports_ips:
519+
LOG.debug("No valid VIF ports were found for network %s, "
520+
"tearing the namespace down if needed", net_name)
521+
self.teardown_datapath(datapath_uuid, net_name)
522+
return
523+
524+
return net_name, datapath_ports_ips, metadata_port_info
525+
526+
def provision_datapath(self, datapath):
527+
"""Provision the datapath so that it can serve metadata.
528+
529+
This function will create the namespace and VETH pair if needed
530+
and assign the IP addresses to the interface corresponding to the
531+
metadata port of the network. It will also remove existing IP from
532+
the namespace if they are no longer needed.
474533
534+
:param datapath: datapath object.
535+
:return: The metadata namespace name for the datapath or None
536+
if namespace was not provisioned
537+
"""
538+
539+
provision_params = self._get_provision_params(datapath)
540+
if not provision_params:
541+
return
542+
net_name, datapath_ports_ips, metadata_port_info = provision_params
543+
544+
LOG.info("Provisioning metadata for network %s", net_name)
475545
# Create the VETH pair if it's not created. Also the add_veth function
476546
# will create the namespace for us.
477547
namespace = self._get_namespace_name(net_name)
@@ -496,22 +566,24 @@ def provision_datapath(self, datapath, net_name):
496566
ip2.link.set_up()
497567

498568
# Configure the MAC address.
499-
ip2.link.set_address(metadata_port.mac)
500-
dev_info = ip2.addr.list()
501-
502-
# Configure the IP addresses on the VETH pair and remove those
503-
# that we no longer need.
504-
current_cidrs = {dev['cidr'] for dev in dev_info}
505-
for ipaddr in current_cidrs - metadata_port.ip_addresses:
506-
ip2.addr.delete(ipaddr)
507-
for ipaddr in metadata_port.ip_addresses - current_cidrs:
569+
ip2.link.set_address(metadata_port_info.mac)
570+
571+
cidrs_to_add, cidrs_to_delete = self._process_cidrs(
572+
{dev['cidr'] for dev in ip2.addr.list()},
573+
datapath_ports_ips,
574+
metadata_port_info.ip_addresses
575+
)
576+
# Delete any non active addresses from the network namespace
577+
for cidr in cidrs_to_delete:
578+
ip2.addr.delete(cidr)
579+
for cidr in cidrs_to_add:
508580
# NOTE(dalvarez): metadata only works on IPv4. We're doing this
509581
# extra check here because it could be that the metadata port has
510582
# an IPv6 address if there's an IPv6 subnet with SLAAC in its
511583
# network. Neutron IPAM will autoallocate an IPv6 address for every
512584
# port in the network.
513-
if utils.get_ip_version(ipaddr) == 4:
514-
ip2.addr.add(ipaddr)
585+
if utils.get_ip_version(cidr) == n_const.IP_VERSION_4:
586+
ip2.addr.add(cidr)
515587

516588
# Check that this port is not attached to any other OVS bridge. This
517589
# can happen when the OVN bridge changes (for example, during a
@@ -536,7 +608,8 @@ def provision_datapath(self, datapath, net_name):
536608
veth_name[0]).execute()
537609
self.ovs_idl.db_set(
538610
'Interface', veth_name[0],
539-
('external_ids', {'iface-id': port.logical_port})).execute()
611+
('external_ids', {'iface-id':
612+
metadata_port_info.logical_port})).execute()
540613

541614
# Ensure the correct checksum in the metadata traffic.
542615
self._ensure_datapath_checksum(namespace)
@@ -546,14 +619,3 @@ def provision_datapath(self, datapath, net_name):
546619
self._process_monitor, namespace, n_const.METADATA_PORT,
547620
self.conf, bind_address=n_const.METADATA_V4_IP,
548621
network_id=net_name)
549-
550-
def ensure_all_networks_provisioned(self, nets):
551-
"""Ensure that all requested datapaths are provisioned.
552-
553-
This function will make sure that requested datapaths have their
554-
namespaces, VETH pair and OVS ports created and metadata proxies are up
555-
and running.
556-
"""
557-
# Make sure that all those datapaths are serving metadata
558-
for datapath, net_name in nets:
559-
self.provision_datapath(datapath, net_name)

0 commit comments

Comments
 (0)