[#1405] do not allocate in pool reservations when out of pool is specified

src/bin/dhcp6/tests/sarr_unittest.cc

@@ -74,7 +74,7 @@ const char* CONFIGS[] = {
         " } ],"
         "\"valid-lifetime\": 4000 }",
 
-// Configuration 1
+    // Configuration 1
     "{ \"interfaces-config\": {"
         "  \"interfaces\": [ \"*\" ]"
         "},"
@@ -99,7 +99,7 @@ const char* CONFIGS[] = {
         "     \"qualifying-suffix\" : \"example.com\" }"
     "}",
 
-// Configuration 2
+    // Configuration 2
     "{ \"interfaces-config\": {"
         "  \"interfaces\": [ \"*\" ]"
         "},"
@@ -180,7 +180,33 @@ const char* CONFIGS[] = {
         "    \"interface-id\": \"\","
         "    \"interface\": \"eth0\""
         " } ],"
-        "\"valid-lifetime\": 4000 }"
+        "\"valid-lifetime\": 4000"
+    "}",
+
+    // Configuration 4
+    "{ \"interfaces-config\": {"
+        "  \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"preferred-lifetime\": 3000,"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet6\": [ { "
+        "    \"pools\": [ { \"pool\": \"2001:db8:1::1 - 2001:db8:1::10\" } ],"
+        "    \"subnet\": \"2001:db8:1::/48\", "
+        "    \"interface\": \"eth0\", "
+        "    \"reservation-mode\": \"out-of-pool\","
+        "    \"reservations\": [ "
+        "       {"
+        "         \"duid\": \"aa:bb:cc:dd:ee:ff\","
+        "         \"ip-addresses\": [\"2001:db8:1::20\"]"
+        "       },"
+        "       {"
+        "         \"duid\": \"11:22:33:44:55:66\","
+        "         \"ip-addresses\": [\"2001:db8:1::5\"]"
+        "       }"
+        "    ]"
+        "} ]"
+    "}"
 };
 
 /// @brief Test fixture class for testing 4-way exchange: Solicit-Advertise,
@@ -624,5 +650,62 @@ TEST_F(SARRTest, pkt6ReceiveDropStat3) {
     EXPECT_EQ(1, pkt6_recv_drop->getInteger().first);
 }
 
+// This test verifies that in pool reservations are ignored when the
+// reservation mode is set to "out-of-pool".
+TEST_F(SARRTest, reservationModeOutOfPool) {
+    // Create the first client for which we have a reservation out of the
+    // dynamic pool.
+    Dhcp6Client client;
+    configure(CONFIGS[4], *client.getServer());
+    client.setDUID("aa:bb:cc:dd:ee:ff");
+    client.setInterface("eth0");
+    client.requestAddress(1234, IOAddress("2001:db8:1::3"));
+
+    // Perform 4-way exchange.
+    ASSERT_NO_THROW(client.doSARR());
+    // Server should have assigned a prefix.
+    ASSERT_EQ(1, client.getLeaseNum());
+
+    Lease6 lease = client.getLease(0);
+    // Check that the server allocated the reserved address.
+    ASSERT_EQ("2001:db8:1::20", lease.addr_.toText());
+
+    client.clearConfig();
+    // Create another client which has a reservation within the pool.
+    // The server should ignore this reservation in the current mode.
+    client.setDUID("11:22:33:44:55:66");
+    // This client is requesting a different address than reserved. The
+    // server should allocate this address to the client.
+    // Perform 4-way exchange.
+    ASSERT_NO_THROW(client.doSARR());
+    // Server should have assigned a prefix.
+    ASSERT_EQ(1, client.getLeaseNum());
+
+    lease = client.getLease(0);
+    // Check that the requested address was assigned.
+    ASSERT_EQ("2001:db8:1::3", lease.addr_.toText());
+}
+
+// This test verifies that the in-pool reservation can be assigned to
+// the client not owning this reservation when the reservation mode is
+// set to "out-of-pool".
+TEST_F(SARRTest, reservationIgnoredInOutOfPoolMode) {
+    // Create the first client for which we have a reservation out of the
+    // dynamic pool.
+    Dhcp6Client client;
+    configure(CONFIGS[4], *client.getServer());
+    client.setDUID("12:34:56:78:9A:BC");
+    client.setInterface("eth0");
+    client.requestAddress(1234, IOAddress("2001:db8:1::5"));
+
+    // Perform 4-way exchange.
+    ASSERT_NO_THROW(client.doSARR());
+    // Server should have assigned a prefix.
+    ASSERT_EQ(1, client.getLeaseNum());
+
+    Lease6 lease = client.getLease(0);
+    // Check that the server allocated the reserved address.
+    ASSERT_EQ("2001:db8:1::5", lease.addr_.toText());
+}
 
 } // end of anonymous namespace

src/lib/dhcpsrv/alloc_engine.cc

@@ -536,12 +536,20 @@ ConstHostPtr
 AllocEngine::ClientContext6::currentHost() const {
     Subnet6Ptr subnet = host_subnet_ ? host_subnet_ : subnet_;
     if (subnet) {
-        SubnetID id = (subnet_->getHostReservationMode() & Network::HR_GLOBAL ?
+        SubnetID id = subnet->getID();
-                       SUBNET_ID_GLOBAL : subnet->getID());
+        // if reservation mode is explicitly set to global search only by
+        // SUBNET_ID_GLOBAL.
+        if (subnet_->getHostReservationMode() == Network::HR_GLOBAL) {
+            id = SUBNET_ID_GLOBAL;
+        }
 
         auto host = hosts_.find(id);
         if (host != hosts_.cend()) {
             return (host->second);
+        } else if (id != SUBNET_ID_GLOBAL) {
+            // nothing found for specific subnet ID leads to search for
+            // SUBNET_ID_GLOBAL if HR_GLOBAL_FLAG is set.
+            return (globalHost());
         }
     }
 
@@ -551,7 +559,7 @@ AllocEngine::ClientContext6::currentHost() const {
 ConstHostPtr
 AllocEngine::ClientContext6::globalHost() const {
     Subnet6Ptr subnet = host_subnet_ ? host_subnet_ : subnet_;
-    if (subnet && (subnet_->getHostReservationMode() & Network::HR_GLOBAL)) {
+    if (subnet && (subnet_->getHostReservationMode() & Network::HR_GLOBAL_FLAG)) {
         auto host = hosts_.find(SUBNET_ID_GLOBAL);
         if (host != hosts_.cend()) {
             return (host->second);
@@ -599,7 +607,7 @@ void AllocEngine::findReservation(ClientContext6& ctx) {
     SharedNetwork6Ptr network;
     subnet->getSharedNetwork(network);
 
-    if (subnet->getHostReservationMode() & Network::HR_GLOBAL) {
+    if (subnet->getHostReservationMode() & Network::HR_GLOBAL_FLAG) {
         ConstHostPtr ghost = findGlobalReservation(ctx);
         if (ghost) {
             ctx.hosts_[SUBNET_ID_GLOBAL] = ghost;
@@ -931,7 +939,13 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
             // it has been reserved for us we would have already allocated a lease.
 
             ConstHostCollection hosts;
-            if (hr_mode != Network::HR_DISABLED) {
+            bool in_subnet = (hr_mode & Network::HR_IN_SUBNET_FLAG);
+            bool out_of_pool = (hr_mode & Network::HR_OUT_OF_POOL_FLAG);
+            // The HR_OUT_OF_POOL_FLAG indicates that no client should be assigned reservations
+            // from within the dynamic pool, and for that reason we only look at reservations that
+            // are outside the pools, hence the inPool check.
+            if ((in_subnet && !out_of_pool) ||
+                (out_of_pool && (!ctx.subnet_->inPool(ctx.currentIA().type_, hint)))) {
                 hosts = getIPv6Resrv(subnet->getID(), hint);
             }
 
@@ -965,7 +979,13 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
 
             // If the lease is expired, we may likely reuse it, but...
             ConstHostCollection hosts;
-            if (hr_mode != Network::HR_DISABLED) {
+            bool in_subnet = (hr_mode & Network::HR_IN_SUBNET_FLAG);
+            bool out_of_pool = (hr_mode & Network::HR_OUT_OF_POOL_FLAG);
+            // The HR_OUT_OF_POOL_FLAG indicates that no client should be assigned reservations
+            // from within the dynamic pool, and for that reason we only look at reservations that
+            // are outside the pools, hence the inPool check.
+            if ((in_subnet && !out_of_pool) ||
+                (out_of_pool && (!ctx.subnet_->inPool(ctx.currentIA().type_, hint)))) {
                 hosts = getIPv6Resrv(subnet->getID(), hint);
             }
 
@@ -1260,6 +1280,12 @@ AllocEngine::allocateReservedLeases6(ClientContext6& ctx,
 
         ConstHostPtr host = ctx.hosts_[subnet_id];
 
+        // Check which host reservation mode is supported in this subnet.
+        Network::HRMode hr_mode = subnet->getHostReservationMode();
+
+        bool in_subnet = (hr_mode & Network::HR_IN_SUBNET_FLAG);
+        bool out_of_pool = (hr_mode & Network::HR_OUT_OF_POOL_FLAG);
+
         // Get the IPv6 reservations of specified type.
         const IPv6ResrvRange& reservs = host->getIPv6Reservations(type);
         BOOST_FOREACH(IPv6ResrvTuple type_lease_tuple, reservs) {
@@ -1273,6 +1299,15 @@ AllocEngine::allocateReservedLeases6(ClientContext6& ctx,
                 continue;
             }
 
+            // The HR_OUT_OF_POOL_FLAG indicates that no client should be assigned reservations
+            // from within the dynamic pool, and for that reason we only look at reservations that
+            // are outside the pools, hence the inPool check.
+            if ((in_subnet && !out_of_pool) ||
+                (out_of_pool && (!subnet->inPool(ctx.currentIA().type_, addr)))) {
+            } else {
+                continue;
+            }
+
             // If there's a lease for this address, let's not create it.
             // It doesn't matter whether it is for this client or for someone else.
             if (!LeaseMgrFactory::instance().getLease6(ctx.currentIA().type_,
@@ -2954,10 +2989,16 @@ namespace {
 /// @return true if the address is reserved for another client.
 bool
 addressReserved(const IOAddress& address, const AllocEngine::ClientContext4& ctx) {
+    if (!ctx.subnet_) {
+        return false;
+    }
     bool in_subnet = (ctx.subnet_->getHostReservationMode() & Network::HR_IN_SUBNET_FLAG);
     bool out_of_pool = (ctx.subnet_->getHostReservationMode() & Network::HR_OUT_OF_POOL_FLAG);
-    if (ctx.subnet_ && ((in_subnet && !out_of_pool) ||
+    // The HR_OUT_OF_POOL_FLAG indicates that no client should be assigned reservations
-        (out_of_pool && (!ctx.subnet_->inPool(Lease::TYPE_V4, address))))) {
+    // from within the dynamic pool, and for that reason we only look at reservations that
+    // are outside the pools, hence the inPool check.
+    if ((in_subnet && !out_of_pool) ||
+        (out_of_pool && (!ctx.subnet_->inPool(Lease::TYPE_V4, address)))) {
         // The global parameter ip-reservations-unique controls whether it is allowed
         // to specify multiple reservations for the same IP address or delegated prefix
         // or IP reservations must be unique. Some host backends do not support the
@@ -3019,7 +3060,7 @@ hasAddressReservation(AllocEngine::ClientContext4& ctx) {
 
     Subnet4Ptr subnet = ctx.subnet_;
     while (subnet) {
-        if (subnet->getHostReservationMode() & Network::HR_GLOBAL) {
+        if (subnet->getHostReservationMode() & Network::HR_GLOBAL_FLAG) {
             auto host = ctx.hosts_.find(SUBNET_ID_GLOBAL);
             bool found = (host != ctx.hosts_.end() &&
                     !(host->second->getIPv4Reservation().isV4Zero()));
@@ -3037,6 +3078,9 @@ hasAddressReservation(AllocEngine::ClientContext4& ctx) {
         auto host = ctx.hosts_.find(subnet->getID());
         bool in_subnet = (subnet->getHostReservationMode() & Network::HR_IN_SUBNET_FLAG);
         bool out_of_pool = (subnet->getHostReservationMode() & Network::HR_OUT_OF_POOL_FLAG);
+        // The HR_OUT_OF_POOL_FLAG indicates that no client should be assigned reservations
+        // from within the dynamic pool, and for that reason we only look at reservations that
+        // are outside the pools, hence the inPool check.
         if (host != ctx.hosts_.end()) {
             auto reservation = host->second->getIPv4Reservation();
             if (!reservation.isV4Zero() &&
@@ -3219,12 +3263,20 @@ AllocEngine::ClientContext4::ClientContext4(const Subnet4Ptr& subnet,
 ConstHostPtr
 AllocEngine::ClientContext4::currentHost() const {
     if (subnet_) {
-        SubnetID id = (subnet_->getHostReservationMode() & Network::HR_GLOBAL ?
+        SubnetID id = subnet_->getID();
-                       SUBNET_ID_GLOBAL : subnet_->getID());
+        // if reservation mode is explicitly set to global search only by
+        // SUBNET_ID_GLOBAL.
+        if (subnet_->getHostReservationMode() == Network::HR_GLOBAL) {
+            id = SUBNET_ID_GLOBAL;
+        }
 
         auto host = hosts_.find(id);
         if (host != hosts_.cend()) {
             return (host->second);
+        } else if (id != SUBNET_ID_GLOBAL) {
+            // nothing found for specific subnet ID leads to search for
+            // SUBNET_ID_GLOBAL if HR_GLOBAL_FLAG is set.
+            return (globalHost());
         }
     }
     return (ConstHostPtr());
@@ -3232,7 +3284,7 @@ AllocEngine::ClientContext4::currentHost() const {
 
 ConstHostPtr
 AllocEngine::ClientContext4::globalHost() const {
-    if (subnet_ && (subnet_->getHostReservationMode() & Network::HR_GLOBAL)) {
+    if (subnet_ && (subnet_->getHostReservationMode() & Network::HR_GLOBAL_FLAG)) {
         auto host = hosts_.find(SUBNET_ID_GLOBAL);
         if (host != hosts_.cend()) {
             return (host->second);
@@ -3319,7 +3371,7 @@ AllocEngine::findReservation(ClientContext4& ctx) {
     SharedNetwork4Ptr network;
     subnet->getSharedNetwork(network);
 
-    if (subnet->getHostReservationMode() & Network::HR_GLOBAL) {
+    if (subnet->getHostReservationMode() & Network::HR_GLOBAL_FLAG) {
         ConstHostPtr ghost = findGlobalReservation(ctx);
         if (ghost) {
             ctx.hosts_[SUBNET_ID_GLOBAL] = ghost;