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 90c1200a861facbaec3eef0da849002a3ca0b002..1fea47858628a4012b2f07b7da80e290eef2b99b 100644 (file)
--- 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 b3ec08becb67a86de35572171b6317954f811a06..a0f656b29f710d669933a7896125964b7040e45b 100644 (file)
--- 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 6fd86174a65bfb865b7ce23305aea75b403fb186..25a416cd2ebe3af51b9e77be92ce5c37c38841e2 100644 (file)
--- 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'