[5306] Changes after review:

 - max attempts set to 0 in unit-tests (matching production code)
 - couple typos
 - new unit-test added (selectSharedNetworkByRelayAddressSubnetLevel)
 - ClientContext4 is now noncopyable
 - unused network parameter removed

src/lib/dhcpsrv/alloc_engine.cc

@@ -329,6 +329,9 @@ AllocEngine::findReservationInternal(ContextType& ctx,
             }
         }
 
+        // We need to get to the next subnet if this is a shared network. If it
+        // is not (a plain subnet), getNextSubnet will return NULL and we're
+        // done here.
         subnet = subnet->getNextSubnet(ctx.subnet_, ctx.query_->getClasses());
     }
 }
@@ -442,10 +445,6 @@ AllocEngine::allocateLeases6(ClientContext6& ctx) {
         //       no: release existing leases, assign new ones based on reservations
         // Case 4/catch-all. if there are no leases and no reservations...
         //       assign new leases
-        //
-        // We could implement those checks as nested ifs, but the performance
-        // gain would be minimal and the code readability loss would be substantial.
-        // Hence independent checks.
 
         // Case 1: There are no leases and there's a reservation for this host.
         if (leases.empty() && ctx.currentHost()) {
@@ -2281,9 +2280,6 @@ void findClientLease(AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease)
 /// a pool within any of the subnets belonging to the current shared network.
 bool
 inAllowedPool(AllocEngine::ClientContext4& ctx, const IOAddress& address) {
-    SharedNetwork4Ptr network;
-    ctx.subnet_->getSharedNetwork(network);
-
     // If the subnet belongs to a shared network we will be iterating
     // over the subnets that belong to this shared network.
     Subnet4Ptr current_subnet = ctx.subnet_;
@@ -2835,7 +2831,11 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
         // version of dynamic_pointer_cast.
         Subnet4Ptr subnet4 = boost::dynamic_pointer_cast<Subnet4>(ctx.subnet_);
 
-        // Pass the parameters
+        // Pass the parameters. Note the clientid is passed only if match-client-id
+        // is set. This is done that way, because the lease4-renew hook point is
+        // about renewing a lease and the configuration parameter says the
+        // client-id should be ignored. Hence no clientid value if match-client-id
+        // is false.
         ctx.callout_handle_->setArgument("query4", ctx.query_);
         ctx.callout_handle_->setArgument("subnet4", subnet4);
         ctx.callout_handle_->setArgument("clientid", subnet4->getMatchClientId() ?

src/lib/dhcpsrv/alloc_engine.h

@@ -1038,7 +1038,7 @@ public:
     /// 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 {
+    struct ClientContext4 : public boost::noncopyable {
         /// @brief Subnet selected for the client by the server.
         Subnet4Ptr subnet_;
 

src/lib/dhcpsrv/cfg_subnets4.cc

@@ -128,7 +128,7 @@ CfgSubnets4::selectSubnet(const SubnetSelector& selector) const {
         for (Subnet4Collection::const_iterator subnet = subnets_.begin();
              subnet != subnets_.end(); ++subnet) {
 
-            // If relay information specified for this subnet it must match.
+            // If relay information is specified for this subnet, it must match.
             // Otherwise, we ignore this subnet.
             if (!(*subnet)->getRelayInfo().addr_.isV4Zero()) {
                 if (selector.giaddr_ != (*subnet)->getRelayInfo().addr_) {

src/lib/dhcpsrv/subnet.h

@@ -406,7 +406,7 @@ public:
     ///
     /// If the current subnet doesn't belong to any shared network or if
     /// the next subnet is the same as first subnet (specified in the
-    /// arguments) a NULL pointer is returned.
+    /// argument) a NULL pointer is returned.
     ///
     /// @param first_subnet Pointer to the subnet from which iterations have
     /// started.

src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc

@@ -49,7 +49,7 @@ TEST_F(AllocEngine4Test, constructor) {
 TEST_F(AllocEngine4Test, simpleAlloc4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     // Assigned addresses should be zero.
@@ -84,7 +84,7 @@ TEST_F(AllocEngine4Test, simpleAlloc4) {
 TEST_F(AllocEngine4Test, fakeAlloc4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     // Assigned addresses should be zero.
@@ -120,7 +120,7 @@ TEST_F(AllocEngine4Test, fakeAlloc4) {
 TEST_F(AllocEngine4Test, allocWithValidHint4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
@@ -155,7 +155,7 @@ TEST_F(AllocEngine4Test, allocWithValidHint4) {
 TEST_F(AllocEngine4Test, allocWithUsedHint4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     // Let's create a lease and put it in the LeaseMgr
@@ -202,7 +202,7 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
 TEST_F(AllocEngine4Test, allocBogusHint4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     // Client would like to get a 10.1.1.1 lease, which does not belong to any
@@ -234,7 +234,7 @@ TEST_F(AllocEngine4Test, allocBogusHint4) {
 TEST_F(AllocEngine4Test, allocateLease4Nulls) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     // Allocations without subnet are not allowed
@@ -284,7 +284,7 @@ TEST_F(AllocEngine4Test, allocateLease4Nulls) {
 TEST_F(AllocEngine4Test, simpleRenew4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     EXPECT_TRUE(testStatistics("assigned-addresses", 0, subnet_->getID()));
@@ -393,7 +393,7 @@ TEST_F(AllocEngine4Test, IterativeAllocator_manyPools4) {
 TEST_F(AllocEngine4Test, smallPool4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     IOAddress addr("192.0.2.17");
@@ -438,7 +438,7 @@ TEST_F(AllocEngine4Test, smallPool4) {
 TEST_F(AllocEngine4Test, outOfAddresses4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     IOAddress addr("192.0.2.17");
@@ -479,7 +479,7 @@ TEST_F(AllocEngine4Test, outOfAddresses4) {
 // different subnet than orginally selected, when the address pool in
 // the first subnet is exhausted.
 TEST_F(AllocEngine4Test, discoverSharedNetwork) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create two subnets, each with a single address pool. The first subnet
     // has only one address in its address pool to make it easier to simulate
@@ -507,7 +507,7 @@ TEST_F(AllocEngine4Test, discoverSharedNetwork) {
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
     // Create context which will be used to try to allocate leases from the
-    // shared network. The context poits to subnet1, which address space
+    // shared network. The context points to subnet1, which address space
     // is exhausted. We expect the allocation engine to find another subnet
     // within the same shared network and offer an address from there.
     AllocEngine::ClientContext4
@@ -550,7 +550,7 @@ TEST_F(AllocEngine4Test, discoverSharedNetwork) {
 // different subnet than orginally selected, when the address pool in
 // the first subnet is exhausted.
 TEST_F(AllocEngine4Test, discoverSharedNetworkClassification) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create two subnets, each with a single address pool. The first subnet
     // has only one address in its address pool to make it easier to simulate
@@ -620,7 +620,7 @@ TEST_F(AllocEngine4Test, discoverSharedNetworkClassification) {
 // existing leases regardless in which subnet belonging to a shared network
 // reservations belong.
 TEST_F(AllocEngine4Test, discoverSharedNetworkReservations) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create two subnets, each with a single address pool. The first subnet
     // has only one address in its address pool to make it easier to simulate
@@ -674,7 +674,7 @@ TEST_F(AllocEngine4Test, discoverSharedNetworkReservations) {
 // different subnet than orginally selected, when the address pool in
 // the first subnet is exhausted.
 TEST_F(AllocEngine4Test, reuqestSharedNetwork) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create two subnets, each with a single address pool. The first subnet
     // has only one address in its address pool to make it easier to simulate
@@ -702,7 +702,7 @@ TEST_F(AllocEngine4Test, reuqestSharedNetwork) {
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
     // Create context which will be used to try to allocate leases from the
-    // shared network. The context poits to subnet1, which address space
+    // shared network. The context points to subnet1, which address space
     // is exhausted. We expect the allocation engine to find another subnet
     // within the same shared network and offer an address from there.
     AllocEngine::ClientContext4
@@ -747,7 +747,7 @@ TEST_F(AllocEngine4Test, reuqestSharedNetwork) {
 // different subnet than orginally selected, when the address pool in
 // the first subnet is exhausted.
 TEST_F(AllocEngine4Test, requestSharedNetworkClassification) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create two subnets, each with a single address pool. The first subnet
     // has only one address in its address pool to make it easier to simulate
@@ -818,7 +818,7 @@ TEST_F(AllocEngine4Test, requestSharedNetworkClassification) {
 // existing leases regardless in which subnet belonging to a shared network
 // reservations belong (DHCPREQUEST case).
 TEST_F(AllocEngine4Test, requestSharedNetworkReservations) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create two subnets, each with a single address pool. The first subnet
     // has only one address in its address pool to make it easier to simulate
@@ -876,7 +876,7 @@ TEST_F(AllocEngine4Test, requestSharedNetworkReservations) {
 TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     IOAddress addr("192.0.2.15");
@@ -945,7 +945,7 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
 TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     IOAddress addr("192.0.2.105");
@@ -1007,7 +1007,7 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
 TEST_F(AllocEngine4Test, discoverReuseDeclinedLease4) {
 
     AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                          100, false));
+                                          0, false));
     ASSERT_TRUE(engine);
 
     // Now prepare a configuration with single address pool.
@@ -1045,7 +1045,7 @@ TEST_F(AllocEngine4Test, discoverReuseDeclinedLease4Stats) {
 
     // Now prepare for DISCOVER processing
     AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                          100, false));
+                                          0, false));
     ASSERT_TRUE(engine);
 
     // Now prepare a configuration with single address pool.
@@ -1080,7 +1080,7 @@ TEST_F(AllocEngine4Test, discoverReuseDeclinedLease4Stats) {
 TEST_F(AllocEngine4Test, requestReuseDeclinedLease4) {
 
     AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                          100, false));
+                                          0, false));
     ASSERT_TRUE(engine);
 
     // Now prepare a configuration with single address pool.
@@ -1116,7 +1116,7 @@ TEST_F(AllocEngine4Test, requestReuseDeclinedLease4) {
 TEST_F(AllocEngine4Test, requestReuseDeclinedLease4Stats) {
 
     AllocEnginePtr engine(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                          100, false));
+                                          0, false));
     ASSERT_TRUE(engine);
 
     // Now prepare a configuration with single address pool.
@@ -1158,7 +1158,7 @@ TEST_F(AllocEngine4Test, identifyClientLease) {
                                100, 30, 60, time(NULL), subnet_->getID()));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
                                     IOAddress::IPV4_ZERO_ADDRESS(),
                                     false, false, "", true);
@@ -1237,7 +1237,7 @@ TEST_F(AllocEngine4Test, requestOtherClientLease) {
     LeaseMgrFactory::instance().addLease(lease);
     LeaseMgrFactory::instance().addLease(lease2);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // First client requests the lease which belongs to the second client.
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("192.0.2.102"),
@@ -1275,7 +1275,7 @@ TEST_F(AllocEngine4Test, reservedAddressNoHint) {
     CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
     CfgMgr::instance().commit();
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, 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
@@ -1314,7 +1314,7 @@ TEST_F(AllocEngine4Test,reservedAddressNoHintFakeAllocation) {
     CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
     CfgMgr::instance().commit();
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Query allocation engine for the lease to be assigned to this
     // client without specifying the address to be assigned.
@@ -1355,7 +1355,7 @@ TEST_F(AllocEngine4Test, reservedAddressHint) {
     CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
     CfgMgr::instance().commit();
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     AllocEngine::ClientContext4 ctx1(subnet_, clientid_, hwaddr_,
                                     IOAddress("192.0.2.234"), false, false,
@@ -1404,7 +1404,7 @@ TEST_F(AllocEngine4Test, reservedAddressHintFakeAllocation) {
     CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
     CfgMgr::instance().commit();
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, 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.
@@ -1452,7 +1452,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLease) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Request allocation of the reserved address.
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
@@ -1501,7 +1501,7 @@ TEST_F(AllocEngine4Test, reservedAddressHijacked) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Try to allocate the reserved lease to client B.
     AllocEngine::ClientContext4 ctx1(subnet_, clientid_, hwaddr_,
@@ -1558,7 +1558,7 @@ TEST_F(AllocEngine4Test, reservedAddressHijackedFakeAllocation) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, 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
@@ -1618,7 +1618,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Try to allocate a lease and specify a different address than reserved
     // and different from the one that client is currently using.
@@ -1674,7 +1674,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseFakeAllocation) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Try to allocate a lease and use a completely different address
     // as a hint.
@@ -1739,7 +1739,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseNoHint) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Try to allocate a lease with providing no hint.
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
@@ -1791,7 +1791,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseNoHintFakeAllocation) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Query the allocation engine for the lease to be allocated for the
     // client.
@@ -1855,7 +1855,7 @@ TEST_F(AllocEngine4Test, reservedAddressConflictResolution) {
                                false, false, ""));
     LeaseMgrFactory::instance().addLease(lease);
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
 
     // Client B sends a DHCPREQUEST to allocate a reserved lease. The
@@ -1939,7 +1939,7 @@ TEST_F(AllocEngine4Test, reservedAddressVsDynamicPool) {
     CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
     CfgMgr::instance().commit();
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Different client tries to allocate a lease. Note, that we're using
     // an iterative allocator which would pick the first address from the
@@ -1971,7 +1971,7 @@ TEST_F(AllocEngine4Test, reservedAddressHintUsedByOtherClient) {
     CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
     CfgMgr::instance().commit();
 
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Different client is requesting this address.
     AllocEngine::ClientContext4 ctx1(subnet_, ClientIdPtr(), hwaddr_,
@@ -2002,7 +2002,7 @@ TEST_F(AllocEngine4Test, reservedAddressHintUsedByOtherClient) {
 // address when the pool is exhausted, and the only available
 // address is reserved for a different client.
 TEST_F(AllocEngine4Test, reservedAddressShortPool) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create short pool with only one address.
     initSubnet(IOAddress("192.0.2.100"), IOAddress("192.0.2.100"));
@@ -2044,7 +2044,7 @@ TEST_F(AllocEngine4Test, reservedAddressShortPool) {
 // dynamic pool if the client's reservation is made for a hostname but
 // not for an address.
 TEST_F(AllocEngine4Test, reservedHostname) {
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Create a reservation for a hostname. Address is set to 0 which
     // indicates that there is no reservation.
@@ -2079,7 +2079,7 @@ TEST_F(AllocEngine4Test, reservedHostname) {
 // the value of NULL in the host_ field of the client context.
 TEST_F(AllocEngine4Test, findReservation) {
     // Create the instance of the allocation engine.
-    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 0, false);
 
     // Context is required to call the AllocEngine::findReservation.
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_,
@@ -2154,7 +2154,7 @@ TEST_F(AllocEngine4Test, findReservation) {
 TEST_F(AllocEngine4Test, simpleAlloc4Stats) {
     boost::scoped_ptr<AllocEngine> engine;
     ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE,
-                                                 100, false)));
+                                                 0, false)));
     ASSERT_TRUE(engine);
 
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"),

src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc

@@ -223,6 +223,7 @@ TEST(CfgSubnets4Test, selectSharedNetworkByIface) {
     SharedNetwork4Ptr network_returned;
     selected->getSharedNetwork(network_returned);
     ASSERT_TRUE(network_returned);
+    EXPECT_EQ(network, network_returned);
 
     const Subnet4Collection* subnets_eth1 = network_returned->getAllSubnets();
     EXPECT_EQ(2, subnets_eth1->size());
@@ -248,6 +249,7 @@ TEST(CfgSubnets4Test, selectSharedNetworkByIface) {
     selector.iface_name_ = "eth0";
     selected = cfg.selectSubnet(selector);
     ASSERT_TRUE(selected);
+    EXPECT_EQ(subnet1, selected);
 }
 
 // This test verifies that when the classification information is specified for
@@ -367,7 +369,7 @@ TEST(CfgSubnets4Test, selectSharedNetworkByClasses) {
     selector.client_classes_ = client_classes;
     EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
 
-    // Switch to device-type1 and expect that we're assigned a subnet from
+    // Switch to device-type1 and expect that we assigned a subnet from
     // another shared network.
     client_classes.clear();
     client_classes.insert("device-type1");
@@ -459,7 +461,7 @@ TEST(CfgSubnets4Test, selectSubnetByRelayAddress) {
 
 // This test verifies that the relay information specified on the shared
 // network level can be used to select a subnet.
-TEST(CfgSubnets4Test, selectSharedNetworkByRelayAddress) {
+TEST(CfgSubnets4Test, selectSharedNetworkByRelayAddressNetworkLevel) {
     CfgSubnets4 cfg;
 
     // Create 3 subnets.
@@ -492,6 +494,45 @@ TEST(CfgSubnets4Test, selectSharedNetworkByRelayAddress) {
     EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
 }
 
+// This test verifies that the relay information specified on the subnet
+// level can be used to select a subnet and the fact that a subnet belongs
+// to a shared network doesn't affect the process.
+TEST(CfgSubnets4Test, selectSharedNetworkByRelayAddressSubnetLevel) {
+    CfgSubnets4 cfg;
+
+    // Create 3 subnets.
+    Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.2.0"), 26, 1, 2, 3));
+    Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.64"), 26, 1, 2, 3));
+    Subnet4Ptr subnet3(new Subnet4(IOAddress("192.0.2.128"), 26, 1, 2, 3));
+
+    // Add them to the configuration.
+    cfg.add(subnet1);
+    cfg.add(subnet2);
+    cfg.add(subnet3);
+
+    SharedNetwork4Ptr network1(new SharedNetwork4("network1"));
+    network1->add(subnet1);
+
+    SharedNetwork4Ptr network2(new SharedNetwork4("network2"));
+    network2->add(subnet2);
+
+    SubnetSelector selector;
+
+    // Now specify relay info. Note that for the second subnet we specify
+    // relay info on the network level.
+    subnet1->setRelayInfo(IOAddress("10.0.0.1"));
+    subnet2->setRelayInfo(IOAddress("10.0.0.2"));
+    subnet3->setRelayInfo(IOAddress("10.0.0.3"));
+
+    // And try again. This time relay-info is there and should match.
+    selector.giaddr_ = IOAddress("10.0.0.1");
+    EXPECT_EQ(subnet1, cfg.selectSubnet(selector));
+    selector.giaddr_ = IOAddress("10.0.0.2");
+    EXPECT_EQ(subnet2, cfg.selectSubnet(selector));
+    selector.giaddr_ = IOAddress("10.0.0.3");
+    EXPECT_EQ(subnet3, cfg.selectSubnet(selector));
+}
+
 // This test verifies that the subnet can be selected for the client
 // using a source address if the client hasn't set the ciaddr.
 TEST(CfgSubnets4Test, selectSubnetNoCiaddr) {