From: Tomek Mrugalski Date: Tue, 20 Jan 2015 20:15:25 +0000 (+0100) Subject: [3563] CfgHosts::get6(subnet_id, addr) implemented, with unit-tests. X-Git-Tag: trac3712_base~21^2~9 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=06246ea31973025b7d7ce44a11c30d8cbf809ab4;p=thirdparty%2Fkea.git [3563] CfgHosts::get6(subnet_id, addr) implemented, with unit-tests. --- diff --git a/src/lib/dhcpsrv/cfg_hosts.cc b/src/lib/dhcpsrv/cfg_hosts.cc index 5d7a7010bd..2b8659324e 100644 --- a/src/lib/dhcpsrv/cfg_hosts.cc +++ b/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 +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(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)); } } diff --git a/src/lib/dhcpsrv/cfg_hosts.h b/src/lib/dhcpsrv/cfg_hosts.h index 61d28e9f13..0cf511da8d 100644 --- a/src/lib/dhcpsrv/cfg_hosts.h +++ b/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 + 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. diff --git a/src/lib/dhcpsrv/host_container.h b/src/lib/dhcpsrv/host_container.h index bc22d24e38..c59142a44a 100644 --- a/src/lib/dhcpsrv/host_container.h +++ b/src/lib/dhcpsrv/host_container.h @@ -22,6 +22,7 @@ #include #include #include +#include namespace isc { namespace dhcp { @@ -101,17 +102,29 @@ typedef std::pairgetIPv6SubnetID() : 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, - /// 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 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 HostContainer6Index1Range; -} -} +}; // end of isc::dhcp namespace +}; // end of isc namespace #endif // HOST_CONTAINER_H diff --git a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc index 3b22d0a41b..a1548143a2 100644 --- a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include 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),