[4212] Applied two patches by Adam:

 - addressed most review comments
 - added new unit-tests
 - remove reservation for temporary addresses TA
 - updated AUTHORS file

AUTHORS

@@ -82,7 +82,8 @@ We have received the following contributions:
    2015-01: Extract MAC address from remote-id
    2015-05: MySQL schema extended to cover host reservation
    2015-10: Common MySQL Connector Pool
-   2015-12: MySQL host data source implemented.
+   2015-12: MySQL host data source implemented
+   2016-02: IPv6 reservations implemented
 
  - Jinmei Tatuya
    2015-10: Pkt4o6 class improvements

src/lib/dhcpsrv/host.h

@@ -46,10 +46,8 @@ public:
     /// @brief Type of the reservation.
     ///
     /// Currently supported types are NA and PD.
-    /// Added TA type for IPv6 Reservation implementation.
     enum Type {
         TYPE_NA,
-        TYPE_TA,
         TYPE_PD
     };
 

src/lib/dhcpsrv/mysql_host_data_source.cc

@@ -14,8 +14,7 @@
 
 #include <config.h>
 
-#define __STDC_FORMAT_MACROS
-#include <inttypes.h>
+#include <stdint.h>
 
 #include <asiolink/io_address.h>
 #include <dhcp/duid.h>
@@ -58,9 +57,9 @@ TaggedStatement tagged_statements[] = {
             "dhcp4_client_classes, dhcp6_client_classes) "
          "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"},
     {MySqlHostDataSource::INSERT_V6_RESRV,
-         "INSERT INTO ipv6_reservations(reservation_id, address, prefix_len, type, "
+         "INSERT INTO ipv6_reservations(address, prefix_len, type, "
             "dhcp6_iaid, host_id) "
-         "VALUES (?,?,?,?,?,?)"},
+         "VALUES (?,?,?,?,?)"},
     {MySqlHostDataSource::GET_V6_RESRV,
          "SELECT reservation_id, address, prefix_len, type, dhcp6_iaid, host_id "
          "FROM ipv6_reservations "
@@ -563,6 +562,7 @@ private:
     HostPtr     host_;			// Pointer to Host object
 };
 
+/// @brief This class represents the exchanges related to IPv6 Reservations.
 class MySqlIPv6ReservationExchange {
     /// @brief Set number of database columns for this reservation structure
     static const size_t RESRV_COLUMNS = 6;
@@ -668,47 +668,39 @@ public:
         // Set up the structures for the various components of the host structure.
 
         try {
-            // reservation_id INT UNSIGNED NOT NULL
-            // The host_id is auto_incremented by MySQL database,
-            // so we need to pass the NULL value
-            reservation_id_ = static_cast<uint32_t>(NULL);
-            bind_[0].buffer_type = MYSQL_TYPE_LONG;
-            bind_[0].buffer = reinterpret_cast<char*>(&reservation_id_);
-            bind_[0].is_unsigned = MLM_TRUE;
-
             // address VARCHAR(39)
             address_ = resv.getPrefix().toText();
             address_len_ = address_.length();
-            bind_[1].buffer_type = MYSQL_TYPE_BLOB;
-            bind_[1].buffer = reinterpret_cast<char*>
+            bind_[0].buffer_type = MYSQL_TYPE_BLOB;
+            bind_[0].buffer = reinterpret_cast<char*>
                 (const_cast<char*>(address_.c_str()));
-            bind_[1].buffer_length = address_len_;
-            bind_[1].length = &address_len_;
+            bind_[0].buffer_length = address_len_;
+            bind_[0].length = &address_len_;
 
             // prefix_len tinyint
             prefix_len_ = resv.getPrefixLen();
-            bind_[2].buffer_type = MYSQL_TYPE_TINY;
-            bind_[2].buffer = reinterpret_cast<char*>(&prefix_len_);
-            bind_[2].is_unsigned = MLM_TRUE;
+            bind_[1].buffer_type = MYSQL_TYPE_TINY;
+            bind_[1].buffer = reinterpret_cast<char*>(&prefix_len_);
+            bind_[1].is_unsigned = MLM_TRUE;
 
             // type tinyint
             // See lease6_types for values (0 = IA_NA, 1 = IA_TA, 2 = IA_PD)
             type_ = resv.getType() == IPv6Resrv::TYPE_NA ? 0 : 2;
-            bind_[3].buffer_type = MYSQL_TYPE_TINY;
-            bind_[3].buffer = reinterpret_cast<char*>(&type_);
-            bind_[3].is_unsigned = MLM_TRUE;
+            bind_[2].buffer_type = MYSQL_TYPE_TINY;
+            bind_[2].buffer = reinterpret_cast<char*>(&type_);
+            bind_[2].is_unsigned = MLM_TRUE;
 
             // dhcp6_iaid INT UNSIGNED
             /// @todo: We don't support iaid in the IPv6Resrv yet.
             iaid_ = 0;
-            bind_[4].buffer_type = MYSQL_TYPE_LONG;
-            bind_[4].buffer = reinterpret_cast<char*>(&iaid_);
-            bind_[4].is_unsigned = MLM_TRUE;
+            bind_[3].buffer_type = MYSQL_TYPE_LONG;
+            bind_[3].buffer = reinterpret_cast<char*>(&iaid_);
+            bind_[3].is_unsigned = MLM_TRUE;
 
             // host_id INT UNSIGNED NOT NULL
-            bind_[5].buffer_type = MYSQL_TYPE_LONG;
-            bind_[5].buffer = reinterpret_cast<char*>(&host_id_);
-            bind_[5].is_unsigned = MLM_TRUE;
+            bind_[4].buffer_type = MYSQL_TYPE_LONG;
+            bind_[4].buffer = reinterpret_cast<char*>(&host_id_);
+            bind_[4].is_unsigned = MLM_TRUE;
 
         } catch (const std::exception& ex) {
             isc_throw(DbOperationError,
@@ -718,7 +710,8 @@ public:
 
         // Add the data to the vector.  Note the end element is one after the
         // end of the array.
-        return (std::vector<MYSQL_BIND>(&bind_[0], &bind_[RESRV_COLUMNS]));
+        // RESRV_COLUMNS -1 as we do not set reservation_id.
+        return (std::vector<MYSQL_BIND>(&bind_[0], &bind_[RESRV_COLUMNS-1]));
     }
 
 
@@ -789,7 +782,7 @@ public:
     /// @return IPv6Resrv object (containing IPv6 address or prefix reservation)
     IPv6Resrv getIPv6ReservData(){
 
-        // Set the IPv6 Reservation type (0 = IA_NA, 1 = IA_TA, 2 = IA_PD)
+        // Set the IPv6 Reservation type (0 = IA_NA, 2 = IA_PD)
         IPv6Resrv::Type type = IPv6Resrv::TYPE_NA;
 
         switch (type_) {
@@ -797,10 +790,6 @@ public:
             type = IPv6Resrv::TYPE_NA;
             break;
 
-        case 1:
-            type = IPv6Resrv::TYPE_TA;
-            break;
-
         case 2:
             type = IPv6Resrv::TYPE_PD;
             break;
@@ -809,7 +798,7 @@ public:
             isc_throw(BadValue,
                     "invalid IPv6 reservation type returned: "
                     << static_cast<int>(type_)
-                    << ". Only 0, 1 or 2 are allowed.");
+                    << ". Only 0 or 2 are allowed.");
         }
 
         IPv6Resrv r(type, IOAddress(address_), prefix_len_);
@@ -852,7 +841,7 @@ public:
 
 private:
     uint64_t	host_id_;        /// Host unique identifier
-    uint64_t    reservation_id_; /// Host unique identifier
+    uint64_t    reservation_id_; /// Reservation unique identifier
     size_t      host_id_length_; /// Length of the host unique ID
     std::string address_;        ///< Address (or prefix)
     size_t      address_len_;    ///< Length of the textual address representation
@@ -1007,7 +996,8 @@ MySqlHostDataSource::getIPv6ReservationCollection(StatementIndex stindex,
 
     // Set up the fetch "release" object to release resources associated
     // with the call to mysql_stmt_fetch when this method exits, then
-    // retrieve the data.
+    // retrieve the data. mysql_stmt_fetch return value equal to 0 represents
+    // successful data fetch.
     MySqlFreeResult fetch_release(conn_.statements_[stindex]);
     while ((status = mysql_stmt_fetch(conn_.statements_[stindex])) == 0) {
         try {
@@ -1022,6 +1012,7 @@ MySqlHostDataSource::getIPv6ReservationCollection(StatementIndex stindex,
     }
 
     // How did the fetch end?
+    // If mysql_stmt_fetch return value is equal to 1 an error occurred.
     if (status == 1) {
         // Error - unable to fetch results
         checkError(status, stindex, "unable to fetch results");
@@ -1089,7 +1080,8 @@ MySqlHostDataSource::getHostCollection(StatementIndex stindex, MYSQL_BIND* bind,
 
     // Set up the fetch "release" object to release resources associated
     // with the call to mysql_stmt_fetch when this method exits, then
-    // retrieve the data.
+    // retrieve the data. mysql_stmt_fetch return value equal to 0 represents
+    // successful data fetch.
     MySqlFreeResult fetch_release(conn_.statements_[stindex]);
     int count = 0;
     HostPtr host;
@@ -1113,6 +1105,7 @@ MySqlHostDataSource::getHostCollection(StatementIndex stindex, MYSQL_BIND* bind,
     }
 
     // How did the fetch end?
+    // If mysql_stmt_fetch return value is equal to 1 an error occurred.
     if (status == 1) {
         // Error - unable to fetch results
         checkError(status, stindex, "unable to fetch results");
@@ -1453,6 +1446,7 @@ std::pair<uint32_t, uint32_t> MySqlHostDataSource::getVersion() const {
 
     // Fetch the data and set up the "release" object to release associated
     // resources when this method exits then retrieve the data.
+    // mysql_stmt_fetch return value other than 0 means error occurrence.
     MySqlFreeResult fetch_release(conn_.statements_[stindex]);
     status = mysql_stmt_fetch(conn_.statements_[stindex]);
     if (status != 0) {

src/lib/dhcpsrv/mysql_host_data_source.h

@@ -165,21 +165,6 @@ public:
     virtual ConstHostPtr
     get6(const asiolink::IOAddress& prefix, const uint8_t prefix_len) const;
 
-    /// @brief Returns all IPv6 reservations assigned to single host
-    ///
-    /// @param host_id ID of a host owning IPv6 reservations
-    ///
-    /// @return Collection of IPv6 reservations
-    virtual IPv6ResrvCollection
-    getAllReservations(HostID host_id) const;
-
-    /// @brief Retrieves all IPv6 reservations for a single host and then
-    ///         adds them to that host.
-    ///
-    /// @param host Pointer to a host to be populated with IPv6 reservations.
-    void
-    assignReservations(HostPtr& host) const;
-
     /// @brief Adds a new host to the collection.
     ///
     /// The implementations of this method should guard against duplicate
@@ -312,6 +297,21 @@ private:
             boost::shared_ptr<MySqlIPv6ReservationExchange> exchange,
             IPv6ResrvCollection& result) const;
 
+    /// @brief Returns all IPv6 reservations assigned to single host
+    ///
+    /// @param host_id ID of a host owning IPv6 reservations
+    ///
+    /// @return Collection of IPv6 reservations
+    virtual IPv6ResrvCollection
+    getAllReservations(HostID host_id) const;
+
+    /// @brief Retrieves all IPv6 reservations for a single host and then
+    ///         adds them to that host.
+    ///
+    /// @param host Pointer to a host to be populated with IPv6 reservations.
+    void
+    assignReservations(HostPtr& host) const;
+
 
     /// @brief Check Error and Throw Exception
     ///

src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc

@@ -298,11 +298,20 @@ GenericHostDataSourceTest::compareReservations6(IPv6ResrvRange resrv1,
     }
 
     if (std::distance(resrv1.first, resrv1.second) > 0) {
-        if (expect_match){
-            for (; resrv1.first != resrv1.second; resrv1.first++, resrv2.first++){
-                EXPECT_EQ(resrv1.first->second.getType(), resrv2.first->second.getType());
-                EXPECT_EQ(resrv1.first->second.getPrefixLen(), resrv2.first->second.getPrefixLen());
-                EXPECT_EQ(resrv1.first->second.getPrefix(), resrv2.first->second.getPrefix());
+        for (; resrv1.first != resrv1.second; resrv1.first++) {
+            IPv6ResrvIterator iter = resrv2.first;
+            while (iter != resrv2.second) {
+                if((resrv1.first->second.getType() == iter->second.getType()) &&
+                        (resrv1.first->second.getPrefixLen() == iter->second.getPrefixLen()) &&
+                        (resrv1.first->second.getPrefix() == iter->second.getPrefix())) {
+                    break;
+                }
+                iter++;
+                if (iter == resrv2.second && expect_match) {
+                    ADD_FAILURE()<< "Reservation comparison failed, "
+                    "no match for reservation: "
+                    << resrv1.first->second.getPrefix().toText();
+                }
             }
         }
     }
@@ -752,7 +761,7 @@ void GenericHostDataSourceTest::testGetByIPv6(BaseHostDataSource::IdType id,
     EXPECT_FALSE(hdsptr_->get6(IOAddress("2001:db8::5"), len));
 }
 
-void GenericHostDataSourceTest::testAddDuplicate() {
+void GenericHostDataSourceTest::testAddDuplicate6WithSameDUID() {
     // Make sure we have the pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
 
@@ -764,7 +773,34 @@ void GenericHostDataSourceTest::testAddDuplicate() {
 
     // Then try to add it again, it should throw an exception.
     ASSERT_THROW(hdsptr_->add(host), DuplicateEntry);
+}
+
+void GenericHostDataSourceTest::testAddDuplicate6WithSameHWAddr() {
+    // Make sure we have the pointer to the host data source.
+    ASSERT_TRUE(hdsptr_);
+
+    // Create a host reservations.
+    HostPtr host = initializeHost6("2001:db8::1", BaseHostDataSource::ID_HWADDR, true);
 
+    // Add this reservation once.
+    ASSERT_NO_THROW(hdsptr_->add(host));
+
+    // Then try to add it again, it should throw an exception.
+    ASSERT_THROW(hdsptr_->add(host), DuplicateEntry);
+}
+
+void GenericHostDataSourceTest::testAddDuplicate4() {
+    // Make sure we have the pointer to the host data source.
+    ASSERT_TRUE(hdsptr_);
+
+    // Create a host reservations.
+    HostPtr host = initializeHost4("192.0.2.1", false);
+
+    // Add this reservation once.
+    ASSERT_NO_THROW(hdsptr_->add(host));
+
+    // Then try to add it again, it should throw an exception.
+    ASSERT_THROW(hdsptr_->add(host), DuplicateEntry);
 }
 
 void GenericHostDataSourceTest::testAddr6AndPrefix(){
@@ -802,10 +838,12 @@ void GenericHostDataSourceTest::testMultipletReservations(){
     IPv6Resrv resv1(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::6"), len);
     IPv6Resrv resv2(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::7"), len);
     IPv6Resrv resv3(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::8"), len);
+    IPv6Resrv resv4(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::9"), len);
 
     host->addReservation(resv1);
     host->addReservation(resv2);
     host->addReservation(resv3);
+    host->addReservation(resv4);
 
     ASSERT_NO_THROW(hdsptr_->add(host));
 
@@ -817,6 +855,34 @@ void GenericHostDataSourceTest::testMultipletReservations(){
 
     // Check if hosts are the same
     compareHosts(host, from_hds);
+}
+
+void GenericHostDataSourceTest::testMultipletReservationsDifferentOrder(){
+    // Make sure we have the pointer to the host data source.
+    ASSERT_TRUE(hdsptr_);
+    uint8_t len = 128;
+
+    HostPtr host1 = initializeHost6("2001:db8::1", BaseHostDataSource::ID_DUID, false);
+    HostPtr host2 = initializeHost6("2001:db8::1", BaseHostDataSource::ID_DUID, false);
+
+    // Add some reservations
+    IPv6Resrv resv1(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::6"), len);
+    IPv6Resrv resv2(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::7"), len);
+    IPv6Resrv resv3(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::8"), len);
+    IPv6Resrv resv4(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::9"), len);
+
+    host1->addReservation(resv1);
+    host1->addReservation(resv2);
+    host1->addReservation(resv3);
+    host1->addReservation(resv4);
+
+    host2->addReservation(resv4);
+    host2->addReservation(resv3);
+    host2->addReservation(resv2);
+    host2->addReservation(resv1);
+
+    // Check if reservations are the same
+    compareReservations6(host1->getIPv6Reservations(), host2->getIPv6Reservations(), true);
 
 }
 

src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h

@@ -215,6 +215,10 @@ public:
     ///         retrieved correctly.
     void testMultipletReservations();
 
+    /// @brief Tests if compareIPv6Reservations() method treats same pool of
+    ///         reservations but added in different order as equal.
+    void testMultipletReservationsDifferentOrder();
+
     /// @brief Test if host reservations made for different IPv6 subnets
     ///        are handled correctly.
     ///
@@ -224,10 +228,20 @@ public:
     /// @param id identifier type (ID_HWADDR or ID_DUID)
     void testSubnetId6(int subnets, BaseHostDataSource::IdType id);
 
-    /// @brief Test if the duplicate host instances can't be inserted.
+    /// @brief Test if the duplicate host with same DUID can't be inserted.
+    ///
+    /// Uses gtest macros to report failures.
+    void testAddDuplicate6WithSameDUID();
+
+    /// @brief Test if the duplicate host with same HWAddr can't be inserted.
+    ///
+    /// Uses gtest macros to report failures.
+    void testAddDuplicate6WithSameHWAddr();
+
+    /// @brief Test if the duplicate IPv4 host with can't be inserted.
     ///
     /// Uses gtest macros to report failures.
-    void testAddDuplicate();
+    void testAddDuplicate4();
 
     /// @brief Returns DUID with identical content as specified HW address
     ///

src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc

@@ -325,6 +325,12 @@ TEST_F(MySqlHostDataSourceTest, multipleReservations){
     testMultipletReservations();
 }
 
+// Tests if compareIPv6Reservations() method treats same pool of reservations
+// but added in different order as equal.
+TEST_F(MySqlHostDataSourceTest, multipleReservationsDifferentOrder){
+    testMultipletReservationsDifferentOrder();
+}
+
 // Test verifies if multiple client classes for IPv4 can be stored.
 TEST_F(MySqlHostDataSourceTest, DISABLED_multipleClientClasses4) {
     /// @todo: Implement this test as part of #4213.
@@ -381,8 +387,24 @@ TEST_F(MySqlHostDataSourceTest, subnetId6) {
 // Test if the duplicate host instances can't be inserted. The test logic is as
 // follows: try to add multiple instances of the same host reservation and
 // verify that the second and following attempts will throw exceptions.
-TEST_F(MySqlHostDataSourceTest, addDuplicate) {
-    testAddDuplicate();
+// Hosts with same DUID.
+TEST_F(MySqlHostDataSourceTest, addDuplicate6WithDUID) {
+    testAddDuplicate6WithSameDUID();
+}
+
+// Test if the duplicate host instances can't be inserted. The test logic is as
+// follows: try to add multiple instances of the same host reservation and
+// verify that the second and following attempts will throw exceptions.
+// Hosts with same HWAddr.
+TEST_F(MySqlHostDataSourceTest, addDuplicate6WithHWAddr) {
+    testAddDuplicate6WithSameHWAddr();
+}
+
+// Test if the duplicate IPv4 host instances can't be inserted. The test logic is as
+// follows: try to add multiple instances of the same host reservation and
+// verify that the second and following attempts will throw exceptions.
+TEST_F(MySqlHostDataSourceTest, addDuplicate4) {
+    testAddDuplicate4();
 }
 
 }; // Of anonymous namespace