[3565] reservation-mode is now configurable

diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc
index 0a086d591d8037812654a75d288ba5f3256e15ee..f8abcf1221360215aac2fab33e93d6e691aa64c8 100644 (file)
--- a/src/bin/dhcp4/json_config_parser.cc
+++ b/src/bin/dhcp4/json_config_parser.cc
@@ -182,7 +182,8 @@ protected:
         } else if ((config_id.compare("subnet") == 0) ||
                    (config_id.compare("interface") == 0) ||
                    (config_id.compare("client-class") == 0) ||
-                   (config_id.compare("next-server") == 0)) {
+                   (config_id.compare("next-server") == 0) ||
+                   (config_id.compare("reservation-mode") == 0)) {
             parser = new StringParser(config_id, string_values_);
         } else if (config_id.compare("pools") == 0) {
             parser = new Pools4ListParser(config_id, pools_);

diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index b7674132b189cab5b3f4f8fab7f031db850f819a..14fc0cf869cb6120e923549ad3ba52519905f874 100644 (file)
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -3478,8 +3478,68 @@ TEST_F(Dhcp4ParserTest, reservationBogus) {
 
     EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
     checkResult(x, 1);
+}
+
+/// The goal of this test is to verify that Host Reservation modes can be
+/// specified on a per-subnet basis.
+TEST_F(Dhcp4ParserTest, hostReservationPerSubnet) {
+
+    /// - Configuration:
+    ///   - only addresses (no prefixes)
+    ///   - 4 subnets with:
+    ///       - 192.0.2.0/24 (all reservations enabled)
+    ///       - 192.0.3.0/24 (out-of-pool reservations)
+    ///       - 192.0.4.0/24 (reservations disabled)
+    ///       - 192.0.5.0/24 (reservations not specified)
+    const char* hr_config =
+        "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"192.0.2.0/24\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\", "
+        "    \"reservation-mode\": \"all\","
+        "    \"interface\": \"eth0\""
+        " },"
+        " {"
+        "    \"pools\": [ { \"pool\": \"192.0.3.0/24\" } ],"
+        "    \"subnet\": \"192.0.3.0/24\", "
+        "    \"reservation-mode\": \"out-of-pool\","
+        "    \"interface\": \"eth1\""
+        " },"
+        " {"
+        "    \"pools\": [ { \"pool\": \"192.0.4.0/24\" } ],"
+        "    \"subnet\": \"192.0.4.0/24\", "
+        "    \"reservation-mode\": \"disabled\","
+        "    \"interface\": \"eth1\""
+        " },"
+        " {"
+        "    \"pools\": [ { \"pool\": \"192.0.5.0/24\" } ],"
+        "    \"subnet\": \"192.0.5.0/24\", "
+        "    \"interface\": \"eth1\""
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
 
+    ElementPtr json = Element::fromJSON(hr_config);
+    CfgMgr::instance().clear();
+    ConstElementPtr result;
+    EXPECT_NO_THROW(result = configureDhcp4Server(*srv_, json));
+
+    // returned value should be 0 (success)
+    checkResult(result, 0);
 
+    const Subnet4Collection* subnets =
+        CfgMgr::instance().getStagingCfg()->getCfgSubnets4()->getAll();
+    ASSERT_TRUE(subnets);
+    ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
+
+    // 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());
 }
 
+
+
 }

diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc
index 1ea45bacd982d7444ae82691581edc1353688118..690de2f77b1a82f8d92dc5cef6cd3c465502a8e6 100644 (file)
--- a/src/bin/dhcp6/json_config_parser.cc
+++ b/src/bin/dhcp6/json_config_parser.cc
@@ -389,7 +389,8 @@ protected:
         } else if ((config_id.compare("subnet") == 0) ||
                    (config_id.compare("interface") == 0) ||
                    (config_id.compare("client-class") == 0) ||
-                   (config_id.compare("interface-id") == 0)) {
+                   (config_id.compare("interface-id") == 0) ||
+                   (config_id.compare("reservation-mode") == 0)) {
             parser = new StringParser(config_id, string_values_);
         } else if (config_id.compare("pools") == 0) {
             parser = new Pools6ListParser(config_id, pools_);

diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index fe96975df92445fdb5ce5022666046db723b7b58..6dea50fe7500baf1af0848008bd40702b55ac0e7 100644 (file)
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -3691,4 +3691,66 @@ TEST_F(Dhcp6ParserTest, macSourcesBogus) {
     checkResult(status, 1);
 }
 
+/// The goal of this test is to verify that Host Reservation modes can be
+/// specified on a per-subnet basis.
+TEST_F(Dhcp6ParserTest, hostReservationPerSubnet) {
+
+    /// - Configuration:
+    ///   - only addresses (no prefixes)
+    ///   - 4 subnets with:
+    ///       - 2001:db8:1::/64 (all reservations enabled)
+    ///       - 2001:db8:2::/64 (out-of-pool reservations)
+    ///       - 2001:db8:3::/64 (reservations disabled)
+    ///       - 2001:db8:3::/64 (reservations not specified)
+    const char* HR_CONFIG =
+        "{ \"interfaces\": [ \"*\" ],"
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ { "
+        "    \"pools\": [ { \"pool\": \"2001:db8:1::/64\" } ],"
+        "    \"subnet\": \"2001:db8:1::/48\", "
+        "    \"reservation-mode\": \"all\","
+        "    \"interface\": \"eth0\""
+        " },"
+        " {"
+        "    \"pools\": [ { \"pool\": \"2001:db8:2::/64\" } ],"
+        "    \"subnet\": \"2001:db8:2::/48\", "
+        "    \"reservation-mode\": \"out-of-pool\","
+        "    \"interface\": \"eth1\""
+        " },"
+        " {"
+        "    \"pools\": [ { \"pool\": \"2001:db8:3::/64\" } ],"
+        "    \"subnet\": \"2001:db8:3::/48\", "
+        "    \"reservation-mode\": \"disabled\","
+        "    \"interface\": \"eth1\""
+        " },"
+        " {"
+        "    \"pools\": [ { \"pool\": \"2001:db8:4::/64\" } ],"
+        "    \"subnet\": \"2001:db8:4::/48\", "
+        "    \"interface\": \"eth1\""
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    ConstElementPtr status;
+
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_,
+                    Element::fromJSON(HR_CONFIG)));
+
+    // returned value should be 0 (success)
+    checkResult(status, 0);
+    CfgMgr::instance().commit();
+
+    const Subnet6Collection* subnets =
+        CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    ASSERT_TRUE(subnets);
+    ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
+
+    // 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());
+}
+
 };

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 4b54d19a600f26dbdf44aa2516f3812bda702721..28b258d998c9a0892529b1a92654c01a57831fe4 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -588,7 +588,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
         /// In-pool reservations: Check if this address is reserved for someone
         /// else. There is no need to check for whom it is reserved, because if
         /// it has been reserved for us we would have already allocated a lease.
-        if (hr_mode == Subnet::HR_IN_POOL &&
+        if (hr_mode == Subnet::HR_ALL &&
             HostMgr::instance().get6(ctx.subnet_->getID(), candidate)) {
 
             // Don't allocate.

diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index bf9a47731f1677756824763616b6b395bc8ddc1d..794e9cc0e8743c5d94a4a55c6e60af66bf89440d 100644 (file)
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -1124,6 +1124,21 @@ SubnetConfigParser::build(ConstElementPtr subnet) {
     }
 }
 
+Subnet::HRMode
+HRModeFromText(const std::string& txt) {
+    if ( (txt.compare("disabled") == 0) ||
+         (txt.compare("off") == 0) )  {
+        return (Subnet::HR_DISABLED);
+    } else if (txt.compare("out-of-pool") == 0) {
+        return (Subnet::HR_OUT_OF_POOL);
+    } else if (txt.compare("all") == 0) {
+        return (Subnet::HR_ALL);
+    } else {
+        isc_throw(BadValue, "Can't convert '" << txt
+                  << "' into any valid reservation-mode values");
+    }
+}
+
 void
 SubnetConfigParser::createSubnet() {
     std::string subnet_txt;
@@ -1177,6 +1192,24 @@ 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
+    }
+
+    try {
+        // This may throw if the user didn't specify one of allowed values
+        if (!hr_mode.empty()) {
+            subnet_->setHostReservationMode(HRModeFromText(hr_mode));
+        }
+    } catch (const BadValue& ex) {
+        isc_throw(DhcpConfigError, "Failed to process specified value "
+                  " of reservation-mode parameter: " << ex.what()
+                  << string_values_->getPosition("reservation-mode"));
+    }
+
     if (!iface.empty()) {
         if (!IfaceMgr::instance().getIface(iface)) {
             isc_throw(DhcpConfigError, "Specified interface name " << iface

diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc
index 44efbeecc1ca3acca3f9b65fc272fa97281041ed..5a962719be2fecc0ad2c94615bc1913299e4943e 100644 (file)
--- a/src/lib/dhcpsrv/subnet.cc
+++ b/src/lib/dhcpsrv/subnet.cc
@@ -38,8 +38,7 @@ Subnet::Subnet(const isc::asiolink::IOAddress& prefix, uint8_t len,
      last_allocated_ia_(lastAddrInPrefix(prefix, len)),
      last_allocated_ta_(lastAddrInPrefix(prefix, len)),
      last_allocated_pd_(lastAddrInPrefix(prefix, len)), relay_(relay),
-     cfg_option_(new CfgOption())
-
+     host_reservation_mode_(HR_ALL), cfg_option_(new CfgOption())
       {
     if ((prefix.isV6() && len > 128) ||
         (prefix.isV4() && len > 32)) {

diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h
index d037fcecaf6684587d4a4c153337f04adf966dbf..ab63d903dd7d02206dc8d9cbc356e42c16629db2 100644 (file)
--- a/src/lib/dhcpsrv/subnet.h
+++ b/src/lib/dhcpsrv/subnet.h
@@ -84,7 +84,7 @@ public:
         /// there is a non-trivial performance penalty for it, as the
         /// AllocEngine code has to check whether there are reservations, even
         /// when dealing with reservations from within the dynamic pools.
-        HR_IN_POOL
+        HR_ALL
     } HRMode;
 
     /// Pointer to the RelayInfo structure
@@ -324,14 +324,29 @@ public:
     /// not in the dynamic pool). HR may also be completely disabled for
     /// performance reasons.
     ///
-    /// @todo: implement this.
-    ///
     /// @return whether in-pool host reservations are allowed.
     HRMode
     getHostReservationMode() {
-        return (Subnet::HR_IN_POOL);
+        return (host_reservation_mode_);
+    }
+
+    /// @brief Sets host reservation mode.
+    ///
+    /// See @getHostReservationMode for details.
+    ///
+    /// @param mode mode to be set
+    void setHostReservationMode(HRMode mode) {
+        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)
     ///
@@ -477,6 +492,10 @@ protected:
     /// so it may be a while until we support this.
     ClientClasses white_list_;
 
+    /// @brief Specifies host reservation mode
+    ///
+    /// See @ref HRMode type for details.
+    HRMode host_reservation_mode_;
 private:
 
     /// @brief Pointer to the option data configuration for this subnet.