From eac9fb143cf3e6f4dd2b3413f81dab92d42354ca Mon Sep 17 00:00:00 2001 From: Jack McCann Date: Wed, 15 Apr 2015 22:32:51 +0000 Subject: [PATCH] Remove hack for sending gratuitous arp from fip ns I just saw this note toward bottom of the lartc [1] page: "On Linux 2.4, you may need to execute 'echo 1 > /proc/sys/net/ipv4/ip_nonlocal_bind' before being able to send out unsolicited ARP messages!" I wonder if we set that in fip ns, if it will let us send grat arp without adding/removing the IP. It couldn't be that easy, could it? [1] http://lartc.org/howto/lartc.bridging.proxy-arp.html Change-Id: Ie55b402a6c46af00c8d4875264489fc4318544b3 --- neutron/agent/l3/dvr_fip_ns.py | 4 +++ neutron/agent/l3/dvr_router.py | 8 ++--- neutron/agent/linux/ip_lib.py | 24 -------------- neutron/tests/unit/agent/l3/test_agent.py | 4 --- .../tests/unit/agent/l3/test_dvr_router.py | 2 +- neutron/tests/unit/agent/linux/test_ip_lib.py | 32 ------------------- 6 files changed, 9 insertions(+), 65 deletions(-) diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index 050cb6331..e2e63eb27 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -126,6 +126,10 @@ class FipNamespace(namespaces.Namespace): def create(self): # TODO(Carl) Get this functionality from mlavelle's namespace baseclass ip_wrapper_root = ip_lib.IPWrapper() + ip_wrapper_root.netns.execute(['sysctl', + '-w', + 'net.ipv4.ip_nonlocal_bind=1'], + run_as_root=True) ip_wrapper = ip_wrapper_root.ensure_namespace(self.get_name()) ip_wrapper.netns.execute(['sysctl', '-w', 'net.ipv4.ip_forward=1']) if self.use_ipv6: diff --git a/neutron/agent/l3/dvr_router.py b/neutron/agent/l3/dvr_router.py index dfd9cf2c5..8c1313acc 100755 --- a/neutron/agent/l3/dvr_router.py +++ b/neutron/agent/l3/dvr_router.py @@ -100,10 +100,10 @@ class DvrRouter(router.RouterInfo): interface_name = ( self.fip_ns.get_ext_device_name( self.fip_ns.agent_gateway_port['id'])) - ip_lib.send_garp_for_proxyarp(fip_ns_name, - interface_name, - floating_ip, - self.agent_conf.send_arp_for_ha) + ip_lib.send_gratuitous_arp(fip_ns_name, + interface_name, + floating_ip, + self.agent_conf.send_arp_for_ha) # update internal structures self.dist_fip_count = self.dist_fip_count + 1 diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index a22b46b4f..407ccf716 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -732,30 +732,6 @@ def send_gratuitous_arp(ns_name, iface_name, address, count): eventlet.spawn_n(arping) -def send_garp_for_proxyarp(ns_name, iface_name, address, count): - """ - Send a gratuitous arp using given namespace, interface, and address - - This version should be used when proxy arp is in use since the interface - won't actually have the address configured. We actually need to configure - the address on the interface and then remove it when the proxy arp has been - sent. - """ - def arping_with_temporary_address(): - # Configure the address on the interface - device = IPDevice(iface_name, namespace=ns_name) - net = netaddr.IPNetwork(str(address)) - device.addr.add(str(net)) - - _arping(ns_name, iface_name, address, count) - - # Delete the address from the interface - device.addr.delete(str(net)) - - if count > 0: - eventlet.spawn_n(arping_with_temporary_address) - - def add_namespace_to_cmd(cmd, namespace=None): """Add an optional namespace to the command.""" diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index e628cd465..47dbd109e 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -324,10 +324,6 @@ class BasicRouterOperationsFramework(base.BaseTestCase): 'neutron.agent.linux.ip_lib.send_gratuitous_arp') self.send_arp = self.send_arp_p.start() - self.send_arp_proxyarp_p = mock.patch( - 'neutron.agent.linux.ip_lib.send_garp_for_proxyarp') - self.send_arp_proxyarp = self.send_arp_proxyarp_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() diff --git a/neutron/tests/unit/agent/l3/test_dvr_router.py b/neutron/tests/unit/agent/l3/test_dvr_router.py index ecd092e47..fbbf08c43 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_router.py @@ -56,7 +56,7 @@ class TestDvrRouterOperations(base.BaseTestCase): self.assertEqual([{'host': mock.sentinel.myhost}], fips) - @mock.patch.object(ip_lib, 'send_garp_for_proxyarp') + @mock.patch.object(ip_lib, 'send_gratuitous_arp') @mock.patch.object(ip_lib, 'IPDevice') @mock.patch.object(ip_lib, 'IPRule') def test_floating_ip_added_dist(self, mIPRule, mIPDevice, mock_arp): diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 7b34d3537..01ddf3999 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -1041,38 +1041,6 @@ class TestArpPing(TestIPCmdBase): self._test_arping( ip_lib.send_gratuitous_arp, '20.0.0.1', spawn_n, mIPWrapper) - @mock.patch.object(ip_lib, 'IPDevice') - @mock.patch.object(ip_lib, 'IPWrapper') - @mock.patch('eventlet.spawn_n') - def test_send_garp_for_proxy_arp(self, spawn_n, mIPWrapper, mIPDevice): - addr = '20.0.0.1' - ip_wrapper = mIPWrapper(namespace=mock.sentinel.ns_name) - mIPWrapper.reset_mock() - device = mIPDevice(mock.sentinel.iface_name, - namespace=mock.sentinel.ns_name) - mIPDevice.reset_mock() - - # Check that the address was added to the interface before arping - def check_added_address(*args, **kwargs): - mIPDevice.assert_called_once_with(mock.sentinel.iface_name, - namespace=mock.sentinel.ns_name) - device.addr.add.assert_called_once_with(addr + '/32') - self.assertFalse(device.addr.delete.called) - device.addr.reset_mock() - - ip_wrapper.netns.execute.side_effect = check_added_address - - self._test_arping( - ip_lib.send_garp_for_proxyarp, addr, spawn_n, mIPWrapper) - - # Test that the address was removed after arping - device = mIPDevice(mock.sentinel.iface_name, - namespace=mock.sentinel.ns_name) - device.addr.delete.assert_called_once_with(addr + '/32') - - # If this was called then check_added_address probably had a assert - self.assertFalse(device.addr.add.called) - class TestAddNamespaceToCmd(base.BaseTestCase): def test_add_namespace_to_cmd_with_namespace(self): -- 2.45.2