[3563] Six unit-tests implemented for AllocEngine6 + Reservations

src/lib/dhcpsrv/alloc_engine.cc

@@ -306,9 +306,16 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
             isc_throw(InvalidOperation, "DUID is mandatory for allocation");
         }
 
-        // Check if there's a host reservation for this client.
-        ctx.host_ = HostMgr::instance().get6(ctx.subnet_->getID(), ctx.duid_,
-                                             ctx.hwaddr_);
+        // Check which host reservation mode is supported in this subnet.
+        Subnet::HRMode hr_mode = ctx.subnet_->getHostReservationMode();
+
+        // Check if there's a host reservation for this client. Attempt to get
+        // host info only if reservations are not disabled.
+        if (hr_mode != Subnet::HR_DISABLED) {
+
+            ctx.host_ = HostMgr::instance().get6(ctx.subnet_->getID(), ctx.duid_,
+                                                 ctx.hwaddr_);
+        }
 
         // Check if there are existing leases for that subnet/duid/iaid
         // combination.
@@ -337,7 +344,7 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
         // Hence independent checks.
 
         // Case 1: There are no leases and there's a reservation for this host.
-        if (leases.empty() && ctx.host_) {
+        if ((hr_mode != Subnet::HR_DISABLED) && leases.empty() && ctx.host_) {
 
             // Try to allocate leases that match reservations. Typically this will
             // succeed, except cases where the reserved addresses are used by
@@ -460,6 +467,10 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
                   << Lease6::typeToText(ctx.type_));
     }
 
+
+    // Check which host reservation mode is supported in this subnet.
+    Subnet::HRMode hr_mode = ctx.subnet_->getHostReservationMode();
+
     Lease6Collection leases;
 
     IOAddress hint("::");
@@ -482,10 +493,12 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
             // else. 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.
 
-            /// @todo: BROKEN this call is broken. It tries to convert getID()
-            /// to IOAddress::uint32_t()
-            if (!ctx.subnet_->allowInPoolReservations() ||
-                !HostMgr::instance().get6(ctx.subnet_->getID(), hint)) {
+            ConstHostPtr host;
+            if (hr_mode != Subnet::HR_DISABLED) {
+                host = HostMgr::instance().get6(ctx.subnet_->getID(), hint);
+            }
+
+            if (!host) {
                 // If the in-pool reservations are disabled, or there is no
                 // reservation for a given hint, we're good to go.
 
@@ -510,9 +523,13 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
             // If the lease is expired, we may likely reuse it, but...
             if (lease->expired()) {
 
+                ConstHostPtr host;
+                if (hr_mode != Subnet::HR_DISABLED) {
+                    host = HostMgr::instance().get6(ctx.subnet_->getID(), hint);
+                }
+
                 // Let's check if there is a reservation for this address.
-                if (!ctx.subnet_->allowInPoolReservations() ||
-                    !HostMgr::instance().get6(ctx.subnet_->getID(), hint)) {
+                if (!host) {
 
                     // Copy an existing, expired lease so as it can be returned
                     // to the caller.
@@ -553,7 +570,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
         /// In-pool reservations: Check if this address is reserved for someone
         /// else. 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 (ctx.subnet_->allowInPoolReservations() &&
+        if (hr_mode == Subnet::HR_IN_POOL &&
             HostMgr::instance().get6(ctx.subnet_->getID(), candidate)) {
 
             // Don't allocate.
@@ -619,7 +636,7 @@ void
 AllocEngine::allocateReservedLeases6(ClientContext6& ctx, Lease6Collection& existing_leases) {
 
     // If there are no reservations or the reservation is v4, there's nothing to do.
-    if (!ctx.host_ || ctx.host_->hasIPv6Reservation()) {
+    if (!ctx.host_ || !ctx.host_->hasIPv6Reservation()) {
         return;
     }
 

src/lib/dhcpsrv/subnet.h

@@ -66,6 +66,27 @@ public:
         isc::asiolink::IOAddress addr_;
     };
 
+    /// @brief Specifies allowed host reservation mode.
+    ///
+    typedef enum  {
+
+        /// None - host reservation is disabled. No reservation types
+        /// are allowed.
+        HR_DISABLED,
+
+        /// Only out-of-pool reservations is allowed. This mode
+        /// allows AllocEngine to skip reservation checks when
+        /// dealing with with addresses that are in pool.
+        HR_OUT_OF_POOL,
+
+        /// Both out-of-pool and in-pool reservations are allowed. This is the
+        /// most flexible mode, where sysadmin have biggest liberty. However,
+        /// there is a non-trivial performance penalty for it, as the
+        /// AllocEngine code has to check whether there are reservations, even
+        /// when dealing with reservations from within the dynamic pools.
+        HR_IN_POOL
+    } HRMode;
+
     /// Pointer to the RelayInfo structure
     typedef boost::shared_ptr<Subnet::RelayInfo> RelayInfoPtr;
 
@@ -284,19 +305,19 @@ public:
     void
     allowClientClass(const isc::dhcp::ClientClass& class_name);
 
-    /// @brief Specifies whether in-pool host reservations are allowed.
+    /// @brief Specifies what type of Host Reservations are supported.
     ///
     /// Host reservations may be either in-pool (they reserve an address that
     /// is in the dynamic pool) or out-of-pool (they reserve an address that is
-    /// not in the dynamic pool).
+    /// not in the dynamic pool). HR may also be completely disabled for
+    /// performance reasons.
     ///
     /// @todo: implement this.
     ///
     /// @return whether in-pool host reservations are allowed.
-    bool
-    allowInPoolReservations() {
-        /// @todo: Make this configurable.
-        return (true);
+    HRMode
+    getHostReservationMode() {
+        return (Subnet::HR_IN_POOL);
     }
 
 protected:

src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -378,6 +378,29 @@ public:
         detailCompareLease(lease, from_mgr);
     }
 
+    /// @brief Utility function that creates a host reservation
+    ///
+    /// @param add_to_host_mgr true if the reservation should be added
+    /// @param type specifies reservation type
+    /// @param addr specifies reserved address or prefix
+    /// @param prefix_len prefix length (should be 128 for addresses)
+    /// @return created Host object.
+    HostPtr
+    createHost6(bool add_to_host_mgr, IPv6Resrv::Type type,
+                const asiolink::IOAddress& addr, uint8_t prefix_len) {
+        HostPtr host(new Host(&duid_->getDuid()[0], duid_->getDuid().size(),
+                              Host::IDENT_DUID, SubnetID(0), subnet_->getID(),
+                              IOAddress("0.0.0.0")));
+        IPv6Resrv resv(type, addr, prefix_len);
+        host->addReservation(resv);
+
+        if (add_to_host_mgr) {
+            CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+            CfgMgr::instance().commit();
+        }
+        return (host);
+    }
+
     virtual ~AllocEngine6Test() {
         factory_.destroy();
     }
@@ -1005,6 +1028,153 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     detailCompareLease(lease, from_mgr);
 }
 
+// --- v6 host reservation ---
+
+// Checks that a client gets the address reserved (in-pool case)
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has an in-pool reservation.
+// - Client sends SOLICIT without any hints.
+// - Client is allocated a reserved address.
+//
+// Note that DHCPv6 client can, but don't have to send any hints in its
+// Solicit message.
+TEST_F(AllocEngine6Test, reservedAddressSolicitNoHint) {
+    // Create reservation for the client (this is in-pool reservation,
+    // as the pool is 2001:db8:1::10 - "2001:db8:1::20
+    createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:1::1c"), 128);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), true);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+}
+
+// Checks that a client gets the address reserved (in-pool case)
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has an in-pool reservation.
+// - Client sends REQUEST without any hints.
+// - Client is allocated a reserved address.
+//
+// Note that DHCPv6 client must send an address in Request that the server
+// offered in Advertise. Nevertheless, the client may ignore this requirement.
+TEST_F(AllocEngine6Test, reservedAddressRequestNoHint) {
+    // Create reservation for the client (this is in-pool reservation,
+    // as the pool is 2001:db8:1::10 - "2001:db8:1::20
+    createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:1::1c"), 128);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("::"), false);
+    ASSERT_TRUE(lease);
+    EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+}
+
+// Checks that a client gets the address reserved (in-pool case)
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has an in-pool reservation.
+// - Client sends SOLICIT with a hint that does not match reservation
+// - Client is allocated a reserved address, not the hint.
+//
+// Note that DHCPv6 client can, but don't have to send any hints in its
+// Solicit message.
+TEST_F(AllocEngine6Test, reservedAddressSolicitValidHint) {
+    // Create reservation for the client (this is in-pool reservation,
+    // as the pool is 2001:db8:1::10 - "2001:db8:1::20
+    createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:1::1c"), 128);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Let's pretend the client sends hint 2001:db8:1::10.
+    Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), true);
+    ASSERT_TRUE(lease);
+
+    // The hint should be ignored and the reserved address should be assigned
+    EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+}
+
+// Checks that a client gets the address reserved (in-pool case)
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has an in-pool reservation.
+// - Client sends Request with a hint that does not match reservation
+// - Client is allocated a reserved address, not the hint.
+//
+// Note that DHCPv6 client must send an address in Request that the server
+// offered in Advertise. Nevertheless, the client may ignore this requirement.
+TEST_F(AllocEngine6Test, reservedAddressRequestValidHint) {
+    // Create reservation for the client (this is in-pool reservation,
+    // as the pool is 2001:db8:1::10 - "2001:db8:1::20
+    createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:1::1c"), 128);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Let's pretend the client sends hint 2001:db8:1::10.
+    Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::10"), false);
+    ASSERT_TRUE(lease);
+
+    // The hint should be ignored and the reserved address should be assigned
+    EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+}
+
+// Checks that a client gets the address reserved (in-pool case)
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has an in-pool reservation.
+// - Client sends SOLICIT with a hint that does matches reservation
+// - Client is allocated a reserved address, not the hint.
+//
+// Note that DHCPv6 client can, but don't have to send any hints in its
+// Solicit message.
+TEST_F(AllocEngine6Test, reservedAddressSolicitMatchingHint) {
+    // Create reservation for the client (this is in-pool reservation,
+    // as the pool is 2001:db8:1::10 - "2001:db8:1::20
+    createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:1::1c"), 128);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Let's pretend the client sends hint 2001:db8:1::10.
+    Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), true);
+    ASSERT_TRUE(lease);
+
+    // The hint should be ignored and the reserved address should be assigned
+    EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+}
+
+// Checks that a client gets the address reserved (in-pool case)
+// This test checks the behavior of the allocation engine in the following
+// scenario:
+// - Client has no lease in the database.
+// - Client has an in-pool reservation.
+// - Client sends Request with a hint that does not match reservation
+// - Client is allocated a reserved address, not the hint.
+//
+// Note that DHCPv6 client must send an address in Request that the server
+// offered in Advertise. Nevertheless, the client may ignore this requirement.
+TEST_F(AllocEngine6Test, reservedAddressRequestMatchingHint) {
+    // Create reservation for the client (this is in-pool reservation,
+    // as the pool is 2001:db8:1::10 - "2001:db8:1::20
+    createHost6(true, IPv6Resrv::TYPE_NA, IOAddress("2001:db8:1::1c"), 128);
+
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Let's pretend the client sends hint 2001:db8:1::10.
+    Lease6Ptr lease = simpleAlloc6Test(pool_, IOAddress("2001:db8:1::1c"), false);
+    ASSERT_TRUE(lease);
+
+    // The hint should be ignored and the reserved address should be assigned
+    EXPECT_EQ("2001:db8:1::1c", lease->addr_.toText());
+}
+
+
 // --- IPv4 ---
 
 // This test checks if the v4 Allocation Engine can be instantiated, parses