From 5f2758bb800c0376efd3e0526f808d73b9ad1bc0 Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Mon, 30 Sep 2019 11:03:49 +0800 Subject: [PATCH] Move arp device check out of loop This could be time-consuming if there are lots of ports under the router. So this patch moves the same device check out of the loop. Closes-Bug: #1856839 Change-Id: I2da856712aaafb77878628c52d19e0a5c7cdee0f --- neutron/agent/l3/dvr.py | 6 ++- neutron/agent/l3/dvr_local_router.py | 41 +++++++++++++------ .../unit/agent/l3/test_dvr_local_router.py | 12 +++--- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py index 44e840621e9..f267cf739f3 100644 --- a/neutron/agent/l3/dvr.py +++ b/neutron/agent/l3/dvr.py @@ -53,8 +53,10 @@ class AgentMixin(object): ip = arp_table['ip_address'] mac = arp_table['mac_address'] subnet_id = arp_table['subnet_id'] - - ri._update_arp_entry(ip, mac, subnet_id, action) + device, device_exists = ri.get_arp_related_dev(subnet_id) + ri._update_arp_entry(ip, mac, subnet_id, action, + device, + device_exists=device_exists) def add_arp_entry(self, context, payload): """Add arp entry into router namespace. Called from RPC.""" diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index d0c8cb4b8c9..99b81b9df52 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -223,12 +223,15 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): def _process_arp_cache_for_internal_port(self, subnet_id): """Function to process the cached arp entries.""" arp_remove = set() + device, device_exists = self.get_arp_related_dev(subnet_id) for arp_entry in self._pending_arp_set: if subnet_id == arp_entry.subnet_id: try: state = self._update_arp_entry( arp_entry.ip, arp_entry.mac, - arp_entry.subnet_id, arp_entry.operation) + arp_entry.subnet_id, arp_entry.operation, + device=device, + device_exists=device_exists) except Exception: state = False if state: @@ -246,18 +249,13 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): arp_delete.add(arp_entry) self._pending_arp_set -= arp_delete - def _update_arp_entry(self, ip, mac, subnet_id, operation): + def _update_arp_entry( + self, ip, mac, subnet_id, operation, device, + device_exists=True): """Add or delete arp entry into router namespace for the subnet.""" - port = self._get_internal_port(subnet_id) - # update arp entry only if the subnet is attached to the router - if not port: - return False try: - # TODO(mrsmith): optimize the calls below for bulk calls - interface_name = self.get_internal_device_name(port['id']) - device = ip_lib.IPDevice(interface_name, namespace=self.ns_name) - if device.exists(): + if device_exists: if operation == 'add': device.neigh.add(ip, mac) elif operation == 'delete': @@ -276,6 +274,16 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): with excutils.save_and_reraise_exception(): LOG.exception("DVR: Failed updating arp entry") + def get_arp_related_dev(self, subnet_id): + port = self._get_internal_port(subnet_id) + # update arp entry only if the subnet is attached to the router + if not port: + return None, False + interface_name = self.get_internal_device_name(port['id']) + device = ip_lib.IPDevice(interface_name, namespace=self.ns_name) + device_exists = device.exists() + return device, device_exists + def _set_subnet_arp_info(self, subnet_id): """Set ARP info retrieved from Plugin for existing ports.""" # TODO(Carl) Can we eliminate the need to make this RPC while @@ -284,6 +292,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): ignored_device_owners = ( lib_constants.ROUTER_INTERFACE_OWNERS + tuple(common_utils.get_dvr_allowed_address_pair_device_owners())) + device, device_exists = self.get_arp_related_dev(subnet_id) for p in subnet_ports: if p['device_owner'] not in ignored_device_owners: @@ -291,7 +300,9 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self._update_arp_entry(fixed_ip['ip_address'], p['mac_address'], subnet_id, - 'add') + 'add', + device=device, + device_exists=device_exists) self._process_arp_cache_for_internal_port(subnet_id) @staticmethod @@ -525,10 +536,14 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self.enable_snat_redirect_rules(ex_gw_port) for port in self.get_snat_interfaces(): for ip in port['fixed_ips']: + subnet_id = ip['subnet_id'] + device, device_exists = self.get_arp_related_dev(subnet_id) self._update_arp_entry(ip['ip_address'], port['mac_address'], - ip['subnet_id'], - 'add') + subnet_id, + 'add', + device=device, + device_exists=device_exists) def external_gateway_updated(self, ex_gw_port, interface_name): pass 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 576bcecd6d9..bdf40a4cd30 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -561,14 +561,14 @@ class TestDvrRouterOperations(base.BaseTestCase): payload = {'arp_table': arp_table, 'router_id': router['id']} agent.add_arp_entry(None, payload) - def test__update_arp_entry_with_no_subnet(self): + def test_get_arp_related_dev_no_subnet(self): self._set_ri_kwargs(mock.sentinel.agent, 'foo_router_id', {'distributed': True, 'gw_port_host': HOSTNAME}) ri = dvr_router.DvrLocalRouter(HOSTNAME, **self.ri_kwargs) - ri.get_internal_device_name = mock.Mock() - ri._update_arp_entry(mock.ANY, mock.ANY, 'foo_subnet_id', 'add') - self.assertFalse(ri.get_internal_device_name.call_count) + with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as f: + ri.get_arp_related_dev('foo_subnet_id') + self.assertFalse(f.call_count) def _setup_test_for_arp_entry_cache(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) @@ -585,9 +585,9 @@ class TestDvrRouterOperations(base.BaseTestCase): state = True with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as rtrdev,\ mock.patch.object(ri, '_cache_arp_entry') as arp_cache: - rtrdev.return_value.exists.return_value = False state = ri._update_arp_entry( - mock.ANY, mock.ANY, subnet_id, 'add') + mock.ANY, mock.ANY, subnet_id, 'add', + mock.ANY, device_exists=False) self.assertFalse(state) self.assertTrue(arp_cache.called) arp_cache.assert_called_once_with(mock.ANY, mock.ANY,