From: Tomek Mrugalski Date: Wed, 7 Mar 2018 13:03:25 +0000 (+0100) Subject: [5533a] Extra comments added X-Git-Tag: trac5527_base~1^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4ffeedbbe3c0e6dd0832323bb6e115982aa98e9a;p=thirdparty%2Fkea.git [5533a] Extra comments added --- diff --git a/src/lib/dhcpsrv/host.h b/src/lib/dhcpsrv/host.h index 20d8e4e554..a92cda079f 100644 --- a/src/lib/dhcpsrv/host.h +++ b/src/lib/dhcpsrv/host.h @@ -548,7 +548,7 @@ public: /// @brief Sets the negative cached flag. /// - /// @param negative New valie for negative cache flag. + /// @param negative sets whether this is a negative cached host. void setNegative(bool negative) { negative_ = negative; } @@ -618,6 +618,10 @@ private: CfgOptionPtr cfg_option6_; /// @brief Negative cached flag. + /// + /// This flag determines whether this object is a negative cache, i.e. + /// we queried other backends for specific host and there was no + /// entry for it. bool negative_; }; diff --git a/src/lib/dhcpsrv/host_data_source_factory.cc b/src/lib/dhcpsrv/host_data_source_factory.cc index 2eb6cd6b9c..08695f5427 100644 --- a/src/lib/dhcpsrv/host_data_source_factory.cc +++ b/src/lib/dhcpsrv/host_data_source_factory.cc @@ -59,8 +59,8 @@ HostDataSourceFactory::add(HostDataSourceList& sources, // No match? if (index == map_.end()) { - isc_throw(InvalidType, "Hosts database access parameter 'type': " << - db_type << " is invalid"); + isc_throw(InvalidType, "The type of host backend: '" << + db_type << "' is not currently supported"); } // Call the factory and push the pointer on sources. @@ -75,8 +75,7 @@ HostDataSourceFactory::add(HostDataSourceList& sources, } bool -HostDataSourceFactory::del(HostDataSourceList& sources, - const string& db_type) { +HostDataSourceFactory::del(HostDataSourceList& sources, const string& db_type) { for (auto it = sources.begin(); it != sources.end(); ++it) { if ((*it)->getType() != db_type) { continue; diff --git a/src/lib/dhcpsrv/host_data_source_factory.h b/src/lib/dhcpsrv/host_data_source_factory.h index dc399c7d70..a4cefe75c2 100644 --- a/src/lib/dhcpsrv/host_data_source_factory.h +++ b/src/lib/dhcpsrv/host_data_source_factory.h @@ -51,7 +51,7 @@ public: /// a keyword/value pair of the form "type=dbtype" giving the database /// type, e.q. "mysql" or "sqlite3". /// - /// @param sources host data source list. + /// @param sources host data source list (new backend will be added here) /// @param dbaccess Database access parameters. These are in the form of /// "keyword=value" pairs, separated by spaces. They are backend- /// -end specific, although must include the "type" keyword which diff --git a/src/lib/dhcpsrv/host_mgr.cc b/src/lib/dhcpsrv/host_mgr.cc index 775997d12a..f9783e9807 100644 --- a/src/lib/dhcpsrv/host_mgr.cc +++ b/src/lib/dhcpsrv/host_mgr.cc @@ -171,6 +171,9 @@ HostMgr::get4Any(const SubnetID& subnet_id, const size_t identifier_len) const { ConstHostPtr host = getCfgHosts()->get4(subnet_id, identifier_type, identifier_begin, identifier_len); + + // Found it the config file or there are no backends configured? + // Then we're done here. if (host || alternate_sources_.empty()) { return (host); } @@ -181,6 +184,8 @@ HostMgr::get4Any(const SubnetID& subnet_id, .arg(Host::getIdentifierAsText(identifier_type, identifier_begin, identifier_len)); + // Try to find a host in each configured backend. We return as soon + // as we find first hit. for (auto it = alternate_sources_.begin(); it != alternate_sources_.end(); ++it) { host = (*it)->get4(subnet_id, identifier_type, diff --git a/src/lib/dhcpsrv/host_mgr.h b/src/lib/dhcpsrv/host_mgr.h index 1e7110adb3..1727bf20e4 100644 --- a/src/lib/dhcpsrv/host_mgr.h +++ b/src/lib/dhcpsrv/host_mgr.h @@ -393,8 +393,9 @@ public: protected: /// @brief The negative caching flag. /// - /// When true and the first data source is a cache negative answers - /// to get[46] for aubnet and identifier are cached. + /// When true and the first cache source will be also + /// caching negative answers. This works for get[46] + /// for a subnet and an identifier. bool negative_caching_; /// @brief Cache an answer. diff --git a/src/lib/dhcpsrv/libdhcpsrv.dox b/src/lib/dhcpsrv/libdhcpsrv.dox index 04775b6977..22bc8b8888 100644 --- a/src/lib/dhcpsrv/libdhcpsrv.dox +++ b/src/lib/dhcpsrv/libdhcpsrv.dox @@ -106,7 +106,8 @@ host reservations in the MySQL database is required to establish connection to the MySQL database and issue specific queries. A factory method creating an instance of a base host data source object must be registered (at global object initialization for built-in backends, -dynamically for backends loaded at run-time). Note, that this instance is +dynamically for backends loaded at run-time). See host_data_source_factory.cc +for example code that registers MySQL and PostgreSQL. Note, that this instance is created as "alternate host data source" as opposed to the primary data source which returns host reservations specified in the configuration file. The primary data source is implemented internally in the diff --git a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc index 3efa01163c..0850bbd19a 100644 --- a/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc @@ -103,7 +103,7 @@ public: /// Closes the database and re-open it. Anything committed should be /// visible. /// - /// Parameter is ignored for CQL backend as the v4 and v6 leases share + /// Parameter is ignored for CQL backend as the v4 and v6 hosts share /// the same database. void reopen(Universe) { HostMgr::create(); @@ -116,7 +116,7 @@ public: /// /// This test checks if the CqlHostDataSource can be instantiated. This happens /// only if the database can be opened. Note that this is not part of the -/// CqlLeaseMgr test fixure set. This test checks that the database can be +/// CqlHostMgr test fixure set. This test checks that the database can be /// opened: the fixtures assume that and check basic operations. TEST(CqlHostDataSource, OpenDatabase) { @@ -124,7 +124,7 @@ TEST(CqlHostDataSource, OpenDatabase) { destroyCqlSchema(false, true); createCqlSchema(false, true); - // Check that lease manager open the database opens correctly and tidy up. + // Check that host manager open the database opens correctly and tidy up. // If it fails, print the error message. try { HostMgr::create(); @@ -137,7 +137,7 @@ TEST(CqlHostDataSource, OpenDatabase) { << "*** before the CQL tests will run correctly.\n"; } - // Check that lease manager open the database opens correctly with a longer + // Check that host manager open the database opens correctly with a longer // timeout. If it fails, print the error message. try { std::string connection_string = validCqlConnectionString() + std::string(" ") + diff --git a/src/lib/dhcpsrv/tests/host_cache_unittest.cc b/src/lib/dhcpsrv/tests/host_cache_unittest.cc index ec3ce0cb2b..c8579f01e3 100644 --- a/src/lib/dhcpsrv/tests/host_cache_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_cache_unittest.cc @@ -161,7 +161,7 @@ TEST_F(HostCacheTest, identifier4) { // Create a host reservation. HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1", Host::IDENT_HWADDR); - ASSERT_TRUE(host); // Make sure the host is generate properly. + ASSERT_TRUE(host); // Make sure the host is generated properly. const IOAddress& address = host->getIPv4Reservation(); // Try to add it to the host data source. @@ -215,7 +215,7 @@ TEST_F(HostCacheTest, identifier6) { HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1", Host::IDENT_DUID, false); - ASSERT_TRUE(host); // Make sure the host is generate properly. + ASSERT_TRUE(host); // Make sure the host is generated properly. // Get the address. IPv6ResrvRange resrvs = host->getIPv6Reservations(); @@ -273,7 +273,7 @@ TEST_F(HostCacheTest, address4) { // Create a host reservation. HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1", Host::IDENT_HWADDR); - ASSERT_TRUE(host); // Make sure the host is generate properly. + ASSERT_TRUE(host); // Make sure the host is generated properly. const IOAddress& address = host->getIPv4Reservation(); // Try to add it to the host data source. @@ -325,7 +325,7 @@ TEST_F(HostCacheTest, address6) { HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1", Host::IDENT_DUID, false); - ASSERT_TRUE(host); // Make sure the host is generate properly. + ASSERT_TRUE(host); // Make sure the host is generated properly. // Get the address. IPv6ResrvRange resrvs = host->getIPv6Reservations(); @@ -380,11 +380,10 @@ TEST_F(HostCacheTest, negativeIdentifier4) { ASSERT_FALSE(HostMgr::instance().getNegativeCaching()); // Create a host reservation. - HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1", - Host::IDENT_HWADDR); - ASSERT_TRUE(host); // Make sure the host is generate properly. + // We will not add it anywhere, just will use its values. + HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1", Host::IDENT_HWADDR); + ASSERT_TRUE(host); - // Do not add it to the host data source. // Try to get it cached. ConstHostPtr got = HostMgr::instance().get4(host->getIPv4SubnetID(), @@ -403,7 +402,8 @@ TEST_F(HostCacheTest, negativeIdentifier4) { HostMgr::instance().setNegativeCaching(true); ASSERT_TRUE(HostMgr::instance().getNegativeCaching()); - // Try it but it will be cached only the second time. + // Try it again. There is no such host, but this time negative cache is enabled, + // so this negative response will be added to the cache. got = HostMgr::instance().get4Any(host->getIPv4SubnetID(), host->getIdentifierType(), &host->getIdentifier()[0], @@ -450,10 +450,11 @@ TEST_F(HostCacheTest, negativeIdentifier6) { ASSERT_TRUE(memptr_); // Create a host reservation. + // We will not add it anywhere, just will use its values. HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1", Host::IDENT_DUID, false); - ASSERT_TRUE(host); // Make sure the host is generate properly. + ASSERT_TRUE(host); // Make sure the host is generated properly. // Do not add it to the host data source. @@ -521,9 +522,10 @@ TEST_F(HostCacheTest, negativeAddress4) { ASSERT_TRUE(memptr_); // Create a host reservation. + // We will not add it anywhere, just will use its values. HostPtr host = HostDataSourceUtils::initializeHost4("192.0.2.1", Host::IDENT_HWADDR); - ASSERT_TRUE(host); // Make sure the host is generate properly. + ASSERT_TRUE(host); // Make sure the host is generated properly. const IOAddress& address = host->getIPv4Reservation(); // Do not add it to the host data source. @@ -550,10 +552,11 @@ TEST_F(HostCacheTest, negativeAddress6) { ASSERT_TRUE(memptr_); // Create a host reservation. + // We will not add it anywhere, just will use its values. HostPtr host = HostDataSourceUtils::initializeHost6("2001:db8::1", Host::IDENT_DUID, false); - ASSERT_TRUE(host); // Make sure the host is generate properly. + ASSERT_TRUE(host); // Make sure the host is generated properly. // Get the address. IPv6ResrvRange resrvs = host->getIPv6Reservations(); diff --git a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc index b05ea47438..3210c84ecd 100644 --- a/src/lib/dhcpsrv/tests/host_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_mgr_unittest.cc @@ -110,7 +110,7 @@ protected: void testGet4(BaseHostDataSource& data_source); /// @brief This test verifies that it is possible to retrieve negative - /// cached reservation with and only with get4Any, + /// cached reservation with and only with get4Any. void testGet4Any(); /// @brief This test verifies that it is possible to retrieve an IPv6 @@ -121,7 +121,7 @@ protected: void testGet6(BaseHostDataSource& data_source); /// @brief This test verifies that it is possible to retrieve negative - /// cached reservation with and only with get6Any, + /// cached reservation with and only with get6Any. void testGet6Any(); /// @brief This test verifies that it is possible to retrieve an IPv6 @@ -336,13 +336,13 @@ HostMgrTest::testGet4Any() { // Set the negative cache flag on the host. new_host->setNegative(true); - // Get4 can't get it. + // get4 is not supposed to get it. host = HostMgr::instance().get4(SubnetID(1), Host::IDENT_DUID, &duids_[0]->getDuid()[0], duids_[0]->getDuid().size()); EXPECT_FALSE(host); - // But Get4Any can. + // But get4Any should. host = HostMgr::instance().get4Any(SubnetID(1), Host::IDENT_DUID, &duids_[0]->getDuid()[0], duids_[0]->getDuid().size()); @@ -409,13 +409,13 @@ HostMgrTest::testGet6Any() { // Set the negative cache flag on the host. new_host->setNegative(true); - // Get6 can't get it. + // get6 is not supposed to get it. host = HostMgr::instance().get6(SubnetID(2), Host::IDENT_HWADDR, &hwaddrs_[0]->hwaddr_[0], hwaddrs_[0]->hwaddr_.size()); EXPECT_FALSE(host); - // But Get4Any can. + // But get6Any should. host = HostMgr::instance().get6Any(SubnetID(2), Host::IDENT_HWADDR, &hwaddrs_[0]->hwaddr_[0], hwaddrs_[0]->hwaddr_.size()); diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index b7b46affc9..36f7fe8e5f 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -148,7 +148,7 @@ TEST(MySqlHostDataSource, OpenDatabase) { destroyMySQLSchema(); createMySQLSchema(); - // Check that lease manager open the database opens correctly and tidy up. + // Check that host manager open the database opens correctly and tidy up. // If it fails, print the error message. try { HostMgr::create(); @@ -161,7 +161,7 @@ TEST(MySqlHostDataSource, OpenDatabase) { << "*** before the MySQL tests will run correctly.\n"; } - // Check that lease manager open the database opens correctly with a longer + // Check that host manager open the database opens correctly with a longer // timeout. If it fails, print the error message. try { string connection_string = validMySQLConnectionString() + string(" ") + @@ -176,7 +176,7 @@ TEST(MySqlHostDataSource, OpenDatabase) { << "*** before the MySQL tests will run correctly.\n"; } - // Check that attempting to get an instance of the lease manager when + // Check that attempting to get an instance of the host manager when // none is set throws an exception. EXPECT_FALSE(HostMgr::instance().getHostDataSource()); diff --git a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc index 559065589b..2824728702 100644 --- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc @@ -148,7 +148,7 @@ TEST(PgSqlHostDataSource, OpenDatabase) { destroyPgSQLSchema(); createPgSQLSchema(); - // Check that lease manager open the database opens correctly and tidy up. + // Check that host manager open the database opens correctly and tidy up. // If it fails, print the error message. try { HostMgr::create(); @@ -161,7 +161,7 @@ TEST(PgSqlHostDataSource, OpenDatabase) { << "*** before the PostgreSQL tests will run correctly.\n"; } - // Check that lease manager open the database opens correctly with a longer + // Check that host manager open the database opens correctly with a longer // timeout. If it fails, print the error message. try { string connection_string = validPgSQLConnectionString() + string(" ") + @@ -175,7 +175,7 @@ TEST(PgSqlHostDataSource, OpenDatabase) { << "*** before the PostgreSQL tests will run correctly.\n"; } - // Check that attempting to get an instance of the lease manager when + // Check that attempting to get an instance of the host manager when // none is set throws an exception. EXPECT_FALSE(HostMgr::instance().getHostDataSource()); diff --git a/src/lib/dhcpsrv/testutils/memory_host_data_source.h b/src/lib/dhcpsrv/testutils/memory_host_data_source.h index 9fda2839ed..02182ed438 100644 --- a/src/lib/dhcpsrv/testutils/memory_host_data_source.h +++ b/src/lib/dhcpsrv/testutils/memory_host_data_source.h @@ -31,6 +31,10 @@ public: /// @brief Return all hosts for the specified HW address or DUID. /// + /// This may return hosts from multiple subnets. + /// + /// Currently not implemented. + /// /// @param hwaddr HW address of the client or NULL if no HW address /// available. /// @param duid client id or NULL if not available, e.g. DHCPv4 client case. @@ -41,6 +45,10 @@ public: /// @brief Return all hosts connected to any subnet for which reservations /// have been made using a specified identifier. /// + /// This may return hosts from multiple subnets. + /// + /// Currently not implemented. + /// /// @param identifier_type Identifier type. /// @param identifier_begin Pointer to a beginning of a buffer containing /// an identifier. @@ -53,6 +61,8 @@ public: /// @brief Returns a collection of hosts using the specified IPv4 address. /// + /// Currently not implemented. + /// /// @param address IPv4 address for which the @c Host object is searched. /// @return Empty collection of const @c Host objects. virtual ConstHostCollection @@ -60,6 +70,8 @@ public: /// @brief Returns a host connected to the IPv4 subnet. /// + /// Currently not implemented. + /// /// @param subnet_id Subnet identifier. /// @param hwaddr HW address of the client or NULL if no HW address /// available. @@ -97,6 +109,8 @@ public: /// @brief Returns a host connected to the IPv6 subnet. /// + /// Currently not implemented. + /// /// @param subnet_id Subnet identifier. /// @param hwaddr HW address of the client or NULL if no HW address /// available.