Parcourir la source

[master] Merge branch 'trac5364'

# Conflicts:
#	src/bin/dhcp6/tests/config_parser_unittest.cc
Marcin Siodelski il y a 7 ans
Parent
commit
07ce52ee4c

+ 7 - 0
doc/guide/dhcp6-srv.xml

@@ -3182,6 +3182,13 @@ If not specified, the default value is:
     2001:db8:cafe::1 and no server unicast.
     </para>
 
+    <para>Some parameters must be the same in all subnets in the same shared
+    network. This restriction applies to <command>interface</command> and
+    <command>rapid-commit</command> settings. The most convenient way is to
+    define them on shared network scope, but you may specify them for each
+    subnet. However, care should be taken for each subnet to have the same
+    value.</para>
+
     <section>
       <title>Local and relayed traffic in shared networks</title>
 

+ 8 - 0
src/bin/dhcp4/dhcp4_messages.mes

@@ -663,6 +663,14 @@ the client. The first argument includes the client and the
 transaction identification information. The second arguments
 includes the subnet details.
 
+% DHCP4_SUBNET_DYNAMICALLY_CHANGED %1: changed selected subnet %2 to subnet %3 from shared network %4 for client assignments
+This debug message indicates that the server is using another subnet
+than initially selected for client assignments. This newly selected
+subnet belongs to the same shared network as the original subnet.
+Some reasons why the new subnet was selected include: address pool
+exhaustion in the original subnet or the fact that the new subnet
+includes some static reservations for this client.
+
 % DHCP4_SUBNET_SELECTED %1: the subnet with ID %2 was selected for client assignments
 This is a debug message noting the selection of a subnet to be used for
 address and option assignment. Subnet selection is one of the early

+ 12 - 1
src/bin/dhcp4/dhcp4_srv.cc

@@ -1849,7 +1849,18 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
 
     // Subnet may be modified by the allocation engine, if the initial subnet
     // belongs to a shared network.
-    subnet = ctx->subnet_;
+    if (subnet->getID() != ctx->subnet_->getID()) {
+        SharedNetwork4Ptr network;
+        subnet->getSharedNetwork(network);
+        if (network) {
+            LOG_DEBUG(packet4_logger, DBG_DHCP4_BASIC_DATA, DHCP4_SUBNET_DYNAMICALLY_CHANGED)
+                .arg(query->getLabel())
+                .arg(subnet->toText())
+                .arg(ctx->subnet_->toText())
+                .arg(network->getName());
+        }
+        subnet = ctx->subnet_;
+    }
 
     if (lease) {
         // We have a lease! Let's set it in the packet and send it back to

+ 3 - 0
src/bin/dhcp4/tests/dhcp4_client.cc

@@ -300,6 +300,9 @@ Dhcp4Client::doInform(const bool set_ciaddr) {
 
 void
 Dhcp4Client::doRelease() {
+    // There is no response for Release message.
+    context_.response_.reset();
+
     if (config_.lease_.addr_.isV4Zero()) {
         isc_throw(Dhcp4ClientError, "failed to send the release"
                   " message because client doesn't have a lease");

+ 228 - 44
src/bin/dhcp4/tests/shared_network_unittest.cc

@@ -6,19 +6,25 @@
 
 #include <config.h>
 #include <asiolink/io_address.h>
+#include <cc/data.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/tests/iface_mgr_test_config.h>
 #include <dhcp/option.h>
 #include <dhcp/option_int.h>
 #include <dhcp/option_string.h>
+#include <dhcpsrv/cfgmgr.h>
+#include <dhcpsrv/cfg_subnets4.h>
+#include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcp4/tests/dhcp4_client.h>
 #include <dhcp4/tests/dhcp4_test_utils.h>
 #include <stats/stats_mgr.h>
 #include <boost/pointer_cast.hpp>
 #include <boost/shared_ptr.hpp>
+#include <functional>
 
 using namespace isc;
 using namespace isc::asiolink;
+using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
 using namespace isc::stats;
@@ -816,6 +822,23 @@ public:
         StatsMgr::instance().removeAll();
     }
 
+    /// @brief Returns subnet having specified address in range.
+    ///
+    /// @param address Address for which subnet is being searched.
+    /// @return Pointer to the subnet having an address in range or null pointer
+    /// if no subnet found.
+    Subnet4Ptr getConfiguredSubnet(const IOAddress& address) {
+        CfgSubnets4Ptr cfg = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4();
+        const Subnet4Collection* subnets = cfg->getAll();
+        for (auto subnet_it = subnets->cbegin(); subnet_it != subnets->cend();
+             ++subnet_it) {
+            if ((*subnet_it)->inRange(address)) {
+                return (*subnet_it);
+            }
+        }
+        return (Subnet4Ptr());
+    }
+
     /// @brief Perform DORA exchange and checks the result
     ///
     /// This convenience method conducts DORA exchange with client
@@ -844,6 +867,13 @@ public:
         EXPECT_EQ((ack ? DHCPACK : DHCPNAK), resp->getType());
         if (ack) {
             EXPECT_EQ(exp_addr, resp->getYiaddr().toText());
+            Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(IOAddress(resp->getYiaddr()));
+            ASSERT_TRUE(lease);
+            // Make sure that the subnet id in the lease database is not messed up.
+            Subnet4Ptr subnet = getConfiguredSubnet(resp->getYiaddr());
+            ASSERT_TRUE(subnet);
+            ASSERT_EQ(subnet->getID(), lease->subnet_id_);
+
         } else {
             EXPECT_EQ("0.0.0.0", resp->getYiaddr().toText());
         }
@@ -902,9 +932,74 @@ public:
         } else {
             EXPECT_EQ(DHCPACK, resp->getType());
             EXPECT_EQ(exp_addr, resp->getYiaddr().toText());
+            Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(IOAddress(resp->getYiaddr()));
+            ASSERT_TRUE(lease);
+            // Make sure that the subnet id in the lease database is not messed up.
+            Subnet4Ptr subnet = getConfiguredSubnet(resp->getYiaddr());
+            ASSERT_TRUE(subnet);
+            ASSERT_EQ(subnet->getID(), lease->subnet_id_);
         }
     }
 
+    /// @brief Verifies lease statistics against values held by StatsMgr.
+    ///
+    /// This method retrieves lease statistics from the database and then compares it
+    /// against values held by the StatsMgr. The compared statistics are number of
+    /// assigned addresses and prefixes for a subnet.
+    void verifyAssignedStats() {
+        LeaseStatsQueryPtr query = LeaseMgrFactory::instance().startLeaseStatsQuery4();
+        LeaseStatsRow row;
+        while (query->getNextRow(row)) {
+            // Only check valid leases.
+            if (row.lease_state_ == Lease::STATE_DEFAULT) {
+                ASSERT_EQ(row.state_count_, getStatsAssignedAddresses(row.subnet_id_))
+                    << "test failed for subnet id " << row.subnet_id_;
+            }
+        }
+    }
+
+    /// @brief Retrieves subnet[id].assigned-addresses statistics for a subnet.
+    ///
+    /// @param subnet_id Identifier of a subnet for which statistics should be
+    /// retrieved.
+    /// @return Number of assigned addresses for a subnet.
+    int64_t getStatsAssignedAddresses(const SubnetID& subnet_id) const {
+        // Retrieve statistics name, e.g. subnet[1234].assigned-addresses.
+        const std::string stats_name = StatsMgr::generateName("subnet", subnet_id, "assigned-addresses");
+        // Top element is a map with a subnet[id].assigned-addresses parameter.
+        ConstElementPtr top_element = StatsMgr::instance().get(stats_name);
+        if (top_element && (top_element->getType() == Element::map)) {
+            // It contains two lists (nested).
+            ConstElementPtr first_list = top_element->get(stats_name);
+            if (first_list && (first_list->getType() == Element::list) &&
+                (first_list->size() > 0)) {
+                // Get the nested list which should have two elements, of which first
+                // is the statistics value we're looking for.
+                ConstElementPtr second_list = first_list->get(0);
+                if (second_list && (second_list->getType() == Element::list) &&
+                    (second_list->size() == 2)) {
+                    ConstElementPtr addresses_element = second_list->get(0);
+                    if (addresses_element && (addresses_element->getType() == Element::integer)) {
+                        return (addresses_element->intValue());
+                    }
+                }
+            }
+        }
+
+        // Statistics invalid or not found.
+        return (0);
+    }
+
+    /// @brief Launches specific operation and verifies lease statistics before and
+    /// after this operation.
+    ///
+    /// @param operation Operation to be launched.
+    void testAssigned(const std::function<void()>& operation) {
+        ASSERT_NO_FATAL_FAILURE(verifyAssignedStats());
+        operation();
+        ASSERT_NO_FATAL_FAILURE(verifyAssignedStats());
+    }
+
     /// @brief Destructor.
     virtual ~Dhcpv4SharedNetworkTest() {
         StatsMgr::instance().removeAll();
@@ -926,33 +1021,45 @@ TEST_F(Dhcpv4SharedNetworkTest, poolInSharedNetworkShortage) {
 
     // Client #1 requests an address in first subnet within a shared network.
     // We'll send a hint of 192.0.2.63 and expect to get it.
-    doDORA(client1, "192.0.2.63", "192.0.2.63");
+    testAssigned([this, &client1]() {
+        doDORA(client1, "192.0.2.63", "192.0.2.63");
+    });
 
     // Client #2 The second client will request a lease and should be assigned
     // an address from the second subnet.
     Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
     client2.setIfaceName("eth1");
-    doDORA(client2, "10.0.0.16");
+    testAssigned([this, &client2]() {
+        doDORA(client2, "10.0.0.16");
+    });
 
     // Client #3. It sends DHCPDISCOVER which should be dropped by the server because
     // the server has no more addresses to assign.
     Dhcp4Client client3(client1.getServer(), Dhcp4Client::SELECTING);
     client3.setIfaceName("eth1");
-    ASSERT_NO_THROW(client3.doDiscover());
-    Pkt4Ptr resp3 = client3.getContext().response_;
-    ASSERT_FALSE(resp3);
+    testAssigned([this, &client3]() {
+        ASSERT_NO_THROW(client3.doDiscover());
+        Pkt4Ptr resp3 = client3.getContext().response_;
+        ASSERT_FALSE(resp3);
+    });
 
     // Client #3 should be assigned an address if subnet 3 is selected for this client.
     client3.setIfaceName("eth0");
-    doDORA(client3, "192.0.2.65");
+    testAssigned([this, &client3]() {
+        doDORA(client3, "192.0.2.65");
+    });
 
     // Client #1 should be able to renew its address.
     client1.setState(Dhcp4Client::RENEWING);
-    doRequest(client1, "192.0.2.63");
+    testAssigned([this, &client1]() {
+        doRequest(client1, "192.0.2.63");
+    });
 
     // Client #2 should be able to renew its address.
     client2.setState(Dhcp4Client::RENEWING);
-    doRequest(client2, "10.0.0.16");
+    testAssigned([this, &client2]() {
+        doRequest(client2, "10.0.0.16");
+    });
 }
 
 // Shared network is selected based on giaddr value (relay specified
@@ -968,13 +1075,17 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectedByRelay1) {
     configure(NETWORKS_CONFIG[1], *client1.getServer());
 
     // Client #1 should be assigned an address from shared network.
-    doDORA(client1, "192.0.2.63", "192.0.2.63");
+    testAssigned([this, &client1] {
+        doDORA(client1, "192.0.2.63", "192.0.2.63");
+    });
 
     // Create client #2. This is a relayed client which is using relay
     // address matching subnet outside of the shared network.
     Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
     client2.useRelay(true, IOAddress("192.1.2.3"), IOAddress("10.0.0.3"));
-    doDORA(client2, "192.0.2.65", "192.0.2.63");
+    testAssigned([this, &client2] {
+        doDORA(client2, "192.0.2.65", "192.0.2.63");
+    });
 }
 
 // Shared network is selected based on giaddr value (relay specified
@@ -992,19 +1103,25 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectedByRelay2) {
     configure(NETWORKS_CONFIG[14], *client1.getServer());
 
     // Client #1 should be assigned an address from shared network.
-    doDORA(client1, "192.0.2.63");
+    testAssigned([this, &client1] {
+        doDORA(client1, "192.0.2.63");
+    });
 
     // Create client #2. This is a relayed client which is using relay
     // address that is used for subnet 2 in the shared network.
     Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
     client2.useRelay(true, IOAddress("192.2.2.2"), IOAddress("10.0.0.3"));
-    doDORA(client2, "10.0.0.16");
+    testAssigned([this, &client2] {
+        doDORA(client2, "10.0.0.16");
+    });
 
     // Create client #3. This is a relayed client which is using relay
     // address matching subnet outside of the shared network.
     Dhcp4Client client3(client1.getServer(), Dhcp4Client::SELECTING);
     client3.useRelay(true, IOAddress("192.3.3.3"), IOAddress("10.0.0.4"));
-    doDORA(client3, "192.0.2.65");
+    testAssigned([this, &client3] {
+        doDORA(client3, "192.0.2.65");
+    });
 }
 
 // Providing a hint for any address belonging to a shared network.
@@ -1019,15 +1136,22 @@ TEST_F(Dhcpv4SharedNetworkTest, hintWithinSharedNetwork) {
 
     // Provide a hint to an existing address within first subnet. This address
     // should be offered out of this subnet.
-    doDiscover(client, "192.0.2.63", "192.0.2.63");
+    testAssigned([this, &client] {
+        doDiscover(client, "192.0.2.63", "192.0.2.63");
+    });
 
     // Similarly, we should be offered an address from another subnet within
     // the same shared network when we ask for it.
-    doDiscover(client, "10.0.0.16", "10.0.0.16");
+    testAssigned([this, &client] {
+        doDiscover(client, "10.0.0.16", "10.0.0.16");
+    });
 
     // Asking for an address that is not in address pool should result in getting
     // an address from one of the subnets, but generally hard to tell from which one.
-    ASSERT_NO_THROW(client.doDiscover(boost::shared_ptr<IOAddress>(new IOAddress("10.0.0.23"))));
+    testAssigned([this, &client] {
+        ASSERT_NO_THROW(client.doDiscover(boost::shared_ptr<IOAddress>(new IOAddress("10.0.0.23"))));
+    });
+
     Pkt4Ptr resp = client.getContext().response_;
     ASSERT_TRUE(resp);
 
@@ -1052,24 +1176,32 @@ TEST_F(Dhcpv4SharedNetworkTest, subnetInSharedNetworkSelectedByClass) {
 
     // Client #1 requests an address in the restricted subnet but can't be assigned
     // this address because the client doesn't belong to a certain class.
-    doDORA(client1, "10.0.0.16", "192.0.2.63");
+    testAssigned([this, &client1] {
+        doDORA(client1, "10.0.0.16", "192.0.2.63");
+    });
 
     // Release the lease that the client has got, because we'll need this address
     // further in the test.
-    ASSERT_NO_THROW(client1.doRelease());
+    testAssigned([this, &client1] {
+        ASSERT_NO_THROW(client1.doRelease());
+    });
 
     // Add option93 which would cause the client to be classified as "a-devices".
     OptionPtr option93(new OptionUint16(Option::V4, 93, 0x0001));
     client1.addExtraOption(option93);
 
     // This time, the allocation of the address provided as hint should be successful.
-    doDORA(client1, "192.0.2.63", "192.0.2.63");
+    testAssigned([this, &client1] {
+        doDORA(client1, "192.0.2.63", "192.0.2.63");
+    });
 
     // Client 2 should be assigned an address from the unrestricted subnet.
     Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
     client2.useRelay(true, IOAddress("192.3.5.6"));
     client2.setIfaceName("eth1");
-    doDORA(client2, "10.0.0.16");
+    testAssigned([this, &client2] {
+        doDORA(client2, "10.0.0.16");
+    });
 
     // Now, let's reconfigure the server to also apply restrictions on the
     // subnet to which client2 now belongs.
@@ -1078,12 +1210,18 @@ TEST_F(Dhcpv4SharedNetworkTest, subnetInSharedNetworkSelectedByClass) {
     // The client should be refused to renew the lease because it doesn't belong
     // to "b-devices" class.
     client2.setState(Dhcp4Client::RENEWING);
-    doRequest(client2, "");
+    testAssigned([this, &client2] {
+        doRequest(client2, "");
+    });
 
     // If we add option93 with a value matching this class, the lease should
     // get renewed.
     OptionPtr option93_bis(new OptionUint16(Option::V4, 93, 0x0002));
     client2.addExtraOption(option93_bis);
+
+    testAssigned([this, &client2] {
+        doRequest(client2, "10.0.0.16");
+    });
 }
 
 // IPv4 address reservation exists in one of the subnets within
@@ -1101,7 +1239,9 @@ TEST_F(Dhcpv4SharedNetworkTest, reservationInSharedNetwork) {
     configure(NETWORKS_CONFIG[4], *client1.getServer());
 
     // Client #1 should get his reserved address from the second subnet.
-    doDORA(client1, "10.0.0.29", "192.0.2.28");
+    testAssigned([this, &client1] {
+        doDORA(client1, "10.0.0.29", "192.0.2.28");
+    });
 
     // Create client #2
     Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
@@ -1109,7 +1249,9 @@ TEST_F(Dhcpv4SharedNetworkTest, reservationInSharedNetwork) {
     client2.setHWAddress("aa:bb:cc:dd:ee:ff");
 
     // Client #2 should get its reserved address from the first subnet.
-    doDORA(client2, "192.0.2.28");
+    testAssigned([this, &client2] {
+        doDORA(client2, "192.0.2.28");
+    });
 
     // Reconfigure the server. Now, the first client gets second client's
     // reservation and vice versa.
@@ -1117,18 +1259,24 @@ TEST_F(Dhcpv4SharedNetworkTest, reservationInSharedNetwork) {
 
     // The first client is trying to renew the lease and should get a DHCPNAK.
     client1.setState(Dhcp4Client::RENEWING);
-    doRequest(client1, "");
+    testAssigned([this, &client1] {
+        doRequest(client1, "");
+    });
 
     // Similarly, the second client is trying to renew the lease and should
     // get a DHCPNAK.
     client2.setState(Dhcp4Client::RENEWING);
-    doRequest(client2, "");
+    testAssigned([this, &client2] {
+        doRequest(client2, "");
+    });
 
     // But the client should get a lease, if it does 4-way exchange. However, it
     // must not get any of the reserved addresses because one of them is reserved
     // for another client and for another one there is a valid lease.
     client1.setState(Dhcp4Client::SELECTING);
-    ASSERT_NO_THROW(client1.doDORA());
+    testAssigned([this, &client1] {
+        ASSERT_NO_THROW(doDORA(client1, "192.0.2.29", "192.0.2.29"));
+    });
     Pkt4Ptr resp1 = client1.getContext().response_;
     ASSERT_TRUE(resp1);
     EXPECT_EQ(DHCPACK, resp1->getType());
@@ -1138,11 +1286,15 @@ TEST_F(Dhcpv4SharedNetworkTest, reservationInSharedNetwork) {
     // Client #2 is now doing 4-way exchange and should get its newly reserved
     // address, released by the 4-way transaction of client 1.
     client2.setState(Dhcp4Client::SELECTING);
-    doDORA(client2, "10.0.0.29");
+    testAssigned([this, &client2] {
+        doDORA(client2, "10.0.0.29");
+    });
 
     // Same for client #1.
     client1.setState(Dhcp4Client::SELECTING);
-    doDORA(client1, "192.0.2.28");
+    testAssigned([this, &client1] {
+        doDORA(client1, "192.0.2.28");
+    });
 }
 
 // Reserved address can't be assigned as long as access to a subnet is
@@ -1160,7 +1312,9 @@ TEST_F(Dhcpv4SharedNetworkTest, reservationAccessRestrictedByClass) {
 
     // Assigned address should be allocated from the second subnet, because the
     // client doesn't belong to the "a-devices" class.
-    doDORA(client, "10.0.0.16");
+    testAssigned([this, &client] {
+        doDORA(client, "10.0.0.16");
+    });
 
     // Add option 93 which would cause the client to be classified as "a-devices".
     OptionPtr option93(new OptionUint16(Option::V4, 93, 0x0001));
@@ -1169,11 +1323,15 @@ TEST_F(Dhcpv4SharedNetworkTest, reservationAccessRestrictedByClass) {
     // Client renews its lease and should get DHCPNAK because this client now belongs
     // to the "a-devices" class and can be assigned a reserved address instead.
     client.setState(Dhcp4Client::RENEWING);
-    doRequest(client, "");
+    testAssigned([this, &client] {
+        doRequest(client, "");
+    });
 
     // Perform 4-way exchange again. It should be assigned a reserved address this time.
     client.setState(Dhcp4Client::SELECTING);
-    doDORA(client, "192.0.2.28");
+    testAssigned([this, &client] {
+        doDORA(client, "192.0.2.28");
+    });
 }
 
 // Some options are specified on the shared subnet level, some on the
@@ -1262,7 +1420,9 @@ TEST_F(Dhcpv4SharedNetworkTest, initReboot) {
 
     // Perform 4-way exchange to obtain a lease. The client should get the lease from
     // the second subnet.
-    doDORA(client1, "10.0.0.16", "10.0.0.16");
+    testAssigned([this, &client1] {
+        doDORA(client1, "10.0.0.16", "10.0.0.16");
+    });
 
     // The client1 transitions to INIT-REBOOT state in which the client1 remembers the
     // lease and sends DHCPREQUEST to all servers (server id) is not specified. If
@@ -1270,7 +1430,9 @@ TEST_F(Dhcpv4SharedNetworkTest, initReboot) {
     // drop the request. We want to make sure that the server responds (resp1) regardless
     // of the subnet from which the lease has been allocated.
     client1.setState(Dhcp4Client::INIT_REBOOT);
-    doRequest(client1, "10.0.0.16");
+    testAssigned([this, &client1] {
+        doRequest(client1, "10.0.0.16");
+    });
 
     // Create client #2.
     Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
@@ -1278,7 +1440,9 @@ TEST_F(Dhcpv4SharedNetworkTest, initReboot) {
 
     // Let's make sure that the behavior is the same for the other subnet within the
     // same shared network.
-    doDORA(client2, "192.0.2.63", "192.0.2.63");
+    testAssigned([this, &client2] {
+        doDORA(client2, "192.0.2.63", "192.0.2.63");
+    });
 
     // The client2 transitions to INIT-REBOOT state in which the client2 remembers the
     // lease and sends DHCPREQUEST to all servers (server id) is not specified. If
@@ -1286,7 +1450,9 @@ TEST_F(Dhcpv4SharedNetworkTest, initReboot) {
     // drop the request. We want to make sure that the server responds (resp2) regardless
     // of the subnet from which the lease has been allocated.
     client2.setState(Dhcp4Client::INIT_REBOOT);
-    doRequest(client2, "192.0.2.63");
+    testAssigned([this, &client2] {
+        doRequest(client2, "192.0.2.63");
+    });
 }
 
 // Host reservations include hostname, next server and client class.
@@ -1305,7 +1471,9 @@ TEST_F(Dhcpv4SharedNetworkTest, variousFieldsInReservation) {
     configure(NETWORKS_CONFIG[10], *client.getServer());
 
     // Perform 4-way exchange.
-    ASSERT_NO_THROW(client.doDORA());
+    testAssigned([this, &client] {
+        ASSERT_NO_THROW(client.doDORA());
+    });
     Pkt4Ptr resp = client.getContext().response_;
     ASSERT_TRUE(resp);
     EXPECT_EQ(DHCPACK, resp->getType());
@@ -1349,7 +1517,9 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectionByInterface) {
     configure(NETWORKS_CONFIG[8], *client1.getServer());
 
     // Perform 4-way exchange.
-    ASSERT_NO_THROW(client1.doDORA());
+    testAssigned([this, &client1] {
+        ASSERT_NO_THROW(client1.doDORA());
+    });
     Pkt4Ptr resp1 = client1.getContext().response_;
     ASSERT_TRUE(resp1);
     EXPECT_EQ(DHCPACK, resp1->getType());
@@ -1362,7 +1532,9 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectionByInterface) {
     client2.setIfaceName("eth0");
 
     // Perform 4-way exchange.
-    ASSERT_NO_THROW(client2.doDORA());
+    testAssigned([this, &client2] {
+        ASSERT_NO_THROW(client2.doDORA());
+    });
     Pkt4Ptr resp2 = client2.getContext().response_;
     ASSERT_TRUE(resp2);
     EXPECT_EQ(DHCPACK, resp2->getType());
@@ -1382,7 +1554,9 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectionByRelay) {
     configure(NETWORKS_CONFIG[9], *client1.getServer());
 
     // Perform 4-way exchange.
-    ASSERT_NO_THROW(client1.doDORA());
+    testAssigned([this, &client1] {
+        ASSERT_NO_THROW(client1.doDORA());
+    });
     Pkt4Ptr resp1 = client1.getContext().response_;
     ASSERT_TRUE(resp1);
     EXPECT_EQ(DHCPACK, resp1->getType());
@@ -1395,7 +1569,9 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectionByRelay) {
     client2.useRelay(true, IOAddress("192.1.2.3"));
 
     // Perform 4-way exchange.
-    ASSERT_NO_THROW(client2.doDORA());
+    testAssigned([this, &client2] {
+        ASSERT_NO_THROW(client2.doDORA());
+    });
     Pkt4Ptr resp2 = client2.getContext().response_;
     ASSERT_TRUE(resp2);
     EXPECT_EQ(DHCPACK, resp2->getType());
@@ -1417,7 +1593,9 @@ TEST_F(Dhcpv4SharedNetworkTest, matchClientId) {
     configure(NETWORKS_CONFIG[11], *client.getServer());
 
     // Perform 4-way exchange.
-    ASSERT_NO_THROW(client.doDORA());
+    testAssigned([this, &client] {
+        ASSERT_NO_THROW(client.doDORA());
+    });
     Pkt4Ptr resp1 = client.getContext().response_;
     ASSERT_TRUE(resp1);
     ASSERT_EQ(DHCPACK, resp1->getType());
@@ -1432,7 +1610,9 @@ TEST_F(Dhcpv4SharedNetworkTest, matchClientId) {
     client.setState(Dhcp4Client::RENEWING);
 
     // Try to renew the lease with modified MAC address.
-    ASSERT_NO_THROW(client.doRequest());
+    testAssigned([this, &client] {
+        ASSERT_NO_THROW(client.doRequest());
+    });
     Pkt4Ptr resp2 = client.getContext().response_;
     ASSERT_TRUE(resp2);
     ASSERT_EQ(DHCPACK, resp2->getType());
@@ -1457,7 +1637,9 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectedByClass) {
     configure(NETWORKS_CONFIG[13], *client1.getServer());
 
     // Simply send DHCPDISCOVER to avoid allocating a lease.
-    ASSERT_NO_THROW(client1.doDiscover());
+    testAssigned([this, &client1] {
+        ASSERT_NO_THROW(client1.doDiscover());
+    });
     Pkt4Ptr resp1 = client1.getContext().response_;
     ASSERT_TRUE(resp1);
     ASSERT_EQ(DHCPOFFER, resp1->getType());
@@ -1474,7 +1656,9 @@ TEST_F(Dhcpv4SharedNetworkTest, sharedNetworkSelectedByClass) {
 
     // Send DHCPDISCOVER. There is no lease in the lease database so the
     // client should be offered a lease based on the client class selection.
-    doDiscover(client2, "192.0.2.63", "");
+    testAssigned([this, &client2] {
+        doDiscover(client2, "192.0.2.63", "");
+    });
 }
 
 } // end of anonymous namespace

+ 8 - 0
src/bin/dhcp6/dhcp6_messages.mes

@@ -711,6 +711,14 @@ the client. The first argument includes the client and the
 transaction identification information. The second argument
 includes the subnet details.
 
+% DHCP6_SUBNET_DYNAMICALLY_CHANGED %1: changed selected subnet %2 to subnet %3 from shared network %4 for client assignments
+This debug message indicates that the server is using another subnet
+than initially selected for client assignments. This newly selected
+subnet belongs to the same shared network as the original subnet.
+Some reasons why the new subnet was selected include: address pool
+exhaustion in the original subnet or the fact that the new subnet
+includes some static reservations for this client.
+
 % DHCP6_SUBNET_SELECTED %1: the subnet with ID %2 was selected for client assignments
 This is a debug message noting the selection of a subnet to be used for
 address and option assignment. Subnet selection is one of the early

+ 16 - 0
src/bin/dhcp6/dhcp6_srv.cc

@@ -1195,6 +1195,8 @@ void
 Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer,
                         AllocEngine::ClientContext6& ctx) {
 
+    Subnet6Ptr subnet = ctx.subnet_;
+
     // We need to allocate addresses for all IA_NA options in the client's
     // question (i.e. SOLICIT or REQUEST) message.
     // @todo add support for IA_TA
@@ -1232,6 +1234,20 @@ Dhcpv6Srv::assignLeases(const Pkt6Ptr& question, Pkt6Ptr& answer,
             break;
         }
     }
+
+    // Subnet may be modified by the allocation engine, if the initial subnet
+    // belongs to a shared network.
+    if (ctx.subnet_ && subnet && (subnet->getID() != ctx.subnet_->getID())) {
+        SharedNetwork6Ptr network;
+        subnet->getSharedNetwork(network);
+        if (network) {
+            LOG_DEBUG(packet6_logger, DBG_DHCP6_BASIC_DATA, DHCP6_SUBNET_DYNAMICALLY_CHANGED)
+                .arg(question->getLabel())
+                .arg(subnet->toText())
+                .arg(ctx.subnet_->toText())
+                .arg(network->getName());
+        }
+    }
 }
 
 

+ 29 - 4
src/bin/dhcp6/json_config_parser.cc

@@ -245,8 +245,33 @@ public:
 
             const Subnet6Collection* subnets = (*net)->getAllSubnets();
             if (subnets) {
+
+                bool rapid_commit;
+
                 // For each subnet, add it to a list of regular subnets.
                 for (auto subnet = subnets->begin(); subnet != subnets->end(); ++subnet) {
+
+                    // Rapid commit must either be enabled or disabled in all subnets
+                    // in the shared network.
+                    if (subnet == subnets->begin()) {
+                        // If this is the first subnet, remember the value.
+                        rapid_commit = (*subnet)->getRapidCommit();
+                    } else {
+                        // Ok, this is the second or following subnets. The value
+                        // must match what was set in the first subnet.
+                        if (rapid_commit != (*subnet)->getRapidCommit()) {
+                            isc_throw(DhcpConfigError, "All subnets in a shared network "
+                                      "must have the same rapid-commit value. Subnet "
+                                      << (*subnet)->toText()
+                                      << " has specified rapid-commit "
+                                      << ( (*subnet)->getRapidCommit() ? "true" : "false")
+                                      << ", but earlier subnet in the same shared-network"
+                                      << " or the shared-network itself used rapid-commit "
+                                      << (rapid_commit ? "true" : "false"));
+                        }
+                    }
+
+
                     if (iface.empty()) {
                         iface = (*subnet)->getIface();
                         continue;
@@ -284,8 +309,8 @@ public:
 
         }
     }
-    
-    
+
+
 };
 
 } // anonymous namespace
@@ -562,7 +587,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set,
         // it checks that there is no conflict between plain subnets and those
         // defined as part of shared networks.
         global_parser.sanityChecks(srv_config, mutable_cfg);
-        
+
     } catch (const isc::Exception& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL)
                   .arg(config_pair.first).arg(ex.what());
@@ -597,7 +622,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set,
 
             // Setup the command channel.
             configureCommandChannel();
-            
+
             // No need to commit interface names as this is handled by the
             // CfgMgr::commit() function.
 

+ 131 - 2
src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -5676,7 +5676,7 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDerive) {
         "        },\n"
         "        \"valid-lifetime\": 400, \n"
         "        \"interface-id\": \"twotwo\",\n"
-        "        \"rapid-commit\": false,\n"
+        "        \"rapid-commit\": true,\n"
         "        \"reservation-mode\": \"out-of-pool\"\n"
         "    }\n"
         "    ]\n"
@@ -5732,7 +5732,7 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDerive) {
     ASSERT_TRUE(s->getInterfaceId());
     EXPECT_TRUE(iface_id2.equals(s->getInterfaceId()));
     EXPECT_EQ(IOAddress("2222::2"), s->getRelayInfo().addr_);
-    EXPECT_FALSE(s->getRapidCommit());
+    EXPECT_TRUE(s->getRapidCommit());
     EXPECT_EQ(Network::HR_OUT_OF_POOL, s->getHostReservationMode());
 
     // Ok, now check the second shared subnet.
@@ -5837,6 +5837,38 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDeriveInterfaces) {
     EXPECT_EQ("", s->getIface());
 }
 
+
+// It is not allowed to have different values for interfaces names is subnets
+// in the same shared network.
+TEST_F(Dhcp6ParserTest, sharedNetworksInterfacesMixed) {
+
+    // We need to fake the interfaces present, because we want to test
+    // interface names inheritance. However, there are sanity checks
+    // on subnet level that would refuse the value if the interface
+    // is not present.
+    IfaceMgrTestConfig iface_config(true);
+
+    string config = "{\n"
+        "\"shared-networks\": [ {\n"
+        "    \"name\": \"foo\"\n,"
+        "    \"subnet6\": [\n"
+        "    { \n"
+        "        \"subnet\": \"2001:db1::/48\",\n"
+        "        \"interface\": \"eth0\"\n"
+        "    },\n"
+        "    { \n"
+        "        \"subnet\": \"2001:db2::/48\",\n"
+        "        \"interface\": \"eth1\"\n"
+        "    }\n"
+        "    ]\n"
+        " } ]\n"
+        "} \n";
+
+    configure(config, CONTROL_RESULT_ERROR, "Subnet 2001:db2::/48 has specified "
+              "interface eth1, but earlier subnet in the same shared-network "
+              "or the shared-network itself used eth0");
+}
+
 // This test checks if client-class is derived properly.
 TEST_F(Dhcp6ParserTest, sharedNetworksDeriveClientClass) {
 
@@ -5921,4 +5953,101 @@ TEST_F(Dhcp6ParserTest, sharedNetworksDeriveClientClass) {
     EXPECT_TRUE(classes.empty());
 }
 
+// Tests if rapid-commit is derived properly.
+TEST_F(Dhcp6ParserTest, sharedNetworksRapidCommit) {
+
+    string config = "{\n"
+        "\"shared-networks\": [ {\n"
+        "    \"name\": \"frog\"\n,"
+        "    \"rapid-commit\": true,\n"
+        "    \"subnet6\": [\n"
+        "    { \n"
+        "        \"subnet\": \"2001:db1::/48\",\n"
+        "        \"pools\": [ { \"pool\": \"2001:db1::/64\" } ]\n"
+        "    },\n"
+        "    { \n"
+        "        \"subnet\": \"2001:db2::/48\",\n"
+        "        \"pools\": [ { \"pool\": \"2001:db2::/64\" } ],\n"
+        "        \"client-class\": \"beta\"\n"
+        "    }\n"
+        "    ]\n"
+        " },\n"
+        "{ // second shared-network starts here\n"
+        "    \"name\": \"bar\",\n"
+        "    \"rapid-commit\": false,\n"
+        "    \"subnet6\": [\n"
+        "    {\n"
+        "        \"subnet\": \"2001:db3::/48\",\n"
+        "        \"pools\": [ { \"pool\": \"2001:db3::/64\" } ]\n"
+        "    }\n"
+        "    ]\n"
+        "} ]\n"
+        "} \n";
+
+    configure(config, CONTROL_RESULT_SUCCESS, "");
+
+    // Now verify that the shared network was indeed configured.
+    CfgSharedNetworks6Ptr cfg_net = CfgMgr::instance().getStagingCfg()
+        ->getCfgSharedNetworks6();
+
+    // Two shared networks are expeced.
+    ASSERT_TRUE(cfg_net);
+    const SharedNetwork6Collection* nets = cfg_net->getAll();
+    ASSERT_TRUE(nets);
+    ASSERT_EQ(2, nets->size());
+
+    // Let's check the first one.
+    SharedNetwork6Ptr net = nets->at(0);
+    ASSERT_TRUE(net);
+
+    const Subnet6Collection * subs = net->getAllSubnets();
+    ASSERT_TRUE(subs);
+    ASSERT_EQ(2, subs->size());
+    EXPECT_TRUE(subs->at(0)->getRapidCommit());
+    EXPECT_TRUE(subs->at(1)->getRapidCommit());
+
+    // Ok, now check the second shared network. It doesn't have
+    // anything defined on shared-network or subnet level, so
+    // everything should have default values.
+    net = nets->at(1);
+    ASSERT_TRUE(net);
+
+    subs = net->getAllSubnets();
+    ASSERT_TRUE(subs);
+    EXPECT_EQ(1, subs->size());
+
+    // This subnet should derive its renew-timer from global scope.
+    EXPECT_FALSE(subs->at(0)->getRapidCommit());
+}
+
+// Tests that non-matching rapid-commit setting for subnets belonging to a
+// shared network cause configuration error.
+TEST_F(Dhcp6ParserTest, sharedNetworksRapidCommitMix) {
+
+    string config = "{\n"
+        "\"shared-networks\": [ {\n"
+        "    \"name\": \"frog\"\n,"
+        "    \"subnet6\": [\n"
+        "    { \n"
+        "        \"subnet\": \"2001:db1::/48\",\n"
+        "        \"rapid-commit\": true,\n"
+        "        \"pools\": [ { \"pool\": \"2001:db1::/64\" } ]\n"
+        "    },\n"
+        "    { \n"
+        "        \"subnet\": \"2001:db2::/48\",\n"
+        "        \"rapid-commit\": false,\n"
+        "        \"pools\": [ { \"pool\": \"2001:db2::/64\" } ],\n"
+        "        \"client-class\": \"beta\"\n"
+        "    }\n"
+        "    ]\n"
+        " } ]\n"
+        "} \n";
+
+    configure(config, CONTROL_RESULT_ERROR, "All subnets in a shared network "
+              "must have the same rapid-commit value. Subnet 2001:db2::/48 has "
+              "specified rapid-commit false, but earlier subnet in the same "
+              "shared-network or the shared-network itself used rapid-commit true");
+}
+
+
 };

+ 40 - 2
src/bin/dhcp6/tests/dhcp6_client.cc

@@ -24,6 +24,7 @@
 #include <algorithm>
 #include <cstdlib>
 #include <time.h>
+#include <utility>
 
 using namespace isc::asiolink;
 using namespace isc::dhcp;
@@ -108,7 +109,8 @@ Dhcp6Client::Dhcp6Client() :
     use_client_id_(true),
     use_rapid_commit_(false),
     client_ias_(),
-    fqdn_() {
+    fqdn_(),
+    interface_id_() {
 }
 
 Dhcp6Client::Dhcp6Client(boost::shared_ptr<NakedDhcpv6Srv>& srv) :
@@ -125,7 +127,8 @@ Dhcp6Client::Dhcp6Client(boost::shared_ptr<NakedDhcpv6Srv>& srv) :
     use_client_id_(true),
     use_rapid_commit_(false),
     client_ias_(),
-    fqdn_() {
+    fqdn_(),
+    interface_id_() {
 }
 
 void
@@ -899,7 +902,14 @@ Dhcp6Client::sendMsg(const Pkt6Ptr& msg) {
             relay.peeraddr_ = asiolink::IOAddress("fe80::1");
             relay.msg_type_ = DHCPV6_RELAY_FORW;
             relay.hop_count_ = 1;
+
+            // Interface identifier, if specified.
+            if (interface_id_) {
+                relay.options_.insert(std::make_pair(D6O_INTERFACE_ID, interface_id_));
+            }
+
             msg->relay_info_.push_back(relay);
+
         } else {
             // The test provided relay_info_, let's use that.
             msg->relay_info_ = relay_info_;
@@ -933,6 +943,12 @@ Dhcp6Client::requestPrefix(const uint32_t iaid,
 }
 
 void
+Dhcp6Client::useInterfaceId(const std::string& interface_id) {
+    OptionBuffer buf(interface_id.begin(), interface_id.end());
+    interface_id_.reset(new Option(Option::V6, D6O_INTERFACE_ID, buf));
+}
+
+void
 Dhcp6Client::useFQDN(const uint8_t flags, const std::string& fqdn_name,
                      Option6ClientFqdn::DomainNameType fqdn_type) {
     fqdn_.reset(new Option6ClientFqdn(flags, fqdn_name, fqdn_type));
@@ -943,6 +959,28 @@ Dhcp6Client::addExtraOption(const OptionPtr& opt) {
     extra_options_.insert(std::make_pair(opt->getType(), opt));
 }
 
+void
+Dhcp6Client::clearExtraOptions() {
+    extra_options_.clear();
+}
+
+void
+Dhcp6Client::printConfiguration() const {
+
+    // Print DUID
+    std::cout << "Client " << (duid_ ? duid_->toText() : "(without duid)")
+              << " got " << getLeaseNum() << " lease(s): ";
+
+    // Print leases
+    for (int i = 0; i < getLeaseNum(); i++) {
+        Lease6 lease = getLease(i);
+        std::cout << lease.addr_.toText() << " ";
+    }
+
+    /// @todo: Print many other parameters.
+    std::cout << std::endl;
+}
+
 } // end of namespace isc::dhcp::test
 } // end of namespace isc::dhcp
 } // end of namespace isc

+ 17 - 0
src/bin/dhcp6/tests/dhcp6_client.h

@@ -639,6 +639,11 @@ public:
         relay_link_addr_ = link_addr;
     }
 
+    /// @brief Sets interface id value to be inserted into relay agent option.
+    ///
+    /// @param interface_id Value of the interface id as string.
+    void useInterfaceId(const std::string& interface_id);
+
     /// @brief Controls whether the client should send a client-id or not
     /// @param send should the client-id be sent?
     void useClientId(const bool send) {
@@ -741,6 +746,15 @@ public:
     /// @param opt additional option to be sent
     void addExtraOption(const OptionPtr& opt);
 
+    /// @brief Configures the client to not send any extra options.
+    void clearExtraOptions();
+
+    /// @brief Debugging method the prints currently held configuration
+    ///
+    /// @todo: This is mostly useful when debugging tests. This method
+    /// is incomplete. Please extend it when needed.
+    void printConfiguration() const;
+
 private:
 
     /// @brief Applies the new leases for the client.
@@ -906,6 +920,9 @@ private:
 
     /// @brief FQDN requested by the client.
     Option6ClientFqdnPtr fqdn_;
+
+    /// @brief Interface id.
+    OptionPtr interface_id_;
 };
 
 } // end of namespace isc::dhcp::test

Fichier diff supprimé car celui-ci est trop grand
+ 922 - 180
src/bin/dhcp6/tests/shared_network_unittest.cc


+ 22 - 4
src/lib/dhcpsrv/alloc_engine.cc

@@ -813,6 +813,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
                 // there's no existing lease for selected candidate, so it is
                 // free. Let's allocate it.
 
+                ctx.subnet_ = subnet;
                 Lease6Ptr lease = createLease6(ctx, candidate, prefix_len);
                 if (lease) {
                     // We are allocating a new lease (not renewing). So, the
@@ -838,6 +839,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) {
                     Lease6Ptr old_lease(new Lease6(*existing));
                     ctx.currentIA().old_leases_.push_back(old_lease);
 
+                    ctx.subnet_ = subnet;
                     existing = reuseExpiredLease(existing, ctx, prefix_len);
 
                     leases.push_back(existing);
@@ -1508,9 +1510,25 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
         return;
     }
 
-    // Check if the lease still belongs to the subnet. If it doesn't,
-    // we'll need to remove it.
-    if ((lease->type_ != Lease::TYPE_PD) && !ctx.subnet_->inRange(lease->addr_)) {
+    // It is likely that the lease for which we're extending the lifetime doesn't
+    // belong to the current but a sibling subnet.
+    if (ctx.subnet_->getID() != lease->subnet_id_) {
+        SharedNetwork6Ptr network;
+        ctx.subnet_->getSharedNetwork(network);
+        if (network) {
+            Subnet6Ptr subnet = network->getSubnet(SubnetID(lease->subnet_id_));
+            // Found the actual subnet this lease belongs to. Stick to this
+            // subnet.
+            if (subnet) {
+                ctx.subnet_ = subnet;
+            }
+        }
+    }
+
+    // Check if the lease still belongs to the subnet and that the use of this subnet
+    // is allowed per client classification. If not, remove this lease.
+    if (((lease->type_ != Lease::TYPE_PD) && !ctx.subnet_->inRange(lease->addr_)) ||
+        !ctx.subnet_->clientSupported(ctx.query_->getClasses())) {
         // Oh dear, the lease is no longer valid. We need to get rid of it.
 
         // Remove this lease from LeaseMgr
@@ -2859,7 +2877,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
 
         // Need to decrease statistic for assigned addresses.
         StatsMgr::instance().addValue(
-            StatsMgr::generateName("subnet", ctx.subnet_->getID(), "assigned-addresses"),
+            StatsMgr::generateName("subnet", client_lease->subnet_id_, "assigned-addresses"),
             static_cast<int64_t>(-1));
     }
 

+ 12 - 21
src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -325,11 +325,7 @@ public:
     ///
     /// The result set is populated by iterating over the IPv4 leases in
     /// storage, in ascending order by address, accumulating the lease state
-    /// counts per subnet. Note that walking the leases by address should
-    /// inherently group them by subnet, and while this does not guarantee
-    /// ascending order of subnet id, it should be sufficient to accumulate
-    /// state counts per subnet.  This avoids introducing an additional
-    /// subnet_id index.
+    /// counts per subnet.
     /// At the completion of all entries for a given subnet, the counts are
     /// used to create LeaseStatsRow instances which are appended to an
     /// internal vector.  The process results in a vector containing one entry
@@ -340,8 +336,8 @@ public:
     /// - Lease::STATE_DEFAULT (i.e. assigned)
     /// - Lease::STATE_DECLINED
     void start() {
-        const Lease4StorageAddressIndex& idx
-            = storage4_.get<AddressIndexTag>();
+        const Lease4StorageSubnetIdIndex& idx
+            = storage4_.get<SubnetIdIndexTag>();
 
         // Iterate over the leases in order by subnet, accumulating per
         // subnet counts for each state of interest.  As we finish each
@@ -349,7 +345,7 @@ public:
         SubnetID cur_id = 0;
         int64_t assigned = 0;
         int64_t declined = 0;
-        for(Lease4StorageAddressIndex::const_iterator lease = idx.begin();
+        for(Lease4StorageSubnetIdIndex::const_iterator lease = idx.begin();
             lease != idx.end(); ++lease) {
             // If we've hit the next subnet, add rows for the current subnet
             // and wipe the accumulators
@@ -417,16 +413,11 @@ public:
     /// @brief Creates the IPv6 lease statistical data result set
     ///
     /// The result set is populated by iterating over the IPv6 leases in
-    /// storage, in ascending order by address, accumulating the lease state
-    /// counts per subnet. Note that walking the leases by address should
-    /// inherently group them by subnet, and while this does not guarantee
-    /// ascending order of subnet id, it should be sufficient to accumulate
-    /// state counts per subnet.  This avoids introducing an additional
-    /// subnet_id index.
-    /// At the completion of all entries for a given subnet, the counts
-    /// are used to create LeaseStatsRow instances which are appended to an
-    /// internal vector.  The process results in a vector containing one entry
-    /// per state per lease type per subnet.
+    /// storage, in ascending order by subnet id, accumulating the lease state
+    /// counts per subnet. At the completion of all entries for a given subnet,
+    /// the counts are used to create LeaseStatsRow instances which are appended
+    /// to an internal vector.  The process results in a vector containing one
+    /// entry per state per lease type per subnet.
     ///
     /// Currently the states counted are:
     ///
@@ -434,8 +425,8 @@ public:
     /// - Lease::STATE_DECLINED
     virtual void start() {
         // Get the subnet_id index
-        const Lease6StorageAddressIndex& idx
-            = storage6_.get<AddressIndexTag>();
+        const Lease6StorageSubnetIdIndex& idx
+            = storage6_.get<SubnetIdIndexTag>();
 
         // Iterate over the leases in order by subnet, accumulating per
         // subnet counts for each state of interest.  As we finish each
@@ -445,7 +436,7 @@ public:
         int64_t declined = 0;
         int64_t assigned_pds = 0;
 
-        for(Lease6StorageAddressIndex::const_iterator lease = idx.begin();
+        for(Lease6StorageSubnetIdIndex::const_iterator lease = idx.begin();
             lease != idx.end(); ++lease) {
 
             // If we've hit the next subnet, add rows for the current subnet