[4303] Values in 'host-reservation-identifiers' used by the servers.

Both DHCPv4 and DHCPv6.

src/bin/dhcp4/json_config_parser.cc

@@ -448,6 +448,8 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id,
         parser = new ExpirationConfigParser();
     } else if (config_id.compare("client-classes") == 0) {
         parser = new ClientClassDefListParser(config_id, globalContext());
+    } else if (config_id.compare("host-reservation-identifiers") == 0) {
+        parser = new HostReservationIdsParser4();
     } else {
         isc_throw(DhcpConfigError,
                 "unsupported global configuration parameter: "

src/bin/dhcp4/tests/dora_unittest.cc

@@ -57,6 +57,11 @@ namespace {
 ///   - 1 pool: 10.0.0.10-10.0.0.100
 ///   - match-client-id flag is set to false, thus the server
 ///     uses HW address for lease lookup, rather than client id
+///
+/// - Configuration 4:
+///   - The same as configuration 2, but using different values in
+///     'host-reservation-identifiers'
+///
 const char* DORA_CONFIGS[] = {
 // Configuration 0
     "{ \"interfaces-config\": {"
@@ -154,6 +159,32 @@ const char* DORA_CONFIGS[] = {
         "    } ]"
         " } ]"
     "}",
+
+// Configuration 4
+    "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"host-reservation-identifiers\": [ \"circuit-id\", \"hw-address\" ],"
+        "\"valid-lifetime\": 600,"
+        "\"subnet4\": [ { "
+        "    \"subnet\": \"10.0.0.0/24\", "
+        "    \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ],"
+        "    \"reservations\": [ "
+        "       {"
+        "         \"hw-address\": \"aa:bb:cc:dd:ee:ff\","
+        "         \"ip-address\": \"10.0.0.7\""
+        "       },"
+        "       {"
+        "         \"duid\": \"01:02:03:04:05\","
+        "         \"ip-address\": \"10.0.0.8\""
+        "       },"
+        "       {"
+        "         \"circuit-id\": \"'charter950'\","
+        "         \"ip-address\": \"10.0.0.9\""
+        "       }"
+        "    ]"
+        "} ]"
+    "}"
 };
 
 /// @brief Test fixture class for testing 4-way (DORA) exchanges.
@@ -697,9 +728,8 @@ TEST_F(DORATest, reservation) {
 }
 
 // This test checks that it is possible to make a reservation by
-// circuit-id inserted by the relay agent..
+// circuit-id inserted by the relay agent.
 TEST_F(DORATest, reservationByCircuitId) {
-    // Client A is a one which will have a reservation.
     Dhcp4Client client(Dhcp4Client::SELECTING);
     // Use relay agent so as the circuit-id can be inserted.
     client.useRelay(true, IOAddress("10.0.0.1"), IOAddress("10.0.0.2"));
@@ -721,6 +751,47 @@ TEST_F(DORATest, reservationByCircuitId) {
     ASSERT_EQ("10.0.0.9", client.config_.lease_.addr_.toText());
 }
 
+// This test verifies that order in which host identifiers are used to
+// retrieve host reservations can be controlled.
+TEST_F(DORATest, hostIdentifiersOrder) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    client.setHWAddress("aa:bb:cc:dd:ee:ff");
+    // Use relay agent so as the circuit-id can be inserted.
+    client.useRelay(true, IOAddress("10.0.0.1"), IOAddress("10.0.0.2"));
+    // Specify circuit-id.
+    client.setCircuitId("charter950");
+
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[2], *client.getServer());
+    // Perform 4-way exchange to obtain reserved address.
+    // The client has in fact two reserved addresses, but the one assigned
+    // should be by hw-address.
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // Make sure that the client has got the lease for the reserved address.
+    ASSERT_EQ("10.0.0.7", client.config_.lease_.addr_.toText());
+
+    // Reconfigure the server to change the preference order of the
+    // host identifiers. The 'circuit-id' should now take precedence over
+    // the hw-address.
+    configure(DORA_CONFIGS[4], *client.getServer());
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // Make sure that the client has got the lease for the reserved address.
+    ASSERT_EQ("10.0.0.9", client.config_.lease_.addr_.toText());
+
+}
+
 // This test checks that setting the match-client-id value to false causes
 // the server to ignore changing client identifier when the client is
 // using consistent HW address.

src/bin/dhcp6/json_config_parser.cc

@@ -702,6 +702,8 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id,
         parser = new ClientClassDefListParser(config_id, globalContext());
     } else if (config_id.compare("server-id") == 0) {
         parser = new DUIDConfigParser();
+    } else if (config_id.compare("host-reservation-identifiers") == 0) {
+        parser = new HostReservationIdsParser6();
     } else {
         isc_throw(DhcpConfigError,
                 "unsupported global configuration parameter: "

src/bin/dhcp6/tests/host_unittest.cc

@@ -23,6 +23,9 @@ namespace {
 ///   Single subnet with two reservations, one with a hostname, one without
 /// - Configuration 1:
 ///   Multiple reservations using different host identifiers.
+/// - Configuration 2:
+///   Same as configuration 1 but 'host-reservation-identifiers' specified
+///   in non-default order.
 const char* CONFIGS[] = {
     // Configuration 0:
     "{ "
@@ -76,8 +79,35 @@ const char* CONFIGS[] = {
         "        \"ip-addresses\": [ \"2001:db8:1::2\" ]"
         "    } ]"
         " } ]"
-    "}"
+    "}",
 
+    // Configuration 2:
+    "{ "
+        "\"interfaces-config\": {"
+        "  \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"host-reservation-identifiers\": [ \"duid\", \"hw-address\" ],"
+        "\"valid-lifetime\": 4000, "
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"mac-sources\": [ \"ipv6-link-local\" ], "
+        "\"subnet6\": [ "
+        " { "
+        "    \"subnet\": \"2001:db8:1::/48\", "
+        "    \"pools\": [ { \"pool\": \"2001:db8:1::/64\" } ],"
+        "    \"interface\" : \"eth0\" , "
+        "    \"reservations\": ["
+        "    {"
+        "        \"hw-address\": \"38:60:77:d5:ff:ee\","
+        "        \"ip-addresses\": [ \"2001:db8:1::1\" ]"
+        "    },"
+        "    {"
+        "        \"duid\": \"01:02:03:05\","
+        "        \"ip-addresses\": [ \"2001:db8:1::2\" ]"
+        "    } ]"
+        " } ]"
+    "}"
 };
 
 /// @brief Test fixture class for testing host reservations
@@ -97,11 +127,14 @@ public:
     /// @param client Reference to a client to be used in the test.
     /// The client should be preconfigured to insert a specific identifier
     /// into the message, e.g. DUID, HW address etc.
+    /// @param config_index Index of the configuration to use in the CONFIGS
+    /// table.
     /// @param exp_ip_address Expected IPv6 address in the returned
     /// reservation.
     void testReservationByIdentifier(Dhcp6Client& client,
+                                     const unsigned int config_index,
                                      const std::string exp_ip_address) {
-        configure(CONFIGS[1], *client.getServer());
+        configure(CONFIGS[config_index], *client.getServer());
 
         const Subnet6Collection* subnets = CfgMgr::instance().getCurrentCfg()->
             getCfgSubnets6()->getAll();
@@ -315,7 +348,7 @@ TEST_F(HostTest, reservationByDUID) {
     // Set DUID matching the one used to create host reservations.
     client.setDUID("01:02:03:05");
     // Run the actual test.
-    testReservationByIdentifier(client, "2001:db8:1::2");
+    testReservationByIdentifier(client, 1, "2001:db8:1::2");
 }
 
 // This test verfies that the host reservation by HW address is found
@@ -327,9 +360,20 @@ TEST_F(HostTest, reservationByHWAddress) {
     // decoded address will be used to search for host reservations.
     client.setLinkLocal(IOAddress("fe80::3a60:77ff:fed5:ffee"));
     // Run the actual test.
-    testReservationByIdentifier(client, "2001:db8:1::1");
+    testReservationByIdentifier(client, 1, "2001:db8:1::1");
 }
 
-
+// This test verifies that order in which host identifiers are used to
+// retrieve host reservations can be controlled.
+TEST_F(HostTest, hostIdentifiersOrder) {
+    Dhcp6Client client;
+    // Set DUID matching the one used to create host reservations.
+    client.setDUID("01:02:03:05");
+    // Set link local address for the client which the server will
+    // use to decode the HW address as 38:60:77:d5:ff:ee. This
+    // decoded address will be used to search for host reservations.
+    client.setLinkLocal(IOAddress("fe80::3a60:77ff:fed5:ffee"));
+    testReservationByIdentifier(client, 2, "2001:db8:1::2");
+}
 
 } // end of anonymous namespace

src/lib/dhcpsrv/alloc_engine.cc

@@ -12,6 +12,7 @@
 #include <dhcp_ddns/ncr_msg.h>
 #include <dhcpsrv/alloc_engine.h>
 #include <dhcpsrv/alloc_engine_log.h>
+#include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/host_mgr.h>
 #include <dhcpsrv/host.h>
@@ -27,6 +28,7 @@
 
 #include <boost/foreach.hpp>
 
+#include <algorithm>
 #include <cstring>
 #include <sstream>
 #include <limits>
@@ -301,6 +303,40 @@ AllocEngine::AllocatorPtr AllocEngine::getAllocator(Lease::Type type) {
     return (alloc->second);
 }
 
+template<typename ContextType>
+void
+AllocEngine::findReservationInternal(ContextType& ctx,
+                                     const ConstCfgHostReservationsPtr& cfg,
+                                     const AllocEngine::HostGetFunc& host_get) {
+    ctx.host_.reset();
+
+    // We can only search for the reservation if a subnet has been selected.
+    if (ctx.subnet_) {
+        // Check which host reservation mode is supported in this subnet.
+        Subnet::HRMode hr_mode = ctx.subnet_->getHostReservationMode();
+
+        // Check if there is a host reseravtion for this client. Attempt to
+        // get host information
+        if (hr_mode != Subnet::HR_DISABLED) {
+            // Iterate over configured identifiers in the order of preference
+            // and try to use each of them to search for the reservations.
+            BOOST_FOREACH(const Host::IdentifierType& id, cfg->getIdentifierTypes()) {
+                IdentifierMap::const_iterator id_ctx = ctx.host_identifiers_.find(id);
+                if (id_ctx != ctx.host_identifiers_.end()) {
+                    // Attempt to find a host using a specified identifier.
+                    ctx.host_ = host_get(ctx.subnet_->getID(), id,
+                                         &id_ctx->second[0], id_ctx->second.size());
+                    // If we found matching host, return.
+                    if (ctx.host_) {
+                        return;
+                    }
+                }
+            }
+        }
+    }    
+}
+
+
 // ##########################################################################
 // #    DHCPv6 lease allocation code starts here.
 // ##########################################################################
@@ -339,30 +375,12 @@ AllocEngine::ClientContext6::ClientContext6(const Subnet6Ptr& subnet, const Duid
 }
 
 
-void AllocEngine::findReservation(ClientContext6& ctx) const {
+void AllocEngine::findReservation(ClientContext6& ctx) {
-    if (!ctx.subnet_ || !ctx.duid_) {
+    ConstCfgHostReservationsPtr cfg =
-        return;
+        CfgMgr::instance().getCurrentCfg()->getCfgHostReservations6();
-    }
+    findReservationInternal(ctx, cfg, boost::bind(&HostMgr::get6,
-
+                                                  &HostMgr::instance(),
-    ctx.host_.reset();
+                                                  _1, _2, _3, _4));
-
-    // Check which host reservation mode is supported in this subnet.
-    Subnet::HRMode hr_mode = ctx.subnet_->getHostReservationMode();
-
-    // Check if there's a host reservation for this client. Attempt to get
-    // host info only if reservations are not disabled.
-    if (hr_mode != Subnet::HR_DISABLED) {
-
-        BOOST_FOREACH(const IdentifierPair& id, ctx.host_identifiers_) {
-            ctx.host_ = HostMgr::instance().get6(ctx.subnet_->getID(),
-                                                 id.first, &id.second[0],
-                                                 id.second.size());
-            // If we found matching host, return.
-            if (ctx.host_) {
-                return;
-            }
-        }
-    }
 }
 
 Lease6Collection
@@ -2117,27 +2135,11 @@ AllocEngine::allocateLease4(ClientContext4& ctx) {
 
 void
 AllocEngine::findReservation(ClientContext4& ctx) {
-    ctx.host_.reset();
+    ConstCfgHostReservationsPtr cfg =
-
+        CfgMgr::instance().getCurrentCfg()->getCfgHostReservations4();
-    // We can only search for the reservation if a subnet has been selected.
+    findReservationInternal(ctx, cfg, boost::bind(&HostMgr::get4,
-    if (ctx.subnet_) {
+                                                  &HostMgr::instance(),
-        // Check which host reservation mode is supported in this subnet.
+                                                  _1, _2, _3, _4));
-        Subnet::HRMode hr_mode = ctx.subnet_->getHostReservationMode();
-
-        // Check if there is a host reseravtion for this client. Attempt to
-        // get host information
-        if (hr_mode != Subnet::HR_DISABLED) {
-            BOOST_FOREACH(const IdentifierPair& id, ctx.host_identifiers_) {
-                ctx.host_ = HostMgr::instance().get4(ctx.subnet_->getID(),
-                                                     id.first, &id.second[0],
-                                                     id.second.size());
-                // If we found matching host, return.
-                if (ctx.host_) {
-                    return;
-                }
-            }
-        }
-    }
 }
 
 Lease4Ptr

src/lib/dhcpsrv/alloc_engine.h

@@ -13,6 +13,7 @@
 #include <dhcp/pkt4.h>
 #include <dhcp/pkt6.h>
 #include <dhcp/option6_ia.h>
+#include <dhcpsrv/cfg_host_reservations.h>
 #include <dhcpsrv/host.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/lease_mgr.h>
@@ -617,10 +618,33 @@ public:
     /// Attempts to find appropriate host reservation in HostMgr. If found, it
     /// will be set in ctx.host_.
     /// @param ctx Client context that contains all necessary information.
-    void findReservation(ClientContext6& ctx) const;
+    static void findReservation(ClientContext6& ctx);
 
 private:
 
+    /// @brief Type of the function used by @ref findReservationInternal to
+    /// retrieve reservations by subnet identifier and host identifier.
+    typedef boost::function<ConstHostPtr(const SubnetID&,
+                                         const Host::IdentifierType&,
+                                         const uint8_t*, const size_t)> HostGetFunc;
+
+    /// @brief Common function for searching host reservations.
+    ///
+    /// This is a common function called by variants of @ref findReservation
+    /// functions.
+    ///
+    /// @param ctx Reference to a @ref ClientContext6 or @ref ClientContext4.
+    /// @param cfg Pointer to object holding general configuration for host
+    /// reservations. It uses this object to read the desired order of
+    /// host identifiers to be used to search for reservations.
+    /// @param host_get Pointer to the @ref HostMgr functions to be used
+    /// to retrieve reservation by subnet identifier and host identifier.
+    /// @tparam ContextType Either @ref ClientContext6 or @ref ClientContext4.
+    template<typename ContextType>
+    static void findReservationInternal(ContextType& ctx,
+                                        const ConstCfgHostReservationsPtr& cfg,
+                                        const HostGetFunc& host_get);
+
     /// @brief creates a lease and inserts it in LeaseMgr if necessary
     ///
     /// Creates a lease based on specified parameters and tries to insert it