From: Mark McClain Date: Fri, 13 Sep 2013 21:48:20 +0000 (-0400) Subject: Dynamically adjust max number of leases X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fbc02fd569983122a9f5284a71f3c3aaaa03d260;p=openstack-build%2Fneutron-build.git Dynamically adjust max number of leases This change dynamically adjusts the maximum number of leases based on the size of the subnets associated with a network. The upper bound is limited by a configurable option to keep the max reasonable and prevent denial of service. Closes bug: 1225200 Change-Id: I75c3907bcf45cd991eadf5dd8c8ad7f1eaab3c85 --- diff --git a/etc/dhcp_agent.ini b/etc/dhcp_agent.ini index 90c1200a8..1fea47858 100644 --- a/etc/dhcp_agent.ini +++ b/etc/dhcp_agent.ini @@ -62,6 +62,9 @@ # Use another DNS server before any in /etc/resolv.conf. # dnsmasq_dns_server = +# Limit number of leases to prevent a denial-of-service. +# dnsmasq_lease_max = 16777216 + # Location to DHCP lease relay UNIX domain socket # dhcp_lease_relay_socket = $state_path/dhcp/lease_relay diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index b3ec08bec..a0f656b29 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -50,6 +50,10 @@ OPTS = [ cfg.StrOpt('dnsmasq_dns_server', help=_('Use another DNS server before any in ' '/etc/resolv.conf.')), + cfg.IntOpt( + 'dnsmasq_lease_max', + default=(2 ** 24), + help=_('Limit number of leases to prevent a denial-of-service.')), cfg.StrOpt('interface_driver', help=_("The driver used to manage the virtual interface.")), ] @@ -309,13 +313,12 @@ class Dnsmasq(DhcpLocalProcess): '--except-interface=lo', '--pid-file=%s' % self.get_conf_file_name( 'pid', ensure_conf_dir=True), - #TODO (mark): calculate value from cidr (defaults to 150) - #'--dhcp-lease-max=%s' % ?, '--dhcp-hostsfile=%s' % self._output_hosts_file(), '--dhcp-optsfile=%s' % self._output_opts_file(), '--leasefile-ro', ] + possible_leases = 0 for i, subnet in enumerate(self.network.subnets): # if a subnet is specified to have dhcp disabled if not subnet.enable_dhcp: @@ -330,11 +333,20 @@ class Dnsmasq(DhcpLocalProcess): set_tag = 'set:' else: set_tag = '' + + cidr = netaddr.IPNetwork(subnet.cidr) + cmd.append('--dhcp-range=%s%s,%s,%s,%ss' % (set_tag, self._TAG_PREFIX % i, - netaddr.IPNetwork(subnet.cidr).network, + cidr.network, mode, self.conf.dhcp_lease_duration)) + possible_leases += cidr.size + + # Cap the limit because creating lots of subnets can inflate + # this possible lease cap. + cmd.append('--dhcp-lease-max=%d' % + min(possible_leases, self.conf.dnsmasq_lease_max)) cmd.append('--conf-file=%s' % self.conf.dnsmasq_config_file) if self.conf.dnsmasq_dns_server: diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 6fd86174a..25a416cd2 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -142,12 +142,14 @@ class FakeV4Network: id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' subnets = [FakeV4Subnet()] ports = [FakePort1()] + namespace = 'qdhcp-ns' class FakeV6Network: id = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb' subnets = [FakeV6Subnet()] ports = [FakePort2()] + namespace = 'qdhcp-ns' class FakeDualNetwork: @@ -534,9 +536,10 @@ class TestDhcpLocalProcess(TestBase): class TestDnsmasq(TestBase): - def _test_spawn(self, extra_options): + def _test_spawn(self, extra_options, network=FakeDualNetwork(), + max_leases=16777216): def mock_get_conf_file_name(kind, ensure_conf_dir=False): - return '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/%s' % kind + return '/dhcp/%s/%s' % (network.id, kind) def fake_argv(index): if index == 0: @@ -550,7 +553,7 @@ class TestDnsmasq(TestBase): 'exec', 'qdhcp-ns', 'env', - 'NEUTRON_NETWORK_ID=cccccccc-cccc-cccc-cccc-cccccccccccc', + 'NEUTRON_NETWORK_ID=%s' % network.id, 'dnsmasq', '--no-hosts', '--no-resolv', @@ -558,12 +561,17 @@ class TestDnsmasq(TestBase): '--bind-interfaces', '--interface=tap0', '--except-interface=lo', - '--pid-file=/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/pid', - '--dhcp-hostsfile=/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/host', - '--dhcp-optsfile=/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/opts', - '--leasefile-ro', - '--dhcp-range=set:tag0,192.168.0.0,static,86400s', - '--dhcp-range=set:tag1,fdca:3ba5:a17a:4ba3::,static,86400s'] + '--pid-file=/dhcp/%s/pid' % network.id, + '--dhcp-hostsfile=/dhcp/%s/host' % network.id, + '--dhcp-optsfile=/dhcp/%s/opts' % network.id, + '--leasefile-ro'] + + expected.extend( + '--dhcp-range=set:tag%d,%s,static,86400s' % + (i, s.cidr.split('/')[0]) + for i, s in enumerate(network.subnets) + ) + expected.append('--dhcp-lease-max=%d' % max_leases) expected.extend(extra_options) self.execute.return_value = ('', '') @@ -576,14 +584,13 @@ class TestDnsmasq(TestBase): with mock.patch.multiple(dhcp.Dnsmasq, **attrs_to_mock) as mocks: mocks['get_conf_file_name'].side_effect = mock_get_conf_file_name mocks['_output_opts_file'].return_value = ( - '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/opts' + '/dhcp/%s/opts' % network.id ) mocks['interface_name'].__get__ = mock.Mock(return_value='tap0') with mock.patch.object(dhcp.sys, 'argv') as argv: argv.__getitem__.side_effect = fake_argv - dm = dhcp.Dnsmasq(self.conf, FakeDualNetwork(), - version=float(2.59)) + dm = dhcp.Dnsmasq(self.conf, network, version=float(2.59)) dm.spawn_process() self.assertTrue(mocks['_output_opts_file'].called) self.execute.assert_called_once_with(expected, @@ -607,6 +614,12 @@ class TestDnsmasq(TestBase): '--server=8.8.8.8', '--domain=openstacklocal']) + def test_spawn_max_leases_is_smaller_than_cap(self): + self._test_spawn( + ['--conf-file=', '--domain=openstacklocal'], + network=FakeV4Network(), + max_leases=256) + def test_output_opts_file(self): fake_v6 = 'gdca:3ba5:a17a:4ba3::1' fake_v6_cidr = 'gdca:3ba5:a17a:4ba3::/64'