From: Thomas Markwalder Date: Wed, 17 Feb 2021 14:04:32 +0000 (-0500) Subject: [#1635] V4 Allocation now uses class values for valid lifetime X-Git-Tag: Kea-1.9.5~27 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c692edf1aababed010df6f04b79a55363a83cb1e;p=thirdparty%2Fkea.git [#1635] V4 Allocation now uses class values for valid lifetime src/lib/dhcpsrv/alloc_engine.* AllocEngine::getValidLft(ctx4) - new method which returns the appropriate value for valid-leasetime based on the context content AllocEngine::createLease4 AllocEngine::updateLease4Information() - call getValidLft() src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc TEST_F(AllocEngine4Test, getValidLft4) - new test --- diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 134a912411..6c12becb5d 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -3840,6 +3840,45 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { return (new_lease); } +uint32_t +AllocEngine::getValidLft(const ClientContext4& ctx) { + + // If it's BOOTP, use infinite valid lifetime. + if (ctx.query_->inClass("BOOTP")) { + return(Lease::INFINITY_LFT); + } + + // If the value is specified in one of our classes use it. + // Use the value from the first class that specifies it. + const ClientClasses classes = ctx.query_->getClasses(); + if (!classes.empty()) { + // Let's get class definitions + const ClientClassDictionaryPtr& dict = + CfgMgr::instance().getCurrentCfg()->getClientClassDictionary(); + + // Iterate over the assigned class defintions. + for (ClientClasses::const_iterator name = classes.cbegin(); + name != classes.cend(); ++name) { + ClientClassDefPtr cl = dict->findClass(*name); + if (cl && (!cl->getValid().unspecified())) { + return(cl->getValid()); + } + } + } + + // Use the dhcp-lease-time content from the client if it's there. + OptionPtr opt = ctx.query_->getOption(DHO_DHCP_LEASE_TIME); + if (opt) { + OptionUint32Ptr opt_lft = boost::dynamic_pointer_cast >(opt); + if (opt_lft) { + return(ctx.subnet_->getValid().get(opt_lft->getValue())); + } + } + + // Use the value from the subnet (or above). + return (ctx.subnet_->getValid()); +} + Lease4Ptr AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr, CalloutHandle::CalloutNextStep& callout_status) { @@ -3850,23 +3889,8 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr, isc_throw(BadValue, "Can't create a lease without a subnet"); } - uint32_t valid_lft; - if (ctx.query_->inClass("BOOTP")) { - // BOOTP uses infinite valid lifetime. - valid_lft = Lease::INFINITY_LFT; - } else { - // Use the dhcp-lease-time content or the default for lease length. - OptionPtr opt = ctx.query_->getOption(DHO_DHCP_LEASE_TIME); - OptionUint32Ptr opt_lft; - if (opt) { - opt_lft = boost::dynamic_pointer_cast >(opt); - } - if (opt_lft) { - valid_lft = ctx.subnet_->getValid().get(opt_lft->getValue()); - } else { - valid_lft = ctx.subnet_->getValid(); - } - } + // Get the context appropriate valid lifetime. + uint32_t valid_lft = getValidLft(ctx); time_t now = time(NULL); @@ -4411,22 +4435,10 @@ AllocEngine::updateLease4Information(const Lease4Ptr& lease, lease->client_id_ = ClientIdPtr(); } lease->cltt_ = time(NULL); - if (ctx.query_->inClass("BOOTP")) { - // BOOTP uses infinite valid lifetime. - lease->valid_lft_ = Lease::INFINITY_LFT; - } else { - // Use the dhcp-lease-time content or the default for lease length. - OptionPtr opt = ctx.query_->getOption(DHO_DHCP_LEASE_TIME); - OptionUint32Ptr opt_lft; - if (opt) { - opt_lft = boost::dynamic_pointer_cast >(opt); - } - if (opt_lft) { - lease->valid_lft_ = ctx.subnet_->getValid().get(opt_lft->getValue()); - } else { - lease->valid_lft_ = ctx.subnet_->getValid(); - } - } + + // Get the context appropriate valid lifetime. + lease->valid_lft_ = getValidLft(ctx); + // Reduced valid lifetime is a significant change. if (lease->valid_lft_ < lease->current_valid_lft_) { changed = true; diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index 485c459097..9dc3db3c9b 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -1624,6 +1624,19 @@ public: /// @return Pointer to the reservation found, or an empty pointer. static ConstHostPtr findGlobalReservation(ClientContext4& ctx); + /// @brief Returns the valid lifetime based on the v4 context + /// + /// The value returned is as follows: + /// -# If the query is BOOTP, it returns INFINITY_LFT + /// -# The value from the first class, assigned to the client, which + /// valid-lifetime. Classes are searched in the order they are assigned + /// to the client. + /// -# The value from DHO_DHCP_LEASE_TIME if it was specified in the query + /// -# The value from the subnet assigned to the client (following + /// inheritnance upward as necessary). + /// @param ctx Client context holding various information about the client. + static uint32_t getValidLft(const ClientContext4& ctx); + private: /// @brief Offers the lease. diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index ff2fe31a9f..b9d76f30eb 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -4281,6 +4281,133 @@ TEST_F(AllocEngine4Test, requestCacheHostname4) { detailCompareLease(lease, from_mgr); } +// Verifies that AllocEngine::getValidLft(ctx4) returns the appropriate +// lifetime value based on the context content: +// -# If the query is BOOTP, it returns INFINITY_LFT +// -# The value from the first class, assigned to the client, which +// valid-lifetime. Classes are searched in the order they are assigned +// to the client. +// -# The value from DHO_DHCP_LEASE_TIME if it was specified in the query +// -# The value from the subnet assigned to the client (following +TEST_F(AllocEngine4Test, getValidLft4) { + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false); + + // Let's make three classes, two with valid-lifetime and one without, + // and add them to the dictionary. + ClientClassDictionaryPtr dictionary = CfgMgr::instance().getStagingCfg()->getClientClassDictionary(); + + ClientClassDefPtr class_def(new ClientClassDef("valid_one", ExpressionPtr())); + Tripletvalid_one(125,150,175); + class_def->setValid(valid_one); + dictionary->addClass(class_def); + + class_def.reset(new ClientClassDef("valid_two", ExpressionPtr())); + Tripletvalid_two(225,250,275); + class_def->setValid(valid_two); + dictionary->addClass(class_def); + + class_def.reset(new ClientClassDef("valid_unspec", ExpressionPtr())); + dictionary->addClass(class_def); + + CfgMgr::instance().commit(); + + + // iterate over various class assignments and verify the correct value is + // being used. + struct Scenario { + std::string desc_; + std::vector classes_; + OptionPtr lease_time_opt_; + uint32_t exp_valid_; + }; + + subnet_->setValid(Triplet(500,1000,1500)); + OptionUint32Ptr client_opt(new OptionUint32(Option::V4, DHO_DHCP_LEASE_TIME, 1050)); + OptionUint32Ptr opt_too_small(new OptionUint32(Option::V4, DHO_DHCP_LEASE_TIME, 450)); + OptionUint32Ptr opt_too_big(new OptionUint32(Option::V4, DHO_DHCP_LEASE_TIME, 1550)); + + std::vector scenarios = { + { + "BOOTP", + {"BOOTP"}, + OptionPtr(), + Lease::INFINITY_LFT + }, + { + "no classes, no option", + {}, + OptionPtr(), + subnet_->getValid() + }, + { + "no classes, option", + {}, + client_opt, + client_opt->getValue() + }, + { + "no classes, option too small", + {}, + opt_too_small, + subnet_->getValid().getMin() + }, + { + "no classes, option too big", + {}, + opt_too_big, + subnet_->getValid().getMax() + }, + { + "valid_unspec, no option", + { "valid_unspec" }, + OptionPtr(), + subnet_->getValid() + }, + { + "valid_one, valid_unspec, no option", + { "valid_unspec", "valid_one" }, + OptionPtr(), + valid_one.get() + }, + { + "valid_one, valid_two, no option", + { "valid_one", "valid_two" }, + OptionPtr(), + valid_one.get() + }, + { + "valid_two, valid_one, no option", + { "valid_two", "valid_one" }, + OptionPtr(), + valid_two.get() + } + }; + + for (auto scenario : scenarios) { + SCOPED_TRACE(scenario.desc_); { + // Create a context; + AllocEngine::ClientContext4 ctx(subnet_, ClientIdPtr(), hwaddr_, + IOAddress("0.0.0.0"), false, false, + "", false); + ctx.query_.reset(new Pkt4(DHCPDISCOVER, 1234)); + + // Add client classes (if any) + for (auto class_name : scenario.classes_) { + ctx.query_->addClass(class_name); + } + + // Add client option (if one) + if (scenario.lease_time_opt_) { + ctx.query_->addOption(scenario.lease_time_opt_); + } + + Lease4Ptr lease = engine.allocateLease4(ctx); + ASSERT_TRUE(lease); + EXPECT_EQ(lease->valid_lft_, scenario.exp_valid_); + } + } +} + } // namespace test } // namespace dhcp } // namespace isc