From: Tomek Mrugalski Date: Thu, 12 Feb 2015 18:33:06 +0000 (+0100) Subject: [3565] reservation-mode is now configurable X-Git-Tag: trac3723_base~26^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=34e62083ebda8a7a3ee7f868dfea95c7091466e3;p=thirdparty%2Fkea.git [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 0a086d591d..f8abcf1221 100644 --- 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 b7674132b1..14fc0cf869 100644 --- 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 1ea45bacd9..690de2f77b 100644 --- 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 fe96975df9..6dea50fe75 100644 --- 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 4b54d19a60..28b258d998 100644 --- 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 bf9a47731f..794e9cc0e8 100644 --- 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 44efbeecc1..5a962719be 100644 --- 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 d037fcecaf..ab63d903dd 100644 --- 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.