From 804ad6cacffd0616badeaa72e2ca0aa1bafd66fb Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Mon, 23 Sep 2019 00:24:31 +0200 Subject: [PATCH] [897-add-infinite-valid-lifetime-for-bootp] Added BOOTP client class --- doc/sphinx/arm/classify.rst | 3 + src/bin/dhcp4/dhcp4_srv.cc | 8 +- src/lib/dhcpsrv/alloc_engine.cc | 9 ++ src/lib/dhcpsrv/client_class_def.cc | 2 +- src/lib/dhcpsrv/client_class_def.h | 2 +- .../dhcpsrv/tests/alloc_engine4_unittest.cc | 101 ++++++++++++++++++ 6 files changed, 121 insertions(+), 4 deletions(-) diff --git a/doc/sphinx/arm/classify.rst b/doc/sphinx/arm/classify.rst index 31e1744c3b..184f15a744 100644 --- a/doc/sphinx/arm/classify.rst +++ b/doc/sphinx/arm/classify.rst @@ -152,6 +152,9 @@ server uses an appropriate pool or subnet to allocate IP addresses (and/or prefixes), based on the assigned client classes. The details can be found in :ref:`high-availability-library`. +The BOOTP hook uses the BOOTP class: when an incoming BOOTP query is +recognized it is put in this class and built BOOTP response too. + Other examples are the ALL class, which all incoming packets belong to, and the KNOWN class, assigned when host reservations exist for a particular client. By convention, built-in classes' names begin with all diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 3edfd8231d..01064d01fe 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -2758,13 +2758,17 @@ Dhcpv4Srv::processRequest(Pkt4Ptr& request, AllocEngine::ClientContext4Ptr& cont // or even rebinding. assignLease(ex); - if (!ex.getResponse()) { + Pkt4Ptr response = ex.getResponse(); + if (!response) { // The ack is empty so return it *now*! return (Pkt4Ptr()); + } else if (request->inClass("BOOTP")) { + // Put BOOTP responses in the BOOTP class. + response->addClass("BOOTP"); } // Adding any other options makes sense only when we got the lease. - if (!ex.getResponse()->getYiaddr().isV4Zero()) { + if (!response->getYiaddr().isV4Zero()) { // Assign reserved classes. ex.setReservedClientClasses(); // Required classification diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index d7a5551160..040908c4fc 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -3560,6 +3560,11 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr, valid_lft = ctx.subnet_->getValid().get(opt_lft->getValue()); } + // BOOTP uses infinite valid lifetime. + if (ctx.query_->inClass("BOOTP")) { + valid_lft = Lease::INFINITY_LFT; + } + time_t now = time(NULL); // @todo: remove this kludge? @@ -3992,6 +3997,10 @@ AllocEngine::updateLease4Information(const Lease4Ptr& lease, } else { lease->valid_lft_ = ctx.subnet_->getValid(); } + // BOOTP uses infinite valid lifetime. + if (ctx.query_->inClass("BOOTP")) { + lease->valid_lft_ = Lease::INFINITY_LFT; + } lease->fqdn_fwd_ = ctx.fwd_dns_update_; lease->fqdn_rev_ = ctx.rev_dns_update_; lease->hostname_ = ctx.hostname_; diff --git a/src/lib/dhcpsrv/client_class_def.cc b/src/lib/dhcpsrv/client_class_def.cc index bb18d8c328..ebb692aeee 100644 --- a/src/lib/dhcpsrv/client_class_def.cc +++ b/src/lib/dhcpsrv/client_class_def.cc @@ -345,7 +345,7 @@ builtinNames = { // In fact DROP is set from an expression as callouts can drop // directly the incoming packet. The expression must not depend on // KNOWN/UNKNOWN which are set far after the drop point. - "ALL", "KNOWN", "UNKNOWN" + "ALL", "KNOWN", "UNKNOWN", "BOOTP" }; std::list diff --git a/src/lib/dhcpsrv/client_class_def.h b/src/lib/dhcpsrv/client_class_def.h index 88425d0a97..2eb5743869 100644 --- a/src/lib/dhcpsrv/client_class_def.h +++ b/src/lib/dhcpsrv/client_class_def.h @@ -382,7 +382,7 @@ private: typedef boost::shared_ptr ClientClassDictionaryPtr; /// @brief List of built-in client class names. -/// i.e. ALL, KNOWN and UNKNOWN but not DROP. +/// i.e. ALL, KNOWN, UNKNOWN and BOOTP but not DROP. extern std::list builtinNames; /// @brief List of built-in client class prefixes diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index ac5dea7da2..b7f7423853 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -276,6 +276,49 @@ TEST_F(AllocEngine4Test, maxAlloc4) { detailCompareLease(lease, from_mgr); } +// This test checks that simple allocation handles BOOTP queries. +TEST_F(AllocEngine4Test, bootpAlloc4) { + boost::scoped_ptr engine; + ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, + 0, false))); + ASSERT_TRUE(engine); + + AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), + false, true, "somehost.example.com.", false); + subnet_->setValid(Triplet(1, 3, 5)); + ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234)); + + // Make the query a BOOTP one. + ctx.query_->addClass("BOOTP"); + + Lease4Ptr lease = engine->allocateLease4(ctx); + // The new lease has been allocated, so the old lease should not exist. + EXPECT_FALSE(ctx.old_lease_); + + // Check that we got a lease + ASSERT_TRUE(lease); + + // Check that is belongs to the right subnet and client. + EXPECT_EQ(lease->subnet_id_, subnet_->getID()); + EXPECT_TRUE(subnet_->inRange(lease->addr_)); + EXPECT_TRUE(subnet_->inPool(Lease::TYPE_V4, lease->addr_)); + ASSERT_TRUE(lease->client_id_); + EXPECT_TRUE(*lease->client_id_ == *clientid_); + ASSERT_TRUE(lease->hwaddr_); + EXPECT_TRUE(*lease->hwaddr_ == *hwaddr_); + + // Check the valid lifetime is infinite. + uint32_t infinity_lft = Lease::INFINITY_LFT; + EXPECT_EQ(infinity_lft, lease->valid_lft_); + + // Check that the lease is indeed in LeaseMgr + Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_); + ASSERT_TRUE(from_mgr); + + // Now check that the lease in LeaseMgr has the same parameters + detailCompareLease(lease, from_mgr); +} + // This test checks if the fake allocation (for DHCPDISCOVER) can succeed TEST_F(AllocEngine4Test, fakeAlloc4) { boost::scoped_ptr engine; @@ -681,6 +724,64 @@ TEST_F(AllocEngine4Test, maxRenew4) { EXPECT_EQ(subnet_->getValid().getMax(), lease2->valid_lft_); } +// This test checks simple renewal handles BOOTP queries. +TEST_F(AllocEngine4Test, bootpRenew4) { + boost::scoped_ptr engine; + ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, + 0, false))); + ASSERT_TRUE(engine); + + AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), + false, true, "somehost.example.com.", false); + subnet_->setValid(Triplet(1, 3, 5)); + ctx.query_.reset(new Pkt4(DHCPREQUEST, 1234)); + + // Make the query a BOOTP one. + ctx.query_->addClass("BOOTP"); + + Lease4Ptr lease = engine->allocateLease4(ctx); + + // Check that we got a lease. + ASSERT_TRUE(lease); + + // Check that is belongs to the right subnet and client. + EXPECT_EQ(lease->subnet_id_, subnet_->getID()); + EXPECT_TRUE(subnet_->inRange(lease->addr_)); + EXPECT_TRUE(subnet_->inPool(Lease::TYPE_V4, lease->addr_)); + ASSERT_TRUE(lease->client_id_); + EXPECT_TRUE(*lease->client_id_ == *clientid_); + ASSERT_TRUE(lease->hwaddr_); + EXPECT_TRUE(*lease->hwaddr_ == *hwaddr_); + + // Check the valid lifetime is infinite. + uint32_t infinity_lft = Lease::INFINITY_LFT; + EXPECT_EQ(infinity_lft, lease->valid_lft_); + + // The new lease has been allocated, so the old lease should not exist. + EXPECT_FALSE(ctx.old_lease_); + + // Do it again, this should amount to the renew of an existing lease + Lease4Ptr lease2 = engine->allocateLease4(ctx); + + // Check that we got a lease. + ASSERT_TRUE(lease2); + + // Check that is belongs to the right subnet and client. + EXPECT_EQ(lease2->subnet_id_, subnet_->getID()); + EXPECT_TRUE(subnet_->inRange(lease2->addr_)); + EXPECT_TRUE(subnet_->inPool(Lease::TYPE_V4, lease2->addr_)); + ASSERT_TRUE(lease2->client_id_); + EXPECT_TRUE(*lease2->client_id_ == *clientid_); + ASSERT_TRUE(lease2->hwaddr_); + EXPECT_TRUE(*lease2->hwaddr_ == *hwaddr_); + + // Lease already existed, so old_lease should be set. + EXPECT_TRUE(ctx.old_lease_); + + // Check the renewed valid lifetime has the max value. + EXPECT_EQ(infinity_lft, lease2->valid_lft_); +} + // This test verifies that the allocator picks addresses that belong to the // pool TEST_F(AllocEngine4Test, IterativeAllocator) { -- 2.47.2