From: Dane LeBlanc Date: Tue, 24 Feb 2015 20:47:01 +0000 (-0500) Subject: Stop sending gratuitous arp when ip version is 6 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=5ff082bcfe12647036e5b033bfc2bac514acdb42;p=openstack-build%2Fneutron-build.git 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 9b7eee99a..90e24d129 100644 --- 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 8c1313acc..df3d465e4 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_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 9c7c5bdc7..2b8ccdbaa 100644 --- 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 0dfbc13ef..f698a94d6 100644 --- 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 32fe1f9ac..f04152cf5 100644 --- 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 aeec5c6f1..143c659dd 100644 --- 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 47b8c45e2..b6ee852ad 100644 --- 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 fbbf08c43..8b68478c1 100644 --- 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 2bf4f3035..b34b3cc54 100644 --- 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 01ddf3999..51ac34cfe 100644 --- 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):