[3747] Use client identifier over MAC to get existing client's lease.

src/bin/dhcp4/tests/dora_unittest.cc

@@ -173,71 +173,30 @@ public:
         IfaceMgr::instance().openSockets4();
     }
 
-    /// @brief Test that server doesn't allocate the lease for a client
-    /// which has the same address or client identifier as another client.
+    /// @brief Test that server returns the same lease for the client which is
+    /// sometimes using client identifier, sometimes not.
     ///
     /// This test checks the server's behavior in the following scenario:
-    /// - Client A identifies itself to the server using the hardware address
-    ///   and client identifier or only one of those.
-    /// - Client A performs the 4-way exchange and obtains a lease from the server.
-    /// - Client B uses the same HW address or client identifier as the client A.
-    /// - Client B uses both HW address and client identifier if the client A is using
-    ///   only one of them. Client B uses one of the HW address or client
-    ///   identifier if the client A is using both.
-    /// - Client B sends the DHCPDISCOVER to the server.
-    ///   The server determines that there is a lease for the client A using the
-    ///   same HW address as the client B. Server discards the client's message and
-    ///   doesn't offer the lease for the client B to prevent allocation of the
-    ///   lease without a unique identifier.
-    /// - The client sends the DHCPREQUEST and the server sends the DHCPNAK for the
-    ///   same reason.
-    /// - The client A renews its address successfully.
+    /// - Client identifies itself to the server using HW address, and may use
+    ///   client identifier.
+    /// - Client performs the 4-way exchange and obtains a lease from the server.
+    /// - If the client identifier was in use when the client has acquired the lease,
+    ///   the client uses null client identifier in the next exchange with the server.
+    /// - If the client identifier was not in use when the client has acquired the
+    ///   lease, the client uses client identifier in the next exchange with the
+    ///   server.
+    /// - When the client contacts the server for the second time using the
+    ///   DHCPDISCOVER the server determines (using HW address) that the client
+    ///   already has a lease and returns this lease to the client.
+    /// - The client renews the existing lease.
     ///
-    /// The specific test cases using this test must make sure that one of the
-    /// provided parameters is an empty string. This simulates the situation where
-    /// one of the clients has only one of the identifiers and the other one has
-    /// two.
-    ///
-    /// @param hwaddr_a HW address of client A.
-    /// @param clientid_a Client id of client A.
-    /// @param hwaddr_b HW address of client B.
-    /// @param clientid_b Client id of client B.
-    void oneAllocationOverlapTest(const std::string& hwaddr_a,
-                                  const std::string& clientid_a,
-                                  const std::string& hwaddr_b,
+    /// @param clientid_a Client identifier when the client initially allocates
+    /// the lease. An empty value means "no client identifier".
+    /// @param clientid_b Client identifier when the client sends the DHCPDISCOVER
+    /// and then DHCPREQUEST to renew lease.
+    void oneAllocationOverlapTest(const std::string& clientid_a,
                                   const std::string& clientid_b);
 
-    /// @brief Test that server can allocate the lease for a client having
-    /// the same HW Address or client id as another client.
-    ///
-    /// This test checks the server behavior in the following situation:
-    /// - Client A identifies itself to the server using client identifier
-    ///   and the hardware address and requests allocation of the new lease.
-    /// - Server allocates the lease to the client.
-    /// - Client B has a different hardware address or client identifier than
-    ///   the client A, but the other identifier is equal to the corresponding
-    ///   identifier of the client A.
-    /// - Client B sends DHCPDISCOVER.
-    /// - Server should determine that the client B is not client A, because
-    ///   it is using a different hadrware address or client identifier.
-    ///   As a consequence, the server should offer a different address to the
-    ///   client B.
-    /// - The client B performs the 4-way exchange again, and the server
-    ///   allocates a new address to the client, which should be different
-    ///   than the address used by the client A.
-    /// - Client B is in the renewing state and it successfully renews its
-    ///   address.
-    /// - The client A also renews its address successfully.
-    ///
-    /// @param hwaddr_a HW address of client A.
-    /// @param clientid_a Client id of client A.
-    /// @param hwaddr_b HW address of client B.
-    /// @param clientid_b Client id of client B.
-    void twoAllocationsOverlapTest(const std::string& hwaddr_a,
-                                   const std::string& clientid_a,
-                                   const std::string& hwaddr_b,
-                                   const std::string& clientid_b);
-
     /// @brief Interface Manager's fake configuration control.
     IfaceMgrTestConfig iface_mgr_test_config_;
 
@@ -507,14 +466,67 @@ TEST_F(DORATest, ciaddr) {
 }
 
 void
-DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a,
-                                    const std::string& clientid_a,
-                                    const std::string& hwaddr_b,
-                                    const std::string& clientid_b) {
+DORATest::oneAllocationOverlapTest(const std::string& clientid_a,
+                                   const std::string& clientid_b) {
+    // Allocate a lease by client using the 4-way exchange.
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    client.includeClientId(clientid_a);
+    client.setHWAddress("01:02:03:04:05:06");
+    configure(DORA_CONFIGS[0], *client.getServer());
+    ASSERT_NO_THROW(client.doDORA());
+    // 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()));
+    Lease4Ptr lease_a = LeaseMgrFactory::instance().getLease4(client.config_.lease_.addr_);
+    ASSERT_TRUE(lease_a);
+    // Remember the allocated address.
+    IOAddress yiaddr = lease_a->addr_;
+
+    // Change client identifier. If parameters clientid_a and clientid_b
+    // are specified correctly, this removes the client identifier from
+    // client's requests if the lease has been acquired with the client
+    // identifier, or adds the client identifier otherwise.
+    client.includeClientId(clientid_b);
+
+    // Check if the server will offer the same address.
+    ASSERT_NO_THROW(client.doDiscover());
+    resp = client.getContext().response_;
+    ASSERT_TRUE(resp);
+    EXPECT_EQ(yiaddr, resp->getYiaddr());
+
+    // Client should also be able to renew its address.
+    client.setState(Dhcp4Client::RENEWING);
+    ASSERT_NO_THROW(client.doRequest());
+    ASSERT_TRUE(client.getContext().response_);
+    resp = client.getContext().response_;
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    ASSERT_EQ(yiaddr, client.config_.lease_.addr_);
+}
+
+// This test checks the server behavior in the following situation:
+// - Client A identifies itself to the server using client identifier
+//   and the hardware address and requests allocation of the new lease.
+// - Server allocates the lease to the client.
+// - Client B has the same hardware address but is using a different
+//   client identifier then Client A.
+// - Client B sends DHCPDISCOVER.
+// - Server should determine that the client B is not client A, because
+//   it is using a different client identifier, even though they use the
+//   same HW address. As a consequence, the server should offer a
+//    different address to the client B.
+// - The client B performs the 4-way exchange again and the server
+//   allocates a new address to the client, which should be different
+//   than the address used by the client A.
+// - Client B is in the renewing state and it successfully renews its
+//   address.
+// - Client A also renews its address successfully.
+TEST_F(DORATest, twoAllocationsOverlap) {
     // Allocate a lease by client A using the 4-way exchange.
     Dhcp4Client client_a(Dhcp4Client::SELECTING);
-    client_a.includeClientId(clientid_a);
-    client_a.setHWAddress(hwaddr_a);
+    client_a.includeClientId("12:34");
+    client_a.setHWAddress("01:02:03:04:05:06");
     configure(DORA_CONFIGS[0], *client_a.getServer());
     ASSERT_NO_THROW(client_a.doDORA());
     // Make sure that the server responded.
@@ -529,8 +541,8 @@ DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a,
 
     // Create client B.
     Dhcp4Client client_b(client_a.getServer(), Dhcp4Client::SELECTING);
-    client_b.setHWAddress(hwaddr_b);
-    client_b.includeClientId(clientid_b);
+    client_b.setHWAddress("01:02:03:04:05:06");
+    client_b.includeClientId("45:67");
     // Send DHCPDISCOVER and expect the response.
     ASSERT_NO_THROW(client_b.doDiscover());
     Pkt4Ptr resp_b = client_b.getContext().response_;
@@ -581,86 +593,6 @@ DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a,
     ASSERT_NE(client_a.config_.lease_.addr_, client_b.config_.lease_.addr_);
 }
 
-void
-DORATest::oneAllocationOverlapTest(const std::string& hwaddr_a,
-                                   const std::string& clientid_a,
-                                   const std::string& hwaddr_b,
-                                   const std::string& clientid_b) {
-    // Allocate a lease by client A using the 4-way exchange.
-    Dhcp4Client client_a(Dhcp4Client::SELECTING);
-    client_a.includeClientId(clientid_a);
-    client_a.setHWAddress(hwaddr_a);
-    configure(DORA_CONFIGS[0], *client_a.getServer());
-    ASSERT_NO_THROW(client_a.doDORA());
-    // Make sure that the server responded.
-    ASSERT_TRUE(client_a.getContext().response_);
-    Pkt4Ptr resp_a = client_a.getContext().response_;
-    // Make sure that the server has responded with DHCPACK.
-    ASSERT_EQ(DHCPACK, static_cast<int>(resp_a->getType()));
-    Lease4Ptr lease_a = LeaseMgrFactory::instance().getLease4(client_a.config_.lease_.addr_);
-    ASSERT_TRUE(lease_a);
-
-    // Client B sends a DHCPDISCOVER.
-    Dhcp4Client client_b(client_a.getServer(), Dhcp4Client::SELECTING);
-    client_b.setHWAddress(hwaddr_b);
-    client_b.includeClientId(clientid_b);
-    // Client A and Client B have one common identifier (HW address
-    // or client identifier) and one of them is missing the other
-    // identifier. The allocation engine can't offer an address for
-    // the client which has the same identifier as the other client and
-    // which doesn't have any other (unique) identifier. It should
-    // discard the DHCPDISCOVER.
-    ASSERT_NO_THROW(client_b.doDiscover());
-    Pkt4Ptr resp_b = client_b.getContext().response_;
-    ASSERT_FALSE(resp_b);
-
-    // Now repeat the same test but send the DHCPREQUEST. This time the
-    // server should send the DHCPNAK.
-    client_b.config_.lease_.addr_ = IOAddress::IPV4_ZERO_ADDRESS();
-    client_b.setState(Dhcp4Client::INIT_REBOOT);
-    ASSERT_NO_THROW(client_b.doRequest());
-    resp_b = client_b.getContext().response_;
-    ASSERT_TRUE(resp_b);
-    ASSERT_EQ(DHCPNAK, static_cast<int>(resp_b->getType()));
-
-    // Client A should also be able to renew its address.
-    client_a.setState(Dhcp4Client::RENEWING);
-    ASSERT_NO_THROW(client_a.doRequest());
-    ASSERT_TRUE(client_a.getContext().response_);
-    resp_b = client_a.getContext().response_;
-    ASSERT_EQ(DHCPACK, static_cast<int>(resp_b->getType()));
-    ASSERT_NE(client_a.config_.lease_.addr_, client_b.config_.lease_.addr_);
-}
-
-// This test checks the server behavior in the following situation:
-// - Client A identifies itself to the server using client identifier
-//   and the hardware address and requests allocation of the new lease.
-// - Server allocates the lease to the client.
-// - Client B has a different hardware address but is using the same
-//   client identifier as Client A.
-// - Client B sends DHCPDISCOVER.
-// - Server should determine that the client B is not client A, because
-//   it is using a different hadrware address, even though they use the
-//   same client identifier. As a consequence, the server should offer
-//   a different address to the client B.
-// - The client B performs the 4-way exchange again and the server
-//   allocates a new address to the client, which should be different
-//   than the address used by the client A.
-// - Client B is in the renewing state and it successfully renews its
-//   address.
-// - Client A also renews its address successfully.
-TEST_F(DORATest, twoAllocationsOverlap1) {
-    twoAllocationsOverlapTest("01:02:03:04:05:06", "12:34",
-                              "02:02:03:03:04:04", "12:34");
-}
-
-// This test is similar to twoAllocationsOverlap1, but the
-// clients differ by client identifier.
-TEST_F(DORATest, twoAllocationsOverlap2) {
-    twoAllocationsOverlapTest("01:02:03:04:05:06", "12:34",
-                              "01:02:03:04:05:06", "22:34");
-}
-
 // This test checks the server behavior in the following situation:
 // - Client A identifies itself to the server using the hardware address
 //   and client identifier.
@@ -676,16 +608,14 @@ TEST_F(DORATest, twoAllocationsOverlap2) {
 //   same reason.
 // - Client A renews its address successfully.
 TEST_F(DORATest, oneAllocationOverlap1) {
-    oneAllocationOverlapTest("01:02:03:04:05:06", "12:34",
-                             "01:02:03:04:05:06", "");
+    oneAllocationOverlapTest("12:34", "");
 }
 
 // This test is similar to oneAllocationOverlap2 but this time the client A
 // uses no client identifier, and the client B uses the HW address and the
 // client identifier. The server behaves as previously.
 TEST_F(DORATest, oneAllocationOverlap2) {
-    oneAllocationOverlapTest("01:02:03:04:05:06", "",
-                             "01:02:03:04:05:06", "12:34");
+    oneAllocationOverlapTest("", "12:34");
 }
 
 // This is a simple test for the host reservation. It creates a reservation

src/lib/dhcpsrv/alloc_engine.cc

@@ -1228,69 +1228,43 @@ hasAddressReservation(const AllocEngine::ClientContext4& ctx) {
     return (ctx.host_ && !ctx.host_->getIPv4Reservation().isV4Zero());
 }
 
-/// @brief Check if there is a lease for the client which message is processed.
+/// @brief Finds existing lease in the database.
 ///
-/// This function searches the lease database to find existing lease for the client.
-/// It finds the lease using the client's HW address first. If the lease exists and
-/// appears to belong to the client the lease is returned. Otherwise, the function
-/// will search for the lease using the client identifier (if supplied). If the
-/// lease exists and appears to belong to the client, it is returned.
+/// This function searches for the lease in the database which belongs to the
+/// client requesting allocation. If the client has supplied the client
+/// identifier this identifier is used to look up the lease. If the lease is
+/// not found using the client identifier, an additional lookup is performed
+/// using the HW address, if supplied. If the lease is found using the HW
+/// address, the function also checks if the lease belongs to the client, i.e.
+/// there is no conflict between the client identifiers.
 ///
-/// This function also identifies the conflicts between existing leases and the
-/// lease to be allocated for the client, when the client is using a HW address
-/// or client identifier which is already in use by the client having a lease in
-/// the database. If the client uses an identifier which is already used by another
-/// client and no other unique identifier which could be used to identify the client's
-/// lease this function signals the conflict by returning 'true'.
-///
-/// @param ctx Client context.
-/// @param [out] client_lease Client's lease found in the database.
-///
-/// @return true if there is a conflict of identifiers (HW address or client id)
-/// between the client which message is being processed and the client which has
-/// a lease in the database. When the value 'true' is returned, the caller should
-/// cease the lease allocation for the client.
-bool matchClientLease(const AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) {
-    // Obtain the sole instance of the LeaseMgr.
+/// @param ctx Context holding data extracted from the client's message,
+/// including the HW address and client identifier.
+/// @param [out] client_lease A pointer to the lease returned by this function
+/// or null value if no has been lease found.
+void findClientLease(const AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) {
     LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
-
-    // The server should hand out existing lease to the client, so we have to check
-    // if there is one. First, try to use the client's HW address.
-    client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID());
-    // If there is no lease for this HW address or the lease doesn't seem to be ours,
-    // we will have to use the client identifier. Note that in some situations two
-    // clients may use the same HW address so even if we find the lease for the HW
-    // address it doesn't mean it is ours, because client identifier may not match.
-    if (ctx.clientid_ && ((!client_lease) || (client_lease && !ctx.myLease(*client_lease)))) {
-        // Check if the lease is in conflict with the lease that we want to allocate.
-        // If the lease is in conflict because of using overlapping HW address or
-        // client identifier, we can't allocate the lease for this client.
-        if (client_lease && ctx.isInConflict(*client_lease)) {
-            return (true);
-        }
-        // There is no lease or the lease we found is not conflicting with the lease
-        // which we have found for the HW address, so there is still a chance that
-        // we will allocate the lease. Check if there is a lease using the client
-        // identifier.
+    // If client identifier has been supplied, use it to lookup the lease. This
+    // search will return no lease if the client doesn't have any lease in the
+    // database or if the client didn't use client identifier to allocate the
+    // existing lease (this include cases when the server was explicitly
+    // configured to ignore client identifier).
+    if (ctx.clientid_) {
         client_lease = lease_mgr.getLease4(*ctx.clientid_, ctx.subnet_->getID());
     }
 
-    // Check if the lease we have found belongs to us.
-    if (client_lease && !ctx.myLease(*client_lease)) {
-        // If the lease doesn't belong to us, check if we can add new lease for
-        // the client which message we're processing, or its identifiers are
-        // in conflict with this lease.
-        if (ctx.isInConflict(*client_lease)) {
-            return (true);
+    // If no lease found using the client identifier, try the lookup using
+    // the HW address.
+    if (!client_lease && ctx.hwaddr_) {
+        client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID());
+        // This lookup may return the lease which has conflicting client
+        // identifier and thus is considered to belong to someone else.
+        // If this is the case, we need to toss the result and force the
+        // Allocation Engine to allocate another lease.
+        if (client_lease && !client_lease->belongsToClient(ctx.hwaddr_, ctx.clientid_)) {
+            client_lease.reset();
         }
-        // If there is no conflict we can proceed and try to find the appropriate
-        // lease but we don't use the one we found, because it is assigned to
-        // someone else. Reset the pointer to indicate that we're not
-        // renewing this lease.
-        client_lease.reset();
     }
-
-    return (false);
 }
 
 } // end of anonymous namespace
@@ -1321,37 +1295,6 @@ AllocEngine::ClientContext4::ClientContext4(const Subnet4Ptr& subnet,
       fake_allocation_(fake_allocation), old_lease_(), host_() {
 }
 
-
-
-bool
-AllocEngine::ClientContext4::myLease(const Lease4& lease) const {
-    if ((!hwaddr_ && lease.hwaddr_) || (hwaddr_ && !lease.hwaddr_)) {
-        return (false);
-    }
-
-    if ((hwaddr_ && lease.hwaddr_) && (hwaddr_->hwaddr_ != lease.hwaddr_->hwaddr_)) {
-        return (false);
-    }
-
-    if ((!clientid_ && lease.client_id_) || (clientid_ && !lease.client_id_)) {
-        return (false);
-    }
-
-    if ((clientid_ && lease.client_id_) && (*clientid_ != *lease.client_id_)) {
-        return (false);
-    }
-
-    return (true);
-}
-
-bool
-AllocEngine::ClientContext4::isInConflict(const Lease4& lease) const {
-    return ((!(hwaddr_ && lease.hwaddr_) && (clientid_ && lease.client_id_) &&
-             (*clientid_ == *lease.client_id_)) ||
-            (!(clientid_ && lease.client_id_) && (hwaddr_ && lease.hwaddr_) &&
-             (hwaddr_->hwaddr_ == lease.hwaddr_->hwaddr_)));
-}
-
 Lease4Ptr
 AllocEngine::allocateLease4(ClientContext4& ctx) {
     // The NULL pointer indicates that the old lease didn't exist. It may
@@ -1410,10 +1353,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
     // if there is a conflict with existing lease and the allocation should
     // not be continued.
     Lease4Ptr client_lease;
-    bool conflict = matchClientLease(ctx, client_lease);
-    if (conflict) {
-        return (Lease4Ptr());
-    }
+    findClientLease(ctx, client_lease);
 
     // new_lease will hold the pointer to the lease that we will offer to the
     // caller.
@@ -1499,10 +1439,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
     // if there is a conflict with existing lease and the allocation should
     // not be continued.
     Lease4Ptr client_lease;
-    bool conflict = matchClientLease(ctx, client_lease);
-    if (conflict) {
-        return (Lease4Ptr());
-    }
+    findClientLease(ctx, client_lease);
 
     // Obtain the sole instance of the LeaseMgr.
     LeaseMgr& lease_mgr = LeaseMgrFactory::instance();
@@ -1537,7 +1474,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
         // if the address is in use by our client or another client.
         // If it is in use by another client, the address can't be
         // allocated.
-        if (existing && !existing->expired() && !ctx.myLease(*existing)) {
+        if (existing && !existing->expired() &&
+            !existing->belongsToClient(ctx.hwaddr_, ctx.clientid_)) {
             return (Lease4Ptr());
         }
 

src/lib/dhcpsrv/alloc_engine.h

@@ -760,45 +760,6 @@ public:
                        const bool fwd_dns_update, const bool rev_dns_update,
                        const std::string& hostname, const bool fake_allocation);
 
-        /// @brief Check if the specified lease belongs to the client.
-        ///
-        /// This method compares the hardware address and the client id
-        /// in the lease with the relevant values in the context. That
-        /// way the method determines whether the lease belongs to the
-        /// client which message the server is processing.
-        ///
-        /// @return true if the lease belongs to the client for which
-        /// the context has been created, false otherwise.
-        bool myLease(const Lease4& lease) const;
-
-        /// @brief Check if the lease conflicts with the context.
-        ///
-        /// This method is used in cases when there is a lease in the lease database
-        /// for the client and the server receives the message from another client
-        /// which may be potentially using the same client identifier or HW address.
-        ///
-        /// Currently the allocation engine allows for allocating a lease for the
-        /// client which is using the same HW address or client identifier as the
-        /// client which already has a lease in the database. However, the value of
-        /// one of the identifiers must be unique. In other words, two clients may
-        /// have the same client identifier but different HW addresses and the
-        /// leases can be allocated for both (there is no conflict).
-        ///
-        /// The other possible case is that one of the clients uses both HW address
-        /// and client identifier, and another client is using only one of those
-        /// and its value is the same as for the former client. The allocation
-        /// engine treats it as a conflict because there is no unique value in the
-        /// lease database by which the server could distinguish the clients.
-        /// Hence, it doesn't allocate the lease from the context in conflict.
-        ///
-        /// This method detects the conflict described above.
-        ///
-        /// @param lease Existing lease to be checked.
-        ///
-        /// @return true if the lease is in conflict with the context, false
-        /// otherwise.
-        bool isInConflict(const Lease4& lease) const;
-
     };
 
     /// @brief Pointer to the @c ClientContext4.

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc

@@ -536,6 +536,65 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
     EXPECT_TRUE(*ctx.old_lease_ == original_lease);
 }
 
+// This test checks that the Allocation Engine correcly identifies the
+// existing client's lease in the lease database, using the client
+// identifier and HW address.
+TEST_F(AllocEngine4Test, identifyClientLease) {
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, clientid_,
+                               100, 30, 60, time(NULL), subnet_->getID()));
+    LeaseMgrFactory::instance().addLease(lease);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
+                                    IOAddress::IPV4_ZERO_ADDRESS(),
+                                    false, false, "", true);
+
+    Lease4Ptr identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr2_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr_;
+    ctx.clientid_ = clientid2_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101");
+
+    ctx.clientid_.reset();
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.hwaddr_ = hwaddr2_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101");
+
+    lease->client_id_.reset();
+    ASSERT_NO_THROW(LeaseMgrFactory::instance().updateLease4(lease));
+
+    ctx.hwaddr_ = hwaddr_;
+    ctx.clientid_ = clientid_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.clientid_.reset();
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText());
+
+    ctx.clientid_ = clientid_;
+    ctx.hwaddr_ = hwaddr2_;
+    identified_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(identified_lease);
+    EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101");
+}
+
 // This test checks that when the client requests the address which belongs
 // to another client, the allocation engine returns NULL (for the
 // DHCPREQUEST case) or a lease for the address which belongs to this
@@ -924,7 +983,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) {
     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,
+    Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, ClientIdPtr(),
                                100, 30, 60, time(NULL), subnet_->getID(),
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
@@ -946,6 +1005,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) {
     AllocEngine::ClientContext4 ctx2(subnet_, clientid_, hwaddr_,
                                     IOAddress("192.0.2.101"), false, false,
                                     "", false);
+    AllocEngine::findReservation(ctx2);
     allocated_lease = engine.allocateLease4(ctx2);
     // The client has reservation so the server wants to allocate a
     // reserved address and doesn't want to renew the address that the

src/lib/dhcpsrv/tests/alloc_engine_utils.cc

@@ -351,6 +351,8 @@ AllocEngine4Test::AllocEngine4Test() {
     HostMgr::instance().create();
 
     clientid_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x44)));
+    clientid2_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x56)));
+
     uint8_t mac[] = { 0, 1, 22, 33, 44, 55};
 
     // Let's use odd hardware type to check if there is no Ethernet

src/lib/dhcpsrv/tests/alloc_engine_utils.h

@@ -383,6 +383,7 @@ public:
     }
 
     ClientIdPtr clientid_;      ///< Client-identifier (value used in tests)
+    ClientIdPtr clientid2_;     ///< Alternative client-identifier.
     HWAddrPtr hwaddr_;          ///< Hardware address (value used in tests)
     HWAddrPtr hwaddr2_;         ///< Alternative hardware address.
     Subnet4Ptr subnet_;         ///< Subnet4 (used in tests)