Browse Source

[master] Merge branch 'trac3564'

Marcin Siodelski 10 years ago
parent
commit
7b192fe314

+ 1 - 0
doc/Makefile.am

@@ -5,6 +5,7 @@ EXTRA_DIST = version.ent.in differences.txt Doxyfile Doxyfile-xml
 nobase_dist_doc_DATA  = examples/kea4/single-subnet.json
 nobase_dist_doc_DATA += examples/kea4/several-subnets.json
 nobase_dist_doc_DATA += examples/kea4/multiple-options.json
+nobase_dist_doc_DATA += examples/kea4/reservations.json
 nobase_dist_doc_DATA += examples/kea6/simple.json
 nobase_dist_doc_DATA += examples/kea6/several-subnets.json
 nobase_dist_doc_DATA += examples/kea6/multiple-options.json

+ 72 - 0
doc/examples/kea4/reservations.json

@@ -0,0 +1,72 @@
+# This is an example configuration file for the DHCPv4 server in Kea.
+# It contains one subnet in which there are two static address reservations
+# for the clients identified by the MAC addresses.
+{ "Dhcp4":
+
+{
+# Kea is told to listen on eth0 interface only.
+  "interfaces": [ "eth0" ],
+
+# We need to specify lease type. As of May 2014, three backends are supported:
+# memfile, mysql and pgsql. We'll just use memfile, because it doesn't require
+# any prior set up.
+  "lease-database": {
+    "type": "memfile"
+  },
+
+# Addresses will be assigned with valid lifetimes being 4000. Client
+# is told to start renewing after 1000 seconds. If the server does not respond
+# after 2000 seconds since the lease was granted, client is supposed
+# to start REBIND procedure (emergency renewal that allows switching
+# to a different server).
+  "valid-lifetime": 4000,
+
+# Renew and rebind timers are commented out. This implies that options
+# 58 and 59 will not be sent to the client. In this case it is up to
+# the client to pick the timer values according to RFC2131. Uncomment the
+# timers to send these options to the client.
+#  "renew-timer": 1000,
+#  "rebind-timer": 2000,
+
+# Define a subnet with two reservations for the 192.0.2.202 and
+# 192.0.2.100 address. Note that the latter is a reservation for the
+# address which is within the range of the pool of the dynamically
+# allocated address. The server will exclude this address from this
+# pool and only assign it to the client which has a reservation for
+# it.
+  "subnet4": [
+    {
+       "pools": [ { "pool":  "192.0.2.1 - 192.0.2.200" } ],
+       "subnet": "192.0.2.0/24",
+       "interface": "eth0",
+       "reservations": [
+         {
+             "hw-address": "1a:1b:1c:1d:1e:1f",
+             "ip-address": "192.0.2.202"
+         },
+         {
+             "hw-address": "0a:0b:0c:0d:0e:0f",
+             "ip-address": "192.0.2.100"
+         }
+       ]
+    } 
+  ]
+},
+
+# The following configures logging. It assumes that messages with at least
+# informational level (info, warn, error) will will be logged to stdout.
+"Logging": {
+    "loggers": [
+        {
+            "name": "kea-dhcp4",
+            "output_options": [
+                {
+                    "output": "stdout"
+                }
+            ],
+            "severity": "INFO"
+        }
+    ]
+}
+
+}

+ 11 - 3
src/bin/dhcp4/dhcp4_srv.cc

@@ -957,14 +957,17 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast<
         OptionCustom>(question->getOption(DHO_DHCP_SERVER_IDENTIFIER));
 
-    // Try to get the Requested IP Address option and use the address as a hint
-    // for the allocation engine. If the server doesn't already have a lease
-    // for this client it will try to allocate the one requested.
+    // Check if the client has sent a requested IP address option or
+    // ciaddr.
     OptionCustomPtr opt_requested_address = boost::dynamic_pointer_cast<
         OptionCustom>(question->getOption(DHO_DHCP_REQUESTED_ADDRESS));
     IOAddress hint("0.0.0.0");
     if (opt_requested_address) {
         hint = opt_requested_address->readAddress();
+
+    } else if (question->getCiaddr() != IOAddress("0.0.0.0")) {
+        hint = question->getCiaddr();
+
     }
 
     HWAddrPtr hwaddr = question->getHWAddr();
@@ -1309,6 +1312,11 @@ Dhcpv4Srv::processDiscover(Pkt4Ptr& discover) {
         // include in the response. If client did not request
         // them we append them for him.
         appendBasicOptions(discover, offer);
+
+    } else {
+        // If the server can't offer an address, it drops the packet.
+        return (Pkt4Ptr());
+
     }
 
     // Set the src/dest IP address, port and interface for the outgoing

+ 6 - 1
src/bin/dhcp4/tests/dhcp4_client.cc

@@ -57,7 +57,7 @@ Dhcp4Client::Dhcp4Client(const Dhcp4Client::State& state) :
     use_relay_(false) {
 }
 
-Dhcp4Client::Dhcp4Client(boost::shared_ptr<NakedDhcpv4Srv>& srv,
+Dhcp4Client::Dhcp4Client(boost::shared_ptr<NakedDhcpv4Srv> srv,
                          const Dhcp4Client::State& state) :
     config_(),
     ciaddr_(IOAddress("0.0.0.0")),
@@ -349,6 +349,11 @@ Dhcp4Client::sendMsg(const Pkt4Ptr& msg) {
     srv_->run();
 }
 
+void
+Dhcp4Client::setHWAddress(const std::string& hwaddr_str) {
+    hwaddr_.reset(new HWAddr(HWAddr::fromText(hwaddr_str)));
+}
+
 } // end of namespace isc::dhcp::test
 } // end of namespace isc::dhcp
 } // end of namespace isc

+ 9 - 2
src/bin/dhcp4/tests/dhcp4_client.h

@@ -103,7 +103,7 @@ public:
     ///
     /// @param srv An instance of the DHCPv4 server to be used.
     /// @param state Initial client's state.
-    Dhcp4Client(boost::shared_ptr<NakedDhcpv4Srv>& srv,
+    Dhcp4Client(boost::shared_ptr<NakedDhcpv4Srv> srv,
                 const State& state = SELECTING);
 
     /// @brief Creates a lease for the client using the specified address
@@ -203,7 +203,9 @@ public:
     HWAddrPtr generateHWAddr(const uint8_t htype = HTYPE_ETHER) const;
 
     /// @brief Returns HW address used by the client.
-    HWAddrPtr getHWAddress() const;
+    HWAddrPtr getHWAddress() const {
+        return (hwaddr_);
+    }
 
     /// @brief Returns current context.
     const Context& getContext() const {
@@ -266,6 +268,11 @@ public:
         dest_addr_ = dest_addr;
     }
 
+    /// @brief Sets the explicit hardware address for the client.
+    ///
+    /// @param hwaddr_str String representation of the HW address.
+    void setHWAddress(const std::string& hwaddr_str);
+
     /// @brief Sets client state.
     ///
     /// Depending on the current state the client's behavior is different

+ 20 - 2
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1136,6 +1136,25 @@ TEST_F(Dhcpv4SrvTest, relayAgentInfoEcho) {
 
     NakedDhcpv4Srv srv(0);
 
+    // Use of the captured DHCPDISCOVER packet requires that
+    // subnet 10.254.226.0/24 is in use, because this packet
+    // contains the giaddr which belongs to this subnet and
+    // this giaddr is used to select the subnet
+    std::string config = "{ \"interfaces\": [ \"*\" ],"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"10.254.226.0/25\" } ],"
+        "    \"subnet\": \"10.254.226.0/24\", "
+        "    \"rebind-timer\": 2000, "
+        "    \"renew-timer\": 1000, "
+        "    \"valid-lifetime\": 4000,"
+        "    \"interface\": \"eth0\" "
+        " } ],"
+        "\"valid-lifetime\": 4000 }";
+
+    configure(config);
+
     // Let's create a relayed DISCOVER. This particular relayed DISCOVER has
     // added option 82 (relay agent info) with 3 suboptions. The server
     // is supposed to echo it back in its response.
@@ -1610,7 +1629,7 @@ public:
             192, 0, 2, 1,           // ciaddr
             1, 2, 3, 4,             // yiaddr
             192, 0, 2, 255,         // siaddr
-            255, 255, 255, 255,     // giaddr
+            192, 0, 2, 50,          // giaddr
         };
 
         // Initialize the vector with the header fields defined above.
@@ -2041,7 +2060,6 @@ TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveValueChange) {
     IfaceMgrTestConfig test_config(true);
     IfaceMgr::instance().openSockets4();
 
-
     // Install callback that modifies MAC addr of incoming packet
     EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout(
                         "buffer4_receive", buffer4_receive_change_hwaddr));

+ 22 - 17
src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -283,10 +283,11 @@ void Dhcpv4SrvTest::checkAddressParams(const Pkt4Ptr& rsp,
     }
 }
 
-void Dhcpv4SrvTest::checkResponse(const Pkt4Ptr& rsp, uint8_t expected_message_type,
+void Dhcpv4SrvTest::checkResponse(const Pkt4Ptr& rsp, int expected_message_type,
                                   uint32_t expected_transid) {
     ASSERT_TRUE(rsp);
-    EXPECT_EQ(expected_message_type, rsp->getType());
+    EXPECT_EQ(expected_message_type,
+              static_cast<int>(rsp->getType()));
     EXPECT_EQ(expected_transid, rsp->getTransid());
 }
 
@@ -544,31 +545,33 @@ Dhcpv4SrvTest::testDiscoverRequest(const uint8_t msg_type) {
 
     if (msg_type == DHCPDISCOVER) {
         ASSERT_NO_THROW(rsp = srv->processDiscover(received));
-        // Should return non-NULL packet.
-        ASSERT_TRUE(rsp);
+        // Should return NULL packet.
+        ASSERT_FALSE(rsp);
+
     } else {
         ASSERT_NO_THROW(rsp = srv->processRequest(received));
         // Should return non-NULL packet.
         ASSERT_TRUE(rsp);
+        // We should get the NAK packet with yiaddr set to 0.
+        EXPECT_EQ(DHCPNAK, rsp->getType());
+        ASSERT_EQ("0.0.0.0", rsp->getYiaddr().toText());
+
+        // Make sure that none of the requested options is returned in NAK.
+        // Also options such as Routers or Subnet Mask should not be there,
+        // because lease hasn't been acquired.
+        EXPECT_TRUE(noRequestedOptions(rsp));
+        EXPECT_TRUE(noBasicOptions(rsp));
     }
-    // We should get the NAK packet with yiaddr set to 0.
-    EXPECT_EQ(DHCPNAK, rsp->getType());
-    ASSERT_EQ("0.0.0.0", rsp->getYiaddr().toText());
-
-    // Make sure that none of the requested options is returned in NAK.
-    // Also options such as Routers or Subnet Mask should not be there,
-    // because lease hasn't been acquired.
-    EXPECT_TRUE(noRequestedOptions(rsp));
-    EXPECT_TRUE(noBasicOptions(rsp));
 }
 
 void
-Dhcpv4SrvTest::configure(const std::string& config) {
-    configure(config, srv_);
+Dhcpv4SrvTest::configure(const std::string& config, const bool commit) {
+    configure(config, srv_, commit);
 }
 
 void
-Dhcpv4SrvTest::configure(const std::string& config, NakedDhcpv4Srv& srv) {
+Dhcpv4SrvTest::configure(const std::string& config, NakedDhcpv4Srv& srv,
+                         const bool commit) {
     ElementPtr json = Element::fromJSON(config);
     ConstElementPtr status;
 
@@ -579,7 +582,9 @@ Dhcpv4SrvTest::configure(const std::string& config, NakedDhcpv4Srv& srv) {
     ConstElementPtr comment = config::parseAnswer(rcode, status);
     ASSERT_EQ(0, rcode);
 
-    CfgMgr::instance().commit();
+    if (commit) {
+        CfgMgr::instance().commit();
+    }
  }
 
 

+ 8 - 3
src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -307,7 +307,7 @@ public:
     /// @param rsp response packet to be validated
     /// @param expected_message_type expected message type
     /// @param expected_transid expected transaction-id
-    void checkResponse(const Pkt4Ptr& rsp, uint8_t expected_message_type,
+    void checkResponse(const Pkt4Ptr& rsp, int expected_message_type,
                        uint32_t expected_transid);
 
     /// @brief Checks if the lease sent to client is present in the database
@@ -405,13 +405,18 @@ public:
     /// @brief Runs DHCPv4 configuration from the JSON string.
     ///
     /// @param config String holding server configuration in JSON format.
-    void configure(const std::string& config);
+    /// @param commit A boolean flag indicating if the new configuration
+    /// should be committed (if true), or not (if false).
+    void configure(const std::string& config, const bool commit = true);
 
     /// @brief Configure specified DHCP server using JSON string.
     ///
     /// @param config String holding server configuration in JSON format.
     /// @param srv Instance of the server to be configured.
-    void configure(const std::string& config, NakedDhcpv4Srv& srv);
+    /// @param commit A boolean flag indicating if the new configuration
+    /// should be committed (if true), or not (if false).
+    void configure(const std::string& config, NakedDhcpv4Srv& srv,
+                   const bool commit = true);
 
     /// @brief This function cleans up after the test.
     virtual void TearDown();

+ 268 - 0
src/bin/dhcp4/tests/dora_unittest.cc

@@ -17,6 +17,10 @@
 #include <cc/data.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/tests/iface_mgr_test_config.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/host.h>
+#include <dhcpsrv/host_mgr.h>
+#include <dhcpsrv/subnet_id.h>
 #include <dhcp4/tests/dhcp4_test_utils.h>
 #include <dhcp4/tests/dhcp4_client.h>
 #include <boost/shared_ptr.hpp>
@@ -47,12 +51,19 @@ namespace {
 ///   - Domain Name Server option present: 192.0.2.202, 192.0.2.203.
 ///   - Log Servers option present: 192.0.2.200 and 192.0.2.201
 ///   - Quotes Servers option present: 192.0.2.202, 192.0.2.203.
+///
+/// - Configuration 2:
+///   - Use for testing simple scenarios with host reservations
+///   - 1 subnet: 10.0.0.0/24
+///   - One reservation for the client using MAC address:
+///     aa:bb:cc:dd:ee:ff, reserved address 10.0.0.7
 const char* DORA_CONFIGS[] = {
 // Configuration 0
     "{ \"interfaces\": [ \"*\" ],"
         "\"valid-lifetime\": 600,"
         "\"subnet4\": [ { "
         "    \"subnet\": \"10.0.0.0/24\", "
+        "    \"id\": 1,"
         "    \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ],"
         "    \"option-data\": [ {"
         "        \"name\": \"routers\","
@@ -119,6 +130,21 @@ const char* DORA_CONFIGS[] = {
         "        \"space\": \"dhcp4\""
         "    } ]"
         " } ]"
+    "}",
+
+// Configuration 2
+    "{ \"interfaces\": [ \"*\" ],"
+        "\"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\""
+        "       }"
+        "    ]"
+        "} ]"
     "}"
 };
 
@@ -402,4 +428,246 @@ TEST_F(DORATest, ciaddr) {
     EXPECT_EQ("0.0.0.0", resp->getCiaddr().toText());
 }
 
+// This is a simple test for the host reservation. It creates a reservation
+// for an address for a single client, identified by the HW address. The
+// test verifies that the client using this HW address will obtain a
+// lease for the reserved address. It also checks that the client using
+// a different HW address will obtain an address from the dynamic pool.
+TEST_F(DORATest, reservation) {
+    // Client A is a one which will have a reservation.
+    Dhcp4Client clientA(Dhcp4Client::SELECTING);
+    // Set explicit HW address so as it matches the reservation in the
+    // configuration used below.
+    clientA.setHWAddress("aa:bb:cc:dd:ee:ff");
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[2], *clientA.getServer());
+    // Client A performs 4-way exchange and should obtain a reserved
+    // address.
+    ASSERT_NO_THROW(clientA.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(clientA.getContext().response_);
+    Pkt4Ptr resp = clientA.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", clientA.config_.lease_.addr_.toText());
+
+    // Client B uses the same server as Client A.
+    Dhcp4Client clientB(clientA.getServer(), Dhcp4Client::SELECTING);
+    // Client B has no reservation so it should get the lease from
+    // the dynamic pool.
+    ASSERT_NO_THROW(clientB.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(clientB.getContext().response_);
+    resp = clientB.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // Obtain the subnet to which the returned address belongs.
+    Subnet4Ptr subnet = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->
+        selectSubnet(clientB.config_.lease_.addr_);
+    ASSERT_TRUE(subnet);
+    // Make sure that the address has been allocated from the dynamic pool.
+    ASSERT_TRUE(subnet->inPool(Lease::TYPE_V4, clientB.config_.lease_.addr_));
+}
+
+// This test checks the following scenario:
+// 1. Client A performs 4-way exchange and obrains a lease from the dynamic pool.
+// 2. Reservation is created for the client A using an address out of the dynamic
+//    pool.
+// 3. Client A renews the lease.
+// 4. Server responds with DHCPNAK to indicate that the client should stop using
+//    an address for which it has a lease. Server doesn't want to renew an
+//    address for which the client doesn't have a reservation, while it has
+//    a reservation for a different address.
+// 5. Client A receives a DHCPNAK and returns to the DHCP server discovery.
+// 6. Client A performs a 4-way exchange with a server and the server allocates
+//   a reserved address to the Client A.
+// 7. Client A renews the allocated address and the server returns a DHCPACK.
+// 8. Reservation for the Client A is removed.
+// 9. Client A renews the (previously reserved) lease and the server returns
+//    DHCPNAK because the address in use is neither reserved nor belongs to
+//    the dynamic pool.
+// 10. Client A returns to the DHCP server discovery.
+// 11. Client A uses 4-way exchange to obtain a lease from the dynamic pool.
+// 12. The new address that the Client A is using is reserved for Client B.
+//     Client A still holds this address.
+// 13. Client B uses 4-way exchange to obtain a new lease.
+// 14. The server determines that the Client B has a reservation for the
+//     address which is in use by Client A. The server drops the client's
+//     DHCPDISCOVER message.
+// 15. Client A renews the lease.
+// 16. The server determines that the address that Client A is using is reserved
+//     for Client B. The server returns DHCPNAK to the Client A.
+// 17. Client B uses 4-way echange to obtain the reserved lease but the lease
+//     for the Client A hasn't been removed yet. Client B's DHCPDISCOVER
+//     message is dropped again.
+// 18. Client A uses 4-way exchange to allocate a new lease.
+// 19. The server allocates a new lease from the dynamic pool but it avoids
+//     allocating the address reserved for the Client B.
+// 20. Client B uses 4-way exchange to obtain a new lease.
+// 21. The server finally allocates a reserved address to the Client B.
+TEST_F(DORATest, reservationsWithConflicts) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[0], *client.getServer());
+    // Client A performs 4-way exchange and obrains a lease from the
+    // dynamic pool.
+    ASSERT_NO_THROW(client.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("10.0.0.50"))));
+    // 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 with the requested address.
+    ASSERT_EQ("10.0.0.50", client.config_.lease_.addr_.toText());
+
+    configure(DORA_CONFIGS[0], false);
+    // Reservation is created for the client A using an address out of the
+    // dynamic pool.
+    HostPtr host(new Host(&client.getHWAddress()->hwaddr_[0],
+                          client.getHWAddress()->hwaddr_.size(),
+                          Host::IDENT_HWADDR, SubnetID(1),
+                          SubnetID(0), IOAddress("10.0.0.9")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Let's transition the client to Renewing state.
+    client.setState(Dhcp4Client::RENEWING);
+
+    // Set the unicast destination address to indicate that it is a renewal.
+    client.setDestAddress(IOAddress("10.0.0.1"));
+    ASSERT_NO_THROW(client.doRequest());
+
+    // Client should get the DHCPNAK from the server because the client has
+    // a reservation for a different address that it is trying to renew.
+    resp = client.getContext().response_;
+    ASSERT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
+
+    // A conforming client would go back to the server discovery.
+    client.setState(Dhcp4Client::SELECTING);
+    // Obtain a lease from the server using the 4-way exchange.
+    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 with a reserved
+    // address
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    ASSERT_EQ("10.0.0.9", client.config_.lease_.addr_.toText());
+
+    // Client A renews the allocated address.
+    client.setState(Dhcp4Client::RENEWING);
+    // Set the unicast destination address to indicate that it is a renewal.
+    client.setDestAddress(IOAddress("10.0.0.1"));
+    ASSERT_NO_THROW(client.doRequest());
+    // Make sure the server responded and renewed the client's address.
+    resp = client.getContext().response_;
+    ASSERT_EQ("10.0.0.9", client.config_.lease_.addr_.toText());
+
+    // By reconfiguring the server, we remove the existing reservations.
+    configure(DORA_CONFIGS[0]);
+
+    // Try to renew the existing lease again.
+    ASSERT_NO_THROW(client.doRequest());
+    // The reservation has been removed, so the server should respond with
+    // a DHCPNAK because the address that the client is using doesn't belong
+    // to a dynamic pool.
+    resp = client.getContext().response_;
+    ASSERT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
+
+    // A conforming client would go back to the server discovery.
+    client.setState(Dhcp4Client::SELECTING);
+    // Obtain a lease from the server using the 4-way exchange.
+    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()));
+    // Obtain the subnet to which the returned address belongs.
+    Subnet4Ptr subnet = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->
+        selectSubnet(client.config_.lease_.addr_);
+    ASSERT_TRUE(subnet);
+    // Make sure that the address has been allocated from the dynamic pool.
+    ASSERT_TRUE(subnet->inPool(Lease::TYPE_V4, client.config_.lease_.addr_));
+
+    // Remember the address allocated in the dynamic pool.
+    IOAddress in_pool_addr = client.config_.lease_.addr_;
+
+    // Create Client B.
+    Dhcp4Client clientB(client.getServer());
+    clientB.modifyHWAddr();
+
+    // Create reservation for the Client B, for the address that the
+    // Client A is using.
+    configure(DORA_CONFIGS[0], false);
+    host.reset(new Host(&clientB.getHWAddress()->hwaddr_[0],
+                        clientB.getHWAddress()->hwaddr_.size(),
+                        Host::IDENT_HWADDR, SubnetID(1),
+                        SubnetID(0), in_pool_addr));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Client B performs a DHCPDISCOVER.
+    clientB.setState(Dhcp4Client::SELECTING);
+    // The server determines that the address reserved for Client B is
+    // in use by Client A so it drops the message from the Client B.
+    ASSERT_NO_THROW(clientB.doDiscover(boost::shared_ptr<
+                                       IOAddress>(new IOAddress("0.0.0.0"))));
+    ASSERT_FALSE(clientB.getContext().response_);
+
+    // Client A renews the lease.
+    client.setState(Dhcp4Client::RENEWING);
+    // Set the unicast destination address to indicate that it is a renewal.
+    client.setDestAddress(IOAddress(in_pool_addr));
+    ASSERT_NO_THROW(client.doRequest());
+    // Client A should get a DHCPNAK because it is using an address reserved
+    // for Client B.
+    resp = client.getContext().response_;
+    ASSERT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
+
+    // Client B performs 4-way exchange but still doesn't get an address
+    // because Client A hasn't obtained a new lease, so it is still using
+    // an address reserved for Client B.
+    clientB.setState(Dhcp4Client::SELECTING);
+    // Obtain a lease from the server using the 4-way exchange.
+    ASSERT_NO_THROW(clientB.doDiscover(boost::shared_ptr<
+                                       IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_FALSE(clientB.getContext().response_);
+
+    // Client A performs 4-way exchange.
+    client.setState(Dhcp4Client::SELECTING);
+    // Obtain a lease from the server using the 4-way exchange.
+    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_;
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // The server should have assigned a different address than the one
+    // reserved for the Client B.
+    ASSERT_NE(client.config_.lease_.addr_.toText(), in_pool_addr.toText());
+    subnet = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->
+        selectSubnet(client.config_.lease_.addr_);
+    ASSERT_TRUE(subnet);
+    ASSERT_TRUE(subnet->inPool(Lease::TYPE_V4, client.config_.lease_.addr_));
+
+    // Client B performs 4-way exchange and obtains a lease for the
+    // reserved address.
+    clientB.setState(Dhcp4Client::SELECTING);
+    ASSERT_NO_THROW(clientB.doDORA(boost::shared_ptr<
+                                  IOAddress>(new IOAddress("0.0.0.0"))));
+    // Make sure that the server responded.
+    ASSERT_TRUE(clientB.getContext().response_);
+    resp = clientB.getContext().response_;
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    ASSERT_EQ(in_pool_addr, clientB.config_.lease_.addr_);
+}
+
 } // end of anonymous namespace

+ 364 - 142
src/lib/dhcpsrv/alloc_engine.cc

@@ -14,6 +14,7 @@
 
 #include <dhcpsrv/alloc_engine.h>
 #include <dhcpsrv/dhcpsrv_log.h>
+#include <dhcpsrv/host_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 
 #include <hooks/server_hooks.h>
@@ -493,113 +494,196 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid
 
     try {
 
+        // Set allocator.
         AllocatorPtr allocator = getAllocator(Lease::TYPE_V4);
 
-        // Allocator is always created in AllocEngine constructor and there is
-        // currently no other way to set it, so that check is not really necessary.
-        if (!allocator) {
-            isc_throw(InvalidOperation, "No allocator selected");
-        }
-
         if (!subnet) {
-            isc_throw(InvalidOperation, "Can't allocate IPv4 address without subnet");
+            isc_throw(BadValue, "Can't allocate IPv4 address without subnet");
         }
 
         if (!hwaddr) {
-            isc_throw(InvalidOperation, "HWAddr must be defined");
+            isc_throw(BadValue, "HWAddr must be defined");
         }
 
-        // Check if there's existing lease for that subnet/clientid/hwaddr combination.
-        Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(*hwaddr, subnet->getID());
-        if (existing) {
-            // Save the old lease, before renewal.
-            old_lease.reset(new Lease4(*existing));
-            // We have a lease already. This is a returning client, probably after
-            // its reboot.
-            existing = renewLease4(subnet, clientid, hwaddr,
-                                   fwd_dns_update, rev_dns_update, hostname,
-                                   existing, callout_handle, fake_allocation);
-            if (existing) {
-                return (existing);
+        /// @todo The context for lease allocation should really be created
+        /// by the DHCPv4 server and passed to this function. The reason for
+        /// this is that the server should retrieve the Host object for the
+        /// client because the Host object contains the data not only useful
+        /// for the address allocation but also hostname and DHCP options
+        /// for the client. The Host object should be passed in the context.
+        /// Making this change would require a change to the allocateLease4
+        /// API which would in turn require lots of changes in unit tests.
+        /// The ticket introducing a context and host reservation in the
+        /// allocation engine is complex enough by itself to warrant that
+        /// the API change is done with a separate ticket.
+        ClientContext4 ctx;
+        ctx.subnet_ = subnet;
+        ctx.clientid_ = clientid;
+        ctx.hwaddr_ = hwaddr;
+        ctx.requested_address_ = hint;
+        ctx.fwd_dns_update_ = fwd_dns_update;
+        ctx.rev_dns_update_ = rev_dns_update;
+        ctx.hostname_ = hostname;
+        ctx.fake_allocation_ = fake_allocation;
+        ctx.callout_handle_ = callout_handle;
+        ctx.old_lease_ = old_lease;
+        ctx.host_ = HostMgr::instance().get4(subnet->getID(), hwaddr);
+
+        // If there is a reservation for this client we want to allocate the
+        // reserved address to the client, rather than any other address.
+        if (ctx.host_) {
+            // In some cases the client doesn't supply any address, e.g. when
+            // it sends a DHCPDISCOVER. In such cases, we use the reserved
+            // address as a hint.
+            if (ctx.requested_address_ == IOAddress("0.0.0.0")) {
+                ctx.requested_address_ = ctx.host_->getIPv4Reservation();
+
+            // If the client supplied an address we have to check if this
+            // address is reserved for this client. If not, we will send
+            // DHCPNAK to inform the client that we were not able to
+            // assign a requested address. The fake allocation (DHCPDISCOVER
+            // case) is an exception. In such case we treat the address
+            // supplied by the client as a hint, but we may offer address
+            // other than desired by the client. So, we don't return an
+            // empty lease.
+            } else if (!ctx.fake_allocation_ &&
+                       (ctx.requested_address_ != ctx.host_->getIPv4Reservation())) {
+                return (Lease4Ptr());
             }
+        }
 
-            // If renewal failed (e.g. the lease no longer matches current configuration)
-            // let's continue the allocation process
+        // Check if the client has any leases in the lease database, using HW
+        // address or client identifier.
+        LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
+        Lease4Ptr existing = lease_mgr.getLease4(*hwaddr, ctx.subnet_->getID());
+        if (!existing && clientid) {
+            existing = lease_mgr.getLease4(*clientid, ctx.subnet_->getID());
         }
 
-        if (clientid) {
-            existing = LeaseMgrFactory::instance().getLease4(*clientid, subnet->getID());
-            if (existing) {
-                // Save the old lease before renewal.
-                old_lease.reset(new Lease4(*existing));
-                // we have a lease already. This is a returning client, probably after
-                // its reboot.
-                existing = renewLease4(subnet, clientid, hwaddr,
-                                       fwd_dns_update, rev_dns_update,
-                                       hostname, existing, callout_handle,
-                                       fake_allocation);
-                // @todo: produce a warning. We haven't found him using MAC address, but
-                // we found him using client-id
-                if (existing) {
-                    return (existing);
-                }
+        // If client has a lease there are two choices. The server may need
+        // to renew (no address change) the lease. Or, the server may need
+        // to replace the lease with a different one. This is the case when
+        // the server has a dynamically assigned lease but an administrator
+        // has made a new reservation for the client for a different address.
+        if (existing) {
+            existing = reallocateClientLease(existing, ctx);
+            // The interrupt_processing_ flag indicates that the lease
+            // reallocation was not successful and that the allocation
+            // engine should cease allocation process for this client.
+            // This may occur when the client is trying to renew the
+            // lease which is reserved for someone else. The server should
+            // send DHCPNAK to indicate that the client should try to
+            // start over the allocation process.
+            if (ctx.interrupt_processing_) {
+                old_lease = ctx.old_lease_;
+                return (Lease4Ptr());
+
+            // If we tried to reallocate the reserved lease we return
+            // at this point regardless if reallocation failed or passed.
+            // We also return when allocation passed, no matter if this
+            // was a reserved address or not.
+            } else  if (ctx.host_ || existing) {
+                old_lease = ctx.old_lease_;
+                return (existing);
             }
         }
 
-        // check if the hint is in pool and is available
-        if (subnet->inPool(Lease::TYPE_V4, hint)) {
-            existing = LeaseMgrFactory::instance().getLease4(hint);
-            if (!existing) {
-                /// @todo: Check if the hint is reserved once we have host support
-                /// implemented
+        // There is no lease in the database for this client, so we will
+        // proceed with a new allocation. We will try to allocate a
+        // reserved address or an address from a dynamic pool if there is
+        // no reservation.
+        if (ctx.host_ || subnet->inPool(Lease::TYPE_V4, ctx.requested_address_)) {
+            // If a client is requesting specific IP address, but the
+            // reservation was made for a different address the server returns
+            // NAK to the client. By returning NULL lease here we indicate to
+            // the server that we're not able to fulfil client's request for the
+            // particular IP address. We don't return NULL lease in case of the
+            // fake allocation (DHCPDISCOVER) because in this case the address
+            // supplied by the client is only a hint.
+            if (!ctx.fake_allocation_ && ctx.host_ &&
+                (ctx.requested_address_ != IOAddress("0.0.0.0")) &&
+                (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) {
+                return (Lease4Ptr());
+            }
 
-                // The hint is valid and not currently used, let's create a lease for it
-                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr, hint,
-                                               fwd_dns_update, rev_dns_update,
+            // The reserved address always takes precedence over an address
+            // supplied by the client (renewed address or requested).
+            const IOAddress& candidate = ctx.host_ ?
+                ctx.host_->getIPv4Reservation() : ctx.requested_address_;
+
+            // Once we picked an address we want to allocate, we have to check
+            // if this address is available.
+            existing = LeaseMgrFactory::instance().getLease4(candidate);
+            if (!existing) {
+                // The candidate address is currently unused. Let's create a
+                // lease for it.
+                Lease4Ptr lease = createLease4(subnet, clientid, hwaddr,
+                                               candidate, fwd_dns_update,
+                                               rev_dns_update,
                                                hostname, callout_handle,
                                                fake_allocation);
 
-                // It can happen that the lease allocation failed (we could have lost
-                // the race condition. That means that the hint is lo longer usable and
-                // we need to continue the regular allocation path.
-                if (lease) {
+                // If we have allocated the lease let's return it. Also,
+                // always return when tried to allocate reserved address,
+                // regardless if allocation was successful or not. If it
+                // was not successful, we will return a NULL pointer which
+                // indicates to the server that it should send NAK to the
+                // client.
+                if (lease || ctx.host_) {
                     return (lease);
                 }
+
+            // There is a lease for this address in the lease database but
+            // it is possible that the lease has expired, in which case
+            // we will be able to reuse it.
             } else {
                 if (existing->expired()) {
                     // Save the old lease, before reusing it.
                     old_lease.reset(new Lease4(*existing));
-                    return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
-                                              fwd_dns_update, rev_dns_update,
-                                              hostname, callout_handle,
-                                              fake_allocation));
+                    return (reuseExpiredLease(existing, ctx));
+
+                // The existing lease is not expired (is in use by some
+                // other client). If we are trying to get this lease because
+                // the address has been reserved for the client we have no
+                // choice but to return a NULL lease to indicate that the
+                // allocation has failed.
+                } else if (ctx.host_) {
+                    return (Lease4Ptr());
+
                 }
 
             }
         }
 
-        // Hint is in the pool but is not available. Search the pool until first of
-        // the following occurs:
+        // No address was requested, requested address was not in pool or the
+        // allocation was not successful so far. Let's try to find a different
+        // address for the client.  Search the pool until first of the following
+        // occurs:
         // - we find a free address
         // - we find an address for which the lease has expired
         // - we exhaust the number of tries
         //
-        // @todo: Current code does not handle pool exhaustion well. It will be
-        // improved. Current problems:
-        // 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
-        // 10 addresses), we will iterate over it 100 times before giving up
-        // 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
-        // 3. the whole concept of infinite attempts is just asking for infinite loop
-        // We may consider some form or reference counting (this pool has X addresses
-        // left), but this has one major problem. We exactly control allocation
-        // moment, but we currently do not control expiration time at all
-
+        /// @todo: Current code does not handle pool exhaustion well. It will be
+        /// improved. Current problems:
+        /// 1. with attempts set to too large value (e.g. 1000) and a small pool (e.g.
+        /// 10 addresses), we will iterate over it 100 times before giving up
+        /// 2. attempts 0 mean unlimited (this is really UINT_MAX, not infinite)
+        /// 3. the whole concept of infinite attempts is just asking for infinite loop
+        /// We may consider some form or reference counting (this pool has X addresses
+        /// left), but this has one major problem. We exactly control allocation
+        /// moment, but we currently do not control expiration time at all
         unsigned int i = attempts_;
         do {
-            IOAddress candidate = allocator->pickAddress(subnet, clientid, hint);
-
-            /// @todo: check if the address is reserved once we have host support
-            /// implemented
+            IOAddress candidate = allocator->pickAddress(subnet, clientid,
+                                                         ctx.requested_address_);
+
+            // Check if this address is reserved. There is no need to check for
+            // whom it is reserved, because if it has been reserved for us we would
+            // have already allocated a lease.
+            if (HostMgr::instance().get4(subnet->getID(), candidate)) {
+                // Don't allocate a reserved address.
+                continue;
+            }
 
             Lease4Ptr existing = LeaseMgrFactory::instance().getLease4(candidate);
             if (!existing) {
@@ -620,10 +704,7 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid
                 if (existing->expired()) {
                     // Save old lease before reusing it.
                     old_lease.reset(new Lease4(*existing));
-                    return (reuseExpiredLease(existing, subnet, clientid, hwaddr,
-                                              fwd_dns_update, rev_dns_update,
-                                              hostname, callout_handle,
-                                              fake_allocation));
+                    return (reuseExpiredLease(existing, ctx));
                 }
             }
 
@@ -643,18 +724,30 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid
     return (Lease4Ptr());
 }
 
-Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
-                                   const ClientIdPtr& clientid,
-                                   const HWAddrPtr& hwaddr,
-                                   const bool fwd_dns_update,
-                                   const bool rev_dns_update,
-                                   const std::string& hostname,
-                                   const Lease4Ptr& lease,
-                                   const isc::hooks::CalloutHandlePtr& callout_handle,
-                                   bool fake_allocation /* = false */) {
-
+Lease4Ptr
+AllocEngine::renewLease4(const Lease4Ptr& lease,
+                         AllocEngine::ClientContext4& ctx) {
     if (!lease) {
-        isc_throw(InvalidOperation, "Lease4 must be specified");
+        isc_throw(BadValue, "null lease specified for renewLease4");
+    }
+
+    // The ctx.host_ possibly contains a reservation for the client for which
+    // we are renewing a lease. If this reservation exists, we assume that
+    // there is no conflict in assigning the address to this client. Note
+    // that the reallocateClientLease checks if the address reserved for
+    // the client matches the address in the lease we're renewing here.
+    if (!ctx.host_) {
+        ConstHostPtr host = HostMgr::instance().get4(ctx.subnet_->getID(),
+                                                     lease->addr_);
+        // Do not renew the lease if:
+        // - If address is reserved for someone else or ...
+        // - renewed address doesn't belong to a pool.
+        if ((host && ctx.hwaddr_ && (*host->getHWAddress() != *ctx.hwaddr_)) ||
+            (!ctx.subnet_->inPool(Lease::TYPE_V4, lease->addr_))) {
+            ctx.interrupt_processing_ = !ctx.fake_allocation_;
+            return (Lease4Ptr());
+        }
+
     }
 
     // Let's keep the old data. This is essential if we are using memfile
@@ -663,51 +756,46 @@ Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
     /// @todo: remove this once #3083 is implemented
     Lease4 old_values = *lease;
 
-    lease->subnet_id_ = subnet->getID();
-    lease->hwaddr_ = hwaddr;
-    lease->client_id_ = clientid;
-    lease->cltt_ = time(NULL);
-    lease->t1_ = subnet->getT1();
-    lease->t2_ = subnet->getT2();
-    lease->valid_lft_ = subnet->getValid();
-    lease->fqdn_fwd_ = fwd_dns_update;
-    lease->fqdn_rev_ = rev_dns_update;
-    lease->hostname_ = hostname;
+    // Update the lease with the information from the context.
+    updateLease4Information(lease, ctx);
 
     bool skip = false;
-    // Execute all callouts registered for packet6_send
-    if (HooksManager::getHooksManager().calloutsPresent(Hooks.hook_index_lease4_renew_)) {
+    // Execute all callouts registered for lease4_renew.
+    if (HooksManager::getHooksManager().
+        calloutsPresent(Hooks.hook_index_lease4_renew_)) {
 
         // Delete all previous arguments
-        callout_handle->deleteAllArguments();
+        ctx.callout_handle_->deleteAllArguments();
 
         // Subnet from which we do the allocation. Convert the general subnet
         // pointer to a pointer to a Subnet4.  Note that because we are using
         // boost smart pointers here, we need to do the cast using the boost
         // version of dynamic_pointer_cast.
-        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(subnet);
+        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(ctx.subnet_);
 
         // Pass the parameters
-        callout_handle->setArgument("subnet4", subnet4);
-        callout_handle->setArgument("clientid", clientid);
-        callout_handle->setArgument("hwaddr", hwaddr);
+        ctx.callout_handle_->setArgument("subnet4", subnet4);
+        ctx.callout_handle_->setArgument("clientid", ctx.clientid_);
+        ctx.callout_handle_->setArgument("hwaddr", ctx.hwaddr_);
 
         // Pass the lease to be updated
-        callout_handle->setArgument("lease4", lease);
+        ctx.callout_handle_->setArgument("lease4", lease);
 
         // Call all installed callouts
-        HooksManager::callCallouts(Hooks.hook_index_lease4_renew_, *callout_handle);
+        HooksManager::callCallouts(Hooks.hook_index_lease4_renew_,
+                                   *ctx.callout_handle_);
 
         // Callouts decided to skip the next processing step. The next
         // processing step would to actually renew the lease, so skip at this
         // stage means "keep the old lease as it is".
-        if (callout_handle->getSkip()) {
+        if (ctx.callout_handle_->getSkip()) {
             skip = true;
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_RENEW_SKIP);
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
+                      DHCPSRV_HOOK_LEASE4_RENEW_SKIP);
         }
     }
 
-    if (!fake_allocation && !skip) {
+    if (!ctx.fake_allocation_ && !skip) {
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease4(lease);
     }
@@ -803,42 +891,29 @@ Lease6Ptr AllocEngine::reuseExpiredLease(Lease6Ptr& expired,
     return (expired);
 }
 
-Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
-                                         const SubnetPtr& subnet,
-                                         const ClientIdPtr& clientid,
-                                         const HWAddrPtr& hwaddr,
-                                         const bool fwd_dns_update,
-                                         const bool rev_dns_update,
-                                         const std::string& hostname,
-                                         const isc::hooks::CalloutHandlePtr& callout_handle,
-                                         bool fake_allocation /*= false */ ) {
+Lease4Ptr
+AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
+                               AllocEngine::ClientContext4& ctx) {
+    if (!expired) {
+        isc_throw(BadValue, "null lease specified for reuseExpiredLease");
+    }
 
-    if (!expired->expired()) {
-        isc_throw(BadValue, "Attempt to recycle lease that is still valid");
+    if (!ctx.subnet_) {
+        isc_throw(BadValue, "null subnet specified for the reuseExpiredLease");
     }
 
-    // address, lease type and prefixlen (0) stay the same
-    expired->client_id_ = clientid;
-    expired->hwaddr_ = hwaddr;
-    expired->valid_lft_ = subnet->getValid();
-    expired->t1_ = subnet->getT1();
-    expired->t2_ = subnet->getT2();
-    expired->cltt_ = time(NULL);
-    expired->subnet_id_ = subnet->getID();
+    updateLease4Information(expired, ctx);
     expired->fixed_ = false;
-    expired->hostname_ = hostname;
-    expired->fqdn_fwd_ = fwd_dns_update;
-    expired->fqdn_rev_ = rev_dns_update;
 
     /// @todo: log here that the lease was reused (there's ticket #2524 for
     /// logging in libdhcpsrv)
 
     // Let's execute all callouts registered for lease4_select
-    if (callout_handle &&
-        HooksManager::getHooksManager().calloutsPresent(hook_index_lease4_select_)) {
+    if (ctx.callout_handle_ &&  HooksManager::getHooksManager()
+        .calloutsPresent(hook_index_lease4_select_)) {
 
         // Delete all previous arguments
-        callout_handle->deleteAllArguments();
+        ctx.callout_handle_->deleteAllArguments();
 
         // Pass necessary arguments
 
@@ -846,32 +921,34 @@ Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
         // pointer to a pointer to a Subnet4.  Note that because we are using
         // boost smart pointers here, we need to do the cast using the boost
         // version of dynamic_pointer_cast.
-        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(subnet);
-        callout_handle->setArgument("subnet4", subnet4);
+        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(ctx.subnet_);
+        ctx.callout_handle_->setArgument("subnet4", subnet4);
 
         // Is this solicit (fake = true) or request (fake = false)
-        callout_handle->setArgument("fake_allocation", fake_allocation);
+        ctx.callout_handle_->setArgument("fake_allocation",
+                                         ctx.fake_allocation_);
 
         // The lease that will be assigned to a client
-        callout_handle->setArgument("lease4", expired);
+        ctx.callout_handle_->setArgument("lease4", expired);
 
         // Call the callouts
-        HooksManager::callCallouts(hook_index_lease6_select_, *callout_handle);
+        HooksManager::callCallouts(hook_index_lease4_select_, *ctx.callout_handle_);
 
         // Callouts decided to skip the action. This means that the lease is not
         // assigned, so the client will get NoAddrAvail as a result. The lease
         // won't be inserted into the database.
-        if (callout_handle->getSkip()) {
-            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS, DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
+        if (ctx.callout_handle_->getSkip()) {
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
+                      DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
             return (Lease4Ptr());
         }
 
         // Let's use whatever callout returned. Hopefully it is the same lease
         // we handled to it.
-        callout_handle->getArgument("lease4", expired);
+        ctx.callout_handle_->getArgument("lease4", expired);
     }
 
-    if (!fake_allocation) {
+    if (!ctx.fake_allocation_) {
         // for REQUEST we do update the lease
         LeaseMgrFactory::instance().updateLease4(expired);
     }
@@ -884,6 +961,126 @@ Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
     return (expired);
 }
 
+Lease4Ptr
+AllocEngine::replaceClientLease(Lease4Ptr& lease, ClientContext4& ctx) {
+
+    if (!lease) {
+        isc_throw(BadValue, "null lease specified for replaceClientLease");
+    }
+
+    if (!ctx.subnet_) {
+        isc_throw(BadValue, "null subnet specified for replaceClientLease");
+    }
+
+    if (ctx.requested_address_ == IOAddress("0.0.0.0")) {
+        isc_throw(BadValue, "zero address specified for the"
+                  " replaceClientLease");
+    }
+
+    // Remember the previous address for this lease.
+    IOAddress prev_address = lease->addr_;
+
+    if (!ctx.host_) {
+        ConstHostPtr host = HostMgr::instance().get4(ctx.subnet_->getID(),
+                                                     ctx.requested_address_);
+        // If there is a reservation for the new address and the reservation
+        // is made for another client, do not use this address.
+        if (host && ctx.hwaddr_ && (*host->getHWAddress() != *ctx.hwaddr_)) {
+            ctx.interrupt_processing_ = true;
+            return (Lease4Ptr());
+        }
+        lease->addr_ = ctx.requested_address_;
+    } else {
+        lease->addr_ = ctx.host_->getIPv4Reservation();
+    }
+
+    updateLease4Information(lease, ctx);
+
+    bool skip = false;
+    // Execute callouts registered for lease4_select.
+    if (ctx.callout_handle_ && HooksManager::getHooksManager()
+        .calloutsPresent(hook_index_lease4_select_)) {
+
+        // Delete all previous arguments.
+        ctx.callout_handle_->deleteAllArguments();
+
+        // Pass arguments.
+        Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(ctx.subnet_);
+        ctx.callout_handle_->setArgument("subnet4", subnet4);
+
+        ctx.callout_handle_->setArgument("fake_allocation",
+                                         ctx.fake_allocation_);
+
+        ctx.callout_handle_->setArgument("lease4", lease);
+
+        HooksManager::callCallouts(hook_index_lease4_select_,
+                                   *ctx.callout_handle_);
+
+        if (ctx.callout_handle_->getSkip()) {
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
+                      DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
+            return (Lease4Ptr());
+        }
+
+        // Let's use whatever callout returned.
+        ctx.callout_handle_->getArgument("lease4", lease);
+
+        // Callouts decided to skip the next processing step. The next
+        // processing step would to actually renew the lease, so skip at this
+        // stage means "keep the old lease as it is".
+        if (ctx.callout_handle_->getSkip()) {
+            skip = true;
+            LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_HOOKS,
+                      DHCPSRV_HOOK_LEASE4_SELECT_SKIP);
+        }
+    }
+
+    /// @todo There should be a callout for a deletion of an old lease.
+    /// The lease4_release callout is in appropriate, because by definition
+    /// it is invoked when DHCPRELEASE packet is received.
+
+    if (!ctx.fake_allocation_ && !skip) {
+        // We can't use LeaseMgr::updateLease because it identifies the
+        // lease by an IP address. Instead, we have to delete an old
+        // lease and add a new one.
+        LeaseMgrFactory::instance().deleteLease(prev_address);
+        LeaseMgrFactory::instance().addLease(lease);
+    }
+
+    return (lease);
+}
+
+Lease4Ptr
+AllocEngine::reallocateClientLease(Lease4Ptr& lease,
+                                   AllocEngine::ClientContext4& ctx) {
+    // Save the old lease, before renewal.
+    ctx.old_lease_.reset(new Lease4(*lease));
+
+    /// The client's address will need to be modified in case if:
+    /// - There is a reservation for the client (likely new one) and
+    ///   the currently used address is different.
+    /// - Client requested some IP address and the requested address
+    ///   is different than the currently used one. Note that if this
+    ///   is a DHCPDISCOVER the requested IP address is ignored when
+    ///   it doesn't match the one in use.
+    if ((ctx.host_ && (ctx.host_->getIPv4Reservation() != lease->addr_)) ||
+        (!ctx.fake_allocation_ &&
+         (ctx.requested_address_ != IOAddress("0.0.0.0")) &&
+         (lease->addr_ != ctx.requested_address_))) {
+        lease = replaceClientLease(lease, ctx);
+        return (lease);
+
+    } else {
+        lease = renewLease4(lease, ctx);
+        if (lease) {
+            return (lease);
+        }
+    }
+
+    return (Lease4Ptr());
+}
+
+
 Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
                                     const DuidPtr& duid,
                                     const uint32_t iaid,
@@ -1064,6 +1261,31 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet,
     }
 }
 
+void
+AllocEngine::updateLease4Information(const Lease4Ptr& lease,
+                                     AllocEngine::ClientContext4& ctx) const {
+    // This should not happen in theory.
+    if (!lease) {
+        isc_throw(BadValue, "null lease specified for updateLease4Information");
+    }
+
+    if (!ctx.subnet_) {
+        isc_throw(BadValue, "null subnet specified for"
+                  " updateLease4Information");
+    }
+
+    lease->subnet_id_ = ctx.subnet_->getID();
+    lease->hwaddr_ = ctx.hwaddr_;
+    lease->client_id_ = ctx.clientid_;
+    lease->cltt_ = time(NULL);
+    lease->t1_ = ctx.subnet_->getT1();
+    lease->t2_ = ctx.subnet_->getT2();
+    lease->valid_lft_ = ctx.subnet_->getValid();
+    lease->fqdn_fwd_ = ctx.fwd_dns_update_;
+    lease->fqdn_rev_ = ctx.rev_dns_update_;
+    lease->hostname_ = ctx.hostname_;
+}
+
 Lease6Collection
 AllocEngine::updateFqdnData(const Lease6Collection& leases,
                             const bool fwd_dns_update,

+ 230 - 70
src/lib/dhcpsrv/alloc_engine.h

@@ -18,6 +18,7 @@
 #include <asiolink/io_address.h>
 #include <dhcp/duid.h>
 #include <dhcp/hwaddr.h>
+#include <dhcpsrv/host.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <hooks/callout_handle.h>
@@ -214,6 +215,91 @@ protected:
         ALLOC_RANDOM     // random - an address is randomly selected
     } AllocType;
 
+    /// @brief Context information for the DHCPv4 lease allocation.
+    ///
+    /// This structure holds a set of information provided by the DHCPv4
+    /// server to the allocation engine. In particular, it holds the
+    /// client identifying information, such as HW address or client
+    /// identifier. It also holds the information about the subnet that
+    /// the client is connected to.
+    ///
+    /// This structure is also used to pass  some information from
+    /// the allocation engine back to the server, i.e. the old lease
+    /// which the client had before the allocation.
+    ///
+    /// This structure is meant to be extended in the future, if more
+    /// information should be passed to the allocation engine. Note
+    /// that the big advantage of using the context structure to pass
+    /// information to the allocation engine methods is that adding
+    /// new information doesn't modify the API of the allocation engine.
+    struct ClientContext4 {
+        /// @brief Subnet selected for the client by the server.
+        SubnetPtr subnet_;
+
+        /// @brief Client identifier from the DHCP message.
+        ClientIdPtr clientid_;
+
+        /// @brief HW address from the DHCP message.
+        HWAddrPtr hwaddr_;
+
+        /// @brief An address that the client desires.
+        ///
+        /// If this address is set to 0 it indicates that this address
+        /// is unspecified.
+        asiolink::IOAddress requested_address_;
+
+        /// @brief Perform forward DNS update.
+        bool fwd_dns_update_;
+
+        /// @brief Perform reverse DNS update.
+        bool rev_dns_update_;
+
+        /// @brief Hostname.
+        ///
+        /// The server retrieves the hostname from the Client FQDN option,
+        /// Hostname option or the host reservation record for the client.
+        std::string hostname_;
+
+        /// @brief Callout handle associated with the client's message.
+        hooks::CalloutHandlePtr callout_handle_;
+
+        /// @brief Indicates if this is a real or fake allocation.
+        ///
+        /// The real allocation is when the allocation engine is supposed
+        /// to make an update in a lease database: create new lease, or
+        /// update existing lease.
+        bool fake_allocation_;
+
+        /// @brief A pointer to an old lease that the client had before update.
+        Lease4Ptr old_lease_;
+
+        /// @brief A pointer to the object identifying host reservations.
+        ConstHostPtr host_;
+
+        /// @brief Signals that the allocation should be interrupted.
+        ///
+        /// This flag is set by the downstream methods called by the
+        /// @c AllocEngine::allocateLease4. This flag is set to true to
+        /// indicate that an attempt to allocate a lease should be
+        /// interrupted.
+        ///
+        /// One possible use case is when the allocation engine tries
+        /// to renew the client's lease and the leased address appears
+        /// to be reserved for someone else. In such case, the allocation
+        /// engine should signal to the server that the address that the
+        /// client should stop using this address. The
+        /// @c AllocEngine::renewLease4 sets this flag so as the
+        /// upstream methods return the NULL lease pointer to the server.
+        bool interrupt_processing_;
+
+        /// @brief Default constructor.
+        ClientContext4()
+            : subnet_(), clientid_(), hwaddr_(), requested_address_("0.0.0.0"),
+              fwd_dns_update_(false), rev_dns_update_(false),
+              hostname_(""), callout_handle_(), fake_allocation_(false),
+              old_lease_(), host_(), interrupt_processing_(false) {
+        }
+    };
 
     /// @brief Default constructor.
     ///
@@ -230,18 +316,66 @@ protected:
 
     /// @brief Returns IPv4 lease.
     ///
-    /// This method finds the appropriate lease for the client using the
-    /// following algorithm:
-    /// - If lease exists for the combination of the HW address, client id and
-    /// subnet, try to renew a lease and return it.
-    /// - If lease exists for the combination of the client id and subnet, try
-    /// to renew the lease and return it.
-    /// - If client supplied an address hint and this address is available,
-    /// allocate the new lease with this address.
-    /// - If client supplied an address hint and the lease for this address
-    /// exists in the database, return this lease if it is expired.
-    /// - Pick new address from the pool and try to allocate it for the client,
-    /// if expired lease exists for the picked address, try to reuse this lease.
+    /// This method finds a lease for a client using the following algorithm:
+    /// - If a lease exists for the combination of the HW address or client id
+    ///   and a subnet, try to use this lease for the client. If the client
+    ///   has a reservation for an address for which the lease was created or
+    ///   the client desires to renew the lease for this address (ciaddr or
+    ///   requested IP address option), the server renews the lease for the
+    ///   client. If the client desires a different address or the server has
+    ///   a (potentially new) reservation for a different address for this
+    ///   client, the existing lease is replaced with a new lease.
+    /// - If the client has no lease in the lease database the server will try
+    ///   to allocate a new lease. If the client has a reservation for the
+    ///   particular address or if it has specified a desired address the
+    ///   server will check if the particular address is not allocated to
+    ///   other client. If the address is available, the server will allocate
+    ///   this address for the client.
+    /// - If the desired address is unavailable the server checks if the
+    ///   lease for this address has expired. If the lease is expired, the
+    ///   server will allocate this lease to the client. The relevant
+    ///   information will be updated, e.g. new client HW address, host name
+    ///   etc.
+    /// - If the desired address is in use by other client, the server will try
+    ///   to allocate a different address. The server picks addresses from
+    ///   a dynamic pool and checks if the address is available and that
+    ///   it is not reserved for another client. If it is in use by another
+    ///   client or if it is reserved for another client, this address is not
+    ///   allocated. The server picks next address and repeats this check.
+    ///   Note that the server ceases allocation after configured number
+    ///   of unsuccessful attempts.
+    ///
+    /// The lease allocation process is slightly different for the
+    /// DHCPDISCOVER and DHCPREQUEST messages. In the former case, the client
+    /// may specify the requested IP address option with a desired address and
+    /// the server treats this address as hint. This means that the server may
+    /// allocate a different address on its discretion and send it to the
+    /// client in the DHCPOFFER. If the client accepts this offer it specifies
+    /// this address in the requested IP address option in the DHCPREQUEST.
+    /// At this point, the allocation engine will use the request IP address
+    /// as a hard requirement and if this address can't be allocated for
+    /// any reason, the allocation engine returns NULL lease. As a result,
+    /// the DHCP server sends a DHCPNAK to the client and the client
+    /// falls back to the DHCP server discovery.
+    ///
+    /// The only exception from this rule is when the client doesn't specify
+    /// a requested IP address option (invalid behavior) in which case the
+    /// allocation engine will try to allocate any address.
+    ///
+    /// If there is an address reservation specified for the particular client
+    /// the reserved address always takes precedence over addresses from the
+    /// dynamic pool or even an address currently allocated for this client.
+    ///
+    /// It is possible that the address reserved for the particular client
+    /// is in use by other client, e.g. as a result of pools reconfigruation.
+    /// In this case, when the client requests allocation of the reserved
+    /// address and the server determines that it is leased to someone else,
+    /// the allocation engine doesn't allocate a lease for the client having
+    /// a reservation. When the client having a lease returns to renew, the
+    /// allocation engine doesn't extend the lease for it and returns a NULL
+    /// pointer. The client falls back to the 4-way exchange and a different
+    /// lease is allocated. At this point, the reserved address is freed and
+    /// can be allocated to the client which holds this reservation.
     ///
     /// When a server should do DNS updates, it is required that allocation
     /// returns the information how the lease was obtained by the allocation
@@ -256,6 +390,9 @@ protected:
     /// returned, it is an indication that allocation engine reused/renewed an
     /// existing lease.
     ///
+    /// @todo Replace parameters with a single parameter of a
+    /// @c ClientContext4 type.
+    ///
     /// @param subnet subnet the allocation should come from
     /// @param clientid Client identifier
     /// @param hwaddr Client's hardware address info
@@ -273,7 +410,7 @@ protected:
     ///        lease. The NULL pointer indicates that lease didn't exist prior
     ///        to calling this function (e.g. new lease has been allocated).
     ///
-    /// @return Allocated IPv4 lease (or NULL if allocation failed)
+    /// @return Allocated IPv4 lease (or NULL if allocation failed).
     Lease4Ptr
     allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid,
                    const HWAddrPtr& hwaddr,
@@ -283,38 +420,21 @@ protected:
                    const isc::hooks::CalloutHandlePtr& callout_handle,
                    Lease4Ptr& old_lease);
 
-    /// @brief Renews a IPv4 lease
+    /// @brief Renews an DHCPv4 lease.
     ///
-    /// Since both request and renew are implemented in DHCPv4 as the sending of
-    /// a REQUEST packet, it is difficult to easily distinguish between those
-    /// cases. Therefore renew for DHCPv4 is done in the allocation engine.
-    /// This method is also used when client crashed/rebooted and tries
-    /// to get a new lease. It thinks that it gets a new lease, but in fact
-    /// we are only renewing the still valid lease for that client.
+    /// This method updates the lease with the information from the provided
+    /// context and invokes the lease4_renew callout.
     ///
-    /// @param subnet A subnet the client is attached to
-    /// @param clientid Client identifier
-    /// @param hwaddr Client's hardware address
-    /// @param fwd_dns_update Indicates whether forward DNS update will be
-    ///        performed for the client (true) or not (false).
-    /// @param rev_dns_update Indicates whether reverse DNS update will be
-    ///        performed for the client (true) or not (false).
-    /// @param hostname A string carrying hostname to be used for DNS updates.
-    /// @param lease A lease to be renewed
-    /// @param callout_handle a callout handle (used in hooks). A lease callouts
-    ///        will be executed if this parameter is passed.
-    /// @param fake_allocation Is this real i.e. REQUEST (false) or just picking
-    ///        an address for DISCOVER that is not really allocated (true)
+    /// The address of the lease being renewed is NOT updated.
+    ///
+    /// @param lease A lease to be renewed.
+    /// @param ctx Message processing context. It holds various information
+    /// extracted from the client's message and required to allocate a lease.
+    ///
+    /// @return Returns renewed lease. Note that the lease is only updated when
+    /// it is an actual allocation (not processing DHCPDISCOVER message).
     Lease4Ptr
-    renewLease4(const SubnetPtr& subnet,
-                const ClientIdPtr& clientid,
-                const HWAddrPtr& hwaddr,
-                const bool fwd_dns_update,
-                const bool rev_dns_update,
-                const std::string& hostname,
-                const Lease4Ptr& lease,
-                const isc::hooks::CalloutHandlePtr& callout_handle,
-                bool fake_allocation /* = false */);
+    renewLease4(const Lease4Ptr& lease, ClientContext4& ctx);
 
     /// @brief Allocates an IPv6 lease
     ///
@@ -396,6 +516,25 @@ private:
                            const isc::hooks::CalloutHandlePtr& callout_handle,
                            bool fake_allocation = false);
 
+    /// @brief Updates the specified lease with the information from a context.
+    ///
+    /// The context, specified as an argument to this method, holds various
+    /// information gathered from the client's message and passed to the
+    /// allocation engine. The allocation engine uses this information to make
+    /// lease allocation decisions. Some public methods of the allocation engine
+    /// requires updating the lease information with the data gathered from the
+    /// context, e.g. @c AllocEngine::reuseExpiredLease requires updating the
+    /// expired lease with a fresh information from the context to create a
+    /// lease to be held for the client.
+    ///
+    /// Note that this doesn't update the lease address.
+    ///
+    /// @param [out] lease A pointer to the lease to be updated.
+    /// @param ctx A context containing information from the server about the
+    /// client and its message.
+    void updateLease4Information(const Lease4Ptr& lease,
+                                 ClientContext4& ctx) const;
+
     /// @brief creates a lease and inserts it in LeaseMgr if necessary
     ///
     /// Creates a lease based on specified parameters and tries to insert it
@@ -433,36 +572,57 @@ private:
                            const isc::hooks::CalloutHandlePtr& callout_handle,
                            bool fake_allocation = false);
 
-    /// @brief Reuses expired IPv4 lease
+    /// @brief Reuses expired DHCPv4 lease.
     ///
-    /// Updates existing expired lease with new information. Lease database
-    /// is updated if this is real (i.e. REQUEST, fake_allocation = false), not
-    /// dummy allocation request (i.e. DISCOVER, fake_allocation = true).
+    /// Makes new allocation using an expired lease. The lease is updated with
+    /// the information from the provided context. Typically, an expired lease
+    /// lease which belonged to one client may be assigned to another client
+    /// which asked for the specific address.
     ///
-    /// @param expired Old, expired lease
-    /// @param subnet Subnet the lease is allocated from
-    /// @param clientid Client identifier
-    /// @param hwaddr Client's hardware address
-    /// @param fwd_dns_update Indicates whether forward DNS update will be
-    ///        performed for the client (true) or not (false).
-    /// @param rev_dns_update Indicates whether reverse DNS update will be
-    ///        performed for the client (true) or not (false).
-    /// @param hostname A string carrying hostname to be used for DNS updates.
-    /// @param callout_handle A callout handle (used in hooks). A lease callouts
-    ///        will be executed if this parameter is passed.
-    /// @param fake_allocation Is this real i.e. REQUEST (false) or just picking
-    ///        an address for DISCOVER that is not really allocated (true)
-    /// @return refreshed lease
-    /// @throw BadValue if trying to recycle lease that is still valid
-    Lease4Ptr reuseExpiredLease(Lease4Ptr& expired,
-                                const SubnetPtr& subnet,
-                                const ClientIdPtr& clientid,
-                                const HWAddrPtr& hwaddr,
-                                const bool fwd_dns_update,
-                                const bool rev_dns_update,
-                                const std::string& hostname,
-                                const isc::hooks::CalloutHandlePtr& callout_handle,
-                                bool fake_allocation = false);
+    /// @param expired An old, expired lease.
+    /// @param ctx Message processing context. It holds various information
+    /// extracted from the client's message and required to allocate a lease.
+    ///
+    /// @return Updated lease instance.
+    /// @throw BadValue if trying to reuse a lease which is still valid or
+    /// when the provided parameters are invalid.
+    Lease4Ptr reuseExpiredLease(Lease4Ptr& expired, ClientContext4& ctx);
+
+    /// @brief Updates the existing, non expired lease with a information from
+    /// the context.
+    ///
+    /// This method is invoked when the client requests allocation of the
+    /// (reserved) lease but there is a lease for this client with a different
+    /// address in the database already. In this case the existing lease must
+    /// be updated in the database with a new information. In particular,
+    /// with a new address.
+    ///
+    /// This method invokes the lease4_release and lease4_select callouts.
+    ///
+    /// @param lease A pointer to the lease to be updated.
+    /// @param ctx A context to be used to update the lease.
+    ///
+    /// @return Pointer to the updated lease.
+    /// @throw BadValue if the provided parameters are invalid.
+    Lease4Ptr replaceClientLease(Lease4Ptr& lease, ClientContext4& ctx);
+
+    /// @brief Replace or renew client's lease.
+    ///
+    /// This method is ivoked by the @c AllocEngine::allocateLease4 when it
+    /// finds that the lease for the particular client already exists in the
+    /// database. If the existing lease has the same IP address as the one
+    /// that the client should be allocated the existing lease is renewed.
+    /// If the client should be allocated a different address, e.g. there
+    /// is a static reservation for the client, the existing lease is replaced
+    /// with a new one. This method handles both cases.
+    ///
+    /// @param lease Existing lease.
+    /// @param ctx Context holding parameters to be used for the lease
+    /// allocation.
+    ///
+    /// @return Updated lease, or NULL if allocation was unsucessful.
+    /// @throw BadValue if specified parameters are invalid.
+    Lease4Ptr reallocateClientLease(Lease4Ptr& lease, ClientContext4& ctx);
 
     /// @brief Reuses expired IPv6 lease
     ///

+ 28 - 0
src/lib/dhcpsrv/base_host_data_source.h

@@ -31,6 +31,14 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+/// @brief Exception thrown when invalid IP address has been specified for
+/// @c Host.
+class BadHostAddress : public isc::BadValue {
+public:
+    BadHostAddress(const char* file, size_t line, const char* what) :
+        isc::BadValue(file, line, what) { };
+};
+
 /// @brief Base interface for the classes implementing simple data source
 /// for host reservations.
 ///
@@ -105,6 +113,26 @@ public:
     get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr,
          const DuidPtr& duid = DuidPtr()) const = 0;
 
+    /// @brief Returns a host connected to the IPv4 subnet and having
+    /// a reservation for a specified IPv4 address.
+    ///
+    /// One of the use cases for this method is to detect collisions between
+    /// dynamically allocated addresses and reserved addresses. When the new
+    /// address is assigned to a client, the allocation mechanism should check
+    /// if this address is not reserved for some other host and do not allocate
+    /// this address if reservation is present.
+    ///
+    /// Implementations of this method should guard against invalid addresses,
+    /// such as IPv6 address.
+    ///
+    /// @param subnet_id Subnet identifier.
+    /// @param address reserved IPv4 address.
+    ///
+    /// @return Const @c Host object using a specified IPv4 address.
+    virtual ConstHostPtr
+    get4(const SubnetID& subnet_id,
+         const asiolink::IOAddress& address) const = 0;
+
     /// @brief Returns a host connected to the IPv6 subnet.
     ///
     /// Implementations of this method should guard against the case when

+ 40 - 4
src/lib/dhcpsrv/cfg_hosts.cc

@@ -36,13 +36,17 @@ CfgHosts::getAll(const HWAddrPtr& hwaddr, const DuidPtr& duid) {
 }
 
 ConstHostCollection
-CfgHosts::getAll4(const IOAddress&) const {
-    isc_throw(isc::NotImplemented, "getAll4(address) const is not implemented");
+CfgHosts::getAll4(const IOAddress& address) const {
+    ConstHostCollection collection;
+    getAllInternal4<ConstHostCollection>(address, collection);
+    return (collection);
 }
 
 HostCollection
-CfgHosts::getAll4(const IOAddress&) {
-    isc_throw(isc::NotImplemented, "getAll4(address) is not implemented");
+CfgHosts::getAll4(const IOAddress& address) {
+    HostCollection collection;
+    getAllInternal4<HostCollection>(address, collection);
+    return (collection);
 }
 
 template<typename Storage>
@@ -76,6 +80,25 @@ CfgHosts::getAllInternal(const HWAddrPtr& hwaddr, const DuidPtr& duid,
     }
 }
 
+template<typename Storage>
+void
+CfgHosts::getAllInternal4(const IOAddress& address, Storage& storage) const {
+    // Must not specify address other than IPv4.
+    if (!address.isV4()) {
+        isc_throw(BadHostAddress, "must specify an IPv4 address when searching"
+                  " for a host, specified address was " << address);
+    }
+    // Search for the Host using the reserved IPv4 address as a key.
+    const HostContainerIndex1& idx = hosts_.get<1>();
+    HostContainerIndex1Range r = idx.equal_range(address);
+    // Append each Host object to the storage.
+    for (HostContainerIndex1::iterator host = r.first; host != r.second;
+         ++host) {
+        storage.push_back(*host);
+    }
+}
+
+
 ConstHostPtr
 CfgHosts::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr,
                const DuidPtr& duid) const {
@@ -91,6 +114,19 @@ CfgHosts::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr,
 }
 
 ConstHostPtr
+CfgHosts::get4(const SubnetID& subnet_id, const IOAddress& address) const {
+    ConstHostCollection hosts = getAll4(address);
+    for (ConstHostCollection::const_iterator host = hosts.begin();
+         host != hosts.end(); ++host) {
+        if ((*host)->getIPv4SubnetID() == subnet_id) {
+            return (*host);
+        }
+    }
+    return (ConstHostPtr());
+}
+
+
+ConstHostPtr
 CfgHosts::get6(const SubnetID& subnet_id, const DuidPtr& duid,
                const HWAddrPtr& hwaddr) const {
     // The true value indicates that it is an IPv6 subnet.

+ 27 - 2
src/lib/dhcpsrv/cfg_hosts.h

@@ -80,7 +80,7 @@ public:
     ///
     /// @param address IPv4 address for which the @c Host object is searched.
     ///
-    /// @throw isc::NotImplemented.
+    /// @return Collection of const @c Host objects.
     virtual ConstHostCollection
     getAll4(const asiolink::IOAddress& address) const;
 
@@ -91,7 +91,7 @@ public:
     ///
     /// @param address IPv4 address for which the @c Host object is searched.
     ///
-    /// @throw isc::NotImplemented
+    /// @return Collection of const @c Host objects.
     virtual HostCollection
     getAll4(const asiolink::IOAddress& address);
 
@@ -125,6 +125,16 @@ public:
     get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr,
          const DuidPtr& duid = DuidPtr());
 
+    /// @brief Returns a host connected to the IPv4 subnet and having
+    /// a reservation for a specified IPv4 address.
+    ///
+    /// @param subnet_id Subnet identifier.
+    /// @param address reserved IPv4 address.
+    ///
+    /// @return Const @c Host object using a specified IPv4 address.
+    virtual ConstHostPtr
+    get4(const SubnetID& subnet_id, const asiolink::IOAddress& address) const;
+
     /// @brief Returns a host connected to the IPv6 subnet and matching
     /// the specified identifiers.
     ///
@@ -216,6 +226,21 @@ private:
     void getAllInternal(const HWAddrPtr& hwaddr, const DuidPtr& duid,
                         Storage& storage) const;
 
+    /// @brief Returns @c Host objects for the specified IPv4 address.
+    ///
+    /// This private method is called by the @c CfgHosts::getAll4 methods
+    /// to retrieve the @c Host for which the specified IPv4 address is
+    /// reserved. The retrieved objects are appended to the @c storage
+    /// container.
+    ///
+    /// @param address IPv4 address.
+    /// @param [out] storage Container to which the retrieved objects are
+    /// appended.
+    /// @tparam One of the @c ConstHostCollection or @c HostCollection.
+    template<typename Storage>
+    void getAllInternal4(const asiolink::IOAddress& address,
+                         Storage& storage) const;
+
     /// @brief Returns @c Host object connected to a subnet.
     ///
     /// This private method returns a pointer to the @c Host object identified

+ 18 - 0
src/lib/dhcpsrv/host_container.h

@@ -64,6 +64,14 @@ typedef boost::multi_index_container<
                     &Host::getIdentifierType
                 >
             >
+        >,
+
+        // Second index is used to search for the host using reserved IPv4
+        // address.
+        boost::multi_index::ordered_non_unique<
+            // Index using values returned by the @c Host::getIPv4Resrvation.
+            boost::multi_index::const_mem_fun<Host, const asiolink::IOAddress&,
+                                               &Host::getIPv4Reservation>
         >
     >
 > HostContainer;
@@ -78,6 +86,16 @@ typedef HostContainer::nth_index<0>::type HostContainerIndex0;
 typedef std::pair<HostContainerIndex0::iterator,
                   HostContainerIndex0::iterator> HostContainerIndex0Range;
 
+/// @brief Second index type in the @c HostContainer.
+///
+/// This index allows for searching for @c Host objects using a
+/// reserved IPv4 address.
+typedef HostContainer::nth_index<1>::type HostContainerIndex1;
+
+/// @brief Results range returned using the @c HostContainerIndex1.
+typedef std::pair<HostContainerIndex1::iterator,
+                  HostContainerIndex1::iterator> HostContainerIndex1Range;
+
 }
 }
 

+ 11 - 0
src/lib/dhcpsrv/host_mgr.cc

@@ -89,6 +89,17 @@ HostMgr::get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr,
 }
 
 ConstHostPtr
+HostMgr::get4(const SubnetID& subnet_id,
+              const asiolink::IOAddress& address) const {
+    ConstHostPtr host = getCfgHosts()->get4(subnet_id, address);
+    if (!host && alternate_source) {
+        host = alternate_source->get4(subnet_id, address);
+    }
+    return (host);
+}
+
+
+ConstHostPtr
 HostMgr::get6(const SubnetID& subnet_id, const DuidPtr& duid,
                const HWAddrPtr& hwaddr) const {
     ConstHostPtr host = getCfgHosts()->get6(subnet_id, duid, hwaddr);

+ 20 - 0
src/lib/dhcpsrv/host_mgr.h

@@ -15,8 +15,14 @@
 #ifndef HOST_MGR_H
 #define HOST_MGR_H
 
+#include <dhcp/duid.h>
+#include <dhcp/hwaddr.h>
+#include <dhcpsrv/base_host_data_source.h>
+#include <dhcpsrv/host.h>
+#include <dhcpsrv/subnet_id.h>
 #include <boost/noncopyable.hpp>
 #include <boost/scoped_ptr.hpp>
+#include <string>
 
 namespace isc {
 namespace dhcp {
@@ -133,6 +139,20 @@ public:
     get4(const SubnetID& subnet_id, const HWAddrPtr& hwaddr,
          const DuidPtr& duid = DuidPtr()) const;
 
+    /// @brief Returns a host connected to the IPv4 subnet and having
+    /// a reservation for a specified IPv4 address.
+    ///
+    /// This method returns a single reservation for the particular host
+    /// (identified by the HW address or DUID) as documented in the
+    /// @c BaseHostDataSource::get4.
+    ///
+    /// @param subnet_id Subnet identifier.
+    /// @param address reserved IPv4 address.
+    ///
+    /// @return Const @c Host object using a specified IPv4 address.
+    virtual ConstHostPtr
+    get4(const SubnetID& subnet_id, const asiolink::IOAddress& address) const;
+
     /// @brief Returns a host connected to the IPv6 subnet.
     ///
     /// This method returns a host connected to the IPv6 subnet and identified

+ 35 - 0
src/lib/dhcpsrv/libdhcpsrv.dox

@@ -219,4 +219,39 @@ increaseAddress(addr) call is equivalent to increasePrefix(addr, 128)
 kept, because increaseAddress() is faster and this is a routine that may be
 called many hundred thousands times per second.
 
+@subsection allocEngineDHCPv4HostReservation Host Reservation support
+
+The Allocation Engine supports allocation of statically assigned addresses
+to the DHCPv4 clients, a.k.a. Host Reservation.
+
+When the server receives a DHCPDISCOVER or DHCPREQUEST from the client it
+calls \ref isc::dhcp::AllocEngine::allocateLease4 to obtain the suitable lease
+for the client. If the Allocation Engine determines that the particular client
+has a reservation it will try to allocate a reserved address for it. If the
+client requested allocation or renewal of a different address, the Allocation
+Engine will respond with a NULL lease to indicate that the address
+desired by the client could not be assigned. The DHCP server should send
+a DHCPNAK to the client and the client should fall back to the DHCP
+server discovery. When the client sends DHCPDISCOVER, the Allocation
+Engine offers the reserved address and the client should request the
+offered address in subsequent DHCPREQUEST messages.
+
+There are cases when the Allocation Engine is unable to assign the
+reserved address for the client. This includes the situations when
+the address had been previously reserved for another client or the
+address had been assigned out of the dynamic address pool. Such address
+may still remain in use of the client which obtained it first and the
+Allocation Engine must not assign it to the client for which it is
+reserved until the client using this address releases or the server
+assigns a different address for it.
+
+In order to resolve this conflict the Allocation Engine will refuse to
+renew the lease for the client using the addres not reserved for it.
+This client should fall back to the 4-way exchange and the Allocation
+Engine will assign a different address. As a result, the reserved
+address will be freed for the use of the client for which the reservation
+was made. The client will be offered/allocated a reserved address
+the next time it retries sending a DHCPDISCOVER/DHCPREQUEST message to
+the server.
+
 */

+ 700 - 11
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -19,6 +19,7 @@
 #include <dhcp/dhcp4.h>
 #include <dhcpsrv/alloc_engine.h>
 #include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/host_mgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/memfile_lease_mgr.h>
@@ -410,14 +411,25 @@ public:
     /// Sets clientid_, hwaddr_, subnet_, pool_ fields to example values
     /// used in many tests, initializes cfg_mgr configuration and creates
     /// lease database.
+    ///
+    /// It also re-initializes the Host Manager.
     AllocEngine4Test() {
+        // Create fresh instance of the HostMgr, and drop any previous HostMgr
+        // state.
+        HostMgr::instance().create();
+
         clientid_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x44)));
-        static uint8_t mac[] = { 0, 1, 22, 33, 44, 55};
+        uint8_t mac[] = { 0, 1, 22, 33, 44, 55};
 
         // Let's use odd hardware type to check if there is no Ethernet
         // hardcoded anywhere.
         hwaddr_ = HWAddrPtr(new HWAddr(mac, sizeof(mac), HTYPE_FDDI));
 
+        // Allocate different MAC address for the tests that require two
+        // different MAC addresses.
+        ++mac[sizeof(mac) - 1];
+        hwaddr2_ = HWAddrPtr(new HWAddr(mac, sizeof (mac), HTYPE_FDDI));
+
         // instantiate cfg_mgr
         CfgMgr& cfg_mgr = CfgMgr::instance();
 
@@ -429,6 +441,13 @@ public:
         cfg_mgr.commit();
 
         factory_.create("type=memfile universe=4 persist=false");
+
+        // Create a default context. Note that remaining parameters must be
+        // assigned when needed.
+        ctx_.subnet_ = subnet_;
+        ctx_.clientid_ = clientid_;
+        ctx_.hwaddr_ = hwaddr_;
+        ctx_.callout_handle_ = HooksManager::createCalloutHandle();
     }
 
     /// @brief checks if Lease4 matches expected configuration
@@ -461,12 +480,15 @@ public:
         factory_.destroy();
     }
 
-    ClientIdPtr clientid_;    ///< Client-identifier (value used in tests)
-    HWAddrPtr hwaddr_;        ///< Hardware address (value used in tests)
-    Subnet4Ptr subnet_;       ///< Subnet4 (used in tests)
-    Pool4Ptr pool_;           ///< Pool belonging to subnet_
-    LeaseMgrFactory factory_; ///< Pointer to LeaseMgr factory
-    Lease4Ptr old_lease_;     ///< Holds previous instance of the lease.
+    ClientIdPtr clientid_;      ///< Client-identifier (value used in tests)
+    HWAddrPtr hwaddr_;          ///< Hardware address (value used in tests)
+    HWAddrPtr hwaddr2_;         ///< Alternative hardware address.
+    Subnet4Ptr subnet_;         ///< Subnet4 (used in tests)
+    Pool4Ptr pool_;             ///< Pool belonging to subnet_
+    LeaseMgrFactory factory_;   ///< Pointer to LeaseMgr factory
+    Lease4Ptr old_lease_;       ///< Holds previous instance of the lease.
+    AllocEngine::ClientContext4 ctx_; ///< Context information passed to various
+                                     ///< allocation engine functions.
 };
 
 // This test checks if the v6 Allocation Engine can be instantiated, parses
@@ -1532,7 +1554,6 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
 // called
 TEST_F(AllocEngine4Test, renewLease4) {
     boost::scoped_ptr<AllocEngine> engine;
-    CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
 
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
                                                  100, false)));
@@ -1556,9 +1577,12 @@ TEST_F(AllocEngine4Test, renewLease4) {
     // Lease was assigned 45 seconds ago and is valid for 100 seconds. Let's
     // renew it.
     ASSERT_FALSE(lease->expired());
-    lease = engine->renewLease4(subnet_, clientid_, hwaddr_, true,
-                                true, "host.example.com.", lease,
-                                callout_handle, false);
+    ctx_.fwd_dns_update_ = true;
+    ctx_.rev_dns_update_ = true;
+    ctx_.hostname_ = "host.example.com.";
+    ctx_.fake_allocation_ = false;
+    lease = engine->renewLease4(lease, ctx_);
+
     // Check that he got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr, lease->addr_);
@@ -1574,6 +1598,671 @@ TEST_F(AllocEngine4Test, renewLease4) {
     detailCompareLease(lease, from_mgr);
 }
 
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has a reservation.
+// - Client sends DHCPREQUEST without requested IP Address, nor ciaddr.
+// - Client is allocated a reserved address.
+//
+// Note that client must normally include a requested IP address or ciaddr
+// in its message. But, we still want to provision clients that don't do that.
+// The server simply picks reserved address or any other available one if there
+// is no reservation.
+TEST_F(AllocEngine4Test, reservedAddressNoHint) {
+    // Create reservation for the client.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Try to allocate a lease without specifying a hint. This is actually
+    // incorrect behavior of the client to not send an address it wants to
+    // obtain but the server should handle this gracefully.
+    Lease4Ptr lease = engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                            IOAddress("0.0.0.0"),
+                                            false, false, "",
+                                            false, CalloutHandlePtr(),
+                                            old_lease_);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ("192.0.2.123", lease->addr_.toText());
+
+    // Make sure that the lease has been committed to the lease database.
+    // And that the committed lease is equal to the one returned.
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+    detailCompareLease(lease, from_mgr);
+
+    // Initially, there was no lease for this client, so the returned old
+    // lease should be NULL.
+    EXPECT_FALSE(old_lease_);
+}
+
+// This test checks behavior of the allocation engine in the following scenario:
+// - Client has no lease in the database.
+// - Client has a reservation.
+// - Client sends DHCPDISCOVER without requested IP Address.
+// - Server returns DHCPOFFER with the reserved address.
+TEST_F(AllocEngine4Test,reservedAddressNoHintFakeAllocation) {
+    // Create reservation for the client.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Query allocation engine for the lease to be assigned to this
+    // client without specifying the address to be assigned.
+    Lease4Ptr lease = engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                            IOAddress("0.0.0.0"),
+                                            false, false, "",
+                                            true, CalloutHandlePtr(),
+                                            old_lease_);
+    ASSERT_TRUE(lease);
+    // The allocation engine should return a reserved address.
+    EXPECT_EQ("192.0.2.123", lease->addr_.toText());
+
+    // This is a "fake" allocation so the returned lease should not be committed
+    // to the lease database.
+    EXPECT_FALSE(LeaseMgrFactory::instance().getLease4(lease->addr_));
+
+    // Client had no lease in the database, so the old lease returned should
+    // be NULL.
+    EXPECT_FALSE(old_lease_);
+}
+
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has a reservation.
+// - Client sends DHCPREQUEST with a requested IP address
+// - Server returns DHCPNAK when requested IP address is different than
+//   the reserved address. Note that the allocation engine returns NULL
+//   to indicate to the server that it should send DHCPNAK.
+// - Server allocates a reserved address to the client when the client requests
+// this address using requested IP address option.
+TEST_F(AllocEngine4Test, reservedAddressHint) {
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    Lease4Ptr lease = engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                            IOAddress("192.0.2.234"),
+                                            false, false, "",
+                                            false, CalloutHandlePtr(),
+                                            old_lease_);
+    // The client requested a different address than reserved, so
+    // the allocation engine should return NULL lease. When the server
+    // receives a NULL lease for the client, it will send a DHCPNAK.
+    ASSERT_FALSE(lease);
+    ASSERT_FALSE(old_lease_);
+
+    // Now, request a correct address. The client should obtain it.
+    lease = engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                  IOAddress("192.0.2.123"),
+                                  false, false, "",
+                                  false, CalloutHandlePtr(),
+                                  old_lease_);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ("192.0.2.123", lease->addr_.toText());
+
+    // Make sure that the lease has been committed to the lease database.
+    // And that the committed lease is equal to the one returned.
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+    detailCompareLease(lease, from_mgr);
+
+    EXPECT_FALSE(old_lease_);
+}
+
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has a reservation.
+// - Client sends DHCPDISCOVER with a requested IP address as a hint.
+// - Server offers a reserved address, even though it is different than the
+// requested address.
+TEST_F(AllocEngine4Test, reservedAddressHintFakeAllocation) {
+    // Create a reservation for the client.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Query the allocation engine for the lease to be assigned to the client
+    // and specify a hint being a different address than the reserved one.
+    Lease4Ptr lease = engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                            IOAddress("192.0.2.234"),
+                                            false, false, "",
+                                            true, CalloutHandlePtr(),
+                                            old_lease_);
+    ASSERT_TRUE(lease);
+    // Allocation engine should return reserved address.
+    EXPECT_EQ("192.0.2.123", lease->addr_.toText());
+
+    // This is a "fake" allocation so the returned lease should not be committed
+    // to the lease database.
+    EXPECT_FALSE(LeaseMgrFactory::instance().getLease4(lease->addr_));
+
+    EXPECT_FALSE(old_lease_);
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client has a lease for the address from the dynamic pool in the database.
+// - Client has a reservation for a different address than the one for which
+// the client has a lease.
+// - Client sends DHCPREQUEST, asking for the reserved address (as it has been
+// offered to it when it sent DHCPDISCOVER).
+// - Server allocates a reserved address and removes the lease for the address
+// previously allocated to the client.
+TEST_F(AllocEngine4Test, reservedAddressExistingLease) {
+    // Create the reservation for the client.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for the client with a different address than the reserved
+    // one.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Request allocation of the reserved address.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      IOAddress("192.0.2.123"),
+                                                      false, false, "",
+                                                      false, CalloutHandlePtr(),
+                                                      old_lease_);
+    ASSERT_TRUE(allocated_lease);
+    // The engine should have allocated the reserved address.
+    EXPECT_EQ("192.0.2.123", allocated_lease->addr_.toText());
+
+    // Make sure that the lease has been committed to the lease database.
+    Lease4Ptr from_mgr =
+        LeaseMgrFactory::instance().getLease4(allocated_lease->addr_);
+    ASSERT_TRUE(from_mgr);
+    detailCompareLease(allocated_lease, from_mgr);
+
+    // The previous lease should have been replaced by a new one. The previous
+    // lease should be returned by the allocation engine to the caller.
+    ASSERT_TRUE(old_lease_);
+    EXPECT_EQ("192.0.2.101", old_lease_->addr_.toText());
+    detailCompareLease(old_lease_, lease);
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client A has a lease in the database.
+// - Client B has a reservation for the address in use by client A.
+// - Client B sends a DHCPREQUEST requesting the allocation of the reserved
+// lease (in use by client A).
+// - Server determines that the reserved address is in use by a different client
+// and returns DHCPNAK to client B.
+TEST_F(AllocEngine4Test, reservedAddressHijacked) {
+    // Create host reservation for the client B.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Allocate a lease for the client A for the same address as reserved
+    // for the client B.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.123"), hwaddr2_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Try to allocate the reserved lease to client B.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      IOAddress("192.0.2.123"),
+                                                      false, false, "",
+                                                      false, CalloutHandlePtr(),
+                                                      old_lease_);
+    // The lease is allocated to someone else, so the allocation should not
+    // succeed.
+    ASSERT_FALSE(allocated_lease);
+    EXPECT_FALSE(old_lease_);
+
+    // Make sure that the allocation engine didn't modify the lease of the
+    // client A.
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+    detailCompareLease(lease, from_mgr);
+
+    // Try doing the same thing, but this time do not request any specific
+    // address. It should have the same effect.
+    allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                            hwaddr_,
+                                            IOAddress("0.0.0.0"),
+                                            false, false, "",
+                                            false, CalloutHandlePtr(),
+                                            old_lease_);
+    ASSERT_FALSE(allocated_lease);
+    EXPECT_FALSE(old_lease_);
+
+    from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+    detailCompareLease(lease, from_mgr);
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client A has a lease in the database.
+// - Client B has a reservation for the address in use by client A.
+// - Client B sends a DHCPDISCOVER.
+// - Server determines that the reserved address is in use by a different client
+// and that it can't allocate a lease to the client B.
+//
+// In the scenario presented here, the allocation engine should return a
+// NULL lease to the server. When the server receives NULL pointer from the
+// allocation engine the proper action for the server will be to not
+// respond to the client. Instead it should report to the administrator
+// that it was unable to allocate the (reserved) lease.
+TEST_F(AllocEngine4Test, reservedAddressHijackedFakeAllocation) {
+    // Create a reservation for the client B.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for the client A.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.123"), hwaddr2_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Query allocation engine for the lease to be allocated to the client B.
+    // The allocation engine is not able to allocate the lease to the client
+    // B, because the address is in use by client A.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      IOAddress("192.0.2.123"),
+                                                      false, false, "",
+                                                      true, CalloutHandlePtr(),
+                                                      old_lease_);
+    // The allocation engine should return no lease.
+    ASSERT_FALSE(allocated_lease);
+    EXPECT_FALSE(old_lease_);
+
+    // Do the same test. But, this time do not specify any address to be
+    // allocated.
+    allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                            hwaddr_,
+                                            IOAddress("0.0.0.0"),
+                                            false, false, "",
+                                            true, CalloutHandlePtr(),
+                                            old_lease_);
+    EXPECT_FALSE(allocated_lease);
+    EXPECT_FALSE(old_lease_);
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client has a reservation.
+// - Client has a lease in the database for a different address than reserved.
+// - Client sends a DHCPREQUEST and asks for a different address than reserved,
+// and different than it has in a database.
+// - Server doesn't allocate the reserved address to the client because the
+// client asked for the different address.
+//
+// Note that in this case the client should get the DHCPNAK and should fall back
+// to the DHCPDISCOVER.
+TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) {
+    // Create a reservation for the client.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for the client for a different address than reserved.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Try to allocate a lease and specify a different address than reserved
+    // and different from the one that client is currently using.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      IOAddress("192.0.2.102"),
+                                                      false, false, "",
+                                                      false, CalloutHandlePtr(),
+                                                      old_lease_);
+    ASSERT_FALSE(allocated_lease);
+    ASSERT_FALSE(old_lease_);
+
+    // Repeat the test, but this time ask for the address that the client
+    // has allocated.
+    allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                            hwaddr_,
+                                            IOAddress("192.0.2.101"),
+                                            false, false, "",
+                                            false, CalloutHandlePtr(),
+                                            old_lease_);
+    // The client has reservation so the server wants to allocate a
+    // reserved address and doesn't want to renew the address that the
+    // client is currently using. This is equivalent of the case when
+    // the client tries to renew the lease but there is a new reservation
+    // for this client. The server doesn't allow for the renewal and
+    // responds with DHCPNAK to force the client to return to the
+    // DHCP server discovery.
+    EXPECT_FALSE(allocated_lease);
+    EXPECT_FALSE(old_lease_);
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client has a lease in the database.
+// - Client has a reservation for a different address than the one for which it
+// has a lease.
+// - Client sends a DHCPDISCOVER and asks for a different address than reserved
+// and different from which it has a lease for.
+// - Server ignores the client's hint and offers a reserved address.
+TEST_F(AllocEngine4Test, reservedAddressExistingLeaseFakeAllocation) {
+    // Create a reservation for the client.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for a different address than reserved.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Try to allocate a lease and use a completely different address
+    // as a hint.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      IOAddress("192.0.2.102"),
+                                                      false, false, "",
+                                                      true, CalloutHandlePtr(),
+                                                      old_lease_);
+    // Server should offer a lease for a reserved address.
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_EQ("192.0.2.123", allocated_lease->addr_.toText());
+
+    // The lease should not be allocated until the client sends a DHCPREQUEST.
+    EXPECT_FALSE(LeaseMgrFactory::instance().getLease4(allocated_lease->addr_));
+
+    // Old lease should contain the currently used lease.
+    ASSERT_TRUE(old_lease_);
+    EXPECT_EQ("192.0.2.101", old_lease_->addr_.toText());
+
+    // Repeat the test but this time ask for the address for which the
+    // client has a lease.
+    allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                            hwaddr_,
+                                            IOAddress("192.0.2.101"),
+                                            false, false, "",
+                                            true, CalloutHandlePtr(),
+                                            old_lease_);
+    // The server should offer the lease, but not for the address that
+    // the client requested. The server should offer a reserved address.
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_EQ("192.0.2.123", allocated_lease->addr_.toText());
+
+    // Old lease should contain the currently used lease.
+    ASSERT_TRUE(old_lease_);
+    EXPECT_EQ("192.0.2.101", old_lease_->addr_.toText());
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client has a reservation.
+// - Client has a lease for a different address than reserved.
+// - Client sends a DHCPREQUEST to allocate a lease.
+// - The server determines that the client has a reservation for the
+// different address than it is currently using and should assign
+// a reserved address and remove the previous lease.
+TEST_F(AllocEngine4Test, reservedAddressExistingLeaseNoHint) {
+    // Create a reservation.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for a different address than reserved.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Try to allocate a lease with providing no hint.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      IOAddress("0.0.0.0"),
+                                                      false, false, "",
+                                                      false, CalloutHandlePtr(),
+                                                      old_lease_);
+    // The reserved address should be allocated.
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_EQ("192.0.2.123", allocated_lease->addr_.toText());
+
+    // The previous lease should be removed.
+    EXPECT_FALSE(LeaseMgrFactory::instance().getLease4(lease->addr_));
+
+    // Make sure that the allocated lease is committed in the lease database.
+    Lease4Ptr from_mgr =
+        LeaseMgrFactory::instance().getLease4(allocated_lease->addr_);
+    ASSERT_TRUE(from_mgr);
+    detailCompareLease(allocated_lease, from_mgr);
+
+    // Old lease should be returned.
+    ASSERT_TRUE(old_lease_);
+    detailCompareLease(lease, old_lease_);
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client has a reservation.
+// - Client has a lease for a different address than reserved.
+// - Client sends a DHCPDISCOVER with no hint.
+// - Server determines that there is a reservation for the client and that
+// the current lease should be removed and the reserved address should be
+// allocated.
+TEST_F(AllocEngine4Test, reservedAddressExistingLeaseNoHintFakeAllocation) {
+    // Create a reservation.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.123")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for a different address than reserved.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Query the allocation engine for the lease to be allocated for the
+    // client.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      IOAddress("0.0.0.0"),
+                                                      false, false, "",
+                                                      true, CalloutHandlePtr(),
+                                                      old_lease_);
+    // The server should offer the reserved address.
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_EQ("192.0.2.123", allocated_lease->addr_.toText());
+
+    // The lease should not be committed to the lease database until the
+    // client sends a DHCPREQUEST.
+    EXPECT_FALSE(LeaseMgrFactory::instance().getLease4(allocated_lease->addr_));
+
+    // The old lease should reflect what is in the database.
+    ASSERT_TRUE(old_lease_);
+    Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_);
+    ASSERT_TRUE(from_mgr);
+    detailCompareLease(lease, from_mgr);
+}
+
+// This test checks that the behavior of the allocation engine in the following
+// scenario:
+// - Client A has a lease for the address.
+// - Client B has a reservation for the same address that the Client A is using.
+// - Client B requests allocation of the reserved address.
+// - Server returns DHCPNAK to the client to indicate that the requested address
+// can't be allocated.
+// - Client A renews the lease.
+// - Server determines that the lease that the Client A is trying to renews
+// is for the address reserved for Client B. Therefore, the server returns
+// DHCPNAK to force the client to return to the server discovery.
+// - The Client A sends DHCPDISCOVER.
+// - The server offers an address to the Client A, which is different than
+// the address reserved for Client B.
+// - The Client A requests allocation of the offered address.
+// - The server allocates the new address to Client A.
+// - The Client B sends DHCPDISCOVER to the server.
+// - The server offers a reserved address to the Client B.
+// - The Client B requests the offered address.
+// - The server allocates the reserved address to the Client B.
+TEST_F(AllocEngine4Test, reservedAddressConflictResolution) {
+    // Create a reservation for client B.
+    HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.101")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Create a lease for Client A.
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0,
+                               100, 30, 60, time(NULL), subnet_->getID(),
+                               false, false, ""));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+
+    // Client B sends a DHCPREQUEST to allocate a reserved lease. The
+    // allocation engine declines allocation of the address for the
+    // client because Client A has a lease for it.
+    ASSERT_FALSE(engine.allocateLease4(subnet_, ClientIdPtr(), hwaddr2_,
+                                       IOAddress("192.0.2.101"), false,
+                                       false, "", false, CalloutHandlePtr(),
+                                       old_lease_));
+
+    // Client A tries to renew the lease. The renewal should fail because
+    // server detects that Client A doesn't have reservation for this
+    // address.
+    ASSERT_FALSE(engine.allocateLease4(subnet_, clientid_, hwaddr_,
+                                       IOAddress("192.0.2.101"), false, false,
+                                       "", false, CalloutHandlePtr(),
+                                       old_lease_));
+    ASSERT_TRUE(old_lease_);
+    EXPECT_EQ("192.0.2.101", old_lease_->addr_.toText());
+
+    // Client A returns to DHCPDISCOVER and should be offered a lease.
+    // The offered lease address must be different than the one the
+    // Client B has reservation for.
+    Lease4Ptr offered_lease = engine.allocateLease4(subnet_, clientid_,
+                                                    hwaddr_,
+                                                    IOAddress("192.0.2.101"),
+                                                    false, false, "", true,
+                                                    CalloutHandlePtr(),
+                                                    old_lease_);
+    ASSERT_TRUE(offered_lease);
+    EXPECT_NE(offered_lease->addr_.toText(), "192.0.2.101");
+
+    // Client A tried to acquire the lease. It should succeed. At this point
+    // the previous lease should be released and become available for the
+    // Client B.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, clientid_,
+                                                      hwaddr_,
+                                                      offered_lease->addr_,
+                                                      false, false, "", false,
+                                                      CalloutHandlePtr(),
+                                                      old_lease_);
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_NE(allocated_lease->addr_.toText(), "192.0.2.101");
+
+    // Client B tries to get the lease again. It should be offered
+    // a reserved lease.
+    offered_lease = engine.allocateLease4(subnet_, ClientIdPtr(),
+                                          hwaddr2_,
+                                          IOAddress("0.0.0.0"),
+                                          false, false, "", true,
+                                          CalloutHandlePtr(),
+                                          old_lease_);
+    ASSERT_TRUE(offered_lease);
+    EXPECT_EQ("192.0.2.101", offered_lease->addr_.toText());
+
+    // Client B requests allocation of the lease and it should succeed.
+    allocated_lease = engine.allocateLease4(subnet_, ClientIdPtr(),
+                                            hwaddr2_,
+                                            offered_lease->addr_,
+                                            false, false, "", false,
+                                            CalloutHandlePtr(),
+                                            old_lease_);
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_EQ("192.0.2.101", allocated_lease->addr_.toText());
+}
+
+// This test checks that the address is not assigned from the dynamic
+// pool if it has been reserved for another client.
+TEST_F(AllocEngine4Test, reservedAddressVsDynamicPool) {
+    // Create a reservation for the client.
+    HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress("192.0.2.100")));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Different client tries to allocate a lease. Note, that we're using
+    // an iterative allocator which would pick the first address from the
+    // dynamic pool, i.e. 192.0.2.100. This address is reserved so we expect
+    // that a different address will be allocated.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, ClientIdPtr(),
+                                                      hwaddr_,
+                                                      IOAddress("0.0.0.0"),
+                                                      false, false, "", false,
+                                                      CalloutHandlePtr(),
+                                                      old_lease_);
+    ASSERT_TRUE(allocated_lease);
+    EXPECT_NE(allocated_lease->addr_.toText(), "192.0.2.100");
+}
+
 /// @brief helper class used in Hooks testing in AllocEngine6
 ///
 /// It features a couple of callout functions and buffers to store

+ 30 - 0
src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc

@@ -19,6 +19,7 @@
 #include <dhcpsrv/cfg_hosts.h>
 #include <dhcpsrv/host.h>
 #include <gtest/gtest.h>
+#include <set>
 
 using namespace isc;
 using namespace isc::dhcp;
@@ -188,6 +189,35 @@ TEST_F(CfgHostsTest, getAllRepeatingHosts) {
     }
 }
 
+// This test checks that all reservations for the specified IPv4 address can
+// be retrieved.
+TEST_F(CfgHostsTest, getAll4ByAddress) {
+    CfgHosts cfg;
+    // Add hosts.
+    for (int i = 0; i < 25; ++i) {
+        // Add host identified by the HW address.
+        cfg.add(HostPtr(new Host(hwaddrs_[i]->toText(false),
+                                 "hw-address",
+                                 SubnetID(1 + i), SubnetID(0),
+                                 IOAddress("192.0.2.5"))));
+        // Add host identified by the DUID.
+        cfg.add(HostPtr(new Host(duids_[i]->toText(),
+                                 "duid",
+                                 SubnetID(1 + i), SubnetID(0),
+                                 IOAddress("192.0.2.10"))));
+    }
+
+    HostCollection hosts = cfg.getAll4(IOAddress("192.0.2.10"));
+    std::set<uint32_t> subnet_ids;
+    for (HostCollection::const_iterator host = hosts.begin(); host != hosts.end();
+         ++host) {
+        subnet_ids.insert((*host)->getIPv4SubnetID());
+    }
+    ASSERT_EQ(25, subnet_ids.size());
+    EXPECT_EQ(1, *subnet_ids.begin());
+    EXPECT_EQ(25, *subnet_ids.rbegin());
+}
+
 // This test checks that the reservations can be retrieved for the particular
 // host connected to the specific IPv4 subnet (by subnet id).
 TEST_F(CfgHostsTest, get4) {