From fb4fe8f19eda554da5273decea0116ae0e895cc9 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 9 Mar 2020 18:39:32 +0100 Subject: [PATCH] [#1139] Improved host based classification The DHCPv6 server now supports the case when client classes are specified in the host reservation and can be used to influence the pool selection within a top level subnet. --- src/bin/dhcp6/dhcp6_srv.cc | 56 ++++++++++++++++-------- src/bin/dhcp6/dhcp6_srv.h | 13 ++++-- src/bin/dhcp6/tests/host_unittest.cc | 64 +++++++++++++++++++++++++--- 3 files changed, 106 insertions(+), 27 deletions(-) diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 049f3329b8..97844184a6 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -360,6 +360,7 @@ Dhcpv6Srv::initContext(const Pkt6Ptr& pkt, // Collect host identifiers if host reservations enabled. The identifiers // are stored in order of preference. The server will use them in that // order to search for host reservations. + SharedNetwork6Ptr sn; if (ctx.subnet_) { const ConstCfgHostOperationsPtr cfg = CfgMgr::instance().getCurrentCfg()->getCfgHostOperations6(); @@ -422,17 +423,34 @@ Dhcpv6Srv::initContext(const Pkt6Ptr& pkt, // Find host reservations using specified identifiers. alloc_engine_->findReservation(ctx); + + // Get shared network to see if it is set for a subnet. + ctx.subnet_->getSharedNetwork(sn); } // Global host reservations are independent of a selected subnet. If the // global reservations contain client classes we should use them in case - // they are meant to affect pool selection. + // they are meant to affect pool selection. Also, if the subnet does not + // belong to a shared network we can use the reserved client classes + // because there is no way our subnet could change. Such classes may + // affect selection of a pool within the selected subnet. auto global_host = ctx.globalHost(); - if (global_host && !global_host->getClientClasses6().empty()) { - // Previously evaluated classes must be ignored because having new - // classes fetched from the hosts db may eliminate some of them. - pkt->classes_.clear(); + auto current_host = ctx.currentHost(); + if ((global_host && !global_host->getClientClasses6().empty()) || + (!sn && current_host && !current_host->getClientClasses6().empty())) { + // We have already evaluated client classes and some of them may + // be in conflict with the reserved classes. Therefore, we need to + // remove those that were assigned as a result of evaluation. + // That preserves built-in classes and the classes set explicitly + // by the hooks libraries. + const ClientClassDictionaryPtr& dict = + CfgMgr::instance().getCurrentCfg()->getClientClassDictionary(); + const ClientClassDefListPtr& defs_ptr = dict->getClasses(); + for (auto def : *defs_ptr) { + ctx.query_->classes_.erase(def->getName()); + } setReservedClientClasses(pkt, ctx); + evaluateClasses(pkt, false); } // Set KNOWN builtin class if something was found, UNKNOWN if not. @@ -2990,7 +3008,7 @@ Dhcpv6Srv::processSolicit(AllocEngine::ClientContext6& ctx) { processClientFqdn(solicit, response, ctx); assignLeases(solicit, response, ctx); - setNonGlobalReservedClientClasses(solicit, ctx); + conditionallySetReservedClientClasses(solicit, ctx); requiredClassify(solicit, ctx); copyClientOptions(solicit, response); @@ -3020,7 +3038,7 @@ Dhcpv6Srv::processRequest(AllocEngine::ClientContext6& ctx) { processClientFqdn(request, reply, ctx); assignLeases(request, reply, ctx); - setNonGlobalReservedClientClasses(request, ctx); + conditionallySetReservedClientClasses(request, ctx); requiredClassify(request, ctx); copyClientOptions(request, reply); @@ -3046,7 +3064,7 @@ Dhcpv6Srv::processRenew(AllocEngine::ClientContext6& ctx) { processClientFqdn(renew, reply, ctx); extendLeases(renew, reply, ctx); - setNonGlobalReservedClientClasses(renew, ctx); + conditionallySetReservedClientClasses(renew, ctx); requiredClassify(renew, ctx); copyClientOptions(renew, reply); @@ -3072,7 +3090,7 @@ Dhcpv6Srv::processRebind(AllocEngine::ClientContext6& ctx) { processClientFqdn(rebind, reply, ctx); extendLeases(rebind, reply, ctx); - setNonGlobalReservedClientClasses(rebind, ctx); + conditionallySetReservedClientClasses(rebind, ctx); requiredClassify(rebind, ctx); copyClientOptions(rebind, reply); @@ -3093,7 +3111,7 @@ Pkt6Ptr Dhcpv6Srv::processConfirm(AllocEngine::ClientContext6& ctx) { Pkt6Ptr confirm = ctx.query_; - setNonGlobalReservedClientClasses(confirm, ctx); + conditionallySetReservedClientClasses(confirm, ctx); requiredClassify(confirm, ctx); // Get IA_NAs from the Confirm. If there are none, the message is @@ -3183,7 +3201,7 @@ Pkt6Ptr Dhcpv6Srv::processRelease(AllocEngine::ClientContext6& ctx) { Pkt6Ptr release = ctx.query_; - setNonGlobalReservedClientClasses(release, ctx); + conditionallySetReservedClientClasses(release, ctx); requiredClassify(release, ctx); // Create an empty Reply message. @@ -3209,7 +3227,7 @@ Pkt6Ptr Dhcpv6Srv::processDecline(AllocEngine::ClientContext6& ctx) { Pkt6Ptr decline = ctx.query_; - setNonGlobalReservedClientClasses(decline, ctx); + conditionallySetReservedClientClasses(decline, ctx); requiredClassify(decline, ctx); // Create an empty Reply message. @@ -3504,7 +3522,7 @@ Pkt6Ptr Dhcpv6Srv::processInfRequest(AllocEngine::ClientContext6& ctx) { Pkt6Ptr inf_request = ctx.query_; - setNonGlobalReservedClientClasses(inf_request, ctx); + conditionallySetReservedClientClasses(inf_request, ctx); requiredClassify(inf_request, ctx); // Create a Reply packet, with the same trans-id as the client's. @@ -3656,10 +3674,14 @@ Dhcpv6Srv::setReservedClientClasses(const Pkt6Ptr& pkt, } void -Dhcpv6Srv::setNonGlobalReservedClientClasses(const Pkt6Ptr& pkt, - const AllocEngine::ClientContext6& ctx) { - if (!ctx.globalHost()) { - setReservedClientClasses(pkt, ctx); +Dhcpv6Srv::conditionallySetReservedClientClasses(const Pkt6Ptr& pkt, + const AllocEngine::ClientContext6& ctx) { + if (ctx.subnet_) { + SharedNetwork6Ptr shared_network; + ctx.subnet_->getSharedNetwork(shared_network); + if (shared_network && !ctx.globalHost()) { + setReservedClientClasses(pkt, ctx); + } } } diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h index c2a4943b6c..fcdaba1cd7 100644 --- a/src/bin/dhcp6/dhcp6_srv.h +++ b/src/bin/dhcp6/dhcp6_srv.h @@ -821,13 +821,18 @@ protected: void setReservedClientClasses(const Pkt6Ptr& pkt, const AllocEngine::ClientContext6& ctx); - /// @brief Assigns non-global classes retrieved from host reservation - /// database. + /// @brief Assigns classes retrieved from host reservation database + /// if they haven't been yet set. + /// + /// This function sets reserved client classes in case they haven't + /// been set after fetching host reservations from the database. + /// This is the case when the client has non-global host reservation + /// and the selected subnet belongs to a shared network. /// /// @param pkt Pointer to the packet to which classes will be assigned. /// @param ctx Reference to the client context. - void setNonGlobalReservedClientClasses(const Pkt6Ptr& pkt, - const AllocEngine::ClientContext6& ctx); + void conditionallySetReservedClientClasses(const Pkt6Ptr& pkt, + const AllocEngine::ClientContext6& ctx); /// @brief Assigns incoming packet to zero or more classes (required pass). /// diff --git a/src/bin/dhcp6/tests/host_unittest.cc b/src/bin/dhcp6/tests/host_unittest.cc index d6905ab60f..49b6b2da03 100644 --- a/src/bin/dhcp6/tests/host_unittest.cc +++ b/src/bin/dhcp6/tests/host_unittest.cc @@ -523,6 +523,43 @@ const char* CONFIGS[] = { " }\n" " ]\n" "}]\n" + "}", + + // Configuration 12 client-class reservation and client-class guarded pools. + "{ \"interfaces-config\": {\n" + " \"interfaces\": [ \"*\" ]\n" + "},\n" + "\"client-classes\": [" + "{" + " \"name\": \"reserved_class\"" + "}," + "{" + " \"name\": \"unreserved_class\"," + " \"test\": \"not member('reserved_class')\"" + "}" + "],\n" + "\"valid-lifetime\": 4000,\n" + "\"subnet6\": [\n" + " {\n" + " \"subnet\": \"2001:db8:1::/64\", \n" + " \"id\": 10," + " \"reservations\": [{ \n" + " \"duid\": \"01:02:03:05\",\n" + " \"client-classes\": [ \"reserved_class\" ]\n" + " }],\n" + " \"pools\": [" + " {" + " \"pool\": \"2001:db8:1::10-2001:db8:1::11\"," + " \"client-class\": \"reserved_class\"" + " }," + " {" + " \"pool\": \"2001:db8:1::20-2001:db8:1::21\"," + " \"client-class\": \"unreserved_class\"" + " }" + " ],\n" + " \"interface\": \"eth0\"\n" + " }\n" + "]\n" "}" }; @@ -924,7 +961,13 @@ public: /// /// @param config_idx Index of the server configuration from the /// @c CONFIGS array. - void testGlobalClassSubnetPoolSelection(const int config_idx); + /// @param first_address Address to be allocated from the pool having + /// a reservation. + /// @param second_address Address to be allocated from the pool not + /// having a reservation. + void testGlobalClassSubnetPoolSelection(const int config_idx, + const std::string& first_address = "2001:db8:1::10", + const std::string& second_address = "2001:db8:2::10"); /// @brief Configures client to include 6 IAs without hints. /// @@ -1272,7 +1315,9 @@ HostTest::testOverrideVendorOptions(const uint16_t msg_type) { } void -HostTest::testGlobalClassSubnetPoolSelection(const int config_idx) { +HostTest::testGlobalClassSubnetPoolSelection(const int config_idx, + const std::string& first_address, + const std::string& second_address) { Dhcp6Client client_resrv; // Use DUID for which we have host reservation including client class. @@ -1284,11 +1329,11 @@ HostTest::testGlobalClassSubnetPoolSelection(const int config_idx) { // Let's use the 2001:db8:2::10 as a hint to make sure that the server // refuses allocating it and uses the sole pool available for this // client. - client_resrv.requestAddress(1, IOAddress("2001:db8:2::10")); + client_resrv.requestAddress(1, IOAddress(second_address)); ASSERT_NO_THROW(client_resrv.doSARR()); ASSERT_EQ(1, client_resrv.getLeaseNum()); Lease6 lease_client = client_resrv.getLease(0); - EXPECT_EQ("2001:db8:1::10", lease_client.addr_.toText()); + EXPECT_EQ(first_address, lease_client.addr_.toText()); // This client has no reservation and therefore should be // assigned to the unreserved_class and be given an address @@ -1298,11 +1343,11 @@ HostTest::testGlobalClassSubnetPoolSelection(const int config_idx) { // Let's use the address of 2001:db8:1::10 as a hint to make sure that the // server refuses it in favor of the 2001:db8:2::10. - client_no_resrv.requestAddress(1, IOAddress("2001:db8:1::10")); + client_no_resrv.requestAddress(1, IOAddress(first_address)); ASSERT_NO_THROW(client_no_resrv.doSARR()); ASSERT_EQ(1, client_no_resrv.getLeaseNum()); lease_client = client_no_resrv.getLease(0); - EXPECT_EQ("2001:db8:2::10", lease_client.addr_.toText()); + EXPECT_EQ(second_address, lease_client.addr_.toText()); } void @@ -2306,4 +2351,11 @@ TEST_F(HostTest, clientClassGlobalSubnetSelection) { ASSERT_NO_FATAL_FAILURE(testGlobalClassSubnetPoolSelection(11)); } +// Verifies that client class specified in the reservation may be +// used to influence pool selection within a subnet. +TEST_F(HostTest, clientClassPoolSelection) { + ASSERT_NO_FATAL_FAILURE(testGlobalClassSubnetPoolSelection(12, "2001:db8:1::10", + "2001:db8:1::20")); +} + } // end of anonymous namespace