]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Remove hack for sending gratuitous arp from fip ns
authorJack McCann <jack.mccann@hp.com>
Wed, 15 Apr 2015 22:32:51 +0000 (22:32 +0000)
committerCarl Baldwin <carl.baldwin@hp.com>
Tue, 26 May 2015 16:13:35 +0000 (16:13 +0000)
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
neutron/agent/l3/dvr_router.py
neutron/agent/linux/ip_lib.py
neutron/tests/unit/agent/l3/test_agent.py
neutron/tests/unit/agent/l3/test_dvr_router.py
neutron/tests/unit/agent/linux/test_ip_lib.py

index 050cb6331ae84369a95e7748b8518ae96fe45afb..e2e63eb2700dafb0b7b26fd3e7f437dba6aad37e 100644 (file)
@@ -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:
index dfd9cf2c543accb989d375e0312ea5828ce6d0b7..8c1313acc9f2ab583fd9f8d1c47bc1af9b671e9f 100755 (executable)
@@ -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
 
index a22b46b4f5ab38f4bb3419e7fdf579294b8152f7..407ccf7165acc741fe6806832387f0eb99335126 100644 (file)
@@ -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."""
 
index e628cd465c7e3e2c347bf51b262d85fd32165a52..47dbd109e32d05287524e90a6b8efee99631c910 100644 (file)
@@ -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()
index ecd092e47dbef7d98025b66b6f1b5ed59b6f55e3..fbbf08c43a600d77b07a30c3dec5a8a38256ebb0 100644 (file)
@@ -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):
index 7b34d353742c9804dea8c888b59207e8e6115c65..01ddf39997b30e3e19f4e7b21d5ddb3ad687453b 100644 (file)
@@ -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):