Stop sending gratuitous arp when ip version is 6

This fix prevents calls to the arping utility for IPv6
addresses, thereby eliminating errors reported by arping
for IPv6 addresses.

The assumption is that NDP, DAD, and RAs are sufficient
for address resolution and duplicate address detection
for IPv6, and that unsolicited Neighbor Advertisements (NAs)
are not required for OpenStack services. If this turns out
not to be the case for some service/feature, then a separate
bug should be filed to add support for unsolicited NAs for
that service.

Change-Id: I14f869b7d488d7e691f7316eafcab3064e12cda6
Closes-Bug: 1357068

diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py
index 9b7eee99a88915e507f68768f9eb252a9cceeb10..90e24d129d9bcaf50e67cf271ce9b88332db3c29 100644 (file)
--- a/neutron/agent/l3/dvr_fip_ns.py
+++ b/neutron/agent/l3/dvr_fip_ns.py
@@ -107,10 +107,10 @@ class FipNamespace(namespaces.Namespace):
                             clean_connections=True)
 
         for fixed_ip in ex_gw_port['fixed_ips']:
-            ip_lib.send_gratuitous_arp(ns_name,
-                                       interface_name,
-                                       fixed_ip['ip_address'],
-                                       self.agent_conf.send_arp_for_ha)
+            ip_lib.send_ip_addr_adv_notif(ns_name,
+                                          interface_name,
+                                          fixed_ip['ip_address'],
+                                          self.agent_conf)
 
         for subnet in ex_gw_port['subnets']:
             gw_ip = subnet.get('gateway_ip')

diff --git a/neutron/agent/l3/dvr_router.py b/neutron/agent/l3/dvr_router.py
index 8c1313acc9f2ab583fd9f8d1c47bc1af9b671e9f..df3d465e4d6899f3bee907a353b57424ecb59558 100755 (executable)
--- 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_gratuitous_arp(fip_ns_name,
-                                   interface_name,
-                                   floating_ip,
-                                   self.agent_conf.send_arp_for_ha)
+        ip_lib.send_ip_addr_adv_notif(fip_ns_name,
+                                      interface_name,
+                                      floating_ip,
+                                      self.agent_conf)
         # update internal structures
         self.dist_fip_count = self.dist_fip_count + 1
 

diff --git a/neutron/agent/l3/legacy_router.py b/neutron/agent/l3/legacy_router.py
index 9c7c5bdc7e96074b11d0eab27bd6eb048785477b..2b8ccdbaa96bcf46309319d8d131dc069e3deff5 100644 (file)
--- a/neutron/agent/l3/legacy_router.py
+++ b/neutron/agent/l3/legacy_router.py
@@ -24,8 +24,8 @@ class LegacyRouter(router.RouterInfo):
 
         # As GARP is processed in a distinct thread the call below
         # won't raise an exception to be handled.
-        ip_lib.send_gratuitous_arp(self.ns_name,
-                                   interface_name,
-                                   fip['floating_ip_address'],
-                                   self.agent_conf.send_arp_for_ha)
+        ip_lib.send_ip_addr_adv_notif(self.ns_name,
+                                      interface_name,
+                                      fip['floating_ip_address'],
+                                      self.agent_conf)
         return l3_constants.FLOATINGIP_STATUS_ACTIVE

diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py
index 0dfbc13ef589a4bb454ef83b39e4c3bb72e9f8f4..f698a94d61ce6ed43b5988c75d8ec0dca1590882 100644 (file)
--- a/neutron/agent/l3/router_info.py
+++ b/neutron/agent/l3/router_info.py
@@ -293,10 +293,10 @@ class RouterInfo(object):
         ip_cidrs = common_utils.fixed_ip_cidrs(fixed_ips)
         self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name)
         for fixed_ip in fixed_ips:
-            ip_lib.send_gratuitous_arp(ns_name,
-                                       interface_name,
-                                       fixed_ip['ip_address'],
-                                       self.agent_conf.send_arp_for_ha)
+            ip_lib.send_ip_addr_adv_notif(ns_name,
+                                          interface_name,
+                                          fixed_ip['ip_address'],
+                                          self.agent_conf)
 
     def internal_network_added(self, port):
         network_id = port['network_id']
@@ -465,10 +465,10 @@ class RouterInfo(object):
                             enable_ra_on_gw=enable_ra_on_gw,
                             clean_connections=True)
         for fixed_ip in ex_gw_port['fixed_ips']:
-            ip_lib.send_gratuitous_arp(ns_name,
-                                       interface_name,
-                                       fixed_ip['ip_address'],
-                                       self.agent_conf.send_arp_for_ha)
+            ip_lib.send_ip_addr_adv_notif(ns_name,
+                                          interface_name,
+                                          fixed_ip['ip_address'],
+                                          self.agent_conf)
 
     def is_v6_gateway_set(self, gateway_ips):
         """Check to see if list of gateway_ips has an IPv6 gateway.

diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py
index 32fe1f9ac84b8bcb590517471c6a08b1a17e6efc..f04152cf5382388f35330edd58305a1f2d409cfb 100644 (file)
--- a/neutron/agent/linux/ip_lib.py
+++ b/neutron/agent/linux/ip_lib.py
@@ -757,13 +757,23 @@ def _arping(ns_name, iface_name, address, count):
                             'ns': ns_name})
 
 
-def send_gratuitous_arp(ns_name, iface_name, address, count):
-    """Send a gratuitous arp using given namespace, interface, and address."""
+def send_ip_addr_adv_notif(ns_name, iface_name, address, config):
+    """Send advance notification of an IP address assignment.
+
+    If the address is in the IPv4 family, send gratuitous ARP.
+
+    If the address is in the IPv6 family, no advance notification is
+    necessary, since the Neighbor Discovery Protocol (NDP), Duplicate
+    Address Discovery (DAD), and (for stateless addresses) router
+    advertisements (RAs) are sufficient for address resolution and
+    duplicate address detection.
+    """
+    count = config.send_arp_for_ha
 
     def arping():
         _arping(ns_name, iface_name, address, count)
 
-    if count > 0:
+    if count > 0 and netaddr.IPAddress(address).version == 4:
         eventlet.spawn_n(arping)
 
 

diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index aeec5c6f1c2db38ca9820ed1b4e0f31b47d3c8df..143c659dd122f7c53bb5315cf567eeeedb0f8804 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -323,9 +323,9 @@ class BasicRouterOperationsFramework(base.BaseTestCase):
         self.process_monitor = mock.patch(
             'neutron.agent.linux.external_process.ProcessMonitor').start()
 
-        self.send_arp_p = mock.patch(
-            'neutron.agent.linux.ip_lib.send_gratuitous_arp')
-        self.send_arp = self.send_arp_p.start()
+        self.send_adv_notif_p = mock.patch(
+            'neutron.agent.linux.ip_lib.send_ip_addr_adv_notif')
+        self.send_adv_notif = self.send_adv_notif_p.start()
 
         self.dvr_cls_p = mock.patch('neutron.agent.linux.interface.NullDriver')
         driver_cls = self.dvr_cls_p.start()
@@ -510,8 +510,9 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             ri.internal_network_added(port)
             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.ns_name, interface_name,
-                                                  '99.0.1.9', mock.ANY)
+            self.send_adv_notif.assert_called_once_with(ri.ns_name,
+                                                        interface_name,
+                                                        '99.0.1.9', mock.ANY)
         elif action == 'remove':
             self.device_exists.return_value = True
             ri.internal_network_removed(port)
@@ -622,7 +623,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             self.assertEqual(self.mock_driver.plug.call_count, 1)
             self.assertEqual(self.mock_driver.init_l3.call_count, 1)
             if no_subnet and not dual_stack:
-                self.assertEqual(self.send_arp.call_count, 0)
+                self.assertEqual(self.send_adv_notif.call_count, 0)
                 ip_cidrs = []
                 gateway_ips = []
                 if no_sub_gw:
@@ -640,7 +641,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
                     exp_arp_calls += [mock.call(ri.ns_name, interface_name,
                                                 '2001:192:168:100::2',
                                                 mock.ANY)]
-                self.send_arp.assert_has_calls(exp_arp_calls)
+                self.send_adv_notif.assert_has_calls(exp_arp_calls)
                 ip_cidrs = ['20.0.0.30/24']
                 gateway_ips = ['20.0.0.1']
                 if dual_stack:
@@ -811,7 +812,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             ri.use_ipv6 = True
             exp_arp_calls += [mock.call(ri.ns_name, interface_name,
                                         '2001:192:168:100::2', mock.ANY)]
-        self.send_arp.assert_has_calls(exp_arp_calls)
+        self.send_adv_notif.assert_has_calls(exp_arp_calls)
         ip_cidrs = ['20.0.0.30/24']
         gateway_ips = ['20.0.0.1']
         if dual_stack:
@@ -1148,7 +1149,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         del router[l3_constants.INTERFACE_KEY]
         del router['gw_port']
         ri.process(agent)
-        self.assertEqual(self.send_arp.call_count, 1)
+        self.assertEqual(self.send_adv_notif.call_count, 1)
         distributed = ri.router.get('distributed', False)
         self.assertEqual(ri.process_floating_ip_addresses.called,
                          distributed)
@@ -1385,7 +1386,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         self.assertEqual(len(mangle_rules_delta), 1)
         self._verify_snat_mangle_rules(nat_rules_delta, mangle_rules_delta,
                                        router)
-        self.assertEqual(self.send_arp.call_count, 1)
+        self.assertEqual(self.send_adv_notif.call_count, 1)
 
     def test_process_router_snat_enabled(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
@@ -1412,7 +1413,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         self.assertEqual(len(mangle_rules_delta), 1)
         self._verify_snat_mangle_rules(nat_rules_delta, mangle_rules_delta,
                                        router)
-        self.assertEqual(self.send_arp.call_count, 1)
+        self.assertEqual(self.send_adv_notif.call_count, 1)
 
     def test_process_router_interface_added(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
@@ -1426,8 +1427,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         # Reassign the router object to RouterInfo
         ri.router = router
         ri.process(agent)
-        # send_arp is called both times process is called
-        self.assertEqual(self.send_arp.call_count, 2)
+        # send_ip_addr_adv_notif is called both times process is called
+        self.assertEqual(self.send_adv_notif.call_count, 2)
 
     def _test_process_ipv6_only_or_dual_stack_gw(self, dual_stack=False):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
@@ -1617,8 +1618,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         # Reassign the router object to RouterInfo
         ri.router = router
         ri.process(agent)
-        # send_arp is called both times process is called
-        self.assertEqual(self.send_arp.call_count, 2)
+        # send_ip_addr_adv_notif is called both times process is called
+        self.assertEqual(self.send_adv_notif.call_count, 2)
 
     def test_process_router_ipv6_interface_removed(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)

diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
index 47b8c45e2d0bef286e9aa6098df8bf3ad849c61e..b6ee852ad8333a109adc288bce064ac5d2e64c80 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
+++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
@@ -67,9 +67,10 @@ class TestDvrFipNs(base.BaseTestCase):
 
     @mock.patch.object(ip_lib, 'IPWrapper')
     @mock.patch.object(ip_lib, 'IPDevice')
-    @mock.patch.object(ip_lib, 'send_gratuitous_arp')
+    @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif')
     @mock.patch.object(ip_lib, 'device_exists')
-    def test_gateway_added(self, device_exists, send_arp, IPDevice, IPWrapper):
+    def test_gateway_added(self, device_exists, send_adv_notif,
+                           IPDevice, IPWrapper):
         subnet_id = _uuid()
         agent_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30',
                                         'prefixlen': 24,
@@ -86,10 +87,10 @@ class TestDvrFipNs(base.BaseTestCase):
                                    mock.sentinel.interface_name)
         self.assertEqual(self.driver.plug.call_count, 1)
         self.assertEqual(self.driver.init_l3.call_count, 1)
-        send_arp.assert_called_once_with(self.fip_ns.get_name(),
-                                         mock.sentinel.interface_name,
-                                         '20.0.0.30',
-                                         mock.ANY)
+        send_adv_notif.assert_called_once_with(self.fip_ns.get_name(),
+                                               mock.sentinel.interface_name,
+                                               '20.0.0.30',
+                                               mock.ANY)
 
     @mock.patch.object(ip_lib, 'IPWrapper')
     def test_destroy(self, IPWrapper):

diff --git a/neutron/tests/unit/agent/l3/test_dvr_router.py b/neutron/tests/unit/agent/l3/test_dvr_router.py
index fbbf08c43a600d77b07a30c3dec5a8a38256ebb0..8b68478c1be4d93dfc45eceb371d955e03964cb7 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_dvr_router.py
+++ b/neutron/tests/unit/agent/l3/test_dvr_router.py
@@ -56,10 +56,10 @@ class TestDvrRouterOperations(base.BaseTestCase):
 
         self.assertEqual([{'host': mock.sentinel.myhost}], fips)
 
-    @mock.patch.object(ip_lib, 'send_gratuitous_arp')
+    @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif')
     @mock.patch.object(ip_lib, 'IPDevice')
     @mock.patch.object(ip_lib, 'IPRule')
-    def test_floating_ip_added_dist(self, mIPRule, mIPDevice, mock_arp):
+    def test_floating_ip_added_dist(self, mIPRule, mIPDevice, mock_adv_notif):
         router = mock.MagicMock()
         ri = self._create_router(router)
         ext_net_id = _uuid()

diff --git a/neutron/tests/unit/agent/l3/test_legacy_router.py b/neutron/tests/unit/agent/l3/test_legacy_router.py
index 2bf4f303515b4634bca28c65c068089dc6ce750e..b34b3cc540a659e4e40ee5afd81cbe6bb5c853de 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_legacy_router.py
+++ b/neutron/tests/unit/agent/l3/test_legacy_router.py
@@ -49,28 +49,27 @@ class TestBasicRouterOperations(BasicRouterTestCaseFramework):
         device.delete_addr_and_conntrack_state.assert_called_once_with(cidr)
 
 
-@mock.patch.object(ip_lib, 'send_gratuitous_arp')
+@mock.patch.object(ip_lib, 'send_ip_addr_adv_notif')
 class TestAddFloatingIpWithMockGarp(BasicRouterTestCaseFramework):
-    def test_add_floating_ip(self, send_gratuitous_arp):
+    def test_add_floating_ip(self, send_ip_addr_adv_notif):
         ri = self._create_router()
         ri._add_fip_addr_to_device = mock.Mock(return_value=True)
-        self.agent_conf.send_arp_for_ha = mock.sentinel.arp_count
         ip = '15.1.2.3'
         result = ri.add_floating_ip({'floating_ip_address': ip},
                                     mock.sentinel.interface_name,
                                     mock.sentinel.device)
-        ip_lib.send_gratuitous_arp.assert_called_once_with(
+        ip_lib.send_ip_addr_adv_notif.assert_called_once_with(
             ri.ns_name,
             mock.sentinel.interface_name,
             ip,
-            mock.sentinel.arp_count)
+            self.agent_conf)
         self.assertEqual(l3_constants.FLOATINGIP_STATUS_ACTIVE, result)
 
-    def test_add_floating_ip_error(self, send_gratuitous_arp):
+    def test_add_floating_ip_error(self, send_ip_addr_adv_notif):
         ri = self._create_router()
         ri._add_fip_addr_to_device = mock.Mock(return_value=False)
         result = ri.add_floating_ip({'floating_ip_address': '15.1.2.3'},
                                     mock.sentinel.interface_name,
                                     mock.sentinel.device)
-        self.assertFalse(ip_lib.send_gratuitous_arp.called)
+        self.assertFalse(ip_lib.send_ip_addr_adv_notif.called)
         self.assertEqual(l3_constants.FLOATINGIP_STATUS_ERROR, result)

diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py
index 01ddf39997b30e3e19f4e7b21d5ddb3ad687453b..51ac34cfe95ef3bff2bf65e1eff62b5d55ae02d3 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_ip_lib.py
+++ b/neutron/tests/unit/agent/linux/test_ip_lib.py
@@ -1013,13 +1013,18 @@ class TestIpNeighCommand(TestIPCmdBase):
 
 
 class TestArpPing(TestIPCmdBase):
-    def _test_arping(self, function, address, spawn_n, mIPWrapper):
+    @mock.patch.object(ip_lib, 'IPWrapper')
+    @mock.patch('eventlet.spawn_n')
+    def test_send_ipv4_addr_adv_notif(self, spawn_n, mIPWrapper):
         spawn_n.side_effect = lambda f: f()
         ARPING_COUNT = 3
-        function(mock.sentinel.ns_name,
-                 mock.sentinel.iface_name,
-                 address,
-                 ARPING_COUNT)
+        address = '20.0.0.1'
+        config = mock.Mock()
+        config.send_arp_for_ha = ARPING_COUNT
+        ip_lib.send_ip_addr_adv_notif(mock.sentinel.ns_name,
+                                      mock.sentinel.iface_name,
+                                      address,
+                                      config)
 
         self.assertTrue(spawn_n.called)
         mIPWrapper.assert_called_once_with(namespace=mock.sentinel.ns_name)
@@ -1035,11 +1040,16 @@ class TestArpPing(TestIPCmdBase):
         ip_wrapper.netns.execute.assert_any_call(arping_cmd,
                                                  check_exit_code=True)
 
-    @mock.patch.object(ip_lib, 'IPWrapper')
     @mock.patch('eventlet.spawn_n')
-    def test_send_gratuitous_arp(self, spawn_n, mIPWrapper):
-        self._test_arping(
-            ip_lib.send_gratuitous_arp, '20.0.0.1', spawn_n, mIPWrapper)
+    def test_no_ipv6_addr_notif(self, spawn_n):
+        ipv6_addr = 'fd00::1'
+        config = mock.Mock()
+        config.send_arp_for_ha = 3
+        ip_lib.send_ip_addr_adv_notif(mock.sentinel.ns_name,
+                                      mock.sentinel.iface_name,
+                                      ipv6_addr,
+                                      config)
+        self.assertFalse(spawn_n.called)
 
 
 class TestAddNamespaceToCmd(base.BaseTestCase):