From 2aa23de58f55f7b1001508326c5ac2627ba3a429 Mon Sep 17 00:00:00 2001 From: Sergey Nechaev Date: Tue, 5 Apr 2016 14:40:59 +0000 Subject: [PATCH] Adding support of releasing DHCPv6 leases Original problem is that dhcp_release does not work with IPv6, but IPv6 leases still should be released. For example: 1. Start VM in dhcpv6-stateful network, make it acquire IPv6 address. 2. Delete VM. 3. Start another VM in same network before lease expires. There's a very high chance that the same IPv6 address will be allocated for both of these VMs (same address will be reused after first VM was deleted). On DHCP agent, hosts file would be changed, but not lease file, so dnsmasq will not give second VM address until lease expires. Reducing lease time is not a good solution here. Solution is adding invocation of dhcp_release6 utility when releasing IPv6 address. dhcp_release6 utility appears in dnsmasq 2.76. It crafts DHCP6_Release packet, sends it from passed network interface to IPv6 multicast address and waits for DHCP6_Reply. Closes-Bug: 1521666 Change-Id: I5efab81cdaf0676503b6c7da0d4b4f400d859286 --- etc/neutron/rootwrap.d/dhcp.filters | 1 + neutron/agent/linux/dhcp.py | 105 ++++++++++++++++-- neutron/cmd/sanity/checks.py | 17 +++ neutron/cmd/sanity_check.py | 21 ++++ neutron/tests/unit/agent/linux/test_dhcp.py | 105 +++++++++++++++++- .../add-dhcp_release6-ff1b8d62fd7fe76d.yaml | 7 ++ 6 files changed, 240 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/add-dhcp_release6-ff1b8d62fd7fe76d.yaml diff --git a/etc/neutron/rootwrap.d/dhcp.filters b/etc/neutron/rootwrap.d/dhcp.filters index 156c9cfa545..ab87abb2940 100644 --- a/etc/neutron/rootwrap.d/dhcp.filters +++ b/etc/neutron/rootwrap.d/dhcp.filters @@ -20,6 +20,7 @@ ovs-vsctl: CommandFilter, ovs-vsctl, root ivs-ctl: CommandFilter, ivs-ctl, root mm-ctl: CommandFilter, mm-ctl, root dhcp_release: CommandFilter, dhcp_release, root +dhcp_release6: CommandFilter, dhcp_release6, root # metadata proxy metadata_proxy: CommandFilter, neutron-ns-metadata-proxy, root diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 775f79f45c8..2760b30ee71 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -441,18 +441,24 @@ class Dnsmasq(DhcpLocalProcess): service_name=DNSMASQ_SERVICE_NAME, monitored_process=pm) - def _release_lease(self, mac_address, ip, client_id): + def _release_lease(self, mac_address, ip, client_id=None, + server_id=None, iaid=None): """Release a DHCP lease.""" if netaddr.IPAddress(ip).version == constants.IP_VERSION_6: - # Note(SridharG) dhcp_release is only supported for IPv4 - # addresses. For more details, please refer to man page. - return - - cmd = ['dhcp_release', self.interface_name, ip, mac_address] - if client_id: - cmd.append(client_id) + cmd = ['dhcp_release6', '--iface', self.interface_name, + '--ip', ip, '--client-id', client_id, + '--server-id', server_id, '--iaid', iaid] + else: + cmd = ['dhcp_release', self.interface_name, ip, mac_address] + if client_id: + cmd.append(client_id) ip_wrapper = ip_lib.IPWrapper(namespace=self.network.namespace) - ip_wrapper.netns.execute(cmd, run_as_root=True) + try: + ip_wrapper.netns.execute(cmd, run_as_root=True) + except RuntimeError as e: + # when failed to release single lease there's + # no need to propagate error further + LOG.warning(e) def _output_config_files(self): self._output_hosts_file() @@ -707,10 +713,77 @@ class Dnsmasq(DhcpLocalProcess): LOG.debug('Error while reading hosts file %s', filename) return leases + def _read_v6_leases_file_leases(self, filename): + """ + reading information from leases file which is needed to pass to + dhcp_release6 command line utility if some of these leases are not + needed anymore + + in this method ipv4 entries in leases file are ignored, as info in + hosts file is enough + + each line in dnsmasq leases file is one of the following + * duid entry: duid server_duid + There MUST be single duid entry per file + * ipv4 entry: space separated list + - The expiration time (seconds since unix epoch) or duration + (if dnsmasq is compiled with HAVE_BROKEN_RTC) of the lease. + 0 means infinite. + - The link address, in format XX-YY:YY:YY[...], where XX is the ARP + hardware type. "XX-" may be omitted for Ethernet. + - The IPv4 address + - The hostname (sent by the client or assigned by dnsmasq) + or '*' for none. + - The client identifier (colon-separated hex bytes) + or '*' for none. + + * ipv6 entry: space separated list + - The expiration time or duration + - The IAID as a Big Endian decimal number, prefixed by T for + IA_TAs (temporary addresses). + - The IPv6 address + - The hostname or '*' + - The client DUID (colon-separated hex bytes) or '*' if unknown + + original discussion is in dnsmasq mailing list + http://lists.thekelleys.org.uk/pipermail/\ + dnsmasq-discuss/2016q2/010595.html + + :param filename: leases file + :return: dict, keys are IPv6 addresses, values are dicts containing + iaid, client_id and server_id + """ + leases = {} + server_id = None + if os.path.exists(filename): + with open(filename) as f: + for l in f.readlines(): + if l.startswith('duid'): + if not server_id: + server_id = l.strip().split()[1] + continue + else: + LOG.warning(_LW('Multiple DUID entries in %s ' + 'lease file, dnsmasq is possibly ' + 'not functioning properly'), + filename) + continue + parts = l.strip().split() + (iaid, ip, client_id) = parts[1], parts[2], parts[4] + if netaddr.IPAddress(ip).version == constants.IP_VERSION_4: + continue + leases[ip] = {'iaid': iaid, + 'client_id': client_id, + 'server_id': server_id + } + return leases + def _release_unused_leases(self): filename = self.get_conf_file_name('host') old_leases = self._read_hosts_file_leases(filename) - + leases_filename = self.get_conf_file_name('leases') + # here is dhcpv6 stuff needed to craft dhcpv6 packet + v6_leases = self._read_v6_leases_file_leases(leases_filename) new_leases = set() dhcp_port_exists = False dhcp_port_on_this_host = self.device_manager.get_device_id( @@ -723,7 +796,17 @@ class Dnsmasq(DhcpLocalProcess): dhcp_port_exists = True for ip, mac, client_id in old_leases - new_leases: - self._release_lease(mac, ip, client_id) + entry = v6_leases.get(ip, None) + version = netaddr.IPAddress(ip).version + if entry: + # must release IPv6 lease + self._release_lease(mac, ip, entry['client_id'], + entry['server_id'], entry['iaid']) + # must release only if v4 lease. If we have ipv6 address missing + # in old_leases, that means it's released already and nothing to do + # here + elif version == constants.IP_VERSION_4: + self._release_lease(mac, ip, client_id) if not dhcp_port_exists: self.device_manager.driver.unplug( diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py index 439ce253954..c49e614653e 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -41,6 +41,7 @@ LOG = logging.getLogger(__name__) MINIMUM_DNSMASQ_VERSION = 2.67 +DNSMASQ_VERSION_DHCP_RELEASE6 = 2.76 MINIMUM_DIBBLER_VERSION = '1.0.1' @@ -180,6 +181,10 @@ def get_minimal_dnsmasq_version_supported(): return MINIMUM_DNSMASQ_VERSION +def get_dnsmasq_version_with_dhcp_release6(): + return DNSMASQ_VERSION_DHCP_RELEASE6 + + def dnsmasq_version_supported(): try: cmd = ['dnsmasq', '--version'] @@ -196,6 +201,18 @@ def dnsmasq_version_supported(): return True +def dhcp_release6_supported(): + try: + cmd = ['dhcp_release6', '--help'] + env = {'LC_ALL': 'C'} + agent_utils.execute(cmd, addl_env=env) + except (OSError, RuntimeError, IndexError, ValueError) as e: + LOG.debug("Exception while checking dhcp_release6. " + "Exception: %s", e) + return False + return True + + class KeepalivedIPv6Test(object): def __init__(self, ha_port, gw_port, gw_vip, default_gw): self.ha_port = ha_port diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 0e6aa4e4cb5..dc69fc64d14 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -228,6 +228,24 @@ def check_ip6tables(): 'is installed.')) return result + +def check_dhcp_release6(): + result = checks.dhcp_release6_supported() + if not result: + LOG.error(_LE('No dhcp_release6 tool detected. The installed version ' + 'of dnsmasq does not support releasing IPv6 leases. ' + 'Please update to at least version %s if you need this ' + 'feature. If you do not use IPv6 stateful subnets you ' + 'can continue to use this version of dnsmasq, as ' + 'other IPv6 address assignment mechanisms besides ' + 'stateful DHCPv6 should continue to work without ' + 'the dhcp_release6 utility. ' + 'Current version of dnsmasq is ok if other checks ' + 'pass.'), + checks.get_dnsmasq_version_with_dhcp_release6()) + return result + + # Define CLI opts to test specific features, with a callback for the test OPTS = [ BoolOptCallback('ovs_vxlan', check_ovs_vxlan, default=False, @@ -266,6 +284,9 @@ OPTS = [ help=_('Check ipset installation')), BoolOptCallback('ip6tables_installed', check_ip6tables, help=_('Check ip6tables installation')), + BoolOptCallback('dhcp_release6', check_dhcp_release6, + help=_('Check dhcp_release6 installation')), + ] diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index ea2b35ce38c..e257e02721f 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -1609,9 +1609,19 @@ class TestDnsmasq(TestBase): mac1 = '00:00:80:aa:bb:cc' ip2 = '192.168.1.3' mac2 = '00:00:80:cc:bb:aa' + ip3 = '0001:0002:0003:004:0005:0006:0007:0008' + mac3 = '00:00:80:bb:aa:cc' - old_leases = set([(ip1, mac1, None), (ip2, mac2, None)]) + old_leases = {(ip1, mac1, None), (ip2, mac2, None), (ip3, mac3, None)} dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) + dnsmasq._read_v6_leases_file_leases = mock.Mock( + return_value={ + '0001:0002:0003:004:0005:0006:0007:0008': + {'iaid': 0xff, + 'client_id': 'client_id', + 'server_id': 'server_id'}} + ) + dnsmasq._output_hosts_file = mock.Mock() dnsmasq._release_lease = mock.Mock() dnsmasq.network.ports = [] @@ -1620,7 +1630,12 @@ class TestDnsmasq(TestBase): dnsmasq._release_unused_leases() dnsmasq._release_lease.assert_has_calls([mock.call(mac1, ip1, None), - mock.call(mac2, ip2, None)], + mock.call(mac2, ip2, None), + mock.call(mac3, ip3, + 'client_id', + 'server_id', + 0xff), + ], any_order=True) dnsmasq.device_manager.driver.unplug.assert_has_calls( [mock.call(dnsmasq.interface_name, @@ -1636,13 +1651,24 @@ class TestDnsmasq(TestBase): old_leases = set([(ip1, mac1, None), (ip2, mac2, None)]) dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) + dnsmasq._read_v6_leases_file_leases = mock.Mock( + return_value={'fdca:3ba5:a17a::1': {'iaid': 0xff, + 'client_id': 'client_id', + 'server_id': 'server_id'} + }) ipw = mock.patch( 'neutron.agent.linux.ip_lib.IpNetnsCommand.execute').start() dnsmasq._release_unused_leases() - # Verify that dhcp_release is called only for ipv4 addresses. - self.assertEqual(1, ipw.call_count) + # Verify that dhcp_release is called both for ipv4 and ipv6 addresses. + self.assertEqual(2, ipw.call_count) + ipw.assert_has_calls([mock.call(['dhcp_release6', + '--iface', None, '--ip', ip1, + '--client-id', 'client_id', + '--server-id', 'server_id', + '--iaid', 0xff], + run_as_root=True)]) ipw.assert_has_calls([mock.call(['dhcp_release', None, ip2, mac2], - run_as_root=True)]) + run_as_root=True), ]) def test_release_unused_leases_with_dhcp_port(self): dnsmasq = self._get_dnsmasq(FakeNetworkDhcpPort()) @@ -1650,9 +1676,15 @@ class TestDnsmasq(TestBase): mac1 = '00:00:80:aa:bb:cc' ip2 = '192.168.1.3' mac2 = '00:00:80:cc:bb:aa' + ip6 = '2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d' old_leases = set([(ip1, mac1, None), (ip2, mac2, None)]) dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) + dnsmasq._read_v6_leases_file_leases = mock.Mock( + return_value={ip6: {'iaid': 0xff, + 'client_id': 'client_id', + 'server_id': 'server_id'} + }) dnsmasq._output_hosts_file = mock.Mock() dnsmasq._release_lease = mock.Mock() dnsmasq.device_manager.get_device_id = mock.Mock( @@ -1670,9 +1702,15 @@ class TestDnsmasq(TestBase): ip2 = '192.168.1.3' mac2 = '00:00:80:cc:bb:aa' client_id2 = 'client2' + ip6 = '2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d' old_leases = set([(ip1, mac1, client_id1), (ip2, mac2, client_id2)]) dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) + dnsmasq._read_v6_leases_file_leases = mock.Mock( + return_value={ip6: {'iaid': 0xff, + 'client_id': 'client_id', + 'server_id': 'server_id'} + }) dnsmasq._output_hosts_file = mock.Mock() dnsmasq._release_lease = mock.Mock() dnsmasq.network.ports = [] @@ -1691,9 +1729,15 @@ class TestDnsmasq(TestBase): mac1 = '00:00:80:aa:bb:cc' ip2 = '192.168.0.3' mac2 = '00:00:80:cc:bb:aa' + ip6 = '2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d' old_leases = set([(ip1, mac1, None), (ip2, mac2, None)]) dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) + dnsmasq._read_v6_leases_file_leases = mock.Mock( + return_value={ip6: {'iaid': 0xff, + 'client_id': 'client_id', + 'server_id': 'server_id'} + }) dnsmasq._output_hosts_file = mock.Mock() dnsmasq._release_lease = mock.Mock() dnsmasq.network.ports = [FakePort1()] @@ -1712,10 +1756,16 @@ class TestDnsmasq(TestBase): ip2 = '192.168.0.5' mac2 = '00:00:0f:aa:bb:55' client_id2 = 'test5' + ip6 = '2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d' old_leases = set([(ip1, mac1, client_id1), (ip2, mac2, client_id2)]) dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) dnsmasq._output_hosts_file = mock.Mock() + dnsmasq._read_v6_leases_file_leases = mock.Mock( + return_value={ip6: {'iaid': 0xff, + 'client_id': 'client_id', + 'server_id': 'server_id'} + }) dnsmasq._release_lease = mock.Mock() dnsmasq.network.ports = [FakePort5()] @@ -1770,6 +1820,51 @@ class TestDnsmasq(TestBase): ("fdca:3ba5:a17a::1", "00:00:80:aa:bb:cc", 'client2')]), leases) + def test_read_v6_leases_file_leases(self): + filename = '/path/to/file' + lines = [ + "1472673289 aa:bb:cc:00:00:01 192.168.1.2 host-192-168-1-2 *", + "1472673289 aa:bb:cc:00:00:01 192.168.1.3 host-192-168-1-3 *", + "1472673289 aa:bb:cc:00:00:01 192.168.1.4 host-192-168-1-4 *", + "duid 00:01:00:01:02:03:04:05:06:07:08:09:0a:0b", + "1472597740 1044800001 2001:DB8::a host-2001-db8--a " + "00:04:4a:d0:d2:34:19:2b:49:08:84:e8:34:bd:0c:dc:b9:3b", + "1472597823 1044800002 2001:DB8::b host-2001-db8--b " + "00:04:ce:96:53:3d:f2:c2:4c:4c:81:7d:db:c9:8d:d2:74:22:3b:0a", + "1472599048 1044800003 2001:DB8::c host-2001-db8--c " + "00:04:4f:f0:cd:ca:5e:77:41:bc:9d:7f:5c:33:31:37:5d:80:77:b4" + ] + mock_open = self.useFixture( + tools.OpenFixture(filename, '\n'.join(lines))).mock_open + + dnsmasq = self._get_dnsmasq(FakeDualNetwork()) + with mock.patch('os.path.exists', return_value=True): + leases = dnsmasq._read_v6_leases_file_leases(filename) + server_id = '00:01:00:01:02:03:04:05:06:07:08:09:0a:0b' + entry1 = {'iaid': '1044800001', + 'client_id': '00:04:4a:d0:d2:34:19:2b:49:08:84:' + 'e8:34:bd:0c:dc:b9:3b', + 'server_id': server_id + } + + entry2 = {'iaid': '1044800002', + 'client_id': '00:04:ce:96:53:3d:f2:c2:4c:4c:81:' + '7d:db:c9:8d:d2:74:22:3b:0a', + 'server_id': server_id + } + entry3 = {'iaid': '1044800003', + 'client_id': '00:04:4f:f0:cd:ca:5e:77:41:bc:9d:' + '7f:5c:33:31:37:5d:80:77:b4', + 'server_id': server_id + } + expected = {'2001:DB8::a': entry1, + '2001:DB8::b': entry2, + '2001:DB8::c': entry3 + } + + self.assertEqual(expected, leases) + mock_open.assert_called_once_with(filename) + def test_make_subnet_interface_ip_map(self): with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as ip_dev: ip_dev.return_value.addr.list.return_value = [ diff --git a/releasenotes/notes/add-dhcp_release6-ff1b8d62fd7fe76d.yaml b/releasenotes/notes/add-dhcp_release6-ff1b8d62fd7fe76d.yaml new file mode 100644 index 00000000000..00c1b8afd32 --- /dev/null +++ b/releasenotes/notes/add-dhcp_release6-ff1b8d62fd7fe76d.yaml @@ -0,0 +1,7 @@ +--- +prelude: > + - Call dhcp_release6 command line utility when releasing + unused IPv6 leases for DHCPv6 stateful subnets. + dhcp_release6 first appeared in dnsmasq 2.76 +fixes: + - closes bug 1521666