From 9b2b7faee6eb4b05a05a0a59f16aa5067915f7cf Mon Sep 17 00:00:00 2001 From: Vasyl Saienko Date: Mon, 2 Dec 2024 09:42:38 +0200 Subject: [PATCH] Setup DVR router ARP for AAP with correct MAC * Pick up mac address from AAP when its configured, fallback to the parent port address * Log arp entry operations for DVR L3 * Add unittests Closes-Bug: #2090825 Change-Id: Idad491e3dd5adc74f2047b0fb81d674f396f53c9 --- neutron/agent/l3/dvr_local_router.py | 2 + neutron/db/l3_dvr_db.py | 11 +- .../l3_router/test_l3_dvr_router_plugin.py | 4 +- neutron/tests/unit/db/test_l3_dvr_db.py | 105 ++++++++++++++++-- ...-aap-mac-for-dvr-arp-ce9e00d163cf4ddc.yaml | 8 ++ 5 files changed, 118 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/use-correct-aap-mac-for-dvr-arp-ce9e00d163cf4ddc.yaml diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index 80dffc5b09a..33d8aecf481 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -319,6 +319,8 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): device_exists=True): """Add or delete arp entry into router namespace for the subnet.""" + LOG.debug("Handling ARP entry operation %s for ip: %s mac: %s " + "device: %s", operation, ip, mac, device) try: if device_exists: if operation == 'add': diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index f5ad670564d..fc6c7e725bc 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -1260,13 +1260,16 @@ class _DVRAgentInterfaceMixin: aa_pair_fixed_ips = [] if port_dict.get('allowed_address_pairs'): for address_pair in port_dict['allowed_address_pairs']: + aap_mac_address = address_pair.get("mac_address", + port_dict["mac_address"]) aap_ip_cidr = address_pair['ip_address'].split("/") if len(aap_ip_cidr) == 1 or int(aap_ip_cidr[1]) == 32: subnet_id = self._get_subnet_id_for_given_fixed_ip( context, aap_ip_cidr[0], port_dict) if subnet_id is not None: fixed_ip = {'subnet_id': subnet_id, - 'ip_address': aap_ip_cidr[0]} + 'ip_address': aap_ip_cidr[0], + 'mac_address': aap_mac_address} aa_pair_fixed_ips.append(fixed_ip) else: LOG.debug("Subnet does not match for the given " @@ -1289,8 +1292,9 @@ class _DVRAgentInterfaceMixin: self._get_allowed_address_pair_fixed_ips(context, port_dict)) changed_fixed_ips = fixed_ips + allowed_address_pair_fixed_ips for fixed_ip in changed_fixed_ips: + mac_address = fixed_ip.get("mac_address", port_dict['mac_address']) self._generate_arp_table_and_notify_agent( - context, fixed_ip, port_dict['mac_address'], + context, fixed_ip, mac_address, self.l3_rpc_notifier.add_arp_entry) def delete_arp_entry_for_dvr_service_port(self, context, port_dict, @@ -1311,8 +1315,9 @@ class _DVRAgentInterfaceMixin: self._get_allowed_address_pair_fixed_ips(context, port_dict)) fixed_ips_to_delete = fixed_ips + allowed_address_pair_fixed_ips for fixed_ip in fixed_ips_to_delete: + mac_address = fixed_ip.get("mac_address", port_dict['mac_address']) self._generate_arp_table_and_notify_agent( - context, fixed_ip, port_dict['mac_address'], + context, fixed_ip, mac_address, self.l3_rpc_notifier.del_arp_entry) def _get_address_pair_active_port_with_fip( diff --git a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py index fde74174d03..71a664357fb 100644 --- a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py +++ b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py @@ -1183,7 +1183,7 @@ class L3DvrTestCase(L3DvrTestCaseBase): vrrp_port_subnet_id = vrrp_port_fixed_ips[0]['subnet_id'] vrrp_arp_table1 = { 'ip_address': vrrp_port_fixed_ips[0]['ip_address'], - 'mac_address': vm_port_mac, + 'mac_address': vrrp_port['port']['mac_address'], 'subnet_id': vrrp_port_subnet_id} expected_calls = [ @@ -1335,7 +1335,7 @@ class L3DvrTestCase(L3DvrTestCaseBase): vrrp_port_subnet_id = vrrp_port_fixed_ips[0]['subnet_id'] vrrp_arp_table1 = { 'ip_address': vrrp_port_fixed_ips[0]['ip_address'], - 'mac_address': vm_port_mac, + 'mac_address': vrrp_port['port']['mac_address'], 'subnet_id': vrrp_port_subnet_id} expected_calls = [ diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index d4f0c37be2e..fa4b7c25222 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -1337,8 +1337,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): mock.call(self.ctx, "router_1", expected_arp_table), mock.call(self.ctx, "router_2", expected_arp_table)]) - def _test_update_arp_entry_for_dvr_service_port( - self, device_owner, action): + def _test_arp_entry_for_dvr_service_port_no_aap(self, action): router_dict = {'name': 'test_router', 'admin_state_up': True, 'distributed': True} router = self._create_router(router_dict) @@ -1355,7 +1354,8 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): {'subnet_id': '48534187-f077-4e81-93ff-81ec4cc0ad3b', 'ip_address': 'fd45:1515:7e0:0:f816:3eff:fe1a:1111'}], 'mac_address': 'my_mac', - 'device_owner': device_owner + 'device_owner': 'nova:compute', + 'allowed_address_pairs': [] } dvr_port = { 'id': 'dvr_port_id', @@ -1364,14 +1364,105 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): 'device_id': router['id'] } plugin.get_ports.return_value = [dvr_port] - if action == 'add': + if action == 'update': self.mixin.update_arp_entry_for_dvr_service_port( self.ctx, port) - self.assertEqual(3, l3_notify.add_arp_entry.call_count) - elif action == 'del': + elif action == 'delete': self.mixin.delete_arp_entry_for_dvr_service_port( self.ctx, port) - self.assertEqual(3, l3_notify.del_arp_entry.call_count) + expected_calls = [ + mock.call(self.ctx, router.id, { + 'ip_address': '10.0.0.11', + 'mac_address': 'my_mac', + 'subnet_id': '51edc9e0-24f9-47f2-8e1e-2a41cb691323'}), + mock.call(self.ctx, router.id, { + 'ip_address': '10.0.0.21', + 'mac_address': 'my_mac', + 'subnet_id': '2b7c8a07-6f8e-4937-8701-f1d5da1a807c'}), + mock.call(self.ctx, router.id, { + 'ip_address': 'fd45:1515:7e0:0:f816:3eff:fe1a:1111', + 'mac_address': 'my_mac', + 'subnet_id': '48534187-f077-4e81-93ff-81ec4cc0ad3b'})] + if action == 'update': + l3_notify.add_arp_entry.assert_has_calls(expected_calls) + elif action == 'delete': + l3_notify.del_arp_entry.assert_has_calls(expected_calls) + + def test_update_arp_entry_for_dvr_service_port_no_aap(self): + self._test_arp_entry_for_dvr_service_port_no_aap(action='update') + + def test_delete_arp_entry_for_dvr_service_port_no_aap(self): + self._test_arp_entry_for_dvr_service_port_no_aap(action='delete') + + def _test_arp_entry_for_dvr_service_port_aap(self, action): + router_dict = {'name': 'test_router', 'admin_state_up': True, + 'distributed': True} + router = self._create_router(router_dict) + plugin = mock.Mock() + directory.add_plugin(plugin_constants.CORE, plugin) + l3_notify = self.mixin.l3_rpc_notifier = mock.Mock() + port = { + 'id': 'my_port_id', + 'network_id': 'my_network_id', + 'fixed_ips': [ + {'subnet_id': '51edc9e0-24f9-47f2-8e1e-2a41cb691323', + 'ip_address': '10.0.0.11'}], + 'mac_address': 'my_mac', + 'device_owner': 'nova:compute', + 'allowed_address_pairs': [ + {'ip_address': '10.0.0.12', + 'mac_address': 'aa:bb:cc:dd:ee:ff'}, + {'ip_address': '10.0.0.13'}, + {'ip_address': '10.0.0.0/24'}, + {'ip_address': '10.0.0.14/32', + 'mac_address': 'aa:bb:cc:dd:ee:ff'} + ] + } + dvr_port = { + 'id': 'dvr_port_id', + 'fixed_ips': mock.ANY, + 'device_owner': const.DEVICE_OWNER_DVR_INTERFACE, + 'device_id': router['id'] + } + plugin.get_ports.return_value = [dvr_port] + subnet = {'id': '51edc9e0-24f9-47f2-8e1e-2a41cb691323', + 'cidr': '10.0.0.0/24'} + with mock.patch.object(self.mixin._core_plugin, 'get_subnets', + return_value=[subnet]): + if action == 'update': + self.mixin.update_arp_entry_for_dvr_service_port( + self.ctx, port) + elif action == 'delete': + self.mixin.delete_arp_entry_for_dvr_service_port( + self.ctx, port) + expected_calls = [ + mock.call(self.ctx, router.id, { + 'ip_address': '10.0.0.11', + 'mac_address': 'my_mac', + 'subnet_id': '51edc9e0-24f9-47f2-8e1e-2a41cb691323'} + ), + mock.call(self.ctx, router.id, { + 'ip_address': '10.0.0.12', + 'mac_address': 'aa:bb:cc:dd:ee:ff', + 'subnet_id': '51edc9e0-24f9-47f2-8e1e-2a41cb691323'}), + mock.call(self.ctx, router.id, { + 'ip_address': '10.0.0.13', + 'mac_address': 'my_mac', + 'subnet_id': '51edc9e0-24f9-47f2-8e1e-2a41cb691323'}), + mock.call(self.ctx, router.id, { + 'ip_address': '10.0.0.14', + 'mac_address': 'aa:bb:cc:dd:ee:ff', + 'subnet_id': '51edc9e0-24f9-47f2-8e1e-2a41cb691323'})] + if action == 'update': + l3_notify.add_arp_entry.assert_has_calls(expected_calls) + elif action == 'delete': + l3_notify.del_arp_entry.assert_has_calls(expected_calls) + + def test_update_arp_entry_for_dvr_service_port_aap(self): + self._test_arp_entry_for_dvr_service_port_aap(action='update') + + def test_delete_arp_entry_for_dvr_service_port_aap(self): + self._test_arp_entry_for_dvr_service_port_aap(action='delete') def test_add_router_interface_csnat_ports_failure(self): router_dict = {'name': 'test_router', 'admin_state_up': True, diff --git a/releasenotes/notes/use-correct-aap-mac-for-dvr-arp-ce9e00d163cf4ddc.yaml b/releasenotes/notes/use-correct-aap-mac-for-dvr-arp-ce9e00d163cf4ddc.yaml new file mode 100644 index 00000000000..97b369ef26c --- /dev/null +++ b/releasenotes/notes/use-correct-aap-mac-for-dvr-arp-ce9e00d163cf4ddc.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + An issue when arp entry in DVR router for allowed address pairs (AAP) is + configured for parent port MAC address even when AAP has different MAC address. + Ensure we use MAC address from AAP if it is set and fallback to parent + port mac address. +