]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[3565] Changes after review:
authorTomek Mrugalski <tomasz@isc.org>
Mon, 16 Feb 2015 16:30:15 +0000 (17:30 +0100)
committerTomek Mrugalski <tomasz@isc.org>
Mon, 16 Feb 2015 16:30:15 +0000 (17:30 +0100)
- Documentation for reservation-mode added
- spec files updated
- unit-tests for HR no longer use eth0 or eth1
- @todos added when allow_new_leases_in_renewals_ is expected to be used
- HR unit-test no longer uses at()
- Copyright years updated
- getOptionalParam() used when handling reservation-mode
- hrModeFromText() moved to SubnetConfigParser
- hrModeFromText() starts with lowercase and has its documentation expanded

18 files changed:
doc/guide/dhcp4-srv.xml
doc/guide/dhcp6-srv.xml
src/bin/dhcp4/dhcp4.spec
src/bin/dhcp4/json_config_parser.cc
src/bin/dhcp4/tests/config_parser_unittest.cc
src/bin/dhcp6/dhcp6.spec
src/bin/dhcp6/dhcp6_srv.cc
src/bin/dhcp6/dhcp6_srv.h
src/bin/dhcp6/json_config_parser.cc
src/bin/dhcp6/tests/config_parser_unittest.cc
src/bin/dhcp6/tests/fqdn_unittest.cc
src/lib/dhcpsrv/lease.h
src/lib/dhcpsrv/parsers/dhcp_parsers.cc
src/lib/dhcpsrv/parsers/dhcp_parsers.h
src/lib/dhcpsrv/subnet.h
src/lib/dhcpsrv/tests/alloc_engine_utils.h
src/lib/dhcpsrv/tests/host_unittest.cc
src/lib/dhcpsrv/tests/test_utils.h

index d93f9457897136014a78fe368facd986dc10fc5e..859b9742bc76fab26e33a656d28718f2e954283b 100644 (file)
@@ -1886,6 +1886,72 @@ temporarily override a list of interface names and listen on all interfaces.
       reservation. Such a feature will be added in the upcoming Kea
       releases.</para>
     </section>
+
+    <section id="reservation4-mode">
+      <title>Fine Tuning IPv4 Host Reservation</title>
+
+      <note>
+        <para><command>reservation-mode</command> in DHCPv6 server in 0.9.1 beta is
+        accepted, but not used. Full implementation will be available in the upcoming
+        releases.</para>
+      </note>
+
+      <para>Host reservation capability introduces additional restrictions for the
+      allocation engine during lease selection and renewal. In particular, three
+      major checks are necessary. First, when selecting a new lease, it is not
+      sufficient for a candidate lease to be not used by anyone else. It also must
+      not be reserved for anyone else. Second, when renewing a lease, additional
+      check must be performed whether the address being renewed is not reserved
+      for anyone else. Finally, when a host renews an address, the server has to
+      check whether there's a reservation for this host, so the exisiting (dynamically
+      allocated) address should be revoked and the reserved one be used instead.
+      </para>
+      <para>Some of those checks in certain deployments may be not necessary. Skipping
+      them may improve performance. The Kea server allows configuration of the
+      reservation types allowed for each subnet using <command>reservation-mode</command>.
+      Allowed values are:
+
+      <itemizedlist>
+      <listitem><simpara> <command>all</command> - enables all host reservation
+      types. This is the default value. This setting is the safest and the most
+      flexible. It allows in-pool and out-of-pool reservations. As all checks
+      are conducted, it is also the slowest.
+      </simpara></listitem>
+
+      <listitem><simpara> <command>out-of-pool</command> - allows only out of
+      pool host reservations.  With this setting in place, the server may assume
+      that all host reservations are for addresses that do not belong to the
+      dynamic pool. Therefore it can skip the reservation checks when dealing
+      with in-pool addresses, thus improving performance. Do not use this mode
+      if any of your reservations use in-pool address. Caution is advised when
+      using this setting. Kea 0.9.1 does not sanity check the reservations against
+      <command>reservation-mode</command>. Misconfiguration may cause problems.
+      </simpara></listitem>
+
+      <listitem><simpara>
+      <command>disabled</command> - host reservation support is disabled. As there
+      are no reservations, the server will skip all checks. Any reservations defined
+      will be completely ignored. As the checks are skipped, the server may
+      operate faster in this mode.
+      </simpara></listitem>
+
+      </itemizedlist>
+      </para>
+
+      <para>
+        An example configuration that disables reservation looks like follows:
+       <screen>
+"Dhcp4": {
+    "subnet4": [
+        "subnet": "192.0.2.0/24",
+        <userinput>"reservation-mode": "disabled"</userinput>,
+        ...
+    ]
+}
+</screen>
+      </para>        
+   </section>
+
   </section>
   <!-- end of host reservations section -->
 
index 074f05f7cbdf25e9472c456f9caa7f68d745d46b..6f609ece9690e2e530238f88e72510ed5a7f9f1a 100644 (file)
@@ -1894,6 +1894,70 @@ should include options from the isc option space:
       releases.</para>
     </section>
 
+    <section id="reservation6-mode">
+      <title>Fine Tuning IPv6 Host Reservation</title>
+
+      <note>
+        <para><command>reservation-mode</command> in the DHCPv6 server in 0.9.1 beta is
+        implemented, but was not tested and is considered experimental.</para>
+      </note>
+
+      <para>Host reservation capability introduces additional restrictions for the
+      allocation engine during lease selection and renewal. In particular, three
+      major checks are necessary. First, when selecting a new lease, it is not
+      sufficient for a candidate lease to be not used by anyone else. It also must
+      not be reserved for anyone else. Second, when renewing a lease, additional
+      check must be performed whether the address being renewed is not reserved
+      for anyone else. Finally, when a host renews an address, the server has to
+      check whether there's a reservation for this host, so the exisiting (dynamically
+      allocated) address should be revoked and the reserved one be used instead.
+      </para>
+      <para>Some of those checks in certain deployments may be not necessary. Skipping
+      them may improve performance. The Kea server allows configuration of the
+      reservation types allowed for each subnet using <command>reservation-mode</command>.
+      Allowed values are:
+
+      <itemizedlist>
+      <listitem><simpara> <command>all</command> - enables all host reservation
+      types. This is the default value. This setting is the safest and the most
+      flexible. It allows in-pool and out-of-pool reservations. As all checks
+      are conducted, it is also the slowest.
+      </simpara></listitem>
+
+      <listitem><simpara> <command>out-of-pool</command> - allows only out of
+      pool host reservations.  With this setting in place, the server may assume
+      that all host reservations are for addresses that do not belong to the
+      dynamic pool. Therefore it can skip the reservation checks when dealing
+      with in-pool addresses, thus improving performance. Do not use this mode
+      if any of your reservations use in-pool address. Caution is advised when
+      using this setting. Kea 0.9.1 does not sanity check the reservations against
+      <command>reservation-mode</command>. Misconfiguration may cause problems.
+      </simpara></listitem>
+
+      <listitem><simpara>
+      <command>disabled</command> - host reservation support is disabled. As there
+      are no reservations, the server will skip all checks. Any reservations defined
+      will be completely ignored. As the checks are skipped, the server may
+      operate faster in this mode.
+      </simpara></listitem>
+
+      </itemizedlist>
+      </para>
+
+      <para>
+        An example configuration that disables reservation looks like follows:
+       <screen>
+"Dhcp6": {
+    "subnet6": [
+        "subnet": "2001:db8:1::/64",
+        <userinput>"reservation-mode": "disabled"</userinput>,
+        ...
+    ]
+}
+</screen>
+      </para>        
+   </section>
+
     <!-- @todo: add support for per IA reservation (that specifies IAID in
                 the ip-addresses and prefixes) -->
   </section>
index 8c99576a87686b4de0bcc9b7c9ef0bc7c9e68813..0d3922b71b43f7735ef0fc4f635eb2f683a54c8f 100644 (file)
                         "item_default": "0.0.0.0"
                       } ]
                   }
-                } ]
+                },
+                {
+                  "item_name": "reservation-mode",
+                  "item_type": "string",
+                  "item_optional": true,
+                  "item_default": "all",
+                  "item_description": "Specifies allowed host reservation types. Disabling unused modes may improve performance. Allowed values: disabled, off, out-of-pool, all"
+                }
+             ]
          }
       },
 
index f8abcf1221360215aac2fab33e93d6e691aa64c8..96ad96551c46204bac295b6cb06f98b79f40ec0b 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
index 14fc0cf869cb6120e923549ad3ba52519905f874..85ecb917b0052303d72665b6e2bc036d41bf1c67 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -3498,30 +3498,25 @@ TEST_F(Dhcp4ParserTest, hostReservationPerSubnet) {
         "\"subnet4\": [ { "
         "    \"pools\": [ { \"pool\": \"192.0.2.0/24\" } ],"
         "    \"subnet\": \"192.0.2.0/24\", "
-        "    \"reservation-mode\": \"all\","
-        "    \"interface\": \"eth0\""
+        "    \"reservation-mode\": \"all\""
         " },"
         " {"
         "    \"pools\": [ { \"pool\": \"192.0.3.0/24\" } ],"
         "    \"subnet\": \"192.0.3.0/24\", "
-        "    \"reservation-mode\": \"out-of-pool\","
-        "    \"interface\": \"eth1\""
+        "    \"reservation-mode\": \"out-of-pool\""
         " },"
         " {"
         "    \"pools\": [ { \"pool\": \"192.0.4.0/24\" } ],"
         "    \"subnet\": \"192.0.4.0/24\", "
-        "    \"reservation-mode\": \"disabled\","
-        "    \"interface\": \"eth1\""
+        "    \"reservation-mode\": \"disabled\""
         " },"
         " {"
         "    \"pools\": [ { \"pool\": \"192.0.5.0/24\" } ],"
-        "    \"subnet\": \"192.0.5.0/24\", "
-        "    \"interface\": \"eth1\""
+        "    \"subnet\": \"192.0.5.0/24\""
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
     ElementPtr json = Element::fromJSON(hr_config);
-    CfgMgr::instance().clear();
     ConstElementPtr result;
     EXPECT_NO_THROW(result = configureDhcp4Server(*srv_, json));
 
index 573129f0a8ae09d7293d912ac3beec57d1f9a3ca..618d1720a9a3ad37ff488731167fb370e49d56f3 100644 (file)
                         }
                       } ]
                   }
+                },
+                {
+                  "item_name": "reservation-mode",
+                  "item_type": "string",
+                  "item_optional": true,
+                  "item_default": "all",
+                  "item_description": "Specifies allowed host reservation types. Disabling unused modes may improve performance. Allowed values: disabled, off, out-of-pool, all"
                 } ]
             }
       },
index 13ac19dd3e21fa0d00ac4a75230508975a7b791e..c1460df327b3b0e0a7f8021fb6161322edc11403 100644 (file)
@@ -1587,10 +1587,15 @@ Dhcpv6Srv::extendIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         hints_count++;
     }
 
-    // If this is a Renew, it's ok to allocate new leases
+    /// @todo: This was clarfied in draft-ietf-dhc-dhcpv6-stateful-issues that
+    /// the server is allowed to assign new leases in both Renew and Rebind. For
+    /// now, we only support it in Renew, because it breaks a lot of Rebind
+    /// unit-tests. Ultimately, whether we allow it or not, should be exposed
+    /// as configurable policy. See ticket #3717.
     if (query->getType() == DHCPV6_RENEW) {
         ctx.allow_new_leases_in_renewals_ = true;
     }
+
     Lease6Collection leases = alloc_engine_->renewLeases6(ctx);
 
     // Ok, now we have the leases extended. We have:
@@ -1755,6 +1760,12 @@ Dhcpv6Srv::extendIA_PD(const Subnet6Ptr& subnet, const DuidPtr& duid,
         hints_count++;
     }
 
+    /// @todo: The draft-ietf-dhc-dhcpv6-stateful-issues added a new capability
+    /// of the server to to assign new PD leases in both Renew and Rebind.
+    /// There's allow_new_leases_in_renewals_ in the ClientContext6, but we
+    /// currently not use it in PD yet. This should be implemented as part
+    /// of the stateful-issues implementation effort. See ticket #3718.
+
     // Call Allocation Engine and attempt to renew leases. Number of things
     // may happen. Leases may be extended, revoked (if the lease is no longer
     // valid or reserved for someone else), or new leases may be added.
index f7a57e6f8a3d324053565e008ff721c4a7ec2d40..2f294de5bd78f201643b9e6b380232ba148be2b8 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
index 690de2f77b1a82f8d92dc5cef6cd3c465502a8e6..18e78d407689bd175f5674f1ffb890f751fd4e1a 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
index 6dea50fe7500baf1af0848008bd40702b55ac0e7..c0016d9fcb020ca90af00241cb8631a037ea0e80 100644 (file)
@@ -3710,25 +3710,21 @@ TEST_F(Dhcp6ParserTest, hostReservationPerSubnet) {
         "\"subnet6\": [ { "
         "    \"pools\": [ { \"pool\": \"2001:db8:1::/64\" } ],"
         "    \"subnet\": \"2001:db8:1::/48\", "
-        "    \"reservation-mode\": \"all\","
-        "    \"interface\": \"eth0\""
+        "    \"reservation-mode\": \"all\""
         " },"
         " {"
         "    \"pools\": [ { \"pool\": \"2001:db8:2::/64\" } ],"
         "    \"subnet\": \"2001:db8:2::/48\", "
-        "    \"reservation-mode\": \"out-of-pool\","
-        "    \"interface\": \"eth1\""
+        "    \"reservation-mode\": \"out-of-pool\""
         " },"
         " {"
         "    \"pools\": [ { \"pool\": \"2001:db8:3::/64\" } ],"
         "    \"subnet\": \"2001:db8:3::/48\", "
-        "    \"reservation-mode\": \"disabled\","
-        "    \"interface\": \"eth1\""
+        "    \"reservation-mode\": \"disabled\""
         " },"
         " {"
         "    \"pools\": [ { \"pool\": \"2001:db8:4::/64\" } ],"
-        "    \"subnet\": \"2001:db8:4::/48\", "
-        "    \"interface\": \"eth1\""
+        "    \"subnet\": \"2001:db8:4::/48\" "
         " } ],"
         "\"valid-lifetime\": 4000 }";
 
@@ -3741,16 +3737,30 @@ TEST_F(Dhcp6ParserTest, hostReservationPerSubnet) {
     checkResult(status, 0);
     CfgMgr::instance().commit();
 
-    const Subnet6Collection* subnets =
-        CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    ConstCfgSubnets6Ptr subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6();
+
+    const Subnet6Collection* subnet_col = subnets->getAll();
     ASSERT_TRUE(subnets);
-    ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
+    ASSERT_EQ(4, subnet_col->size()); // We expect 4 subnets
+
+    // Let's check if the parsed subnets have correct HR modes
+    Subnet6Ptr subnet;
+    subnet = subnets->selectSubnet(IOAddress("2001:db8:1::1"));
+    ASSERT_TRUE(subnet);
+    EXPECT_EQ(Subnet::HR_ALL, subnet->getHostReservationMode());
 
-    // Check subnet-ids of each subnet (it should be monotonously increasing)
-    EXPECT_EQ(Subnet::HR_ALL, subnets->at(0)->getHostReservationMode());
-    EXPECT_EQ(Subnet::HR_OUT_OF_POOL, subnets->at(1)->getHostReservationMode());
-    EXPECT_EQ(Subnet::HR_DISABLED, subnets->at(2)->getHostReservationMode());
-    EXPECT_EQ(Subnet::HR_ALL, subnets->at(3)->getHostReservationMode());
+
+    subnet = subnets->selectSubnet(IOAddress("2001:db8:2::1"));
+    ASSERT_TRUE(subnet);
+    EXPECT_EQ(Subnet::HR_OUT_OF_POOL, subnet->getHostReservationMode());
+
+    subnet = subnets->selectSubnet(IOAddress("2001:db8:3::1"));
+    ASSERT_TRUE(subnet);
+    EXPECT_EQ(Subnet::HR_DISABLED, subnet->getHostReservationMode());
+
+    subnet = subnets->selectSubnet(IOAddress("2001:db8:4::1"));
+    ASSERT_TRUE(subnet);
+    EXPECT_EQ(Subnet::HR_ALL, subnet->getHostReservationMode());
 }
 
 };
index 60d69ede32731f7d36fe4e4153d702d55477da99..c87f7a757710f5a6b880bb196509076a3a541ddd 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
index 0c536fb5a2beca4afb0b9f1b2d0e16e1ae730211..4ad892b8952a38173cc552a79dd34ad055b5fc63 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
index 794e9cc0e8743c5d94a4a55c6e60af66bf89440d..46c8cfab1c847e5214120f68a9848cbe6da56cb8 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -1125,7 +1125,7 @@ SubnetConfigParser::build(ConstElementPtr subnet) {
 }
 
 Subnet::HRMode
-HRModeFromText(const std::string& txt) {
+SubnetConfigParser::hrModeFromText(const std::string& txt) {
     if ( (txt.compare("disabled") == 0) ||
          (txt.compare("off") == 0) )  {
         return (Subnet::HR_DISABLED);
@@ -1192,18 +1192,13 @@ SubnetConfigParser::createSubnet() {
         // iface not mandatory so swallow the exception
     }
 
-    std::string hr_mode;
-    try {
-        hr_mode = string_values_->getParam("reservation-mode");
-    } catch (const DhcpConfigError &) {
-        // Host reservation mode is not mandatory, so don't complain
-    }
 
+    // Let's set host reservation mode. If not specified, the default value of
+    // all will be used.
+    std::string hr_mode;
     try {
-        // This may throw if the user didn't specify one of allowed values
-        if (!hr_mode.empty()) {
-            subnet_->setHostReservationMode(HRModeFromText(hr_mode));
-        }
+        hr_mode = string_values_->getOptionalParam("reservation-mode", "all");
+        subnet_->setHostReservationMode(hrModeFromText(hr_mode));
     } catch (const BadValue& ex) {
         isc_throw(DhcpConfigError, "Failed to process specified value "
                   " of reservation-mode parameter: " << ex.what()
index 46e1f7f8657cae836544e08d9481473c29e2eba8..0fb3c351f0590daa4a6dd787d9750861552ba466 100644 (file)
@@ -1093,6 +1093,17 @@ protected:
     /// unspecified value.
     isc::dhcp::Triplet<uint32_t> getOptionalParam(const std::string& name);
 
+    /// @brief Attempts to convert text representation to HRMode enum.
+    ///
+    /// Allowed values are "disabled", "off" (alias for disabled),
+    /// "out-of-pool" and "all". See Subnet::HRMode for their exact meaning.
+    ///
+    /// @throw BadValue if the text cannot be converted.
+    ///
+    /// @param text representation for conversion
+    /// @return one of allowed HRMode values
+    static Subnet::HRMode hrModeFromText(const std::string& txt);
+
 private:
 
     /// @brief Create a new subnet using a data from child parsers.
index ab63d903dd7d02206dc8d9cbc356e42c16629db2..e0f943128efb221ba849828e43b043b418e3f0d0 100644 (file)
@@ -326,7 +326,7 @@ public:
     ///
     /// @return whether in-pool host reservations are allowed.
     HRMode
-    getHostReservationMode() {
+    getHostReservationMode() const {
         return (host_reservation_mode_);
     }
 
@@ -339,14 +339,6 @@ public:
         host_reservation_mode_ = mode;
     }
 
-    /// @brief Attempts to convert text representation to HRMode enum
-    ///
-    /// @throw BadValue if the text cannot be converted
-    ///
-    /// @param text representation for conversion
-    /// @return one of allowed HRMode values
-    static HRMode HRModeFromText(const std::string& txt);
-
 protected:
     /// @brief Returns all pools (non-const variant)
     ///
index d892477920fd2ba963b6aea26149a9f0492f7ebb..951990a642ea9e2710026d1868d7f0fda20ac1d8 100644 (file)
@@ -311,7 +311,7 @@ public:
     }
 
     DuidPtr duid_;            ///< client-identifier (value used in tests)
-    HWAddrPtr hwaddr_;          ///< client's hardware address
+    HWAddrPtr hwaddr_;        ///< client's hardware address
     uint32_t iaid_;           ///< IA identifier (value used in tests)
     Subnet6Ptr subnet_;       ///< subnet6 (used in tests)
     Pool6Ptr pool_;           ///< NA pool belonging to subnet_
index 010278503294f0aac637af6aef9da3f8920cdadd..f89d0445152af505321260b4e238fea7a6170dca 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
index 49f97f9ccfad714322ba65844622da7b7c541c0d..dea983058c096346fd41612033faa9d59e425d60 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above