[3563] CfgHosts::get6(subnet_id, addr) implemented, with unit-tests.

src/lib/dhcpsrv/cfg_hosts.cc

@@ -184,17 +184,64 @@ CfgHosts::get6(const IOAddress&, const uint8_t) {
 }
 
 ConstHostPtr
-CfgHosts::get6(const SubnetID& subnet_id, const IOAddress& address) const {
-    ConstHostCollection hosts = getAll6(address);
-    for (ConstHostCollection::const_iterator host = hosts.begin();
-         host != hosts.end(); ++host) {
-        if ((*host)->getIPv4SubnetID() == subnet_id) {
-            return (*host);
-        }
+CfgHosts::get6(const SubnetID& subnet_id,
+               const asiolink::IOAddress& address) const {
+    ConstHostCollection storage;
+    getAllInternal6(subnet_id, address, storage);
+
+    switch (storage.size()) {
+    case 0:
+        return (ConstHostPtr());
+    case 1:
+        return (*storage.begin());
+    default:
+        isc_throw(DuplicateHost,  "more than one reservation found"
+                  " for the host belonging to the subnet with id '"
+                  << subnet_id << "' and using the address '"
+                  << address.toText() << "'");
     }
-    return (ConstHostPtr());
 }
 
+template<typename Storage>
+void
+CfgHosts::getAllInternal6(const SubnetID& subnet_id,
+                          const asiolink::IOAddress& address,
+                          Storage& storage) const {
+    // Must not specify address other than IPv6.
+    if (!address.isV6()) {
+        isc_throw(BadHostAddress, "must specify an IPv6 address when searching"
+                  " for a host, specified address was " << address);
+    }
+
+    // Let's get all reservations that match subnet_id, address.
+    const HostContainer6Index1& idx = hosts6_.get<1>();
+    HostContainer6Index1Range r = idx.equal_range(boost::make_tuple(subnet_id, address));
+
+    // For each IPv6 reservation, add the host to the results list. Fortunately,
+    // in all sane cases, there will be only one such host. (Each host can have
+    // multiple addresses reserved, but for each (address, subnet_id) there should
+    // be at most one host reserving it).
+    for(HostContainer6Index1::iterator resrv = r.first; resrv != r.second; ++resrv) {
+        storage.push_back(resrv->host_);
+    }
+}
+
+HostPtr
+CfgHosts::get6(const SubnetID& subnet_id, const asiolink::IOAddress& address) {
+    HostCollection storage;
+    getAllInternal6<HostCollection>(subnet_id, address, storage);
+    switch (storage.size()) {
+    case 0:
+        return (HostPtr());
+    case 1:
+        return (*storage.begin());
+    default:
+        isc_throw(DuplicateHost,  "more than one reservation found"
+                  " for the host belonging to the subnet with id '"
+                  << subnet_id << "' and using the address '"
+                  << address.toText() << "'");
+    }
+}
 
 HostPtr
 CfgHosts::getHostInternal(const SubnetID& subnet_id, const bool subnet6,
@@ -253,13 +300,9 @@ CfgHosts::add(const HostPtr& host) {
                   " 0 when adding new host reservation");
     }
 
-    if (host->getIPv4SubnetID() != 0) {
-        add4(host);
-    }
+    add4(host);
 
-    if (host->getIPv6SubnetID() != 0) {
-        add6(host);
-    }
+    add6(host);
 }
 
 void
@@ -294,7 +337,17 @@ CfgHosts::add4(const HostPtr& host) {
                   << "' to the IPv4 subnet id '" << host->getIPv4SubnetID()
                   << "' as this host has already been added");
 
+
+    // Check for duplicates for the specified IPv6 subnet.
+    } else if (host->getIPv6SubnetID() &&
+               get6(host->getIPv6SubnetID(), duid, hwaddr)) {
+        isc_throw(DuplicateHost, "failed to add new host using the HW"
+                  " address '" << (hwaddr ? hwaddr->toText(false) : "(null)")
+                  << " and DUID '" << (duid ? duid->toText() : "(null)")
+                  << "' to the IPv6 subnet id '" << host->getIPv6SubnetID()
+                  << "' as this host has already been added");
     }
+
     /// @todo This may need further sanity checks.
 
     // This is a new instance - add it.
@@ -308,37 +361,30 @@ CfgHosts::add6(const HostPtr& host) {
     HWAddrPtr hwaddr = host->getHWAddress();
     DuidPtr duid = host->getDuid();
 
-    // Check for duplicates for the specified IPv6 subnet.
-    if (host->getIPv6SubnetID() &&
-               get6(host->getIPv6SubnetID(), duid, hwaddr)) {
-        isc_throw(DuplicateHost, "failed to add new host using the HW"
-                  " address '" << (hwaddr ? hwaddr->toText(false) : "(null)")
-                  << " and DUID '" << (duid ? duid->toText() : "(null)")
-                  << "' to the IPv6 subnet id '" << host->getIPv6SubnetID()
-                  << "' as this host has already been added");
-    }
-
-    // Now insert it into hosts_, which will be used for finding hosts
-    // based on their HW or DUID addresses. It cannot be used for
-    // finding IPv6 hosts by their IPv6 addresses, as there may be multiple
-    // addresses for a given host. However, insert only if this
-    // host doesn't have v4 subnet-id. If it does, it was just added
-    // by the previous call to add4().
-    if (! host->getIPv4SubnetID()) {
-        hosts_.insert(host);
-    }
-
     // Get all reservations for this host.
     IPv6ResrvRange reservations = host->getIPv6Reservations();
 
+    // Check if there are any IPv6 reservations.
     if (std::distance(reservations.first, reservations.second) == 0) {
-
-        /// @todo: We don't handle address-less reservations yet
+        // If there aren't, we don't need to add this to hosts6_, which is used
+        // for getting hosts by their IPv6 address reservations.
         return;
     }
 
+    // Now for each reservation, insert corresponding (address, host) tuple.
     for (IPv6ResrvIterator it = reservations.first; it != reservations.second;
          ++it) {
+
+        // If there's an entry for this (subnet-id, address), reject it.
+        if (get6(host->getIPv6SubnetID(), it->second.getPrefix())) {
+            isc_throw(DuplicateHost, "failed to add address reservation for "
+                      << "host using the HW address '"
+                      << (hwaddr ? hwaddr->toText(false) : "(null)")
+                      << " and DUID '" << (duid ? duid->toText() : "(null)")
+                      << "' to the IPv6 subnet id '" << host->getIPv6SubnetID()
+                      << "' for address/prefix " << it->second.getPrefix()
+                      << ": There's already reservation for this address/prefix");
+        }
         hosts6_.insert(HostResrv6Tuple(it->second, host));
     }
 }

src/lib/dhcpsrv/cfg_hosts.h

@@ -297,6 +297,25 @@ private:
     void getAllInternal6(const asiolink::IOAddress& address,
                          Storage& storage) const;
 
+
+    /// @brief Returns @c Host objects for the specified (Subnet-id,IPv6 address) tuple.
+    ///
+    /// This private method is called by the @c CfgHosts::getAll6 methods
+    /// to retrieve the @c Host for which the specified IPv6 address is
+    /// reserved and is in specified subnet-id. The retrieved objects are
+    /// appended to the @c storage container.
+    ///
+    /// @param subnet_id Subnet Identifier.
+    /// @param address IPv6 address.
+    /// @param [out] storage Container to which the retrieved objects are
+    /// appended.
+    /// @tparam One of the @c ConstHostCollection or @c HostCollection.
+    template<typename Storage>
+    void
+    getAllInternal6(const SubnetID& subnet_id,
+                    const asiolink::IOAddress& address,
+                    Storage& storage) const;
+
     /// @brief Returns @c Host object connected to a subnet.
     ///
     /// This private method returns a pointer to the @c Host object identified
@@ -316,8 +335,6 @@ private:
                             const HWAddrPtr& hwaddr,
                             const DuidPtr& duid) const;
 
-
-
     /// @brief Adds a new host to the v4 collection.
     ///
     /// This is an internal method called by public @ref add.

src/lib/dhcpsrv/host_container.h

@@ -22,6 +22,7 @@
 #include <boost/multi_index/mem_fun.hpp>
 #include <boost/multi_index/member.hpp>
 #include <boost/multi_index/ordered_index.hpp>
+#include <boost/multi_index/hashed_index.hpp>
 
 namespace isc {
 namespace dhcp {
@@ -101,17 +102,29 @@ typedef std::pair<HostContainerIndex1::iterator,
 /// @brief Defines one entry for the Host Container for v6 hosts
 ///
 /// It's essentially a pair of (IPv6 reservation, Host pointer).
+/// This structure is used as an intermediate structure in HostContainer6.
+/// For a single host that has reservations for X addresses or prefixes, there
+/// will be X HostResrv6Tuple structures.
 struct HostResrv6Tuple {
 
-    /// @brief Default constructor
-    HostResrv6Tuple(const IPv6Resrv& resrv, const ConstHostPtr& host)
+    /// @brief Default constructor.
+    ///
+    /// @param resrv IPv6 address/prefix reservation
+    /// @param host pointer to the host object
+    HostResrv6Tuple(const IPv6Resrv& resrv, const HostPtr& host)
     :resrv_(resrv), host_(host), subnet_id_(host? host->getIPv6SubnetID() : 0) {
     }
 
+    /// @brief Address or prefix reservation.
     const IPv6Resrv resrv_;
-    ConstHostPtr host_;
+
+    /// @brief Pointer to the host object.
+    HostPtr host_;
+
+    /// @brief Value of the IPv6 Subnet-id
     const SubnetID subnet_id_;
 
+    /// @brief Key extractor (used in the second composite key)
     const asiolink::IOAddress& getKey() const {
         return (resrv_.getPrefix());
     }
@@ -124,48 +137,69 @@ struct HostResrv6Tuple {
 /// for a given IPv6 address or prefix.
 typedef boost::multi_index_container<
 
-    /// This containers stores (IPv6Resrv, HostPtr) tuples
+    // This containers stores (IPv6Resrv, HostPtr) tuples
     HostResrv6Tuple,
 
-    /// Start specification of indexes here.
+    // Start specification of indexes here.
     boost::multi_index::indexed_by<
 
-        /// First index is used to search by an address.
+        // First index is used to search by an address.
         boost::multi_index::ordered_non_unique<
 
-            /// Address is extracted by calling IPv6Resrv::getPrefix()
-            /// and it will return an IOAddress object.
+            // Address is extracted by calling IPv6Resrv::getPrefix()
+            // and it will return an IOAddress object.
             boost::multi_index::const_mem_fun<
-                HostResrv6Tuple, const asiolink::IOAddress&,
-                &HostResrv6Tuple::getKey
-            >
+                HostResrv6Tuple, const asiolink::IOAddress&, &HostResrv6Tuple::getKey>
         >,
 
-        /// Second index is used to search by (subnet_id, address) pair
-        /// @todo: Let's make the first index working first.
-        boost::multi_index::ordered_non_unique<
+        // Second index is used to search by (subnet_id, address) pair.
+        // This is
+        boost::multi_index::ordered_unique<
 
+            /// This is a composite key. It uses two keys: subnet-id and
+            /// IPv6 address reservation.
             boost::multi_index::composite_key<
-                  HostResrv6Tuple,
 
-                  boost::multi_index::member<
-                    HostResrv6Tuple, const SubnetID,
-                    &HostResrv6Tuple::subnet_id_
-                    >,
+                // Composite key uses members of the HostResrv6Tuple class.
+                HostResrv6Tuple,
+
+                // First key extractor. Gets subnet-id as a member of the
+                // HostResrv6Tuple structure.
+                boost::multi_index::member<HostResrv6Tuple, const SubnetID,
+                    &HostResrv6Tuple::subnet_id_>,
 
-                /// Address is extracted by calling IPv6Resrv::getPrefix()
-                /// and it will return an IOAddress object.
+                // Second key extractor. Address is extracted by calling
+                // IPv6Resrv::getPrefix() and it will return an IOAddress object.
                 boost::multi_index::const_mem_fun<
                     HostResrv6Tuple, const asiolink::IOAddress&,
                     &HostResrv6Tuple::getKey
                 >
            >
         >
-
+    >
 > HostContainer6;
 
+/// @brief First index type in the @c HostContainer6.
+///
+/// This index allows for searching for @c Host objects using an
+/// address.
+typedef HostContainer6::nth_index<0>::type HostContainer6Index0;
+
+/// @brief Results range returned using the @c HostContainer6Index0.
+typedef std::pair<HostContainer6Index0::iterator,
+                  HostContainer6Index0::iterator> HostContainer6Index0Range;
+
+/// @brief Second index type in the @c HostContainer6.
+///
+/// This index allows for searching for @c Host objects using a
+/// reserved (SubnetID, IPv6 address) tuple.
+typedef HostContainer6::nth_index<1>::type HostContainer6Index1;
+
+/// @brief Results range returned using the @c HostContainer6Index1.
+typedef std::pair<HostContainer6Index1::iterator,
+                  HostContainer6Index1::iterator> HostContainer6Index1Range;
 
-}
-}
+}; // end of isc::dhcp namespace
+}; // end of isc namespace
 
 #endif // HOST_CONTAINER_H

src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc

@@ -19,6 +19,7 @@
 #include <dhcpsrv/cfg_hosts.h>
 #include <dhcpsrv/host.h>
 #include <gtest/gtest.h>
+#include <sstream>
 #include <set>
 
 using namespace isc;
@@ -322,6 +323,115 @@ TEST_F(CfgHostsTest, get6) {
     EXPECT_THROW(cfg.get6(SubnetID(1), duids_[0], hwaddrs_[0]), DuplicateHost);
 }
 
+// This test checks that the IPv6 reservations can be retrieved for a particular
+// (subnet-id, address) tuple.
+TEST_F(CfgHostsTest, get6ByAddr) {
+    CfgHosts cfg;
+    // Add hosts.
+    for (int i = 0; i < 25; ++i) {
+
+        // Add host identified by DUID.
+        HostPtr host = HostPtr(new Host(duids_[i]->toText(), "duid",
+                                        SubnetID(0), SubnetID(1 + i % 2),
+                                        IOAddress("0.0.0.0")));
+        host->addReservation(IPv6Resrv(IPv6Resrv::TYPE_NA,
+                                       increase(IOAddress("2001:db8:2::1"),
+                                                i)));
+        cfg.add(host);
+    }
+
+    for (int i = 0; i < 25; ++i) {
+        // Retrieve host by (subnet-id,address).
+        HostPtr host = cfg.get6(SubnetID(1 + i % 2),
+                                increase(IOAddress("2001:db8:2::1"), i));
+        ASSERT_TRUE(host);
+
+        EXPECT_EQ(1 + i % 2, host->getIPv6SubnetID());
+        IPv6ResrvRange reservations =
+            host->getIPv6Reservations(IPv6Resrv::TYPE_NA);
+        ASSERT_EQ(1, std::distance(reservations.first, reservations.second));
+        EXPECT_EQ(increase(IOAddress("2001:db8:2::1"), i),
+                  reservations.first->second.getPrefix());
+    }
+}
+
+// This test checks that the IPv6 reservations can be retrieved for a particular
+// (subnet-id, address) tuple.
+TEST_F(CfgHostsTest, get6MultipleAddrs) {
+    CfgHosts cfg;
+
+    // Add 25 hosts. Each host has reservations for 5 addresses.
+    for (int i = 0; i < 25; ++i) {
+
+        // Add host identified by DUID.
+        HostPtr host = HostPtr(new Host(duids_[i]->toText(), "duid",
+                                        SubnetID(0), SubnetID(1 + i % 2),
+                                        IOAddress("0.0.0.0")));
+
+        // Generate 5 unique addresses for this host.
+        for (int j = 0; j < 5; ++j) {
+            std::stringstream tmp;
+            tmp << "2001:db8:" << i << "::" << j;
+            host->addReservation(IPv6Resrv(IPv6Resrv::TYPE_NA, tmp.str()));
+        }
+        cfg.add(host);
+    }
+
+    // We don't care about HW/MAC addresses for now.
+    HWAddrPtr hwaddr_not_used;
+
+    // Now check if we can retrieve each of those 25 hosts by using each
+    // of their addresses.
+    for (int i = 0; i < 25; ++i) {
+
+        // Check that the host is there.
+        HostPtr by_duid = cfg.get6(SubnetID(1 + i % 2), duids_[i], hwaddr_not_used);
+        ASSERT_TRUE(by_duid);
+
+        for (int j = 0; j < 5; ++j) {
+            std::stringstream tmp;
+            tmp << "2001:db8:" << i << "::" << j;
+
+            // Retrieve host by (subnet-id,address).
+            HostPtr by_addr = cfg.get6(SubnetID(1 + i % 2), tmp.str());
+            ASSERT_TRUE(by_addr);
+
+            // The pointers should match. Maybe we should compare contents
+            // rather than just pointers? I think there's no reason why
+            // the code would make any copies of the Host object, so
+            // the pointers should always point to the same object.
+            EXPECT_EQ(by_duid, by_addr);
+        }
+    }
+}
+
+
+// Checks that it's not possible for two hosts to have the same address
+// reserved at the same time.
+TEST_F(CfgHostsTest, add6Invalid2Hosts) {
+    CfgHosts cfg;
+
+    // First host has a reservation for address 2001:db8::1
+    HostPtr host1 = HostPtr(new Host(duids_[0]->toText(), "duid",
+                                     SubnetID(0), SubnetID(1),
+                                     IOAddress("0.0.0.0")));
+    host1->addReservation(IPv6Resrv(IPv6Resrv::TYPE_NA,
+                                    IOAddress("2001:db8::1")));
+    // Adding this should work.
+    EXPECT_NO_THROW(cfg.add(host1));
+
+    // The second host has a reservation for the same address.
+    HostPtr host2 = HostPtr(new Host(duids_[1]->toText(), "duid",
+                                     SubnetID(0), SubnetID(1),
+                                     IOAddress("0.0.0.0")));
+    host2->addReservation(IPv6Resrv(IPv6Resrv::TYPE_NA,
+                                    IOAddress("2001:db8::1")));
+
+    // This second host has a reservation for an address that is already
+    // reserved for the first host, so it should be rejected.
+    EXPECT_THROW(cfg.add(host2), isc::dhcp::DuplicateHost);
+}
+
 TEST_F(CfgHostsTest, zeroSubnetIDs) {
     CfgHosts cfg;
     ASSERT_THROW(cfg.add(HostPtr(new Host(hwaddrs_[0]->toText(false),