[3565] Changes after review:

- 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

doc/guide/dhcp4-srv.xml

@@ -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 -->
 

doc/guide/dhcp6-srv.xml

@@ -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>

src/bin/dhcp4/dhcp4.spec

@@ -367,7 +367,15 @@
                         "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"
+                }
+             ]
          }
       },
 

src/bin/dhcp4/json_config_parser.cc

@@ -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

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -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));
 

src/bin/dhcp6/dhcp6.spec

@@ -425,6 +425,13 @@
                         }
                       } ]
                   }
+                },
+                {
+                  "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"
                 } ]
             }
       },

src/bin/dhcp6/dhcp6_srv.cc

@@ -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.

src/bin/dhcp6/dhcp6_srv.h

@@ -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

src/bin/dhcp6/json_config_parser.cc

@@ -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

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -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());
 }
 
 };

src/bin/dhcp6/tests/fqdn_unittest.cc

@@ -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

src/lib/dhcpsrv/lease.h

@@ -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

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -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()

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -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.

src/lib/dhcpsrv/subnet.h

@@ -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)
     ///

src/lib/dhcpsrv/tests/alloc_engine_utils.h

@@ -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_

src/lib/dhcpsrv/tests/host_unittest.cc

@@ -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

src/lib/dhcpsrv/tests/test_utils.h

@@ -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