[master] Merge branch 'trac3556' (MAC in MySQL backend for kea6)

src/lib/dhcp/hwaddr.cc

@@ -28,18 +28,18 @@ namespace isc {
 namespace dhcp {
 
 HWAddr::HWAddr()
-    :htype_(HTYPE_ETHER) {
+    :htype_(HTYPE_ETHER), source_(0) {
 }
 
 HWAddr::HWAddr(const uint8_t* hwaddr, size_t len, uint16_t htype)
-    :hwaddr_(hwaddr, hwaddr + len), htype_(htype) {
+    :hwaddr_(hwaddr, hwaddr + len), htype_(htype), source_(0) {
     if (len > MAX_HWADDR_LEN) {
         isc_throw(isc::BadValue, "hwaddr length exceeds MAX_HWADDR_LEN");
     }
 }
 
 HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint16_t htype)
-    :hwaddr_(hwaddr), htype_(htype) {
+    :hwaddr_(hwaddr), htype_(htype), source_(0) {
     if (hwaddr.size() > MAX_HWADDR_LEN) {
         isc_throw(isc::BadValue,
             "address vector size exceeds MAX_HWADDR_LEN");

src/lib/dhcp/hwaddr.h

@@ -58,6 +58,14 @@ public:
     /// 16 bits, we need to be able to store that wider format.
     uint16_t htype_;
 
+    /// @brief Hardware address source
+    ///
+    /// This variable specifies how the hardware address was obtained.
+    /// @todo This is a stub implementation. Proper implementation will move
+    /// constants from Pkt::HWADDR_SOURCE_* here. Currently always initialized
+    /// to zero.
+    uint32_t source_;
+
     /// @brief Returns textual representation of a hardware address
     /// (e.g. 00:01:02:03:04:05)
     ///

src/lib/dhcp/pkt.h

@@ -52,6 +52,10 @@ public:
     /// Not really a type, only used in getMAC() calls.
     static const uint32_t HWADDR_SOURCE_ANY = 0xffff;
 
+    /// Used when actual origin is not known, e.g. when reading from a
+    /// lease database that didn't store that information.
+    static const uint32_t HWADDR_SOURCE_UNKNOWN = 0x0000;
+
     /// Obtained first hand from raw socket (100% reliable).
     static const uint32_t HWADDR_SOURCE_RAW = 0x0001;
 

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -161,21 +161,24 @@ TaggedStatement tagged_statements[] = {
                     "SELECT address, duid, valid_lifetime, "
                         "expire, subnet_id, pref_lifetime, "
                         "lease_type, iaid, prefix_len, "
-                        "fqdn_fwd, fqdn_rev, hostname "
+                        "fqdn_fwd, fqdn_rev, hostname, "
+                        "hwaddr, hwtype, hwaddr_source "
                             "FROM lease6 "
                             "WHERE address = ? AND lease_type = ?"},
     {MySqlLeaseMgr::GET_LEASE6_DUID_IAID,
                     "SELECT address, duid, valid_lifetime, "
                         "expire, subnet_id, pref_lifetime, "
                         "lease_type, iaid, prefix_len, "
-                        "fqdn_fwd, fqdn_rev, hostname "
+                        "fqdn_fwd, fqdn_rev, hostname, "
+                        "hwaddr, hwtype, hwaddr_source "
                             "FROM lease6 "
                             "WHERE duid = ? AND iaid = ? AND lease_type = ?"},
     {MySqlLeaseMgr::GET_LEASE6_DUID_IAID_SUBID,
                     "SELECT address, duid, valid_lifetime, "
                         "expire, subnet_id, pref_lifetime, "
                         "lease_type, iaid, prefix_len, "
-                        "fqdn_fwd, fqdn_rev, hostname "
+                        "fqdn_fwd, fqdn_rev, hostname, "
+                        "hwaddr, hwtype, hwaddr_source "
                             "FROM lease6 "
                             "WHERE duid = ? AND iaid = ? AND subnet_id = ? "
                             "AND lease_type = ?"},
@@ -190,8 +193,9 @@ TaggedStatement tagged_statements[] = {
                     "INSERT INTO lease6(address, duid, valid_lifetime, "
                         "expire, subnet_id, pref_lifetime, "
                         "lease_type, iaid, prefix_len, "
-                        "fqdn_fwd, fqdn_rev, hostname) "
-                            "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"},
+                        "fqdn_fwd, fqdn_rev, hostname, "
+                        "hwaddr, hwtype, hwaddr_source) "
+                            "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"},
     {MySqlLeaseMgr::UPDATE_LEASE4,
                     "UPDATE lease4 SET address = ?, hwaddr = ?, "
                         "client_id = ?, valid_lifetime = ?, expire = ?, "
@@ -203,7 +207,7 @@ TaggedStatement tagged_statements[] = {
                         "valid_lifetime = ?, expire = ?, subnet_id = ?, "
                         "pref_lifetime = ?, lease_type = ?, iaid = ?, "
                         "prefix_len = ?, fqdn_fwd = ?, fqdn_rev = ?, "
-                        "hostname = ? "
+                        "hostname = ?, hwaddr = ?, hwtype = ?, hwaddr_source = ? "
                             "WHERE address = ?"},
     // End of list sentinel
     {MySqlLeaseMgr::NUM_STATEMENTS, NULL}
@@ -302,6 +306,7 @@ public:
     /// The initialization of the variables here is only to satisfy cppcheck -
     /// all variables are initialized/set in the methods before they are used.
     MySqlLease4Exchange() : addr4_(0), hwaddr_length_(0), client_id_length_(0),
+                            client_id_null_(MLM_FALSE),
                             fqdn_fwd_(false), fqdn_rev_(false), hostname_length_(0) {
         memset(hwaddr_buffer_, 0, sizeof(hwaddr_buffer_));
         memset(client_id_buffer_, 0, sizeof(client_id_buffer_));
@@ -648,7 +653,7 @@ private:
 
 class MySqlLease6Exchange : public MySqlLeaseExchange {
     /// @brief Set number of database columns for this lease structure
-    static const size_t LEASE_COLUMNS = 12;
+    static const size_t LEASE_COLUMNS = 15;
 
 public:
     /// @brief Constructor
@@ -657,10 +662,12 @@ public:
     /// all variables are initialized/set in the methods before they are used.
     MySqlLease6Exchange() : addr6_length_(0), duid_length_(0),
                             fqdn_fwd_(false), fqdn_rev_(false),
-                            hostname_length_(0) {
+                            hostname_length_(0), hwaddr_length_(0),
+                            hwaddr_null_(MLM_FALSE), hwtype_(0), hwaddr_source_(0) {
         memset(addr6_buffer_, 0, sizeof(addr6_buffer_));
         memset(duid_buffer_, 0, sizeof(duid_buffer_));
         memset(hostname_buffer_, 0, sizeof(hostname_buffer_));
+        memset(hwaddr_buffer_, 0, sizeof(hwaddr_buffer_));
         std::fill(&error_[0], &error_[LEASE_COLUMNS], MLM_FALSE);
 
         // Set the column names (for error messages)
@@ -676,7 +683,10 @@ public:
         columns_[9]  = "fqdn_fwd";
         columns_[10] = "fqdn_rev";
         columns_[11] = "hostname";
-        BOOST_STATIC_ASSERT(8 < LEASE_COLUMNS);
+        columns_[12] = "hwaddr";
+        columns_[13] = "hwtype";
+        columns_[14] = "hwaddr_source";
+        BOOST_STATIC_ASSERT(14 < LEASE_COLUMNS);
     }
 
     /// @brief Create MYSQL_BIND objects for Lease6 Pointer
@@ -725,6 +735,10 @@ public:
                                           // reasons, see memset() above
 
         // duid: varchar(128)
+        if (!lease_->duid_) {
+            isc_throw(DbOperationError, "lease6 for address " << addr6_
+                      << " is missing mandatory client-id.");
+        }
         duid_ = lease_->duid_->getDuid();
         duid_length_ = duid_.size();
 
@@ -820,11 +834,73 @@ public:
         // bind_[11].is_null = &MLM_FALSE; // commented out for performance
                                            // reasons, see memset() above
 
+        // hwaddr: varbinary(20) - hardware/MAC address
+        HWAddrPtr hwaddr = lease_->hwaddr_;
+        if (hwaddr) {
+            hwaddr_ = hwaddr->hwaddr_;
+            hwaddr_length_ = hwaddr->hwaddr_.size();
+
+            bind_[12].buffer_type = MYSQL_TYPE_BLOB;
+            bind_[12].buffer = reinterpret_cast<char*>(&(hwaddr_[0]));
+            bind_[12].buffer_length = hwaddr_length_;
+            bind_[12].length = &hwaddr_length_;
+        } else {
+            bind_[12].buffer_type = MYSQL_TYPE_NULL;
+
+            // According to http://dev.mysql.com/doc/refman/5.5/en/
+            // c-api-prepared-statement-data-structures.html, the other
+            // fields doesn't matter if type is set to MYSQL_TYPE_NULL,
+            // but let's set them to some sane values in case earlier versions
+            // didn't have that assumption.
+            hwaddr_null_ = MLM_TRUE;
+            bind_[12].buffer = NULL;
+            bind_[12].is_null = &hwaddr_null_;
+        }
+
+        // hwtype
+        if (hwaddr) {
+            hwtype_ = lease->hwaddr_->htype_;
+            bind_[13].buffer_type = MYSQL_TYPE_SHORT;
+            bind_[13].buffer = reinterpret_cast<char*>(&hwtype_);
+            bind_[13].is_unsigned = MLM_TRUE;
+        } else {
+            hwtype_ = 0;
+            bind_[13].buffer_type = MYSQL_TYPE_NULL;
+            // According to http://dev.mysql.com/doc/refman/5.5/en/
+            // c-api-prepared-statement-data-structures.html, the other
+            // fields doesn't matter if type is set to MYSQL_TYPE_NULL,
+            // but let's set them to some sane values in case earlier versions
+            // didn't have that assumption.
+            hwaddr_null_ = MLM_TRUE;
+            bind_[13].buffer = NULL;
+            bind_[13].is_null = &hwaddr_null_;
+        }
+
+        /// Hardware source
+        if (hwaddr) {
+            hwaddr_source_ = lease->hwaddr_->source_;
+            bind_[14].buffer_type = MYSQL_TYPE_LONG;
+            bind_[14].buffer = reinterpret_cast<char*>(&hwaddr_source_);
+            bind_[14].is_unsigned = MLM_TRUE;
+        } else {
+            hwaddr_source_ = 0;
+
+            bind_[14].buffer_type = MYSQL_TYPE_NULL;
+            // According to http://dev.mysql.com/doc/refman/5.5/en/
+            // c-api-prepared-statement-data-structures.html, the other
+            // fields doesn't matter if type is set to MYSQL_TYPE_NULL,
+            // but let's set them to some sane values in case earlier versions
+            // didn't have that assumption.
+            hwaddr_null_ = MLM_TRUE;
+            bind_[14].buffer = NULL;
+            bind_[14].is_null = &hwaddr_null_;
+        }
+
         // Add the error flags
         setErrorIndicators(bind_, error_, LEASE_COLUMNS);
 
         // .. and check that we have the numbers correct at compile time.
-        BOOST_STATIC_ASSERT(11 < LEASE_COLUMNS);
+        BOOST_STATIC_ASSERT(14 < LEASE_COLUMNS);
 
         // Add the data to the vector.  Note the end element is one after the
         // end of the array.
@@ -941,11 +1017,31 @@ public:
         // bind_[11].is_null = &MLM_FALSE; // commented out for performance
                                            // reasons, see memset() above
 
+        // hardware address
+        // hwaddr: varbinary(20)
+        hwaddr_null_ = MLM_FALSE;
+        hwaddr_length_ = sizeof(hwaddr_buffer_);
+        bind_[12].buffer_type = MYSQL_TYPE_BLOB;
+        bind_[12].buffer = reinterpret_cast<char*>(hwaddr_buffer_);
+        bind_[12].buffer_length = hwaddr_length_;
+        bind_[12].length = &hwaddr_length_;
+        bind_[12].is_null = &hwaddr_null_;
+
+        // hardware type: unsigned short int (16 bits)
+        bind_[13].buffer_type = MYSQL_TYPE_SHORT;
+        bind_[13].buffer = reinterpret_cast<char*>(&hwtype_);
+        bind_[13].is_unsigned = MLM_TRUE;
+
+        // hardware source: unsigned int (32 bits)
+        bind_[14].buffer_type = MYSQL_TYPE_LONG;
+        bind_[14].buffer = reinterpret_cast<char*>(&hwaddr_source_);
+        bind_[14].is_unsigned = MLM_TRUE;
+
         // Add the error flags
         setErrorIndicators(bind_, error_, LEASE_COLUMNS);
 
         // .. and check that we have the numbers correct at compile time.
-        BOOST_STATIC_ASSERT(11 < LEASE_COLUMNS);
+        BOOST_STATIC_ASSERT(14 < LEASE_COLUMNS);
 
         // Add the data to the vector.  Note the end element is one after the
         // end of the array.
@@ -1001,8 +1097,12 @@ public:
         std::string hostname(hostname_buffer_,
                              hostname_buffer_ + hostname_length_);
 
-        /// @todo: HWAddr is not yet stored, see ticket #3556.
+        /// Set hardware address if it was set
         HWAddrPtr hwaddr;
+        if (hwaddr_null_ == MLM_FALSE) {
+            hwaddr.reset(new HWAddr(hwaddr_buffer_, hwaddr_length_, hwtype_));
+            hwaddr->source_ = hwaddr_source_;
+        }
 
         // Create the lease and set the cltt (after converting from the
         // expire time retrieved from the database).
@@ -1060,7 +1160,13 @@ private:
     char            hostname_buffer_[HOSTNAME_MAX_LEN];
                                         ///< Client hostname
     unsigned long   hostname_length_;   ///< Client hostname length
-
+    uint8_t         hwaddr_buffer_[HWAddr::MAX_HWADDR_LEN];
+                                        ///< Buffer for Hardware address
+    std::vector<uint8_t> hwaddr_;       ///< Hardware address (optional)
+    unsigned long   hwaddr_length_;     ///< Aux. variable denoting hwaddr_ size()
+    my_bool         hwaddr_null_;       ///< Used when HWAddr is null
+    uint16_t        hwtype_;            ///< Hardware type
+    uint32_t        hwaddr_source_;     ///< Source of the hardware address
 };
 
 

src/lib/dhcpsrv/mysql_lease_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -77,7 +77,7 @@ private:
 
 // Define the current database schema values
 
-const uint32_t CURRENT_VERSION_VERSION = 1;
+const uint32_t CURRENT_VERSION_VERSION = 2;
 const uint32_t CURRENT_VERSION_MINOR = 0;
 
 

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc

@@ -916,6 +916,54 @@ GenericLeaseMgrTest::testLease6MAC() {
     EXPECT_FALSE(stored3->hwaddr_);
 }
 
+// Checks whether a hardware address type can be stored and retrieved.
+void
+GenericLeaseMgrTest::testLease6HWTypeAndSource() {
+    // Get the leases to be used for the test.
+    vector<Lease6Ptr> leases = createLeases6();
+
+    HWAddrPtr hwaddr1(new HWAddr(vector<uint8_t>(6, 11), 123));
+    HWAddrPtr hwaddr2(new HWAddr(vector<uint8_t>(6, 22), 456));
+
+    // Those should use defines from Pkt::HWADDR_SOURCE_*, but let's
+    // test an uncommon value (and 0 which means unknown).
+    hwaddr1->source_ = 123456u;
+    hwaddr2->source_ = 0;
+
+    leases[1]->hwaddr_ = hwaddr1;     // Add hardware address to leases 1 and 2
+    leases[2]->hwaddr_ = hwaddr2;
+    leases[3]->hwaddr_ = HWAddrPtr(); // No hardware address for the third one
+
+    // Start the tests.  Add three leases to the database, read them back and
+    // check they are what we think they are.
+    EXPECT_TRUE(lmptr_->addLease(leases[1]));
+    EXPECT_TRUE(lmptr_->addLease(leases[2]));
+    EXPECT_TRUE(lmptr_->addLease(leases[3]));
+    lmptr_->commit();
+
+    // Reopen the database to ensure that they actually got stored.
+    reopen(V6);
+
+    // First lease should have a hardware address in it
+    Lease6Ptr stored1 = lmptr_->getLease6(leasetype6_[1], ioaddress6_[1]);
+    ASSERT_TRUE(stored1);
+    ASSERT_TRUE(stored1->hwaddr_);
+    EXPECT_EQ(123, stored1->hwaddr_->htype_);
+    EXPECT_EQ(123456, stored1->hwaddr_->source_);
+
+    // Second lease should have a hardware address in it
+    Lease6Ptr stored2 = lmptr_->getLease6(leasetype6_[2], ioaddress6_[2]);
+    ASSERT_TRUE(stored2);
+    ASSERT_TRUE(stored2->hwaddr_);
+    EXPECT_EQ(456, stored2->hwaddr_->htype_);
+    EXPECT_EQ(0, stored2->hwaddr_->source_);
+
+    // Third lease should NOT have any hardware address.
+    Lease6Ptr stored3 = lmptr_->getLease6(leasetype6_[3], ioaddress6_[3]);
+    ASSERT_TRUE(stored3);
+    EXPECT_FALSE(stored3->hwaddr_);
+}
+
 void
 GenericLeaseMgrTest::testLease4InvalidHostname() {
     // Get the leases to be used for the test.

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.h

@@ -189,6 +189,9 @@ public:
     /// @brief Checks that Lease6 can be stored with and without a hardware address.
     void testLease6MAC();
 
+    /// @brief Checks that Lease6 stores hardware type and hardware source.
+    void testLease6HWTypeAndSource();
+
     /// @brief Test that IPv6 lease can be added, retrieved and deleted.
     ///
     /// This method checks basic IPv6 lease operations. There's check_t1_t2

src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -135,7 +135,8 @@ void createSchema() {
 
     // Execute creation statements.
     for (int i = 0; create_statement[i] != NULL; ++i) {
-        (void) mysql_query(mysql, create_statement[i]);
+        ASSERT_EQ(0, mysql_query(mysql, create_statement[i]))
+            << "Failed on statement " << i << ": " << create_statement[i];
     }
 }
 
@@ -517,9 +518,18 @@ TEST_F(MySqlLeaseMgrTest, testRecreateLease6) {
 }
 
 /// @brief Checks that null DUID is not allowed.
-/// Test is disabled as MySqlLeaseMgr does not currently defend against a null DUID.
-TEST_F(MySqlLeaseMgrTest, DISABLED_nullDuid) {
+TEST_F(MySqlLeaseMgrTest, nullDuid) {
     testNullDuid();
 }
 
+/// @brief Tests whether memfile can store and retrieve hardware addresses
+TEST_F(MySqlLeaseMgrTest, testLease6Mac) {
+    testLease6MAC();
+}
+
+/// @brief Tests whether memfile can store and retrieve hardware addresses
+TEST_F(MySqlLeaseMgrTest, testLease6HWTypeAndSource) {
+    testLease6HWTypeAndSource();
+}
+
 }; // Of anonymous namespace

src/lib/dhcpsrv/tests/schema_mysql_copy.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -35,6 +35,7 @@ const char* destroy_statement[] = {
     "DROP TABLE lease4",
     "DROP TABLE lease6",
     "DROP TABLE lease6_types",
+    "DROP TABLE lease_hwaddr_source",
     "DROP TABLE schema_version",
     NULL
 };
@@ -42,6 +43,8 @@ const char* destroy_statement[] = {
 // Creation of the new tables.
 
 const char* create_statement[] = {
+
+    // Schema initialization to 1.0 starts here.
     "START TRANSACTION",
     "CREATE TABLE lease4 ("
         "address INT UNSIGNED PRIMARY KEY NOT NULL,"
@@ -93,6 +96,35 @@ const char* create_statement[] = {
     "INSERT INTO schema_version VALUES (1, 0)",
     "COMMIT",
 
+    // Schema initialization to 1.0 ends here.
+
+    // Schema upgrade to 2.0 starts here.
+    "ALTER TABLE lease6 "
+    "ADD COLUMN hwaddr varbinary(20),"
+    "ADD COLUMN hwtype smallint unsigned,"
+    "ADD COLUMN hwaddr_source int unsigned;",
+
+    // Production schema has lease_hwaddr_source table. It is not used by
+    // kea code and is simply useful for formulating more human readable
+    // queries. Hence no need to create it in tests. The actual SQL
+    // code remains here commented out to keep a trace that the omission
+    // is intentional.
+
+    /* "CREATE TABLE lease_hwaddr_source ("
+    "hwaddr_source INT PRIMARY KEY NOT NULL,"
+    "name VARCHAR(40) )",
+
+    "INSERT INTO lease_hwaddr_source VALUES (1, \"HWADDR_SOURCE_RAW\");",
+    "INSERT INTO lease_hwaddr_source VALUES (2, \"HWADDR_SOURCE_IPV6_LINK_LOCAL\");",
+    "INSERT INTO lease_hwaddr_source VALUES (4, \"HWADDR_SOURCE_DUID\");",
+    "INSERT INTO lease_hwaddr_source VALUES (8, \"HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION\");",
+    "INSERT INTO lease_hwaddr_source VALUES (16, \"HWADDR_SOURCE_REMOTE_ID\");",
+    "INSERT INTO lease_hwaddr_source VALUES (32, \"HWADDR_SOURCE_SUBSCRIBER_ID\");",
+    "INSERT INTO lease_hwaddr_source VALUES (64, \"HWADDR_SOURCE_DOCSIS\");", */
+
+    "UPDATE schema_version SET version=\"2\", minor=\"0\";",
+    // Schema upgrade to 2.0 ends here.
+
     NULL
 };