diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 126d49415df..64c4920897a 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -1364,15 +1364,6 @@ class DeviceManager(object): def plug(self, network, port, interface_name): """Plug device settings for the network's DHCP on this host.""" - # Disable acceptance of RAs in the namespace so we don't - # auto-configure an IPv6 address since we explicitly configure - # them on the device. This must be done before any interfaces - # are plugged since it could receive an RA by the time - # plug() returns, so we have to create the namespace first. - if network.namespace: - ip_lib.IPWrapper().ensure_namespace(network.namespace) - self.driver.configure_ipv6_ra(network.namespace, 'default', - n_const.ACCEPT_RA_DISABLED) self.driver.plug(network.id, port.id, interface_name, @@ -1392,6 +1383,19 @@ class DeviceManager(object): self._update_dhcp_port(network, port) interface_name = self.get_interface_name(network, port) + # Disable acceptance of RAs in the namespace so we don't + # auto-configure an IPv6 address since we explicitly configure + # them on the device. This must be done before any interfaces + # are plugged since it could receive an RA by the time + # plug() returns, so we have to create the namespace first. + # It must also be done in the case there is an existing IPv6 + # address here created via SLAAC, since it will be deleted + # and added back statically in the call to init_l3() below. + if network.namespace: + ip_lib.IPWrapper().ensure_namespace(network.namespace) + self.driver.configure_ipv6_ra(network.namespace, 'default', + n_const.ACCEPT_RA_DISABLED) + if ip_lib.ensure_device_is_ready(interface_name, namespace=network.namespace): LOG.debug('Reusing existing device: %s.', interface_name) diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index 0ba91f20db3..7b5c1deacb7 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -117,30 +117,50 @@ class LinuxInterfaceDriver(object): # Neutron, so it would be deleted if we added it to the 'previous' # list here default_ipv6_lla = ip_lib.get_ipv6_lladdr(device.link.address) - previous = {addr['cidr'] for addr in device.addr.list( - filters=['permanent'])} - {default_ipv6_lla} - # add new addresses + cidrs = set() + remove_ips = set() + + # normalize all the IP addresses first for ip_cidr in ip_cidrs: - net = netaddr.IPNetwork(ip_cidr) # Convert to compact IPv6 address because the return values of # "ip addr list" are compact. if net.version == 6: ip_cidr = str(net) - if ip_cidr in previous: - previous.remove(ip_cidr) + cidrs.add(ip_cidr) + + # Determine the addresses that must be added and removed + for address in device.addr.list(): + cidr = address['cidr'] + dynamic = address['dynamic'] + + # skip the IPv6 link-local + if cidr == default_ipv6_lla: continue - device.addr.add(ip_cidr) + if cidr in preserve_ips: + continue - # clean up any old addresses - for ip_cidr in previous: - if ip_cidr not in preserve_ips: - if clean_connections: - device.delete_addr_and_conntrack_state(ip_cidr) - else: - device.addr.delete(ip_cidr) + # Statically created addresses are OK, dynamically created + # addresses must be removed and replaced + if cidr in cidrs and not dynamic: + cidrs.remove(cidr) + continue + + remove_ips.add(cidr) + + # Clean up any old addresses. This must be done first since there + # could be a dynamic address being replaced with a static one. + for ip_cidr in remove_ips: + if clean_connections: + device.delete_addr_and_conntrack_state(ip_cidr) + else: + device.addr.delete(ip_cidr) + + # add any new addresses + for ip_cidr in cidrs: + device.addr.add(ip_cidr) def init_router_port(self, device_name, diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index 3596d9a162d..d045bc2880d 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -1506,15 +1506,13 @@ class TestDeviceManager(base.BaseTestCase): expected_ips = ['172.9.9.9/24', '169.254.169.254/16'] expected = [ mock.call.get_device_name(port), + mock.call.configure_ipv6_ra(net.namespace, 'default', 0), mock.call.init_l3( 'tap12345678-12', expected_ips, namespace=net.namespace)] if not device_is_ready: - expected.insert(1, - mock.call.configure_ipv6_ra(net.namespace, - 'default', 0)) expected.insert(2, mock.call.plug(net.id, port.id, diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py index 01cfc691c78..a634f8ba032 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -121,9 +121,9 @@ class TestABCDriver(TestBase): extra_subnets=[{'cidr': '172.20.0.0/24'}]) self.ip_dev.assert_has_calls( [mock.call('tap0', namespace=ns), - mock.call().addr.list(filters=['permanent']), - mock.call().addr.add('192.168.1.2/24'), + mock.call().addr.list(), mock.call().addr.delete('172.16.77.240/24'), + mock.call().addr.add('192.168.1.2/24'), mock.call('tap0', namespace=ns), mock.call().route.list_onlink_routes(constants.IP_VERSION_4), mock.call().route.list_onlink_routes(constants.IP_VERSION_6), @@ -155,7 +155,7 @@ class TestABCDriver(TestBase): preserve_ips=['192.168.1.3/32']) self.ip_dev.assert_has_calls( [mock.call('tap0', namespace=ns), - mock.call().addr.list(filters=['permanent']), + mock.call().addr.list(), mock.call().addr.add('192.168.1.2/24')]) self.assertFalse(self.ip_dev().addr.delete.called) self.assertFalse(self.ip_dev().delete_addr_and_conntrack_state.called) @@ -198,9 +198,9 @@ class TestABCDriver(TestBase): bc.init_router_port('tap0', [new_cidr], **kwargs) expected_calls = ( [mock.call('tap0', namespace=ns), - mock.call().addr.list(filters=['permanent']), - mock.call().addr.add('2001:db8:a::124/64'), - mock.call().addr.delete('2001:db8:a::123/64')]) + mock.call().addr.list(), + mock.call().addr.delete('2001:db8:a::123/64'), + mock.call().addr.add('2001:db8:a::124/64')]) expected_calls += ( [mock.call('tap0', namespace=ns), mock.call().route.list_onlink_routes(constants.IP_VERSION_4), @@ -222,7 +222,7 @@ class TestABCDriver(TestBase): extra_subnets=[{'cidr': '172.20.0.0/24'}]) self.ip_dev.assert_has_calls( [mock.call('tap0', namespace=ns), - mock.call().addr.list(filters=['permanent']), + mock.call().addr.list(), mock.call().addr.add('192.168.1.2/24'), mock.call().addr.add('2001:db8:a::124/64'), mock.call().addr.delete('172.16.77.240/24'), @@ -269,6 +269,22 @@ class TestABCDriver(TestBase): namespace=ns) self.assertFalse(self.ip_dev().addr.add.called) + def test_l3_init_with_duplicated_ipv6_dynamic(self): + device_name = 'tap0' + cidr = '2001:db8:a::123/64' + ns = '12345678-1234-5678-90ab-ba0987654321' + addresses = [dict(scope='global', + dynamic=True, + cidr=cidr)] + self.ip_dev().addr.list = mock.Mock(return_value=addresses) + bc = BaseChild(self.conf) + bc.init_l3(device_name, [cidr], namespace=ns) + self.ip_dev.assert_has_calls( + [mock.call(device_name, namespace=ns), + mock.call().addr.list(), + mock.call().addr.delete(cidr), + mock.call().addr.add(cidr)]) + def test_add_ipv6_addr(self): device_name = 'tap0' cidr = '2001:db8::/64' diff --git a/releasenotes/notes/dhcp-ipv6-address-update-ff18d1eb0c196bce.yaml b/releasenotes/notes/dhcp-ipv6-address-update-ff18d1eb0c196bce.yaml new file mode 100644 index 00000000000..7070c9b6b2e --- /dev/null +++ b/releasenotes/notes/dhcp-ipv6-address-update-ff18d1eb0c196bce.yaml @@ -0,0 +1,19 @@ +--- +prelude: > + IPv6 addresses in DHCP namespaces will now be + (correctly) statically configured by the DHCP agent. +fixes: + - There is a race condition when adding ports in + DHCP namespaces where an IPv6 address could be + dynamically created via SLAAC from a Router + Advertisement sent from the L3 agent, leading to + a failure to start the DHCP agent. This bug has + been fixed, but care must be taken on an upgrade + dealing with any possibly stale dynamic addresses. + For more information, see bug + `1627902 `_. +upgrade: + - On upgrade, IPv6 addresses in the DHCP namespaces + that have been created dynmically via SLAAC will be + removed, and a static IPv6 address will be added + instead.