[master] Merge branch 'trac2472'

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
 }
 

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();

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

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

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();

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

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<uint32_t, uint32_t> 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...

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_));
     }

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<uint32_t, uint32_t> getVersion() const;

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<uint32_t, uint32_t> getVersion() const {
+    virtual std::pair<uint32_t, uint32_t> 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() {
     }
 };
 

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.
-
-TEST_F(MemfileLeaseMgrTest, addGetDelete) {
+// Checks if the getType() and getName() methods both return "memfile".
+TEST_F(MemfileLeaseMgrTest, getTypeAndName) {
+    const LeaseMgr::ParameterMap pmap;  // Empty parameter map
+    boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr(new Memfile_LeaseMgr(pmap));
+
+    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<Memfile_LeaseMgr> lease_mgr(new Memfile_LeaseMgr(pmap));
 

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<uint32_t, uint32_t> 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<Lease6Ptr> 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<Lease6Ptr> 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<Lease6Ptr> 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<Lease6Ptr> leases = createLeases6();
     EXPECT_LE(3, leases.size());    // Expect to access leases 0 through 5