From ab31406d77a29278a9a0360e375bb3689526aec0 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Fri, 14 May 2021 16:07:14 +0200 Subject: [PATCH] [DVR] Send allowed address pairs info to the L3 agents When new dvr router is going to be created on the node, L3 agent asks server for list of ports plugged to the subnets, to populate arp entries for all fixed IPs from those ports. There was missing info about allowed address pairs there, so those IPs were not populated in the qrouter namespace. Now it's added and L3 agent can add those arp entries to the qrouter namespaces too. Conflicts: neutron/tests/unit/db/test_l3_dvr_db.py Closes-Bug: #1928466 Change-Id: I5d6c72c271ff450d9e43b3e33a99dd59d727882d (cherry picked from commit 7b59b5069b1402730602b430416a15b1609253ea) --- neutron/agent/l3/dvr_local_router.py | 7 +++++++ neutron/db/allowedaddresspairs_db.py | 12 +++++++++++ neutron/db/l3_dvr_db.py | 14 ++++++++++++- .../port/extensions/allowedaddresspairs.py | 10 ++++++++++ .../functional/agent/l3/test_dvr_router.py | 20 +++++++++++-------- .../unit/agent/l3/test_dvr_local_router.py | 10 +++++++--- neutron/tests/unit/db/test_l3_dvr_db.py | 4 ++++ 7 files changed, 65 insertions(+), 12 deletions(-) diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index c7d37d87d43..8bf15897483 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -328,6 +328,13 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): 'add', device=device, device_exists=device_exists) + for allowed_address_pair in p.get('allowed_address_pairs', []): + self._update_arp_entry(allowed_address_pair['ip_address'], + allowed_address_pair['mac_address'], + subnet_id, + 'add', + device=device, + device_exists=device_exists) self._process_arp_cache_for_internal_port(subnet_id) @staticmethod diff --git a/neutron/db/allowedaddresspairs_db.py b/neutron/db/allowedaddresspairs_db.py index 8c737775cc8..056d7fadd2f 100644 --- a/neutron/db/allowedaddresspairs_db.py +++ b/neutron/db/allowedaddresspairs_db.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. # +import collections from neutron_lib.api.definitions import allowedaddresspairs as addr_apidef from neutron_lib.api.definitions import port as port_def @@ -65,6 +66,17 @@ class AllowedAddressPairsMixin(object): return [self._make_allowed_address_pairs_dict(pair.db_obj) for pair in pairs] + def get_allowed_address_pairs_for_ports(self, context, port_ids): + pairs = ( + obj_addr_pair.AllowedAddressPair. + get_allowed_address_pairs_for_ports( + context, port_ids=port_ids)) + result = collections.defaultdict(list) + for pair in pairs: + result[pair.port_id].append( + self._make_allowed_address_pairs_dict(pair.db_obj)) + return result + @staticmethod @resource_extend.extends([port_def.COLLECTION_NAME]) def _extend_port_dict_allowed_address_pairs(port_res, port_db): diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 8ff05eb9d5d..8acd6d4c7ed 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -1334,11 +1334,23 @@ class L3_NAT_with_dvr_db_mixin(_DVRAgentInterfaceMixin, def get_ports_under_dvr_connected_subnet(self, context, subnet_id): query = dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id) ports = [p for p in query.all() if is_port_bound(p)] - return [ + # TODO(slaweq): if there would be way to pass to neutron-lib only + # list of extensions which actually should be processed, than setting + # process_extensions=True below could avoid that second loop and + # getting allowed_address_pairs for each of the ports + ports_list = [ self.l3plugin._core_plugin._make_port_dict( port, process_extensions=False) for port in ports ] + ports_ids = [p['id'] for p in ports_list] + allowed_address_pairs = ( + self._core_plugin.get_allowed_address_pairs_for_ports( + context, ports_ids)) + for port in ports_list: + port['allowed_address_pairs'] = allowed_address_pairs.get( + port['id'], []) + return ports_list def is_distributed_router(router): diff --git a/neutron/objects/port/extensions/allowedaddresspairs.py b/neutron/objects/port/extensions/allowedaddresspairs.py index 0ad7d60863c..f3a77830b46 100644 --- a/neutron/objects/port/extensions/allowedaddresspairs.py +++ b/neutron/objects/port/extensions/allowedaddresspairs.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron_lib.db import api as db_api from neutron_lib.objects import common_types from neutron_lib.utils import net as net_utils @@ -61,3 +62,12 @@ class AllowedAddressPair(base.NeutronDbObject): fields['mac_address'] = net_utils.AuthenticEUI( fields['mac_address']) return fields + + @classmethod + def get_allowed_address_pairs_for_ports(cls, context, port_ids): + with db_api.CONTEXT_READER.using(context): + query = context.session.query(models.AllowedAddressPair).filter( + models.AllowedAddressPair.port_id.in_(port_ids)) + pairs = [cls._load_object(context, db_obj) + for db_obj in query.all()] + return pairs diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index 163def8a7ef..f17b624b394 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -1006,21 +1006,25 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): # cache is properly populated. self.agent.conf.agent_mode = 'dvr_snat' router_info = self.generate_dvr_router_info(enable_snat=True) - expected_neighbor = '35.4.1.10' + expected_neighbors = ['35.4.1.10', '10.0.0.10'] port_data = { - 'fixed_ips': [{'ip_address': expected_neighbor}], + 'fixed_ips': [{'ip_address': expected_neighbors[0]}], 'mac_address': 'fa:3e:aa:bb:cc:dd', - 'device_owner': DEVICE_OWNER_COMPUTE + 'device_owner': DEVICE_OWNER_COMPUTE, + 'allowed_address_pairs': [ + {'ip_address': expected_neighbors[1], + 'mac_address': 'fa:3e:aa:bb:cc:dd'}] } self.agent.plugin_rpc.get_ports_by_subnet.return_value = [port_data] router1 = self.manage_router(self.agent, router_info) internal_device = router1.get_internal_device_name( router_info['_interfaces'][0]['id']) - neighbor = ip_lib.dump_neigh_entries(4, internal_device, - router1.ns_name, - dst=expected_neighbor) - self.assertNotEqual([], neighbor) - self.assertEqual(expected_neighbor, neighbor[0]['dst']) + for expected_neighbor in expected_neighbors: + neighbor = ip_lib.dump_neigh_entries(4, internal_device, + router1.ns_name, + dst=expected_neighbor) + self.assertNotEqual([], neighbor) + self.assertEqual(expected_neighbor, neighbor[0]['dst']) def _assert_rfp_fpr_mtu(self, router, expected_mtu=1500): dev_mtu = self.get_device_mtu( diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index fdbc132f7d4..4c389b16699 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -531,7 +531,10 @@ class TestDvrRouterOperations(base.BaseTestCase): 'device_owner': lib_constants.DEVICE_OWNER_DHCP, 'fixed_ips': [{'ip_address': '1.2.3.4', 'prefixlen': 24, - 'subnet_id': subnet_id}]}, + 'subnet_id': subnet_id}], + 'allowed_address_pairs': [ + {'ip_address': '10.20.30.40', + 'mac_address': '00:11:22:33:44:55'}]}, {'mac_address': '11:22:33:44:55:66', 'device_owner': lib_constants.DEVICE_OWNER_LOADBALANCER, 'fixed_ips': [{'ip_address': '1.2.3.5', @@ -553,8 +556,9 @@ class TestDvrRouterOperations(base.BaseTestCase): '_process_arp_cache_for_internal_port') as parp: ri._set_subnet_arp_info(subnet_id) self.assertEqual(1, parp.call_count) - self.mock_ip_dev.neigh.add.assert_called_once_with( - '1.2.3.4', '00:11:22:33:44:55') + self.mock_ip_dev.neigh.add.assert_has_calls([ + mock.call('1.2.3.4', '00:11:22:33:44:55'), + mock.call('10.20.30.40', '00:11:22:33:44:55')]) # Test negative case router['distributed'] = False diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index c6d08afd792..d609b4840e0 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -1391,6 +1391,8 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): router = self._create_router(router_dict) with self.network() as network,\ self.subnet(network=network) as subnet: + self.mixin._core_plugin.get_allowed_address_pairs_for_ports = ( + mock.Mock()) fake_bound_ports_ids = [] def fake_is_port_bound(port): @@ -1412,6 +1414,8 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): self.ctx, subnet['subnet']['id']) dvr_subnet_ports_ids = [p['id'] for p in dvr_subnet_ports] self.assertItemsEqual(fake_bound_ports_ids, dvr_subnet_ports_ids) + (self.mixin._core_plugin.get_allowed_address_pairs_for_ports. + assert_called_once_with(self.ctx, dvr_subnet_ports_ids)) @mock.patch.object(plugin_utils, 'can_port_be_bound_to_virtual_bridge', return_value=True)