From b847cd02c56dc8fe654f4731306dc2b5493a62eb Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Thu, 25 Oct 2018 14:41:19 -0400 Subject: [PATCH] Enable 'all' IPv6 forwarding knob correctly When the external gateway is plugged and we enable IPv6 forwarding on it, make sure the 'all' sysctl knob is also enabled, else IPv6 packets will not be forwarded. This seems to only affect HA routers that default to disabling this 'all' knob on creation. Also, when we are removing all the IPv6 addresses from a HA router internal interface, set 'accept_ra' to zero so it doesn't accidentally auto-configure an address. Set it back to one when adding them back. Re-homed newly added _wait_until_ipv6_forwarding_has_state() accordingly. Closes-bug: #1787919 Change-Id: Ia1f311ee31d1479089685367a97bf13cf170b342 --- neutron/agent/l3/ha_router.py | 6 +++++ neutron/agent/l3/router_info.py | 5 +++++ .../tests/functional/agent/l3/framework.py | 18 ++++++++++----- .../functional/agent/l3/test_ha_router.py | 22 +++++++++++-------- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index 651784dec6c..8ce0d377ceb 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -27,6 +27,7 @@ from neutron.agent.l3 import router_info as router from neutron.agent.linux import external_process from neutron.agent.linux import ip_lib from neutron.agent.linux import keepalived +from neutron.common import constants as const from neutron.common import utils as common_utils LOG = logging.getLogger(__name__) @@ -290,7 +291,12 @@ class HaRouter(router.RouterInfo): ipv6_lladdr = ip_lib.get_ipv6_lladdr(device.link.address) if self._should_delete_ipv6_lladdr(ipv6_lladdr): + self.driver.configure_ipv6_ra(self.ha_namespace, interface_name, + const.ACCEPT_RA_DISABLED) device.addr.flush(n_consts.IP_VERSION_6) + else: + self.driver.configure_ipv6_ra(self.ha_namespace, interface_name, + const.ACCEPT_RA_WITHOUT_FORWARDING) self._remove_vip(ipv6_lladdr) self._add_vip(ipv6_lladdr, interface_name, scope='link') diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index beac1c8c903..d68a0594955 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -703,6 +703,11 @@ class RouterInfo(object): self.driver.configure_ipv6_ra(ns_name, interface_name, n_const.ACCEPT_RA_DISABLED) self.driver.configure_ipv6_forwarding(ns_name, interface_name, enabled) + # This will make sure the 'all' setting is the same as the interface, + # which is needed for forwarding to work. Don't disable once it's + # been enabled so as to not send spurious MLDv2 packets out. + if enabled: + self.driver.configure_ipv6_forwarding(ns_name, 'all', enabled) def _external_gateway_added(self, ex_gw_port, interface_name, ns_name, preserve_ips): diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index f70786a5c6e..55c036c28e4 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -232,14 +232,22 @@ class L3AgentTestFramework(base.BaseSudoTestCase): 'net.ipv6.conf.%s.accept_ra' % external_device_name]) self.assertEqual(enabled, int(ra_state) != n_const.ACCEPT_RA_DISABLED) - def _assert_ipv6_forwarding(self, router, enabled=True): + def _wait_until_ipv6_forwarding_has_state(self, ns_name, dev_name, state): + + def _ipv6_forwarding_has_state(): + return ip_lib.get_ipv6_forwarding( + device=dev_name, namespace=ns_name) == state + + common_utils.wait_until_true(_ipv6_forwarding_has_state) + + def _assert_ipv6_forwarding(self, router, enabled=True, all_enabled=True): external_port = router.get_ex_gw_port() external_device_name = router.get_external_device_name( external_port['id']) - ip_wrapper = ip_lib.IPWrapper(namespace=router.ns_name) - fwd_state = ip_wrapper.netns.execute(['sysctl', '-b', - 'net.ipv6.conf.%s.forwarding' % external_device_name]) - self.assertEqual(int(enabled), int(fwd_state)) + self._wait_until_ipv6_forwarding_has_state( + router.ns_name, external_device_name, int(enabled)) + self._wait_until_ipv6_forwarding_has_state( + router.ns_name, 'all', int(all_enabled)) def _router_lifecycle(self, enable_ha, ip_version=constants.IP_VERSION_4, dual_stack=False, v6_ext_gw_with_sub=True, diff --git a/neutron/tests/functional/agent/l3/test_ha_router.py b/neutron/tests/functional/agent/l3/test_ha_router.py index eb4dc085e1c..ba2a90dc8f2 100644 --- a/neutron/tests/functional/agent/l3/test_ha_router.py +++ b/neutron/tests/functional/agent/l3/test_ha_router.py @@ -118,7 +118,8 @@ class L3HATestCase(framework.L3AgentTestFramework): router_info['gw_port'] = ex_port router.process() self._assert_ipv6_accept_ra(router, expected_ra) - self._assert_ipv6_forwarding(router, expected_forwarding) + self._assert_ipv6_forwarding(router, expected_forwarding, + expected_forwarding) @testtools.skipUnless(ipv6_utils.is_enabled_and_bind_by_default(), "IPv6 is not enabled") @@ -340,14 +341,6 @@ class L3HATestCase(framework.L3AgentTestFramework): raise self.assertEqual(0, ip_nonlocal_bind_value) - def _wait_until_ipv6_forwarding_has_state(self, ns_name, dev_name, state): - - def _ipv6_forwarding_has_state(): - return ip_lib.get_ipv6_forwarding( - device=dev_name, namespace=ns_name) == state - - common_utils.wait_until_true(_ipv6_forwarding_has_state) - @testtools.skipUnless(ipv6_utils.is_enabled_and_bind_by_default(), "IPv6 is not enabled") def test_ha_router_namespace_has_ipv6_forwarding_disabled(self): @@ -394,6 +387,11 @@ class L3HATestFailover(framework.L3AgentTestFramework): master_router, slave_router = self._get_master_and_slave_routers( router1, router2) + self._assert_ipv6_accept_ra(master_router, True) + self._assert_ipv6_forwarding(master_router, True, True) + self._assert_ipv6_accept_ra(slave_router, False) + self._assert_ipv6_forwarding(slave_router, False, False) + self.fail_ha_router(router1) # NOTE: passing slave_router as first argument, because we expect @@ -403,6 +401,12 @@ class L3HATestFailover(framework.L3AgentTestFramework): self.assertEqual(master_router, new_slave) self.assertEqual(slave_router, new_master) + self._assert_ipv6_accept_ra(new_master, True) + self._assert_ipv6_forwarding(new_master, True, True) + self._assert_ipv6_accept_ra(new_slave, False) + # after transition from master -> slave, 'all' IPv6 forwarding should + # still be enabled + self._assert_ipv6_forwarding(new_slave, False, True) def test_ha_router_lost_gw_connection(self): self.agent.conf.set_override(