[#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

@@ -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()) {
+    auto current_host = ctx.currentHost();
-        // Previously evaluated classes must be ignored because having new
+    if ((global_host && !global_host->getClientClasses6().empty()) ||
-        // classes fetched from the hosts db may eliminate some of them.
+        (!sn && current_host && !current_host->getClientClasses6().empty())) {
-        pkt->classes_.clear();
+        // 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,
+Dhcpv6Srv::conditionallySetReservedClientClasses(const Pkt6Ptr& pkt,
-                                             const AllocEngine::ClientContext6& ctx) {
+                                                 const AllocEngine::ClientContext6& ctx) {
-    if (!ctx.globalHost()) {
+    if (ctx.subnet_) {
-        setReservedClientClasses(pkt, ctx);
+        SharedNetwork6Ptr shared_network;
+        ctx.subnet_->getSharedNetwork(shared_network);
+        if (shared_network && !ctx.globalHost()) {
+            setReservedClientClasses(pkt, ctx);
+        }
     }
 }
 

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
+    /// @brief Assigns classes retrieved from host reservation database
-    /// 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,
+    void conditionallySetReservedClientClasses(const Pkt6Ptr& pkt,
-                                           const AllocEngine::ClientContext6& ctx);
+                                               const AllocEngine::ClientContext6& ctx);
 
     /// @brief Assigns incoming packet to zero or more classes (required pass).
     ///

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