diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 7370583d68..5bcd69e938 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -42,6 +42,7 @@ using namespace std; namespace isc { namespace dhcp { + ControlledDhcpv6Srv* ControlledDhcpv6Srv::server_ = NULL; ConstElementPtr @@ -149,8 +150,8 @@ void ControlledDhcpv6Srv::disconnectSession() { IfaceMgr::instance().set_session_socket(IfaceMgr::INVALID_SOCKET, NULL); } -ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port /*= DHCP6_SERVER_PORT*/) - :Dhcpv6Srv(port), cc_session_(NULL), config_session_(NULL) { +ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port, const char* dbconfig) + : Dhcpv6Srv(port, dbconfig), cc_session_(NULL), config_session_(NULL) { server_ = this; // remember this instance for use in callback } diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index 91fc80acb2..df3b85172b 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -41,7 +41,9 @@ public: /// @brief Constructor /// /// @param port UDP port to be opened for DHCP traffic - ControlledDhcpv6Srv(uint16_t port = DHCP6_SERVER_PORT); + /// @param dbconfig Lease manager database configuration string + ControlledDhcpv6Srv(uint16_t port = DHCP6_SERVER_PORT, + const char* dbconfig = "type=memfile"); /// @brief Destructor. ~ControlledDhcpv6Srv(); diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes index 5f9cd02b68..96649d5890 100644 --- a/src/bin/dhcp6/dhcp6_messages.mes +++ b/src/bin/dhcp6/dhcp6_messages.mes @@ -30,7 +30,7 @@ from the BIND 10 control system by the IPv6 DHCP server. A debug message indicating that the IPv6 DHCP server has received an updated configuration from the BIND 10 configuration system. -% DHCP6_DB_BACKEND_STARTED Lease database started (backend type: %1) +% DHCP6_DB_BACKEND_STARTED lease database started (type: %1, name: %2) This informational message is printed every time DHCPv6 is started. It indicates what database backend type is being to store lease and other information. @@ -47,7 +47,7 @@ interfaces and is therefore shutting down. A debug message issued during startup, this indicates that the IPv6 DHCP server is about to open sockets on the specified port. -% DHCP6_LEASE_ADVERT Lease %1 advertised (client duid=%2, iaid=%3) +% DHCP6_LEASE_ADVERT lease %1 advertised (client duid=%2, iaid=%3) This debug message indicates that the server successfully advertised a lease. It is up to the client to choose one server out of othe advertised and continue allocation with that server. This is a normal behavior and @@ -154,14 +154,16 @@ the response will only contain generic configuration parameters and no addresses or prefixes. % DHCP6_NO_SUBNET_DEF_OPT failed to find subnet for address %1 when adding default options -This warning message indicates that when attempting to add default options to a response, -the server found that it was not configured to support the subnet from which the DHCPv6 -request was received. The packet has been ignored. +This warning message indicates that when attempting to add default options +to a response, the server found that it was not configured to support +the subnet from which the DHCPv6 request was received. The packet has +been ignored. % DHCP6_NO_SUBNET_REQ_OPT failed to find subnet for address %1 when adding requested options -This warning message indicates that when attempting to add requested options to a response, -the server found that it was not configured to support the subnet from which the DHCPv6 -request was received. The packet has been ignored. +This warning message indicates that when attempting to add requested +options to a response, the server found that it was not configured +to support the subnet from which the DHCPv6 request was received. +The packet has been ignored. % DHCP6_CONFIG_LOAD_FAIL failed to load configuration: %1 This critical error message indicates that the initial DHCPv6 diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 1c76b75103..82a4777433 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -80,6 +80,7 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port, const char* dbconfig) // Instantiate LeaseMgr LeaseMgrFactory::create(dbconfig); LOG_INFO(dhcp6_logger, DHCP6_DB_BACKEND_STARTED) + .arg(LeaseMgrFactory::instance().getType()) .arg(LeaseMgrFactory::instance().getName()); // Instantiate allocation engine diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index 8eaf60cb97..c216924859 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -34,6 +34,16 @@ using namespace std; /// Dhcpv6Srv and other classes, see \ref dhcpv6Session. namespace { +// @todo: Replace the next line by extraction from configuration parameters +// This is the "dbconfig" string for the MySQL database. It is likely +// that a long-term solution will be to create the instance of the lease manager +// somewhere other than the Dhcpv6Srv constructor, to give time to extract +// the connection string from the configuration database. +#ifdef HAVE_MYSQL +const char* DBCONFIG = "type=mysql name=kea user=kea password=kea host=localhost"; +#else +const char* DBCONFIG = "type=memfile"; +#endif const char* const DHCP6_NAME = "b10-dhcp6"; @@ -102,7 +112,7 @@ main(int argc, char* argv[]) { int ret = EXIT_SUCCESS; try { - ControlledDhcpv6Srv server(port_number); + ControlledDhcpv6Srv server(port_number, DBCONFIG); if (!stand_alone) { try { server.establishSession(); diff --git a/src/lib/dhcp/lease_mgr.h b/src/lib/dhcp/lease_mgr.h index 40c20cd529..7819aad423 100644 --- a/src/lib/dhcp/lease_mgr.h +++ b/src/lib/dhcp/lease_mgr.h @@ -505,14 +505,26 @@ public: /// @return true if deletion was successful, false if no such lease exists virtual bool deleteLease6(const isc::asiolink::IOAddress& addr) = 0; + /// @brief Return backend type + /// + /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) + /// + /// @return Type of the backend. + virtual std::string getType() const = 0; + /// @brief Returns backend name. /// - /// Each backend have specific name, e.g. "mysql" or "sqlite". + /// If the backend is a database, this is the name of the database or the + /// file. Otherwise it is just the same as the type. + /// + /// @return Name of the backend. virtual std::string getName() const = 0; /// @brief Returns description of the backend. /// /// This description may be multiline text that describes the backend. + /// + /// @return Description of the backend. virtual std::string getDescription() const = 0; /// @brief Returns backend version. @@ -548,7 +560,7 @@ public: /// is currently postponed. /// @brief returns value of the parameter - std::string getParameter(const std::string& name) const; + virtual std::string getParameter(const std::string& name) const; private: /// @brief list of parameters passed in dbconfig diff --git a/src/lib/dhcp/memfile_lease_mgr.h b/src/lib/dhcp/memfile_lease_mgr.h index cb02360db2..9120c5cd6f 100644 --- a/src/lib/dhcp/memfile_lease_mgr.h +++ b/src/lib/dhcp/memfile_lease_mgr.h @@ -133,7 +133,7 @@ public: /// @param addr address of the searched lease /// /// @return smart pointer to the lease (or NULL if a lease is not found) - Lease6Ptr getLease6(const isc::asiolink::IOAddress& addr) const; + virtual Lease6Ptr getLease6(const isc::asiolink::IOAddress& addr) const; /// @brief Returns existing IPv6 lease for a given DUID+IA combination /// @@ -143,7 +143,7 @@ public: /// @param iaid IA identifier /// /// @return collection of IPv6 leases - Lease6Collection getLease6(const DUID& duid, uint32_t iaid) const; + virtual Lease6Collection getLease6(const DUID& duid, uint32_t iaid) const; /// @brief Returns existing IPv6 lease for a given DUID+IA combination /// @@ -154,7 +154,7 @@ public: /// @param subnet_id identifier of the subnet the lease must belong to /// /// @return smart pointer to the lease (or NULL if a lease is not found) - Lease6Ptr getLease6(const DUID& duid, uint32_t iaid, SubnetID subnet_id) const; + virtual Lease6Ptr getLease6(const DUID& duid, uint32_t iaid, SubnetID subnet_id) const; /// @brief Updates IPv4 lease. /// @@ -163,7 +163,7 @@ public: /// @param lease4 The lease to be updated. /// /// If no such lease is present, an exception will be thrown. - void updateLease4(const Lease4Ptr& lease4); + virtual void updateLease4(const Lease4Ptr& lease4); /// @brief Updates IPv4 lease. /// @@ -172,7 +172,7 @@ public: /// @param lease6 The lease to be updated. /// /// If no such lease is present, an exception will be thrown. - void updateLease6(const Lease6Ptr& lease6); + virtual void updateLease6(const Lease6Ptr& lease6); /// @brief Deletes a lease. /// @@ -186,19 +186,38 @@ public: /// @param addr IPv4 address of the lease to be deleted. /// /// @return true if deletion was successful, false if no such lease exists - bool deleteLease6(const isc::asiolink::IOAddress& addr); + virtual bool deleteLease6(const isc::asiolink::IOAddress& addr); + + /// @brief Return backend type + /// + /// Returns the type of the backend. + /// + /// @return Type of the backend. + virtual std::string getType() const { + return (std::string("memfile")); + } /// @brief Returns backend name. /// - /// Each backend have specific name, e.g. "mysql" or "sqlite". - std::string getName() const { return ("memfile"); } + /// As there is no variation, in this case we return the type of the + /// backend. + /// + /// @return Name of the backend. + virtual std::string getName() const { + return ("memfile"); + } /// @brief Returns description of the backend. /// /// This description may be multiline text that describes the backend. - std::string getDescription() const; + /// + /// @return Description of the backend. + virtual std::string getDescription() const; /// @brief Returns backend version. + /// + /// @return Version number as a pair of unsigned integers. "first" is the + /// major version number, "second" the minor number. virtual std::pair getVersion() const { return (std::make_pair(1, 0)); } @@ -215,8 +234,6 @@ public: /// support transactions, this is a no-op. virtual void rollback(); - using LeaseMgr::getParameter; - protected: typedef boost::multi_index_container< // this is a multi-index container... diff --git a/src/lib/dhcp/mysql_lease_mgr.cc b/src/lib/dhcp/mysql_lease_mgr.cc index f030cd3b8e..78013d77a3 100644 --- a/src/lib/dhcp/mysql_lease_mgr.cc +++ b/src/lib/dhcp/mysql_lease_mgr.cc @@ -447,8 +447,9 @@ MySqlLeaseMgr::MySqlLeaseMgr(const LeaseMgr::ParameterMap& parameters) // Open the database openDatabase(); - // Disable autocommit - my_bool result = mysql_autocommit(mysql_, 0); + // Enable autocommit. For maximum speed, the global parameter + // innodb_flush_log_at_trx_commit should be set to 2. + my_bool result = mysql_autocommit(mysql_, 1); if (result != 0) { isc_throw(DbOperationError, mysql_error(mysql_)); } diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h index 8712c33035..bdd99cc1df 100644 --- a/src/lib/dhcp/mysql_lease_mgr.h +++ b/src/lib/dhcp/mysql_lease_mgr.h @@ -239,14 +239,27 @@ public: /// failed. virtual bool deleteLease6(const isc::asiolink::IOAddress& addr); + /// @brief Return backend type + /// + /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) + /// + /// @return Type of the backend. + virtual std::string getType() const { + return (std::string("mysql")); + } + /// @brief Returns backend name. /// /// Each backend have specific name, e.g. "mysql" or "sqlite". + /// + /// @return Name of the backend. virtual std::string getName() const; /// @brief Returns description of the backend. /// /// This description may be multiline text that describes the backend. + /// + /// @return Description of the backend. virtual std::string getDescription() const; /// @brief Returns backend version. @@ -254,16 +267,6 @@ public: /// @return Version number as a pair of unsigned integers. "first" is the /// major version number, "second" the minor number. /// - /// @todo: We will need to implement 3 version functions eventually: - /// A. abstract API version - /// B. backend version - /// C. database version (stored in the database scheme) - /// - /// and then check that: - /// B>=A and B=C (it is ok to have newer backend, as it should be backward - /// compatible) - /// Also if B>C, some database upgrade procedure may be triggered - /// /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. virtual std::pair getVersion() const; diff --git a/src/lib/dhcp/tests/lease_mgr_unittest.cc b/src/lib/dhcp/tests/lease_mgr_unittest.cc index 64ce9b1653..7ad2036930 100644 --- a/src/lib/dhcp/tests/lease_mgr_unittest.cc +++ b/src/lib/dhcp/tests/lease_mgr_unittest.cc @@ -134,7 +134,7 @@ public: /// @param addr address of the searched lease /// /// @return smart pointer to the lease (or NULL if a lease is not found) - Lease6Ptr getLease6(const isc::asiolink::IOAddress&) const { + virtual Lease6Ptr getLease6(const isc::asiolink::IOAddress&) const { return (Lease6Ptr()); } @@ -144,7 +144,7 @@ public: /// @param iaid IA identifier /// /// @return collection of IPv6 leases - Lease6Collection getLease6(const DUID&, uint32_t) const { + virtual Lease6Collection getLease6(const DUID&, uint32_t) const { return (Lease6Collection()); } @@ -155,7 +155,7 @@ public: /// @param subnet_id identifier of the subnet the lease must belong to /// /// @return smart pointer to the lease (or NULL if a lease is not found) - Lease6Ptr getLease6(const DUID&, uint32_t, SubnetID) const { + virtual Lease6Ptr getLease6(const DUID&, uint32_t, SubnetID) const { return (Lease6Ptr()); } @@ -164,21 +164,21 @@ public: /// @param lease4 The lease to be updated. /// /// If no such lease is present, an exception will be thrown. - void updateLease4(const Lease4Ptr&) {} + virtual void updateLease4(const Lease4Ptr&) {} /// @brief Updates IPv4 lease. /// /// @param lease4 The lease to be updated. /// /// If no such lease is present, an exception will be thrown. - void updateLease6(const Lease6Ptr&) {} + virtual void updateLease6(const Lease6Ptr&) {} /// @brief Deletes a lease. /// /// @param addr IPv4 address of the lease to be deleted. /// /// @return true if deletion was successful, false if no such lease exists - bool deleteLease4(const isc::asiolink::IOAddress&) { + virtual bool deleteLease4(const isc::asiolink::IOAddress&) { return (false); } @@ -187,35 +187,49 @@ public: /// @param addr IPv4 address of the lease to be deleted. /// /// @return true if deletion was successful, false if no such lease exists - bool deleteLease6(const isc::asiolink::IOAddress&) { + virtual bool deleteLease6(const isc::asiolink::IOAddress&) { return (false); } + /// @brief Returns backend type. + /// + /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) + /// + /// @return Type of the backend. + virtual std::string getType() const { + return (std::string("concrete")); + } + /// @brief Returns backend name. /// - /// Each backend have specific name, e.g. "mysql" or "sqlite". - std::string getName() const { + /// If the backend is a database, this is the name of the database or the + /// file. Otherwise it is just the same as the type. + /// + /// @return Name of the backend. + virtual std::string getName() const { return (std::string("concrete")); } /// @brief Returns description of the backend. /// /// This description may be multiline text that describes the backend. - std::string getDescription() const { + /// + /// @return Description of the backend. + virtual std::string getDescription() const { return (std::string("This is a dummy concrete backend implementation.")); } /// @brief Returns backend version. - std::pair getVersion() const { + virtual std::pair getVersion() const { return (make_pair(uint32_t(0), uint32_t(0))); } /// @brief Commit transactions - void commit() { + virtual void commit() { } /// @brief Rollback transactions - void rollback() { + virtual void rollback() { } }; diff --git a/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc index c625a16f28..79706e19be 100644 --- a/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc +++ b/src/lib/dhcp/tests/memfile_lease_mgr_unittest.cc @@ -45,14 +45,17 @@ TEST_F(MemfileLeaseMgrTest, constructor) { ASSERT_NO_THROW(lease_mgr.reset(new Memfile_LeaseMgr(pmap))); } -// There's no point in calling any other methods in LeaseMgr, as they -// are purely virtual, so we would only call Memfile_LeaseMgr methods. -// Those methods are just stubs that does not return anything. -// It seems likely that we will need to extend the memfile code for -// allocation engine tests, so we may implement tests that call -// Memfile_LeaseMgr methods then. +// Checks if the getType() and getName() methods both return "memfile". +TEST_F(MemfileLeaseMgrTest, getTypeAndName) { + const LeaseMgr::ParameterMap pmap; // Empty parameter map + boost::scoped_ptr lease_mgr(new Memfile_LeaseMgr(pmap)); -TEST_F(MemfileLeaseMgrTest, addGetDelete) { + EXPECT_EQ(std::string("memfile"), lease_mgr->getType()); + EXPECT_EQ(std::string("memfile"), lease_mgr->getName()); +} + +// Checks that adding/getting/deleting a Lease6 object works. +TEST_F(MemfileLeaseMgrTest, addGetDelete6) { const LeaseMgr::ParameterMap pmap; // Empty parameter map boost::scoped_ptr lease_mgr(new Memfile_LeaseMgr(pmap)); diff --git a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc index a9b09ffbae..d99ae23377 100644 --- a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc +++ b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc @@ -470,8 +470,26 @@ TEST(MySqlOpenTest, OpenDatabase) { destroySchema(); } +// @brief Check the getType() method +// +// getType() returns a string giving the type of the backend, which should +// always be "mysql". +TEST_F(MySqlLeaseMgrTest, getType) { + EXPECT_EQ(std::string("mysql"), lmptr_->getType()); +} + // @brief Check conversion functions -TEST_F(MySqlLeaseMgrTest, CheckTimeConversion) { +// +// The server works using cltt and valid_filetime. In the database, the +// information is stored as expire_time and valid-lifetime, which are +// related by +// +// expire_time = cltt + valid_lifetime +// +// This test checks that the conversion is correct. It does not check that the +// data is entered into the database correctly, only that the MYSQL_TIME +// structure used for the entry is correctly set up. +TEST_F(MySqlLeaseMgrTest, checkTimeConversion) { const time_t cltt = time(NULL); const uint32_t valid_lft = 86400; // 1 day struct tm tm_expire; @@ -509,8 +527,8 @@ TEST_F(MySqlLeaseMgrTest, getName) { // @TODO: check for the negative } -// @brief Check that getVersion() works -TEST_F(MySqlLeaseMgrTest, CheckVersion) { +// @brief Check that getVersion() returns the expected version +TEST_F(MySqlLeaseMgrTest, checkVersion) { // Check version pair version; ASSERT_NO_THROW(version = lmptr_->getVersion()); @@ -518,13 +536,15 @@ TEST_F(MySqlLeaseMgrTest, CheckVersion) { EXPECT_EQ(CURRENT_VERSION_MINOR, version.second); } +// @brief Compare two Lease6 structures for equality void detailCompareLease6(const Lease6Ptr& first, const Lease6Ptr& second) { EXPECT_EQ(first->type_, second->type_); - // Compare address strings - odd things happen when they are different - // as the EXPECT_EQ appears to call the operator uint32_t() function, - // which causes an exception to be thrown for IPv6 addresses. + // Compare address strings. Comparison of address objects is not used, as + // odd things happen when they are different: the EXPECT_EQ macro appears to + // call the operator uint32_t() function, which causes an exception to be + // thrown for IPv6 addresses. EXPECT_EQ(first->addr_.toText(), second->addr_.toText()); EXPECT_EQ(first->prefixlen_, second->prefixlen_); EXPECT_EQ(first->iaid_, second->iaid_); @@ -544,7 +564,7 @@ detailCompareLease6(const Lease6Ptr& first, const Lease6Ptr& second) { // // Tests where a collection of leases can be returned are in the test // Lease6Collection. -TEST_F(MySqlLeaseMgrTest, BasicLease6) { +TEST_F(MySqlLeaseMgrTest, basicLease6) { // Get the leases to be used for the test. vector leases = createLeases6(); @@ -590,7 +610,7 @@ TEST_F(MySqlLeaseMgrTest, BasicLease6) { // // Adds leases to the database and checks that they can be accessed via // a combination of DIUID and IAID. -TEST_F(MySqlLeaseMgrTest, GetLease6Extended1) { +TEST_F(MySqlLeaseMgrTest, getLease6Extended1) { // Get the leases to be used for the test. vector leases = createLeases6(); EXPECT_LE(6, leases.size()); // Expect to access leases 0 through 5 @@ -637,7 +657,7 @@ TEST_F(MySqlLeaseMgrTest, GetLease6Extended1) { // // Adds leases to the database and checks that they can be accessed via // a combination of DIUID and IAID. -TEST_F(MySqlLeaseMgrTest, GetLease6Extended2) { +TEST_F(MySqlLeaseMgrTest, getLease6Extended2) { // Get the leases to be used for the test. vector leases = createLeases6(); EXPECT_LE(6, leases.size()); // Expect to access leases 0 through 5 @@ -678,7 +698,7 @@ TEST_F(MySqlLeaseMgrTest, GetLease6Extended2) { // @brief Lease6 Update Tests // // Checks that we are able to update a lease in the database. -TEST_F(MySqlLeaseMgrTest, UpdateLease6) { +TEST_F(MySqlLeaseMgrTest, updateLease6) { // Get the leases to be used for the test. vector leases = createLeases6(); EXPECT_LE(3, leases.size()); // Expect to access leases 0 through 5