From 3bcc64cf108e6a3999ab803d3c7625eb4f66843e Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 27 Sep 2013 10:57:40 -0400 Subject: [PATCH] Spawn arping in thread to speed-up floating IP Change _send_gratuitous_arp_packet() to spawn a thread to call arping after a floating IP is assigned. This way it doesn't stall _process_routers() from returning quickly due to calling pool.waitall(). Fixes Bug: 1233391 Change-Id: Id1f5eb75c222ba6a0935a294e3973292f50d0559 --- neutron/agent/l3_agent.py | 31 ++++++++------ neutron/tests/unit/test_l3_agent.py | 66 ++++++++++++++++++++--------- 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index b66d441d2ed..16c73fa81d7 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -483,22 +483,25 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): def _get_ex_gw_port(self, ri): return ri.router.get('gw_port') + def _arping(self, ri, interface_name, ip_address): + arping_cmd = ['arping', '-A', + '-I', interface_name, + '-c', self.conf.send_arp_for_ha, + ip_address] + try: + if self.conf.use_namespaces: + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + namespace=ri.ns_name()) + ip_wrapper.netns.execute(arping_cmd, check_exit_code=True) + else: + utils.execute(arping_cmd, check_exit_code=True, + root_helper=self.root_helper) + except Exception as e: + LOG.error(_("Failed sending gratuitous ARP: %s"), str(e)) + def _send_gratuitous_arp_packet(self, ri, interface_name, ip_address): if self.conf.send_arp_for_ha > 0: - arping_cmd = ['arping', '-A', '-U', - '-I', interface_name, - '-c', self.conf.send_arp_for_ha, - ip_address] - try: - if self.conf.use_namespaces: - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - namespace=ri.ns_name()) - ip_wrapper.netns.execute(arping_cmd, check_exit_code=True) - else: - utils.execute(arping_cmd, check_exit_code=True, - root_helper=self.root_helper) - except Exception as e: - LOG.error(_("Failed sending gratuitous ARP: %s"), str(e)) + eventlet.spawn_n(self._arping, ri, interface_name, ip_address) def get_internal_device_name(self, port_id): return (INTERNAL_DEV_PREFIX + port_id)[:self.driver.DEV_NAME_LEN] diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 86e43d60057..95700077b16 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -61,6 +61,10 @@ class TestBasicRouterOperations(base.BaseTestCase): 'neutron.agent.linux.external_process.ProcessManager') self.external_process = self.external_process_p.start() + self.send_arp_p = mock.patch( + 'neutron.agent.l3_agent.L3NATAgent._send_gratuitous_arp_packet') + self.send_arp = self.send_arp_p.start() + self.dvr_cls_p = mock.patch('neutron.agent.linux.interface.NullDriver') driver_cls = self.dvr_cls_p.start() self.mock_driver = mock.MagicMock() @@ -122,6 +126,7 @@ class TestBasicRouterOperations(base.BaseTestCase): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) cidr = '99.0.1.9/24' mac = 'ca:fe:de:ad:be:ef' + interface_name = agent.get_internal_device_name(port_id) if action == 'add': self.device_exists.return_value = False @@ -129,6 +134,8 @@ class TestBasicRouterOperations(base.BaseTestCase): port_id, cidr, mac) self.assertEqual(self.mock_driver.plug.call_count, 1) self.assertEqual(self.mock_driver.init_l3.call_count, 1) + self.send_arp.assert_called_once_with(ri, interface_name, + '99.0.1.9') elif action == 'remove': self.device_exists.return_value = True agent.internal_network_removed(ri, port_id, cidr) @@ -163,16 +170,8 @@ class TestBasicRouterOperations(base.BaseTestCase): interface_name, internal_cidrs) self.assertEqual(self.mock_driver.plug.call_count, 1) self.assertEqual(self.mock_driver.init_l3.call_count, 1) - arping_cmd = ['arping', '-A', '-U', - '-I', interface_name, - '-c', self.conf.send_arp_for_ha, - '20.0.0.30'] - if self.conf.use_namespaces: - self.mock_ip.netns.execute.assert_any_call( - arping_cmd, check_exit_code=True) - else: - self.utils_exec.assert_any_call( - check_exit_code=True, root_helper=self.conf.root_helper) + self.send_arp.assert_called_once_with(ri, interface_name, + '20.0.0.30') elif action == 'remove': self.device_exists.return_value = True @@ -185,6 +184,36 @@ class TestBasicRouterOperations(base.BaseTestCase): def test_agent_add_external_gateway(self): self._test_external_gateway_action('add') + def _test_arping(self, namespace): + if not namespace: + self.conf.set_override('use_namespaces', False) + + router_id = _uuid() + ri = l3_agent.RouterInfo(router_id, self.conf.root_helper, + self.conf.use_namespaces, None) + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + floating_ip = '20.0.0.101' + interface_name = agent.get_external_device_name(router_id) + agent._arping(ri, interface_name, floating_ip) + + arping_cmd = ['arping', '-A', + '-I', interface_name, + '-c', self.conf.send_arp_for_ha, + floating_ip] + if self.conf.use_namespaces: + self.mock_ip.netns.execute.assert_any_call( + arping_cmd, check_exit_code=True) + else: + self.utils_exec.assert_any_call(arping_cmd, + check_exit_code=True, + root_helper=self.conf.root_helper) + + def test_arping_namespace(self): + self._test_arping(namespace=True) + + def test_arping_no_namespace(self): + self._test_arping(namespace=False) + def test_agent_remove_external_gateway(self): self._test_external_gateway_action('remove') @@ -206,16 +235,8 @@ class TestBasicRouterOperations(base.BaseTestCase): if action == 'add': self.device_exists.return_value = False agent.floating_ip_added(ri, ex_gw_port, floating_ip, fixed_ip) - arping_cmd = ['arping', '-A', '-U', - '-I', interface_name, - '-c', self.conf.send_arp_for_ha, - floating_ip] - if self.conf.use_namespaces: - self.mock_ip.netns.execute.assert_any_call( - arping_cmd, check_exit_code=True) - else: - self.utils_exec.assert_any_call( - check_exit_code=True, root_helper=self.conf.root_helper) + self.send_arp.assert_called_once_with(ri, interface_name, + floating_ip) elif action == 'remove': self.device_exists.return_value = True @@ -409,6 +430,7 @@ class TestBasicRouterOperations(base.BaseTestCase): del router[l3_constants.INTERFACE_KEY] del router['gw_port'] agent.process_router(ri) + self.send_arp.assert_called_once() def test_process_router_snat_disabled(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) @@ -429,6 +451,7 @@ class TestBasicRouterOperations(base.BaseTestCase): if r not in ri.iptables_manager.ipv4['nat'].rules] self.assertEqual(len(nat_rules_delta), 2) self._verify_snat_rules(nat_rules_delta, router) + self.send_arp.assert_called_once() def test_process_router_snat_enabled(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) @@ -449,6 +472,7 @@ class TestBasicRouterOperations(base.BaseTestCase): if r not in orig_nat_rules] self.assertEqual(len(nat_rules_delta), 2) self._verify_snat_rules(nat_rules_delta, router) + self.send_arp.assert_called_once() def test_process_router_interface_added(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) @@ -477,6 +501,7 @@ class TestBasicRouterOperations(base.BaseTestCase): if r not in orig_nat_rules] self.assertEqual(len(nat_rules_delta), 1) self._verify_snat_rules(nat_rules_delta, router) + self.send_arp.assert_called_once() def test_process_router_interface_removed(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) @@ -497,6 +522,7 @@ class TestBasicRouterOperations(base.BaseTestCase): if r not in ri.iptables_manager.ipv4['nat'].rules] self.assertEqual(len(nat_rules_delta), 1) self._verify_snat_rules(nat_rules_delta, router, negate=True) + self.send_arp.assert_called_once() def test_handle_router_snat_rules_add_back_jump(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)