[3690] Added unit test to cover change in allocation engine.

Also, further fixes in allocation engine were needed to correctly handle
the case when a client is requesting an address which belongs to
someone else.

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index e306eb76fbf8570aa92b9c0f7819b594c4979104..88d4c243d6cb48237f35cdcbfd2b24c6cfba6f01 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -606,14 +606,28 @@ AllocEngine::allocateLease4(const SubnetPtr& subnet, const ClientIdPtr& clientid
                 return (Lease4Ptr());
             }
 
-            // 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_;
-
-            if (ctx.host_ ||
-                (!ctx.host_ && !HostMgr::instance().get4(ctx.subnet_->getID(),
-                                                         candidate))) {
+            // Now let's pick an address to be allocated to the client. The
+            // candidate address may either be a reserved one or the one that
+            // the client requests.
+            IOAddress candidate = 0;
+            ConstHostPtr other_host;
+            if (ctx.host_) {
+                candidate = ctx.host_->getIPv4Reservation();
+
+            } else {
+                candidate = ctx.requested_address_;
+                // If client is requesting an address we have to check if this address
+                // is not reserved for someone else. Note that for DHCPDISCOVER we
+                // treat the requested address as a hint and we don't return an empty
+                // lease.
+                other_host = HostMgr::instance().get4(ctx.subnet_->getID(), candidate);
+                if (!ctx.fake_allocation_ && other_host) {
+                    return (Lease4Ptr());
+                }
+            }
+
+            // If address is not reserved for another client, let's try allocate it.
+            if (!other_host) {
                 // Once we picked an address we want to allocate, we have to check
                 // if this address is available.
                 existing = LeaseMgrFactory::instance().getLease4(candidate);

diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
index 55ffe109cce18cf212418b72ef402cc0a48bd0f4..9a98fcfc7e48bd8e2a6eaca015f06883d792cdb1 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc
@@ -2263,6 +2263,42 @@ TEST_F(AllocEngine4Test, reservedAddressVsDynamicPool) {
     EXPECT_NE(allocated_lease->addr_.toText(), "192.0.2.100");
 }
 
+// This test checks that the client requesting an address which is
+// reserved for another client will get no lease or a different
+// address will be assigned if the client is sending a DHCPDISCOVER.
+TEST_F(AllocEngine4Test, reservedAddressHintUsedByOtherClient) {
+    // 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 is requesting this address.
+    Lease4Ptr allocated_lease = engine.allocateLease4(subnet_, ClientIdPtr(),
+                                                      hwaddr_,
+                                                      IOAddress("192.0.2.100"),
+                                                      false, false, "", false,
+                                                      CalloutHandlePtr(),
+                                                      old_lease_);
+    // The client should get no lease (DHCPNAK).
+    ASSERT_FALSE(allocated_lease);
+
+    // The same client should get a different lease than requested if
+    // if is sending a DHCPDISCOVER (fake allocation is true).
+    allocated_lease = engine.allocateLease4(subnet_, ClientIdPtr(),
+                                            hwaddr_,
+                                            IOAddress("192.0.2.100"),
+                                            false, false, "", true,
+                                            CalloutHandlePtr(),
+                                            old_lease_);
+    ASSERT_TRUE(allocated_lease);
+    // Make sure the lease obtained is for a different address.
+    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