From 96fd203a1461a74318af0e89b3d049f618c32fde Mon Sep 17 00:00:00 2001 From: Adam Oswick Date: Wed, 19 Jul 2023 12:59:39 +0100 Subject: [PATCH] For hosts in DVR mode, only fetch bound FIPs Currently, agents in DVR mode requesting a router update fetch all the FIPs on a network from the DB rather than just the FIPs that are relevant to the specific host requesting the update. While not noticable in smaller networks with a limited number of floating IPs, this can add significant overhead in larger networks with many FIPs and hosts. That overhead comes from Python mapping the responses from the DB into objects, making extra DB calls per FIP returned and adding additional iterations to the loop in _get_dvr_sync_data. These objects are mostly discarded later on and not updated nor included in the RPC response. This change ensures that we only fetch FIPs from the DB that are bound to the host requesting the update or those which are in a pre-live migration state (as they may be migrated to the host in question). Closes-Bug: #2028185 Change-Id: I199b0b1456aa15dadcc24cafc89db1072d224efd --- neutron/db/l3_db.py | 9 +++++---- neutron/db/l3_dvr_db.py | 11 +++++++++-- neutron/objects/router.py | 17 ++++++++++++++++- neutron/tests/unit/objects/test_router.py | 22 ++++++++++++++++++++-- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index d389e8c92ce..a315f652f53 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -1896,7 +1896,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, d['fixed_ip_address_scope'] = scope_id return d - def _get_sync_floating_ips(self, context, router_ids): + def _get_sync_floating_ips(self, context, router_ids, host=None): """Query floating_ips that relate to list of router_ids with scope. This is different than the regular get_floatingips in that it finds the @@ -1912,7 +1912,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, return [ self._make_floatingip_dict_with_scope(*scoped_fip) for scoped_fip in l3_obj.FloatingIP.get_scoped_floating_ips( - context, router_ids) + context, router_ids, host) ] def _get_sync_interfaces(self, context, router_ids, device_owners=None): @@ -2062,7 +2062,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, router[constants.INTERFACE_KEY] = router_interfaces def _get_router_info_list(self, context, router_ids=None, active=None, - device_owners=None): + device_owners=None, fip_host_filter=None): """Query routers and their related floating_ips, interfaces.""" with db_api.CONTEXT_WRITER.using(context): routers = self._get_sync_routers(context, @@ -2071,7 +2071,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase, router_ids = [router['id'] for router in routers] interfaces = self._get_sync_interfaces( context, router_ids, device_owners) - floating_ips = self._get_sync_floating_ips(context, router_ids) + floating_ips = self._get_sync_floating_ips( + context, router_ids, host=fip_host_filter) return (routers, interfaces, floating_ips) def get_sync_data(self, context, router_ids=None, active=None): diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 0c81929acc5..5b4f1d99532 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -979,9 +979,17 @@ class _DVRAgentInterfaceMixin(object): @log_helper.log_method_call def _get_dvr_sync_data(self, context, host, agent, router_ids=None, active=None): + # If the requesting agent is in normal dvr mode, we can fetch + # only FIPs bound to the particular host requesting the update + requesting_agent_mode = self._get_agent_mode(agent) + fip_host_filter = None + if requesting_agent_mode == const.L3_AGENT_MODE_DVR: + fip_host_filter = host + routers, interfaces, floating_ips = self._get_router_info_list( context, router_ids=router_ids, active=active, - device_owners=const.ROUTER_INTERFACE_OWNERS) + device_owners=const.ROUTER_INTERFACE_OWNERS, + fip_host_filter=fip_host_filter) dvr_router_ids = set(router['id'] for router in routers if is_distributed_router(router)) floating_ip_port_ids = [fip['port_id'] for fip in floating_ips @@ -1014,7 +1022,6 @@ class _DVRAgentInterfaceMixin(object): if len(l3_agent_on_host): l3_agent_mode = self._get_agent_mode( l3_agent_on_host[0]) - requesting_agent_mode = self._get_agent_mode(agent) # Consider the ports where the portbinding host and # request host match. if port_host == host: diff --git a/neutron/objects/router.py b/neutron/objects/router.py index 5362a11c9b2..604ae7bd209 100644 --- a/neutron/objects/router.py +++ b/neutron/objects/router.py @@ -21,6 +21,7 @@ from neutron_lib.utils import net as net_utils from oslo_utils import versionutils from oslo_versionedobjects import fields as obj_fields from sqlalchemy import func +from sqlalchemy import or_ from sqlalchemy import sql from neutron.db.models import dvr as dvr_models @@ -30,6 +31,7 @@ from neutron.db.models import l3agent as rb_model from neutron.db import models_v2 from neutron.objects import base from neutron.objects.qos import binding as qos_binding +from neutron.plugins.ml2 import models as ml2_models @base.NeutronObjectRegistry.register @@ -402,7 +404,7 @@ class FloatingIP(base.NeutronDbObject): @classmethod @db_api.CONTEXT_READER - def get_scoped_floating_ips(cls, context, router_ids): + def get_scoped_floating_ips(cls, context, router_ids, host=None): query = context.session.query(l3.FloatingIP, models_v2.SubnetPool.address_scope_id) query = query.join( @@ -420,6 +422,19 @@ class FloatingIP(base.NeutronDbObject): # Filter out on router_ids query = query.filter(l3.FloatingIP.router_id.in_(router_ids)) + # If a host value is provided, filter output to a specific host + if host is not None: + query = query.outerjoin( + ml2_models.PortBinding, + models_v2.Port.id == ml2_models.PortBinding.port_id) + # Also filter for ports with migrating_to as they may be relevant + # to this host but might not yet have the 'host' column updated + # if the migration is in a pre-live migration state + query = query.filter(or_( + ml2_models.PortBinding.host == host, + ml2_models.PortBinding.profile.like('%migrating_to%'), + )) + # Remove duplicate rows based on FIP IDs and the subnet pool address # scope. Only one subnet pool (per IP version, 4 in this case) can # be assigned to a subnet. The subnet pool address scope for a FIP is diff --git a/neutron/tests/unit/objects/test_router.py b/neutron/tests/unit/objects/test_router.py index 8dff212e49d..6deedfa00f8 100644 --- a/neutron/tests/unit/objects/test_router.py +++ b/neutron/tests/unit/objects/test_router.py @@ -17,11 +17,13 @@ from unittest import mock import netaddr +from neutron_lib.api.definitions import portbindings from neutron_lib import constants from neutron_lib.db import api as db_api from oslo_utils import uuidutils from neutron.db import l3_attrs_db +from neutron.objects import ports from neutron.objects.qos import binding as qos_binding from neutron.objects.qos import policy from neutron.objects import router @@ -328,10 +330,10 @@ class FloatingIPDbObjectTestCase(obj_test_base.BaseDbObjectTestCase, obj_v1_1['versioned_object.data']) def test_get_scoped_floating_ips(self): - def compare_results(router_ids, original_fips): + def compare_results(router_ids, original_fips, host=None): fips_scope = [fip for fip in router.FloatingIP.get_scoped_floating_ips( - self.context, router_ids)] + self.context, router_ids, host=host)] fip_ids = [fip[0].id for fip in fips_scope] as_ids = {fip[1] for fip in fips_scope} self.assertCountEqual(original_fips, fip_ids) @@ -366,11 +368,27 @@ class FloatingIPDbObjectTestCase(obj_test_base.BaseDbObjectTestCase, fip.create() routers[router_id].append(fip.id) + # Associate port with a host + port_binding = ports.PortBinding( + self.context, + port_id=fip.fixed_port_id, + host=f"compute{j}", + vif_type=portbindings.VIF_TYPE_OTHER, + ) + port_binding.create() + # For each router we created, fetch the fips and ensure the # results match what we originally created for router_id, original_fips in routers.items(): compare_results([router_id], original_fips) + # Fetch the first FIP in each router as we can assume that this is + # bound to compute0, then attempt to filter by this compute host + host_filtered_fips = [] + for router_id, original_fips in routers.items(): + host_filtered_fips.append(original_fips[1]) + compare_results(routers.keys(), host_filtered_fips, host="compute1") + # Now try to fetch all the fips for all the routers at once original_fips = list(chain.from_iterable(routers.values())) compare_results(routers.keys(), original_fips)