Browse Source

[3555] Lease4,Lease6 now uses HWAddr structure.

Tomek Mrugalski 10 years ago
parent
commit
8f112316e6

+ 5 - 4
src/bin/dhcp4/dhcp4_srv.cc

@@ -489,8 +489,7 @@ Dhcpv4Srv::computeDhcid(const Lease4Ptr& lease) {
             return (D2Dhcid(lease->client_id_->getClientId(), fqdn_wire));
 
         } else {
-            HWAddrPtr hwaddr(new HWAddr(lease->hwaddr_, HTYPE_ETHER));
-            return (D2Dhcid(hwaddr, fqdn_wire));
+            return (D2Dhcid(lease->hwaddr_, fqdn_wire));
         }
     } catch (const Exception& ex) {
         isc_throw(DhcidComputeError, "unable to compute DHCID: "
@@ -1391,8 +1390,10 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) {
         }
 
         // Does the hardware address match? We don't want one client releasing
-        // second client's leases.
-        if (lease->hwaddr_ != release->getHWAddr()->hwaddr_) {
+        // second client's leases. Note that we're comparing the hardware
+        // addresses only, not hardware types or sources of the hardware
+        // addresses. Thus we don't use HWAddr::equals().
+        if (lease->hwaddr_->hwaddr_ != release->getHWAddr()->hwaddr_) {
             /// @todo: Print hwaddr from lease as part of ticket #2589
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_RELEASE_FAIL_WRONG_HWADDR)
                 .arg(release->getCiaddr().toText())

+ 2 - 4
src/bin/dhcp4/tests/dhcp4_client.cc

@@ -123,8 +123,7 @@ Dhcp4Client::applyConfiguration() {
 
     /// @todo Set the valid lifetime, t1, t2 etc.
     config_.lease_ = Lease4(IOAddress(context_.response_->getYiaddr()),
-                            &context_.response_->getHWAddr()->hwaddr_[0],
-                            context_.response_->getHWAddr()->hwaddr_.size(),
+                            context_.response_->getHWAddr(),
                             0, 0, 0, 0, 0, time(NULL), 0, false, false,
                             "");
 }
@@ -132,8 +131,7 @@ Dhcp4Client::applyConfiguration() {
 void
 Dhcp4Client::createLease(const asiolink::IOAddress& addr,
                          const uint32_t valid_lft) {
-    Lease4 lease(addr, &hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
-                 0, 0, valid_lft, valid_lft / 2, valid_lft,
+    Lease4 lease(addr, hwaddr_, 0, 0, valid_lft, valid_lft / 2, valid_lft,
                  time(NULL), false, false, "");
     config_.lease_ = lease;
 }

+ 18 - 15
src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -816,8 +816,9 @@ TEST_F(Dhcpv4SrvTest, RenewBasic) {
     ASSERT_TRUE(subnet_->inPool(Lease::TYPE_V4, addr));
 
     // let's create a lease and put it in the LeaseMgr
-    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
-    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
+    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2,
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               temp_valid, temp_t1, temp_t2, temp_timestamp,
                               subnet_->getID()));
@@ -983,9 +984,9 @@ TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
     ASSERT_TRUE(subnet_->inPool(Lease::TYPE_V4, addr));
 
     // Let's create a lease and put it in the LeaseMgr
-    uint8_t mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
-    HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
-    Lease4Ptr used(new Lease4(addr, mac_addr, sizeof(mac_addr),
+    uint8_t hwaddr_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr(new HWAddr(hwaddr_data, sizeof(hwaddr_data), HTYPE_ETHER));
+    Lease4Ptr used(new Lease4(addr, hwaddr,
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               temp_valid, temp_t1, temp_t2, temp_timestamp,
                               subnet_->getID()));
@@ -1002,7 +1003,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
     rel->setCiaddr(addr);
     rel->addOption(clientid);
     rel->addOption(srv->getServerID());
-    rel->setHWAddr(hw);
+    rel->setHWAddr(hwaddr);
     rel->setIface("eth0");
 
     // Pass it to the server and hope for a REPLY
@@ -1014,11 +1015,11 @@ TEST_F(Dhcpv4SrvTest, ReleaseBasic) {
     EXPECT_FALSE(l);
 
     // Try to get the lease by hardware address
-    Lease4Collection leases = LeaseMgrFactory::instance().getLease4(hw->hwaddr_);
+    Lease4Collection leases = LeaseMgrFactory::instance().getLease4(*hwaddr);
     EXPECT_EQ(leases.size(), 0);
 
     // Try to get it by hw/subnet_id combination
-    l = LeaseMgrFactory::instance().getLease4(hw->hwaddr_, subnet_->getID());
+    l = LeaseMgrFactory::instance().getLease4(*hwaddr, subnet_->getID());
     EXPECT_FALSE(l);
 
     // Try by client-id
@@ -1083,7 +1084,7 @@ TEST_F(Dhcpv4SrvTest, ReleaseReject) {
     // Let's create a lease and put it in the LeaseMgr
     uint8_t mac_addr[] = { 0, 0x1, 0x2, 0x3, 0x4, 0x5};
     HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
-    Lease4Ptr used(new Lease4(addr, mac_addr, sizeof(mac_addr),
+    Lease4Ptr used(new Lease4(addr, hw,
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               valid, t1, t2, timestamp, subnet_->getID()));
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
@@ -2613,8 +2614,9 @@ TEST_F(HooksDhcpv4SrvTest, lease4RenewSimple) {
     ASSERT_TRUE(subnet_->inPool(Lease::TYPE_V4, addr));
 
     // let's create a lease and put it in the LeaseMgr
-    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
-    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
+    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2,
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               temp_valid, temp_t1, temp_t2, temp_timestamp,
                               subnet_->getID()));
@@ -2700,8 +2702,9 @@ TEST_F(HooksDhcpv4SrvTest, lease4RenewSkip) {
     ASSERT_TRUE(subnet_->inPool(Lease::TYPE_V4, addr));
 
     // let's create a lease and put it in the LeaseMgr
-    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
-    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
+    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2,
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               temp_valid, temp_t1, temp_t2, temp_timestamp,
                               subnet_->getID()));
@@ -2769,7 +2772,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSimple) {
     // Let's create a lease and put it in the LeaseMgr
     uint8_t mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
     HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
-    Lease4Ptr used(new Lease4(addr, mac_addr, sizeof(mac_addr),
+    Lease4Ptr used(new Lease4(addr, hw,
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               temp_valid, temp_t1, temp_t2, temp_timestamp,
                               subnet_->getID()));
@@ -2856,7 +2859,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSkip) {
     // Let's create a lease and put it in the LeaseMgr
     uint8_t mac_addr[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
     HWAddrPtr hw(new HWAddr(mac_addr, sizeof(mac_addr), HTYPE_ETHER));
-    Lease4Ptr used(new Lease4(addr, mac_addr, sizeof(mac_addr),
+    Lease4Ptr used(new Lease4(addr, hw,
                               &client_id_->getDuid()[0], client_id_->getDuid().size(),
                               temp_valid, temp_t1, temp_t2, temp_timestamp,
                               subnet_->getID()));

+ 6 - 4
src/bin/dhcp4/tests/direct_client_unittest.cc

@@ -307,8 +307,9 @@ TEST_F(DirectClientTest, renew) {
     // Create a lease for a client that we will later renewed. By explicitly
     // creating a lease we will get to know the lease parameters, such as
     // leased address etc.
-    const uint8_t hwaddr[] = { 1, 2, 3, 4, 5, 6 };
-    Lease4Ptr lease(new Lease4(IOAddress("10.0.0.10"), hwaddr, sizeof(hwaddr),
+    const uint8_t hwaddr_data[] = { 1, 2, 3, 4, 5, 6 };
+    HWAddrPtr hwaddr(new HWAddr(hwaddr_data, sizeof(hwaddr_data), HTYPE_ETHER));
+    Lease4Ptr lease(new Lease4(IOAddress("10.0.0.10"), hwaddr,
                                &generateClientId()->getData()[0],
                                generateClientId()->getData().size(),
                                100, 50, 75, time(NULL),
@@ -364,8 +365,9 @@ TEST_F(DirectClientTest, rebind) {
                                                       classify_);
     // Create a lease, which will be later renewed. By explicitly creating a
     // lease we will know the lease parameters, such as leased address etc.
-    const uint8_t hwaddr[] = { 1, 2, 3, 4, 5, 6 };
-    Lease4Ptr lease(new Lease4(IOAddress("10.0.0.10"), hwaddr, sizeof(hwaddr),
+    const uint8_t hwaddr_data[] = { 1, 2, 3, 4, 5, 6 };
+    HWAddrPtr hwaddr(new HWAddr(hwaddr_data, sizeof(hwaddr_data), HTYPE_ETHER));
+    Lease4Ptr lease(new Lease4(IOAddress("10.0.0.10"), hwaddr,
                                &generateClientId()->getData()[0],
                                generateClientId()->getData().size(),
                                100, 50, 75, time(NULL),

+ 3 - 2
src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -93,8 +93,9 @@ public:
                           const std::string& hostname,
                           const bool fqdn_fwd,
                           const bool fqdn_rev) {
-        const uint8_t hwaddr[] = { 0, 1, 2, 3, 4, 5, 6 };
-        Lease4Ptr lease(new Lease4(addr, hwaddr, sizeof(hwaddr),
+        const uint8_t hwaddr_data[] = { 0, 1, 2, 3, 4, 5, 6 };
+        HWAddrPtr hwaddr(new HWAddr(hwaddr_data, sizeof(hwaddr_data), HTYPE_ETHER));
+        Lease4Ptr lease(new Lease4(addr, hwaddr,
                                    &generateClientId()->getData()[0],
                                    generateClientId()->getData().size(),
                                    100, 50, 75, time(NULL), subnet_->getID()));

+ 13 - 2
src/bin/dhcp6/dhcp6_srv.cc

@@ -1268,6 +1268,11 @@ Dhcpv6Srv::assignIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
         hostname = fqdn->getDomainName();
     }
 
+    // Attempt to get MAC address using any of available mechanisms.
+    // It's ok if there response is NULL. Hardware address is optional in Lease6
+    /// @todo: Make this configurable after trac 3554 is done.
+    HWAddrPtr hwaddr = query->getMAC(Pkt::HWADDR_SOURCE_ANY);
+
     // Use allocation engine to pick a lease for this client. Allocation engine
     // will try to honour the hint, but it is just a hint - some other address
     // may be used instead. If fake_allocation is set to false, the lease will
@@ -1280,7 +1285,8 @@ Dhcpv6Srv::assignIA_NA(const Subnet6Ptr& subnet, const DuidPtr& duid,
                                                              hostname,
                                                              fake_allocation,
                                                              callout_handle,
-                                                             old_leases);
+                                                             old_leases,
+                                                             hwaddr);
     /// @todo: Handle more than one lease
     Lease6Ptr lease;
     if (!leases.empty()) {
@@ -1383,6 +1389,11 @@ Dhcpv6Srv::assignIA_PD(const Subnet6Ptr& subnet, const DuidPtr& duid,
         hint = hint_opt->getAddress();
     }
 
+    // Attempt to get MAC address using any of available mechanisms.
+    // It's ok if there response is NULL. Hardware address is optional in Lease6
+    /// @todo: Make this configurable after trac 3554 is done.
+    HWAddrPtr hwaddr = query->getMAC(Pkt::HWADDR_SOURCE_ANY);
+
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PROCESS_IA_PD_REQUEST)
         .arg(duid ? duid->toText() : "(no-duid)").arg(ia->getIAID())
         .arg(hint_opt ? hint.toText() : "(no hint)");
@@ -1409,7 +1420,7 @@ Dhcpv6Srv::assignIA_PD(const Subnet6Ptr& subnet, const DuidPtr& duid,
                                                              string(),
                                                              fake_allocation,
                                                              callout_handle,
-                                                             old_leases);
+                                                             old_leases, hwaddr);
 
     if (!leases.empty()) {
 

+ 1 - 0
src/bin/dhcp6/tests/dhcp6_client.cc

@@ -117,6 +117,7 @@ Dhcp6Client::applyRcvdConfiguration(const Pkt6Ptr& reply) {
                                                iaprefix->getPreferred(),
                                                iaprefix->getValid(),
                                                ia->getT1(), ia->getT2(), 0,
+                                               HWAddrPtr(),
                                                iaprefix->getLength());
                     lease_info.lease_.cltt_ = time(NULL);
                 }

+ 4 - 4
src/bin/dhcp6/tests/dhcp6_test_utils.cc

@@ -213,7 +213,7 @@ Dhcpv6SrvTest::testRenewBasic(Lease::Type type, const std::string& existing_addr
     // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(type, existing, duid_, iaid, 501, 502, 503, 504,
-                               subnet_->getID(), prefix_len));
+                               subnet_->getID(), HWAddrPtr(), prefix_len));
     lease->cltt_ = 1234;
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
@@ -358,7 +358,7 @@ Dhcpv6SrvTest::testRenewReject(Lease::Type type, const IOAddress& addr) {
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(type, addr, duid_, valid_iaid,
                                501, 502, 503, 504, subnet_->getID(),
-                               prefix_len));
+                               HWAddrPtr(), prefix_len));
     lease->cltt_ = 123; // Let's use it as an indicator that the lease
                         // was NOT updated.
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
@@ -430,7 +430,7 @@ Dhcpv6SrvTest::testReleaseBasic(Lease::Type type, const IOAddress& existing,
     // Let's prepopulate the database
     Lease6Ptr lease(new Lease6(type, existing, duid_, iaid,
                                501, 502, 503, 504, subnet_->getID(),
-                               prefix_len));
+                               HWAddrPtr(), prefix_len));
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
     // Check that the lease is really in the database
@@ -536,7 +536,7 @@ Dhcpv6SrvTest::testReleaseReject(Lease::Type type, const IOAddress& addr) {
     SCOPED_TRACE("CASE 2: Lease is known and belongs to this client, but to a different IAID");
 
     Lease6Ptr lease(new Lease6(type, addr, duid_, valid_iaid, 501, 502, 503,
-                               504, subnet_->getID(), prefix_len));
+                               504, subnet_->getID(), HWAddrPtr(), prefix_len));
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
     // Let's create a different RELEASE, with a bogus iaid

+ 1 - 1
src/bin/dhcp6/tests/fqdn_unittest.cc

@@ -77,7 +77,7 @@ public:
         generateClientId();
         lease_.reset(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:1::1"),
                                 duid_, 1234, 501, 502, 503,
-                                504, 1, 0));
+                                504, 1, HWAddrPtr(), 0));
         // Config DDNS to be enabled, all controls off
         enableD2();
     }

+ 10 - 5
src/bin/dhcp6/tests/hooks_unittest.cc

@@ -1046,7 +1046,8 @@ TEST_F(HooksDhcpv6SrvTest, basic_lease6_renew) {
     // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid,
-                               501, 502, 503, 504, subnet_->getID(), 0));
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), 0));
     lease->cltt_ = 1234;
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
@@ -1144,7 +1145,8 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdate_lease6_renew) {
     // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid,
-                               501, 502, 503, 504, subnet_->getID(), 0));
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), 0));
     lease->cltt_ = 1234;
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
@@ -1236,7 +1238,8 @@ TEST_F(HooksDhcpv6SrvTest, skip_lease6_renew) {
     // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid,
-                               501, 502, 503, 504, subnet_->getID(), 0));
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), 0));
     lease->cltt_ = 1234;
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
@@ -1313,7 +1316,8 @@ TEST_F(HooksDhcpv6SrvTest, basic_lease6_release) {
     // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid,
-                               501, 502, 503, 504, subnet_->getID(), 0));
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), 0));
     lease->cltt_ = 1234;
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
@@ -1394,7 +1398,8 @@ TEST_F(HooksDhcpv6SrvTest, skip_lease6_release) {
     // Note that preferred, valid, T1 and T2 timers and CLTT are set to invalid
     // value on purpose. They should be updated during RENEW.
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, duid_, iaid,
-                               501, 502, 503, 504, subnet_->getID(), 0));
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), 0));
     lease->cltt_ = 1234;
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 

+ 10 - 10
src/lib/dhcpsrv/alloc_engine.cc

@@ -299,7 +299,7 @@ AllocEngine::allocateLeases6(const Subnet6Ptr& subnet, const DuidPtr& duid,
                              const bool rev_dns_update,
                              const std::string& hostname, bool fake_allocation,
                              const isc::hooks::CalloutHandlePtr& callout_handle,
-                             Lease6Collection& old_leases) {
+                             Lease6Collection& old_leases, const HWAddrPtr& hwaddr) {
 
     try {
         AllocatorPtr allocator = getAllocator(type);
@@ -349,7 +349,7 @@ AllocEngine::allocateLeases6(const Subnet6Ptr& subnet, const DuidPtr& duid,
                 lease = createLease6(subnet, duid, iaid, hint,
                                      pool->getLength(), type,
                                      fwd_dns_update, rev_dns_update,
-                                     hostname, callout_handle, fake_allocation);
+                                     hostname, hwaddr, callout_handle, fake_allocation);
 
                 // It can happen that the lease allocation failed (we could
                 // have lost the race condition. That means that the hint is
@@ -429,7 +429,7 @@ AllocEngine::allocateLeases6(const Subnet6Ptr& subnet, const DuidPtr& duid,
 
                 Lease6Ptr lease = createLease6(subnet, duid, iaid, candidate,
                                                prefix_len, type, fwd_dns_update,
-                                               rev_dns_update, hostname,
+                                               rev_dns_update, hostname, hwaddr,
                                                callout_handle, fake_allocation);
                 if (lease) {
                     // We are allocating a new lease (not renewing). So, the
@@ -664,7 +664,7 @@ Lease4Ptr AllocEngine::renewLease4(const SubnetPtr& subnet,
     Lease4 old_values = *lease;
 
     lease->subnet_id_ = subnet->getID();
-    lease->hwaddr_ = hwaddr->hwaddr_;
+    lease->hwaddr_ = hwaddr;
     lease->client_id_ = clientid;
     lease->cltt_ = time(NULL);
     lease->t1_ = subnet->getT1();
@@ -819,7 +819,7 @@ Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired,
 
     // address, lease type and prefixlen (0) stay the same
     expired->client_id_ = clientid;
-    expired->hwaddr_ = hwaddr->hwaddr_;
+    expired->hwaddr_ = hwaddr;
     expired->valid_lft_ = subnet->getValid();
     expired->t1_ = subnet->getT1();
     expired->t2_ = subnet->getT2();
@@ -893,6 +893,7 @@ Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
                                     const bool fwd_dns_update,
                                     const bool rev_dns_update,
                                     const std::string& hostname,
+                                    const HWAddrPtr& hwaddr,
                                     const isc::hooks::CalloutHandlePtr& callout_handle,
                                     bool fake_allocation /*= false */ ) {
 
@@ -903,7 +904,7 @@ Lease6Ptr AllocEngine::createLease6(const Subnet6Ptr& subnet,
     Lease6Ptr lease(new Lease6(type, addr, duid, iaid,
                                subnet->getPreferred(), subnet->getValid(),
                                subnet->getT1(), subnet->getT2(), subnet->getID(),
-                               prefix_len));
+                               hwaddr, prefix_len));
 
     lease->fqdn_fwd_ = fwd_dns_update;
     lease->fqdn_rev_ = rev_dns_update;
@@ -990,10 +991,9 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet,
         local_copy = clientid->getDuid();
     }
 
-    Lease4Ptr lease(new Lease4(addr, &hwaddr->hwaddr_[0], hwaddr->hwaddr_.size(),
-                               &local_copy[0], local_copy.size(), subnet->getValid(),
-                               subnet->getT1(), subnet->getT2(), now,
-                               subnet->getID()));
+    Lease4Ptr lease(new Lease4(addr, hwaddr, &local_copy[0], local_copy.size(),
+                               subnet->getValid(), subnet->getT1(), subnet->getT2(),
+                               now, subnet->getID()));
 
     // Set FQDN specific lease parameters.
     lease->fqdn_fwd_ = fwd_dns_update;

+ 4 - 2
src/lib/dhcpsrv/alloc_engine.h

@@ -343,6 +343,7 @@ protected:
     ///        collection of new leases, being returned. For newly allocated
     ///        leases (not renewed) the NULL pointers are stored in this
     ///        collection as old leases.
+    /// @param hwaddr Hardware address (optional, may be null for Lease6)
     ///
     /// @return Allocated IPv6 leases (may be empty if allocation failed)
     Lease6Collection
@@ -352,7 +353,7 @@ protected:
                     const bool fwd_dns_update, const bool rev_dns_update,
                     const std::string& hostname, bool fake_allocation,
                     const isc::hooks::CalloutHandlePtr& callout_handle,
-                    Lease6Collection& old_leases);
+                    Lease6Collection& old_leases, const HWAddrPtr& hwaddr);
 
     /// @brief returns allocator for a given pool type
     /// @param type type of pool (V4, IA, TA or PD)
@@ -416,6 +417,7 @@ private:
     ///        responsibility for the reverse DNS Update for this lease
     ///        (if true).
     /// @param hostname A fully qualified domain-name of the client.
+    /// @param hwaddr Hardware address (optional, may be null for Lease6)
     /// @param callout_handle a callout handle (used in hooks). A lease callouts
     ///        will be executed if this parameter is passed (and there are callouts
     ///        registered)
@@ -427,7 +429,7 @@ private:
                            const uint32_t iaid, const isc::asiolink::IOAddress& addr,
                            const uint8_t prefix_len, const Lease::Type type,
                            const bool fwd_dns_update, const bool rev_dns_update,
-                           const std::string& hostname,
+                           const std::string& hostname, const HWAddrPtr& hwaddr,
                            const isc::hooks::CalloutHandlePtr& callout_handle,
                            bool fake_allocation = false);
 

+ 5 - 3
src/lib/dhcpsrv/csv_lease_file4.cc

@@ -29,8 +29,10 @@ void
 CSVLeaseFile4::append(const Lease4& lease) const {
     CSVRow row(getColumnCount());
     row.writeAt(getColumnIndex("address"), lease.addr_.toText());
-    HWAddr hwaddr(lease.hwaddr_, HTYPE_ETHER);
-    row.writeAt(getColumnIndex("hwaddr"), hwaddr.toText(false));
+    if (!lease.hwaddr_) {
+        isc_throw(BadValue, "Lease4 must have hardware address specified.");
+    }
+    row.writeAt(getColumnIndex("hwaddr"), lease.hwaddr_->toText(false));
     // Client id may be unset (NULL).
     if (lease.client_id_) {
         row.writeAt(getColumnIndex("client_id"), lease.client_id_->toText());
@@ -74,7 +76,7 @@ CSVLeaseFile4::next(Lease4Ptr& lease) {
         // that.
         HWAddr hwaddr = readHWAddr(row);
         lease.reset(new Lease4(readAddress(row),
-                               &hwaddr.hwaddr_[0], hwaddr.hwaddr_.size(),
+                               HWAddrPtr(new HWAddr(hwaddr)),
                                client_id_vec.empty() ? NULL : &client_id_vec[0],
                                client_id_len,
                                readValid(row),

+ 37 - 0
src/lib/dhcpsrv/csv_lease_file6.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/csv_lease_file6.h>
 
 using namespace isc::asiolink;
@@ -41,6 +42,10 @@ CSVLeaseFile6::append(const Lease6& lease) const {
     row.writeAt(getColumnIndex("fqdn_fwd"), lease.fqdn_fwd_);
     row.writeAt(getColumnIndex("fqdn_rev"), lease.fqdn_rev_);
     row.writeAt(getColumnIndex("hostname"), lease.hostname_);
+    if (lease.hwaddr_) {
+        // We may not have hardware information
+        row.writeAt(getColumnIndex("hwaddr"), lease.hwaddr_);
+    }
     CSVFile::append(row);
 }
 
@@ -64,6 +69,7 @@ CSVLeaseFile6::next(Lease6Ptr& lease) {
                                readIAID(row), readPreferred(row),
                                readValid(row), 0, 0, // t1, t2 = 0
                                readSubnetID(row),
+                               readHWAddr(row),
                                readPrefixLen(row)));
         lease->cltt_ = readCltt(row);
         lease->fqdn_fwd_ = readFqdnFwd(row);
@@ -94,6 +100,7 @@ CSVLeaseFile6::initColumns() {
     addColumn("fqdn_fwd");
     addColumn("fqdn_rev");
     addColumn("hostname");
+    addColumn("hwaddr");
 }
 
 Lease::Type
@@ -172,5 +179,35 @@ CSVLeaseFile6::readHostname(const CSVRow& row) {
     return (hostname);
 }
 
+HWAddrPtr
+CSVLeaseFile6::readHWAddr(const CSVRow& row) {
+
+    try {
+        const HWAddr& hwaddr = HWAddr::fromText(row.readAt(getColumnIndex("hwaddr")));
+        if (hwaddr.hwaddr_.empty()) {
+            return (HWAddrPtr());
+        }
+
+        /// @todo: HWAddr returns an object, not a pointer. Without HWAddr
+        /// refactoring, at least one copy is unavoidable.
+
+        // Let's return a pointer to new freshly created copy.
+        return (HWAddrPtr(new HWAddr(hwaddr)));
+
+    } catch (const CSVFileError&) {
+        // That's ok, we may be reading old CSV file that didn't store hwaddr
+        return (HWAddrPtr());
+    } catch (const BadValue& ex) {
+        // That's worse. There was something in the file, but its conversion
+        // to HWAddr failed. Let's log it on warning and carry on.
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_MEMFILE_READ_HWADDR_FAIL)
+            .arg(ex.what());
+
+        return (HWAddrPtr());
+    }
+
+
+}
+
 } // end of namespace isc::dhcp
 } // end of namespace isc

+ 6 - 0
src/lib/dhcpsrv/csv_lease_file6.h

@@ -160,6 +160,12 @@ private:
     ///
     /// @param row CSV file holding lease values.
     std::string readHostname(const util::CSVRow& row);
+
+    /// @brief Reads HW address from the CSV file row.
+    ///
+    /// @param row CSV file holding lease values.
+    /// @return pointer to the HWAddr structure that was read
+    HWAddrPtr readHWAddr(const util::CSVRow& row);
     //@}
 
 };

+ 6 - 0
src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -313,6 +313,12 @@ testing but should not be enabled in normal circumstances. Non-persistence
 mode is enabled when 'persist4=no persist6=no' parameters are specified
 in the database access string.
 
+% DHCPSRV_MEMFILE_READ_HWADDR_FAIL failed to read hardware address from disk: %1
+A warning message issued when read attempt of the hardware address stored in
+a disk file failed. The parameter should provide the exact nature of the failure.
+The database read will continue, but that particular lease will no longer
+have hardware address associated with it.
+
 % DHCPSRV_MEMFILE_ROLLBACK rolling back memory file database
 The code has issued a rollback call.  For the memory file database, this is
 a no-op.

+ 31 - 14
src/lib/dhcpsrv/lease.cc

@@ -23,10 +23,10 @@ namespace dhcp {
 Lease::Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2,
              uint32_t valid_lft, SubnetID subnet_id, time_t cltt,
              const bool fqdn_fwd, const bool fqdn_rev,
-             const std::string& hostname)
+             const std::string& hostname, const HWAddrPtr& hwaddr)
     :addr_(addr), t1_(t1), t2_(t2), valid_lft_(valid_lft), cltt_(cltt),
      subnet_id_(subnet_id), fixed_(false), hostname_(hostname),
-     fqdn_fwd_(fqdn_fwd), fqdn_rev_(fqdn_rev) {
+     fqdn_fwd_(fqdn_fwd), fqdn_rev_(fqdn_rev), hwaddr_(hwaddr) {
 }
 
 std::string
@@ -66,8 +66,8 @@ Lease::hasIdenticalFqdn(const Lease& other) const {
 Lease4::Lease4(const Lease4& other)
     : Lease(other.addr_, other.t1_, other.t2_, other.valid_lft_,
             other.subnet_id_, other.cltt_, other.fqdn_fwd_,
-            other.fqdn_rev_, other.hostname_), ext_(other.ext_),
-      hwaddr_(other.hwaddr_) {
+            other.fqdn_rev_, other.hostname_, other.hwaddr_),
+            ext_(other.ext_) {
 
     fixed_ = other.fixed_;
     comments_ = other.comments_;
@@ -91,6 +91,17 @@ Lease4::getClientIdVector() const {
     return (client_id_->getClientId());
 }
 
+const std::vector<uint8_t>&
+Lease4::getRawHWAddr() const {
+    if (!hwaddr_) {
+        // something is wrong, very wrong. Hardware address must always
+        // be present for all IPv4 leases.
+        isc_throw(BadValue, "Lease4 for address " << addr_.toText()
+                  << " is missing a hardware address");
+    }
+    return (hwaddr_->hwaddr_);
+}
+
 bool
 Lease4::matches(const Lease4& other) const {
     if ((client_id_ && !other.client_id_) ||
@@ -105,9 +116,13 @@ Lease4::matches(const Lease4& other) const {
         return false;
     }
 
+    // Note that hwaddr_ is now a poiner to the HWAddr structure.
+    // We can't simply compare smart pointers, we need to compare the
+    // actual objects they point to. It is ok to not check whether they
+    // are non-NULL, as every Lease4 must have hardware address.
     return (addr_ == other.addr_ &&
             ext_ == other.ext_ &&
-            hwaddr_ == other.hwaddr_);
+            *hwaddr_ == *other.hwaddr_);
 
 }
 
@@ -139,12 +154,13 @@ Lease4::operator=(const Lease4& other) {
 
 Lease6::Lease6(Type type, const isc::asiolink::IOAddress& addr,
                DuidPtr duid, uint32_t iaid, uint32_t preferred, uint32_t valid,
-               uint32_t t1, uint32_t t2, SubnetID subnet_id, uint8_t prefixlen)
-    : Lease(addr, t1, t2, valid, subnet_id, 0/*cltt*/, false, false, ""),
+               uint32_t t1, uint32_t t2, SubnetID subnet_id,
+               const HWAddrPtr& hwaddr, uint8_t prefixlen)
+    : Lease(addr, t1, t2, valid, subnet_id, 0/*cltt*/, false, false, "", hwaddr),
       type_(type), prefixlen_(prefixlen), iaid_(iaid), duid_(duid),
       preferred_lft_(preferred) {
     if (!duid) {
-        isc_throw(InvalidOperation, "DUID must be specified for a lease");
+        isc_throw(InvalidOperation, "DUID is mandatory for an IPv6 lease");
     }
 
     cltt_ = time(NULL);
@@ -154,22 +170,23 @@ Lease6::Lease6(Type type, const isc::asiolink::IOAddress& addr,
                DuidPtr duid, uint32_t iaid, uint32_t preferred, uint32_t valid,
                uint32_t t1, uint32_t t2, SubnetID subnet_id,
                const bool fqdn_fwd, const bool fqdn_rev,
-               const std::string& hostname, uint8_t prefixlen)
+               const std::string& hostname, const HWAddrPtr& hwaddr,
+               uint8_t prefixlen)
     : Lease(addr, t1, t2, valid, subnet_id, 0/*cltt*/,
-            fqdn_fwd, fqdn_rev, hostname),
+            fqdn_fwd, fqdn_rev, hostname, hwaddr),
       type_(type), prefixlen_(prefixlen), iaid_(iaid), duid_(duid),
       preferred_lft_(preferred) {
     if (!duid) {
-        isc_throw(InvalidOperation, "DUID must be specified for a lease");
+        isc_throw(InvalidOperation, "DUID is mandatory for an IPv6 lease");
     }
 
     cltt_ = time(NULL);
 }
 
 Lease6::Lease6()
-    : Lease(isc::asiolink::IOAddress("::"), 0, 0, 0, 0, 0, false, false, ""),
-      type_(TYPE_NA), prefixlen_(0), iaid_(0), duid_(DuidPtr()),
-      preferred_lft_(0) {
+    : Lease(isc::asiolink::IOAddress("::"), 0, 0, 0, 0, 0, false, false, "",
+            HWAddrPtr()), type_(TYPE_NA), prefixlen_(0), iaid_(0),
+            duid_(DuidPtr()), preferred_lft_(0) {
 }
 
 const std::vector<uint8_t>&

+ 26 - 13
src/lib/dhcpsrv/lease.h

@@ -59,10 +59,12 @@ struct Lease {
     /// @param fqdn_fwd If true, forward DNS update is performed for a lease.
     /// @param fqdn_rev If true, reverse DNS update is performed for a lease.
     /// @param hostname FQDN of the client which gets the lease.
+    /// @param hwaddr Hardware/MAC address
     Lease(const isc::asiolink::IOAddress& addr, uint32_t t1, uint32_t t2,
           uint32_t valid_lft, SubnetID subnet_id, time_t cltt,
           const bool fqdn_fwd, const bool fqdn_rev,
-          const std::string& hostname);
+          const std::string& hostname,
+          const HWAddrPtr& hwaddr);
 
     /// @brief Destructor
     virtual ~Lease() {}
@@ -126,6 +128,11 @@ struct Lease {
     /// Set true if the DNS PTR record for this lease has been updated.
     bool fqdn_rev_;
 
+    /// @brief Client's MAC/hardware address
+    ///
+    /// This information may not be available in certain cases.
+    HWAddrPtr hwaddr_;
+
     /// @brief Lease comments
     ///
     /// Currently not used. It may be used for keeping comments made by the
@@ -169,9 +176,6 @@ struct Lease4 : public Lease {
     /// should be 0.
     uint32_t ext_;
 
-    /// @brief Hardware address
-    std::vector<uint8_t> hwaddr_;
-
     /// @brief Client identifier
     ///
     /// @todo Should this be a pointer to a client ID or the ID itself?
@@ -181,8 +185,7 @@ struct Lease4 : public Lease {
     /// @brief Constructor
     ///
     /// @param addr IPv4 address.
-    /// @param hwaddr Hardware address buffer
-    /// @param hwaddr_len Length of hardware address buffer
+    /// @param hwaddr A pointer to HWAddr structure
     /// @param clientid Client identification buffer
     /// @param clientid_len Length of client identification buffer
     /// @param valid_lft Lifetime of the lease
@@ -193,14 +196,13 @@ struct Lease4 : public Lease {
     /// @param fqdn_fwd If true, forward DNS update is performed for a lease.
     /// @param fqdn_rev If true, reverse DNS update is performed for a lease.
     /// @param hostname FQDN of the client which gets the lease.
-    Lease4(const isc::asiolink::IOAddress& addr, const uint8_t* hwaddr, size_t hwaddr_len,
+    Lease4(const isc::asiolink::IOAddress& addr, const HWAddrPtr& hwaddr,
            const uint8_t* clientid, size_t clientid_len, uint32_t valid_lft,
            uint32_t t1, uint32_t t2, time_t cltt, uint32_t subnet_id,
            const bool fqdn_fwd = false, const bool fqdn_rev = false,
            const std::string& hostname = "")
         : Lease(addr, t1, t2, valid_lft, subnet_id, cltt, fqdn_fwd, fqdn_rev,
-                hostname),
-        ext_(0), hwaddr_(hwaddr, hwaddr + hwaddr_len) {
+                hostname, hwaddr), ext_(0) {
         if (clientid_len) {
             client_id_.reset(new ClientId(clientid, clientid_len));
         }
@@ -209,7 +211,7 @@ struct Lease4 : public Lease {
     /// @brief Default constructor
     ///
     /// Initialize fields that don't have a default constructor.
-    Lease4() : Lease(0, 0, 0, 0, 0, 0, false, false, "") {
+    Lease4() : Lease(0, 0, 0, 0, 0, 0, false, false, "", HWAddrPtr()) {
     }
 
     /// @brief Copy constructor
@@ -228,6 +230,14 @@ struct Lease4 : public Lease {
     /// or an empty vector if client identifier is NULL.
     const std::vector<uint8_t>& getClientIdVector() const;
 
+    /// @brief Returns raw (as vector) hardware address
+    ///
+    /// This method is needed in multi-index container as key extractor.
+    /// The const reference is only valid as long as the object that returned it.
+    ///
+    /// @return const reference to the hardware address
+    const std::vector<uint8_t>& getRawHWAddr() const;
+
     /// @brief Check if two objects encapsulate the lease for the same
     /// client.
     ///
@@ -319,10 +329,12 @@ struct Lease6 : public Lease {
     /// @param t1 A value of the T1 timer.
     /// @param t2 A value of the T2 timer.
     /// @param subnet_id A Subnet identifier.
-    /// @param prefixlen An address prefix length.
+    /// @param hwaddr hardware/MAC address (optional)
+    /// @param prefixlen An address prefix length (optional, defaults to 128)
     Lease6(Type type, const isc::asiolink::IOAddress& addr, DuidPtr duid,
            uint32_t iaid, uint32_t preferred, uint32_t valid, uint32_t t1,
-           uint32_t t2, SubnetID subnet_id, uint8_t prefixlen = 128);
+           uint32_t t2, SubnetID subnet_id, const HWAddrPtr& hwaddr = HWAddrPtr(),
+           uint8_t prefixlen = 128);
 
     /// @brief Constructor, including FQDN data.
     ///
@@ -338,12 +350,13 @@ struct Lease6 : public Lease {
     /// @param fqdn_fwd If true, forward DNS update is performed for a lease.
     /// @param fqdn_rev If true, reverse DNS update is performed for a lease.
     /// @param hostname FQDN of the client which gets the lease.
+    /// @param hwaddr hardware address (MAC), may be NULL
     /// @param prefixlen An address prefix length.
     Lease6(Type type, const isc::asiolink::IOAddress& addr, DuidPtr duid,
            uint32_t iaid, uint32_t preferred, uint32_t valid, uint32_t t1,
            uint32_t t2, SubnetID subnet_id, const bool fqdn_fwd,
            const bool fqdn_rev, const std::string& hostname,
-           uint8_t prefixlen = 0);
+           const HWAddrPtr& hwaddr = HWAddrPtr(), uint8_t prefixlen = 0);
 
     /// @brief Constructor
     ///

+ 1 - 1
src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -129,7 +129,7 @@ Memfile_LeaseMgr::getLease4(const HWAddr& hwaddr) const {
         lease != idx.end(); ++lease) {
 
         // Every Lease4 has a hardware address, so we can compare it
-        if ((*lease)->hwaddr_ == hwaddr.hwaddr_) {
+        if ( (*(*lease)->hwaddr_) == hwaddr) {
             collection.push_back((*lease));
         }
     }

+ 11 - 7
src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -434,9 +434,11 @@ protected:
                 boost::multi_index::composite_key<
                     Lease4,
                     // The hardware address is held in the hwaddr_ member of the
-                    // Lease4 object.
-                    boost::multi_index::member<Lease4, std::vector<uint8_t>,
-                                               &Lease4::hwaddr_>,
+                    // Lease4 object, which is a HWAddr object. Boost does not
+                    // provide a key extractor for getting a member of a member,
+                    // so we need a simple method for that.
+                    boost::multi_index::const_mem_fun<Lease4, const std::vector<uint8_t>&,
+                                               &Lease4::getRawHWAddr>,
                     // The subnet id is held in the subnet_id_ member of Lease4
                     // class. Note that the subnet_id_ is defined in the base
                     // class (Lease) so we have to point to this class rather
@@ -470,10 +472,12 @@ protected:
                     // calling getClientIdVector const function.
                     boost::multi_index::const_mem_fun<Lease4, const std::vector<uint8_t>&,
                                                       &Lease4::getClientIdVector>,
-                    // The hardware address is held in the hwaddr_ member of the
-                    // Lease4 object.
-                    boost::multi_index::member<Lease4, std::vector<uint8_t>,
-                                               &Lease4::hwaddr_>,
+                    // The hardware address is held in the hwaddr_ object. We can
+                    // access the raw data using lease->hwaddr_->hwaddr_, but Boost
+                    // doesn't seem to provide a way to use member of a member for this,
+                    // so we need a simple key extractor method (getRawHWAddr).
+                    boost::multi_index::const_mem_fun<Lease4, const std::vector<uint8_t>&,
+                                            &Lease4::getRawHWAddr>,
                     // The subnet id is accessed through the subnet_id_ member.
                     boost::multi_index::member<Lease, SubnetID, &Lease::subnet_id_>
                 >

+ 38 - 28
src/lib/dhcpsrv/tests/alloc_engine_unittest.cc

@@ -246,7 +246,7 @@ public:
         Lease6Ptr lease;
         EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                         duid_, iaid_, hint, type, false, false,
-                        "", fake, CalloutHandlePtr(), old_leases_)));
+                        "", fake, CalloutHandlePtr(), old_leases_, HWAddrPtr())));
 
         // Check that we got a lease
         EXPECT_TRUE(lease);
@@ -310,7 +310,7 @@ public:
         Lease6Ptr lease;
         EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                         duid_, iaid_, requested, type, false, false, "", false,
-                        CalloutHandlePtr(), old_leases_)));
+                        CalloutHandlePtr(), old_leases_, HWAddrPtr())));
 
         // Check that we got a lease
         ASSERT_TRUE(lease);
@@ -354,7 +354,8 @@ public:
         Lease6Ptr lease;
         EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                         duid_, iaid_, hint, type, false,
-                        false, "", false, CalloutHandlePtr(), old_leases_)));
+                        false, "", false, CalloutHandlePtr(), old_leases_,
+                        HWAddrPtr())));
 
         // Check that we got a lease
         ASSERT_TRUE(lease);
@@ -449,7 +450,7 @@ public:
         if (lease->client_id_ && clientid_) {
             EXPECT_TRUE(*lease->client_id_ == *clientid_);
         }
-        EXPECT_TRUE(lease->hwaddr_ == hwaddr_->hwaddr_);
+        EXPECT_TRUE(*lease->hwaddr_ == *hwaddr_);
         /// @todo: check cltt
      }
 
@@ -563,13 +564,15 @@ TEST_F(AllocEngine6Test, allocateAddress6Nulls) {
     Lease6Ptr lease;
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(
                     Subnet6Ptr(), duid_, iaid_, IOAddress("::"), Lease::TYPE_NA,
-                    false, false, "", false, CalloutHandlePtr(), old_leases_)));
+                    false, false, "", false, CalloutHandlePtr(), old_leases_,
+                    HWAddrPtr())));
     ASSERT_FALSE(lease);
 
     // Allocations without DUID are not allowed either
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                     DuidPtr(), iaid_, IOAddress("::"), Lease::TYPE_NA, false,
-                    false, "", false, CalloutHandlePtr(), old_leases_)));
+                    false, "", false, CalloutHandlePtr(), old_leases_,
+                    HWAddrPtr())));
     ASSERT_FALSE(lease);
 }
 
@@ -818,7 +821,7 @@ TEST_F(AllocEngine6Test, smallPool6) {
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                     duid_, iaid_, IOAddress("::"), Lease::TYPE_NA, fqdn_fwd_,
                     fqdn_rev_, hostname_, false, CalloutHandlePtr(),
-                    old_leases_)));
+                    old_leases_, HWAddrPtr())));
 
     // Check that we got that single lease
     ASSERT_TRUE(lease);
@@ -863,7 +866,8 @@ TEST_F(AllocEngine6Test, outOfAddresses6) {
     DuidPtr other_duid = DuidPtr(new DUID(vector<uint8_t>(12, 0xff)));
     const uint32_t other_iaid = 3568;
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, other_duid, other_iaid,
-                               501, 502, 503, 504, subnet_->getID(), 0));
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), 0));
     lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
@@ -872,7 +876,7 @@ TEST_F(AllocEngine6Test, outOfAddresses6) {
     Lease6Ptr lease2;
     EXPECT_NO_THROW(lease2 = expectOneLease(engine->allocateLeases6(subnet_,
                     duid_, iaid_, IOAddress("::"), Lease::TYPE_NA, false, false,
-                    "", false, CalloutHandlePtr(), old_leases_)));
+                    "", false, CalloutHandlePtr(), old_leases_, HWAddrPtr())));
     EXPECT_FALSE(lease2);
 
 }
@@ -895,7 +899,8 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) {
     DuidPtr other_duid = DuidPtr(new DUID(vector<uint8_t>(12, 0xff)));
     const uint32_t other_iaid = 3568;
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, other_duid, other_iaid,
-                               501, 502, 503, 504, subnet_->getID(), 0));
+                               501, 502, 503, 504, subnet_->getID(),
+                               HWAddrPtr(), 0));
     lease->cltt_ = time(NULL) - 500; // Allocated 500 seconds ago
     lease->valid_lft_ = 495; // Lease was valid for 495 seconds
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
@@ -907,7 +912,7 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) {
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                     duid_, iaid_, IOAddress("::"), Lease::TYPE_NA, fqdn_fwd_,
                     fqdn_rev_, hostname_, true, CalloutHandlePtr(),
-                    old_leases_)));
+                    old_leases_, HWAddrPtr())));
     // Check that we got that single lease
     ASSERT_TRUE(lease);
     EXPECT_EQ(addr, lease->addr_);
@@ -918,7 +923,7 @@ TEST_F(AllocEngine6Test, solicitReuseExpiredLease6) {
     // CASE 2: Asking specifically for this address
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                     duid_, iaid_, addr, Lease::TYPE_NA, false, false, "",
-                    true, CalloutHandlePtr(), old_leases_)));
+                    true, CalloutHandlePtr(), old_leases_, HWAddrPtr())));
 
     // Check that we got that single lease
     ASSERT_TRUE(lease);
@@ -946,7 +951,8 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     const uint32_t other_iaid = 3568;
     const SubnetID other_subnetid = 999;
     Lease6Ptr lease(new Lease6(Lease::TYPE_NA, addr, other_duid, other_iaid,
-                               501, 502, 503, 504, other_subnetid, 0));
+                               501, 502, 503, 504, other_subnetid, HWAddrPtr(),
+                               0));
     lease->cltt_ = time(NULL) - 500; // Allocated 500 seconds ago
     lease->valid_lft_ = 495; // Lease was valid for 495 seconds
     lease->fqdn_fwd_ = true;
@@ -957,7 +963,7 @@ TEST_F(AllocEngine6Test, requestReuseExpiredLease6) {
     // A client comes along, asking specifically for this address
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                     duid_, iaid_, addr, Lease::TYPE_NA, false, false, "",
-                    false, CalloutHandlePtr(), old_leases_)));
+                    false, CalloutHandlePtr(), old_leases_, HWAddrPtr())));
 
     // Check that he got that single lease
     ASSERT_TRUE(lease);
@@ -1114,10 +1120,11 @@ TEST_F(AllocEngine4Test, allocWithUsedHint4) {
     ASSERT_TRUE(engine);
 
     // Let's create a lease and put it in the LeaseMgr
-    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
     uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
     time_t now = time(NULL);
-    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2, sizeof(hwaddr2),
+    Lease4Ptr used(new Lease4(IOAddress("192.0.2.106"), hwaddr2,
                               clientid2, sizeof(clientid2), 1, 2, 3, now, subnet_->getID()));
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(used));
 
@@ -1371,10 +1378,11 @@ TEST_F(AllocEngine4Test, outOfAddresses4) {
     cfg_mgr.addSubnet4(subnet_);
 
     // Just a different hw/client-id for the second client
-    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
     uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
     time_t now = time(NULL);
-    Lease4Ptr lease(new Lease4(addr, hwaddr2, sizeof(hwaddr2), clientid2,
+    Lease4Ptr lease(new Lease4(addr, hwaddr2, clientid2,
                                sizeof(clientid2), 501, 502, 503, now,
                                subnet_->getID()));
     lease->cltt_ = time(NULL) - 10; // Allocated 10 seconds ago
@@ -1410,11 +1418,11 @@ TEST_F(AllocEngine4Test, discoverReuseExpiredLease4) {
     cfg_mgr.addSubnet4(subnet_);
 
     // Just a different hw/client-id for the second client
-    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
     uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
     time_t now = time(NULL) - 500; // Allocated 500 seconds ago
-    Lease4Ptr lease(new Lease4(addr, clientid2, sizeof(clientid2),
-                               hwaddr2, sizeof(hwaddr2),
+    Lease4Ptr lease(new Lease4(addr, hwaddr2, clientid2, sizeof(clientid2),
                                495, 100, 200, now, subnet_->getID()));
     // Copy the lease, so as it can be compared with the old lease returned
     // by the allocation engine.
@@ -1470,10 +1478,11 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) {
     IOAddress addr("192.0.2.105");
 
     // Just a different hw/client-id for the second client
-    uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
     uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
     time_t now = time(NULL) - 500; // Allocated 500 seconds ago
-    Lease4Ptr lease(new Lease4(addr, clientid2, sizeof(clientid2), hwaddr2,
+    Lease4Ptr lease(new Lease4(addr, hwaddr2, clientid2, sizeof(clientid2),
                                sizeof(hwaddr2), 495, 100, 200, now,
                                subnet_->getID()));
     // Make a copy of the lease, so as we can comapre that with the old lease
@@ -1528,10 +1537,11 @@ TEST_F(AllocEngine4Test, renewLease4) {
     const time_t old_timestamp = time(NULL) - 45; // Allocated 45 seconds ago
 
     // Just a different hw/client-id for the second client
-    const uint8_t hwaddr2[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    const uint8_t hwaddr2_data[] = { 0, 0xfe, 0xfe, 0xfe, 0xfe, 0xfe};
+    HWAddrPtr hwaddr2(new HWAddr(hwaddr2_data, sizeof(hwaddr2_data), HTYPE_ETHER));
     const uint8_t clientid2[] = { 8, 7, 6, 5, 4, 3, 2, 1 };
-    Lease4Ptr lease(new Lease4(addr, clientid2, sizeof(clientid2), hwaddr2,
-                               sizeof(hwaddr2), old_lifetime, old_t1, old_t2,
+    Lease4Ptr lease(new Lease4(addr, hwaddr2, clientid2, sizeof(clientid2),
+                               old_lifetime, old_t1, old_t2,
                                old_timestamp, subnet_->getID()));
     ASSERT_TRUE(LeaseMgrFactory::instance().addLease(lease));
 
@@ -1685,7 +1695,7 @@ TEST_F(HookAllocEngine6Test, lease6_select) {
     Lease6Ptr lease;
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                     duid_, iaid_, IOAddress("::"), Lease::TYPE_NA, false, false,
-                    "", false, callout_handle, old_leases_)));
+                    "", false, callout_handle, old_leases_, HWAddrPtr())));
     // Check that we got a lease
     ASSERT_TRUE(lease);
 
@@ -1756,7 +1766,7 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) {
     Lease6Ptr lease;
     EXPECT_NO_THROW(lease = expectOneLease(engine->allocateLeases6(subnet_,
                     duid_, iaid_, IOAddress("::"), Lease::TYPE_NA, false, false,
-                    "", false, callout_handle, old_leases_)));
+                    "", false, callout_handle, old_leases_, HWAddrPtr())));
     // Check that we got a lease
     ASSERT_TRUE(lease);
 

+ 12 - 4
src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc

@@ -63,10 +63,18 @@ public:
     /// @brief Object providing access to lease file IO.
     LeaseFileIO io_;
 
+    /// @brief hardware address 0 (corresponds to HWADDR0 const)
+    HWAddrPtr hwaddr0_;
+
+    /// @brief hardware address 1 (corresponds to HWADDR1 const)
+    HWAddrPtr hwaddr1_;
+
 };
 
 CSVLeaseFile4Test::CSVLeaseFile4Test()
     : filename_(absolutePath("leases4.csv")), io_(filename_) {
+    hwaddr0_.reset(new HWAddr(HWADDR0, sizeof(HWADDR0), HTYPE_ETHER));
+    hwaddr1_.reset(new HWAddr(HWADDR1, sizeof(HWADDR1), HTYPE_ETHER));
 }
 
 std::string
@@ -103,7 +111,7 @@ TEST_F(CSVLeaseFile4Test, parse) {
 
     // Verify that the lease attributes are correct.
     EXPECT_EQ("192.0.2.1", lease->addr_.toText());
-    HWAddr hwaddr1(lease->hwaddr_, HTYPE_ETHER);
+    HWAddr hwaddr1(*lease->hwaddr_);
     EXPECT_EQ("06:07:08:09:0a:bc", hwaddr1.toText(false));
     EXPECT_FALSE(lease->client_id_);
     EXPECT_EQ(200, lease->valid_lft_);
@@ -122,7 +130,7 @@ TEST_F(CSVLeaseFile4Test, parse) {
     ASSERT_TRUE(lease);
     // Verify that the third lease is correct.
     EXPECT_EQ("192.0.3.15", lease->addr_.toText());
-    HWAddr hwaddr3(lease->hwaddr_, HTYPE_ETHER);
+    HWAddr hwaddr3(*lease->hwaddr_);
     EXPECT_EQ("dd:de:ba:0d:1b:2e:3e:4f", hwaddr3.toText(false));
     ASSERT_TRUE(lease->client_id_);
     EXPECT_EQ("0a:00:01:04", lease->client_id_->toText());
@@ -151,14 +159,14 @@ TEST_F(CSVLeaseFile4Test, recreate) {
     ASSERT_TRUE(io_.exists());
     // Create first lease, with NULL client id.
     Lease4Ptr lease(new Lease4(IOAddress("192.0.3.2"),
-                               HWADDR0, sizeof(HWADDR0),
+                               hwaddr0_,
                                NULL, 0,
                                200, 50, 80, 0, 8, true, true,
                                "host.example.com"));
     ASSERT_NO_THROW(lf->append(*lease));
     // Create second lease, with non-NULL client id.
     lease.reset(new Lease4(IOAddress("192.0.3.10"),
-                           HWADDR1, sizeof(HWADDR1),
+                           hwaddr1_,
                            CLIENTID0, sizeof(CLIENTID0),
                            100, 60, 90, 0, 7));
     ASSERT_NO_THROW(lf->append(*lease));

+ 8 - 8
src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc

@@ -86,14 +86,14 @@ void
 CSVLeaseFile6Test::writeSampleFile() const {
     io_.writeFile("address,duid,valid_lifetime,expire,subnet_id,"
                   "pref_lifetime,lease_type,iaid,prefix_len,fqdn_fwd,"
-                  "fqdn_rev,hostname\n"
+                  "fqdn_rev,hostname,hwaddr\n"
                   "2001:db8:1::1,00:01:02:03:04:05:06:0a:0b:0c:0d:0e:0f,"
                   "200,200,8,100,0,7,0,1,1,host.example.com\n"
                   "2001:db8:1::1,,200,200,8,100,0,7,0,1,1,host.example.com\n"
                   "2001:db8:2::10,01:01:01:01:0a:01:02:03:04:05,300,300,6,150,"
                   "0,8,0,0,0,\n"
                   "3000:1::,00:01:02:03:04:05:06:0a:0b:0c:0d:0e:0f,0,200,8,0,2,"
-                  "16,64,0,0,\n");
+                  "16,64,0,0,,\n");
 }
 
 // This test checks the capability to read and parse leases from the file.
@@ -192,25 +192,25 @@ TEST_F(CSVLeaseFile6Test, recreate) {
     lease.reset(new Lease6(Lease::TYPE_NA, IOAddress("2001:db8:2::10"),
                            makeDUID(DUID1, sizeof(DUID1)),
                            8, 150, 300, 40, 70, 6, false, false,
-                           "", 128));
+                           "", HWAddrPtr(), 128));
     lease->cltt_ = 0;
     ASSERT_NO_THROW(lf->append(*lease));
 
     lease.reset(new Lease6(Lease::TYPE_PD, IOAddress("3000:1:1::"),
                            makeDUID(DUID0, sizeof(DUID0)),
                            7, 150, 300, 40, 70, 10, false, false,
-                           "", 64));
+                           "", HWAddrPtr(), 64));
     lease->cltt_ = 0;
     ASSERT_NO_THROW(lf->append(*lease));
 
     EXPECT_EQ("address,duid,valid_lifetime,expire,subnet_id,pref_lifetime,"
-              "lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname\n"
+              "lease_type,iaid,prefix_len,fqdn_fwd,fqdn_rev,hostname,hwaddr\n"
               "2001:db8:1::1,00:01:02:03:04:05:06:0a:0b:0c:0d:0e:0f,"
-              "200,200,8,100,0,7,0,1,1,host.example.com\n"
+              "200,200,8,100,0,7,0,1,1,host.example.com,\n"
               "2001:db8:2::10,01:01:01:01:0a:01:02:03:04:05"
-              ",300,300,6,150,0,8,128,0,0,\n"
+              ",300,300,6,150,0,8,128,0,0,,\n"
               "3000:1:1::,00:01:02:03:04:05:06:0a:0b:0c:0d:0e:0f,"
-              "300,300,10,150,2,7,64,0,0,\n",
+              "300,300,10,150,2,7,64,0,0,,\n",
               io_.readFile());
 }
 

+ 31 - 35
src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc

@@ -86,7 +86,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
 
     // Set other parameters.  For historical reasons, address 0 is not used.
     if (address == straddress4_[0]) {
-        lease->hwaddr_ = vector<uint8_t>(6, 0x08);
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(6, 0x08), HTYPE_ETHER));
         lease->client_id_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x42)));
         lease->valid_lft_ = 8677;
         lease->cltt_ = 168256;
@@ -96,7 +96,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->hostname_ = "myhost.example.com.";
 
         } else if (address == straddress4_[1]) {
-        lease->hwaddr_ = vector<uint8_t>(6, 0x19);
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(6, 0x19), HTYPE_ETHER));
         lease->client_id_ = ClientIdPtr(
             new ClientId(vector<uint8_t>(8, 0x53)));
         lease->valid_lft_ = 3677;
@@ -107,7 +107,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->hostname_ = "myhost.example.com.";
 
     } else if (address == straddress4_[2]) {
-        lease->hwaddr_ = vector<uint8_t>(6, 0x2a);
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(6, 0x2a), HTYPE_ETHER));
         lease->client_id_ = ClientIdPtr(new ClientId(vector<uint8_t>(8, 0x64)));
         lease->valid_lft_ = 5412;
         lease->cltt_ = 234567;
@@ -117,7 +117,8 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->hostname_ = "";
 
     } else if (address == straddress4_[3]) {
-        lease->hwaddr_ = vector<uint8_t>(6, 0x19);      // Same as lease 1
+        // Hardware address same as lease 1.
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(6, 0x19), HTYPE_ETHER));
         lease->client_id_ = ClientIdPtr(
             new ClientId(vector<uint8_t>(8, 0x75)));
 
@@ -133,7 +134,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->hostname_ = "otherhost.example.com.";
 
     } else if (address == straddress4_[4]) {
-        lease->hwaddr_ = vector<uint8_t>(6, 0x4c);
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(6, 0x4c), HTYPE_ETHER));
         // Same ClientId as straddr4_[1]
         lease->client_id_ = ClientIdPtr(
             new ClientId(vector<uint8_t>(8, 0x53)));    // Same as lease 1
@@ -145,7 +146,8 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->hostname_ = "otherhost.example.com.";
 
     } else if (address == straddress4_[5]) {
-        lease->hwaddr_ = vector<uint8_t>(6, 0x19);      // Same as lease 1
+        // Same as lease 1
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(6, 0x19), HTYPE_ETHER));
         // Same ClientId and IAID as straddress4_1
         lease->client_id_ = ClientIdPtr(
             new ClientId(vector<uint8_t>(8, 0x53)));    // Same as lease 1
@@ -156,7 +158,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->fqdn_fwd_ = false;
         lease->hostname_ = "otherhost.example.com.";
     } else if (address == straddress4_[6]) {
-        lease->hwaddr_ = vector<uint8_t>(6, 0x6e);
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(6, 0x6e), HTYPE_ETHER));
         // Same ClientId as straddress4_1
         lease->client_id_ = ClientIdPtr(
             new ClientId(vector<uint8_t>(8, 0x53)));    // Same as lease 1
@@ -168,7 +170,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) {
         lease->hostname_ = "myhost.example.com.";
 
     } else if (address == straddress4_[7]) {
-        lease->hwaddr_ = vector<uint8_t>();             // Empty
+        lease->hwaddr_.reset(new HWAddr(vector<uint8_t>(), HTYPE_ETHER)); // Empty
         lease->client_id_ = ClientIdPtr();              // Empty
         lease->valid_lft_ = 7975;
         lease->cltt_ = 213876;
@@ -467,7 +469,7 @@ GenericLeaseMgrTest::testLease4NullClientId() {
     EXPECT_FALSE(lmptr_->addLease(leases[1]));
 
     // Check that we can get the lease by HWAddr
-    HWAddr tmp(leases[2]->hwaddr_, HTYPE_ETHER);
+    HWAddr tmp(*leases[2]->hwaddr_);
     Lease4Collection returned = lmptr_->getLease4(tmp);
     ASSERT_EQ(1, returned.size());
     detailCompareLease(leases[2], *returned.begin());
@@ -504,7 +506,7 @@ void
 GenericLeaseMgrTest::testGetLease4HWAddr1() {
     // Let's initialize two different leases 4 and just add the first ...
     Lease4Ptr leaseA = initializeLease4(straddress4_[5]);
-    HWAddr hwaddrA(leaseA->hwaddr_, HTYPE_ETHER);
+    HWAddr hwaddrA(*leaseA->hwaddr_);
     HWAddr hwaddrB(vector<uint8_t>(6, 0x80), HTYPE_ETHER);
 
     EXPECT_TRUE(lmptr_->addLease(leaseA));
@@ -528,7 +530,7 @@ GenericLeaseMgrTest::testGetLease4HWAddr2() {
 
     // Get the leases matching the hardware address of lease 1
     /// @todo: Simply use HWAddr directly once 2589 is implemented
-    HWAddr tmp(leases[1]->hwaddr_, HTYPE_ETHER);
+    HWAddr tmp(*leases[1]->hwaddr_);
     Lease4Collection returned = lmptr_->getLease4(tmp);
 
     // Should be three leases, matching leases[1], [3] and [5].
@@ -546,15 +548,13 @@ GenericLeaseMgrTest::testGetLease4HWAddr2() {
     EXPECT_EQ(straddress4_[5], addresses[2]);
 
     // Repeat test with just one expected match
-    /// @todo: Simply use HWAddr directly once 2589 is implemented
-    returned = lmptr_->getLease4(HWAddr(leases[2]->hwaddr_, HTYPE_ETHER));
+    returned = lmptr_->getLease4(*leases[2]->hwaddr_);
     ASSERT_EQ(1, returned.size());
     detailCompareLease(leases[2], *returned.begin());
 
     // Check that an empty vector is valid
-    EXPECT_TRUE(leases[7]->hwaddr_.empty());
-    /// @todo: Simply use HWAddr directly once 2589 is implemented
-    returned = lmptr_->getLease4(HWAddr(leases[7]->hwaddr_, HTYPE_ETHER));
+    EXPECT_TRUE(leases[7]->hwaddr_->hwaddr_.empty());
+    returned = lmptr_->getLease4(*leases[7]->hwaddr_);
     ASSERT_EQ(1, returned.size());
     detailCompareLease(leases[7], *returned.begin());
 
@@ -573,9 +573,9 @@ GenericLeaseMgrTest::testGetLease4ClientIdHWAddrSubnetId() {
     // a lease may coexist with other leases with non NULL client id.
     leaseC->client_id_.reset();
 
-    HWAddr hwaddrA(leaseA->hwaddr_, HTYPE_ETHER);
-    HWAddr hwaddrB(leaseB->hwaddr_, HTYPE_ETHER);
-    HWAddr hwaddrC(leaseC->hwaddr_, HTYPE_ETHER);
+    HWAddr hwaddrA(*leaseA->hwaddr_);
+    HWAddr hwaddrB(*leaseB->hwaddr_);
+    HWAddr hwaddrC(*leaseC->hwaddr_);
     EXPECT_TRUE(lmptr_->addLease(leaseA));
     EXPECT_TRUE(lmptr_->addLease(leaseB));
     EXPECT_TRUE(lmptr_->addLease(leaseC));
@@ -926,11 +926,11 @@ GenericLeaseMgrTest::testGetLease4HWAddrSize() {
 
     // Now add leases with increasing hardware address size.
     for (uint8_t i = 0; i <= HWAddr::MAX_HWADDR_LEN; ++i) {
-        leases[1]->hwaddr_.resize(i, i);
+        leases[1]->hwaddr_->hwaddr_.resize(i, i);
         EXPECT_TRUE(lmptr_->addLease(leases[1]));
         /// @todo: Simply use HWAddr directly once 2589 is implemented
         Lease4Collection returned =
-            lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER));
+            lmptr_->getLease4(*leases[1]->hwaddr_);
 
         ASSERT_EQ(1, returned.size());
         detailCompareLease(leases[1], *returned.begin());
@@ -939,8 +939,7 @@ GenericLeaseMgrTest::testGetLease4HWAddrSize() {
 
     // Database should not let us add one that is too big
     // (The 42 is a random value put in each byte of the address.)
-    /// @todo: 2589 will make this test impossible
-    leases[1]->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
+    leases[1]->hwaddr_->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
     EXPECT_THROW(lmptr_->addLease(leases[1]), isc::dhcp::DbOperationError);
 }
 
@@ -955,8 +954,8 @@ GenericLeaseMgrTest::testGetLease4HWAddrSubnetId() {
     // Get the leases matching the hardware address of lease 1 and
     // subnet ID of lease 1.  Result should be a single lease - lease 1.
     /// @todo: Simply use HWAddr directly once 2589 is implemented
-    Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
-        HTYPE_ETHER), leases[1]->subnet_id_);
+    Lease4Ptr returned = lmptr_->getLease4(*leases[1]->hwaddr_,
+                                           leases[1]->subnet_id_);
 
     ASSERT_TRUE(returned);
     detailCompareLease(leases[1], returned);
@@ -964,8 +963,7 @@ GenericLeaseMgrTest::testGetLease4HWAddrSubnetId() {
     // Try for a match to the hardware address of lease 1 and the wrong
     // subnet ID.
     /// @todo: Simply use HWAddr directly once 2589 is implemented
-    returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_, HTYPE_ETHER),
-                                 leases[1]->subnet_id_ + 1);
+    returned = lmptr_->getLease4(*leases[1]->hwaddr_, leases[1]->subnet_id_ + 1);
     EXPECT_FALSE(returned);
 
     // Try for a match to the subnet ID of lease 1 (and lease 4) but
@@ -992,9 +990,8 @@ GenericLeaseMgrTest::testGetLease4HWAddrSubnetId() {
     leases[1]->addr_ = leases[2]->addr_;
     EXPECT_TRUE(lmptr_->addLease(leases[1]));
     /// @todo: Simply use HWAddr directly once 2589 is implemented
-    EXPECT_THROW(returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
-                                                    HTYPE_ETHER),
-                                             leases[1]->subnet_id_),
+    EXPECT_THROW(returned = lmptr_->getLease4(*leases[1]->hwaddr_,
+                                              leases[1]->subnet_id_),
                  isc::dhcp::MultipleRecords);
 
 
@@ -1008,11 +1005,10 @@ GenericLeaseMgrTest::testGetLease4HWAddrSubnetIdSize() {
     // Now add leases with increasing hardware address size and check
     // that they can be retrieved.
     for (uint8_t i = 0; i <= HWAddr::MAX_HWADDR_LEN; ++i) {
-        leases[1]->hwaddr_.resize(i, i);
+        leases[1]->hwaddr_->hwaddr_.resize(i, i);
         EXPECT_TRUE(lmptr_->addLease(leases[1]));
         /// @todo: Simply use HWAddr directly once 2589 is implemented
-        Lease4Ptr returned = lmptr_->getLease4(HWAddr(leases[1]->hwaddr_,
-                                                      HTYPE_ETHER),
+        Lease4Ptr returned = lmptr_->getLease4(*leases[1]->hwaddr_,
                                                leases[1]->subnet_id_);
         ASSERT_TRUE(returned);
         detailCompareLease(leases[1], returned);
@@ -1021,7 +1017,7 @@ GenericLeaseMgrTest::testGetLease4HWAddrSubnetIdSize() {
 
     // Database should not let us add one that is too big
     // (The 42 is a random value put in each byte of the address.)
-    leases[1]->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
+    leases[1]->hwaddr_->hwaddr_.resize(HWAddr::MAX_HWADDR_LEN + 100, 42);
     EXPECT_THROW(lmptr_->addLease(leases[1]), isc::dhcp::DbOperationError);
 }
 
@@ -1058,7 +1054,7 @@ GenericLeaseMgrTest::testGetLease4ClientId2() {
 
     // Check that client-id is NULL
     EXPECT_FALSE(leases[7]->client_id_);
-    HWAddr tmp(leases[7]->hwaddr_, HTYPE_ETHER);
+    HWAddr tmp(*leases[7]->hwaddr_);
     returned = lmptr_->getLease4(tmp);
     ASSERT_EQ(1, returned.size());
     detailCompareLease(leases[7], *returned.begin());

+ 42 - 25
src/lib/dhcpsrv/tests/lease_unittest.cc

@@ -52,14 +52,26 @@ Lease4 createLease4(const std::string& hostname, const bool fqdn_fwd,
     return (lease);
 }
 
+/// @brief Fixture class used in Lease4 testing.
+class Lease4Test : public ::testing::Test {
+public:
+
+    /// Default constructor
+    ///
+    /// Currently it only initializes hardware address.
+    Lease4Test() {
+        hwaddr_.reset(new HWAddr(HWADDR, sizeof(HWADDR), HTYPE_ETHER));
+    }
+
+    /// Hardware address, used by tests.
+    HWAddrPtr hwaddr_;
+};
+
 /// Lease4 is also defined in lease_mgr.h, so is tested in this file as well.
 // This test checks if the Lease4 structure can be instantiated correctly
-TEST(Lease4, constructor) {
+TEST_F(Lease4Test, constructor) {
 
     // Random values for the tests
-    const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e};
-    std::vector<uint8_t> hwaddr(HWADDR, HWADDR + sizeof(HWADDR));
-
     const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
     std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
     ClientId clientid(clientid_vec);
@@ -80,14 +92,14 @@ TEST(Lease4, constructor) {
     for (int i = 0; i < sizeof(ADDRESS) / sizeof(ADDRESS[0]); ++i) {
 
         // Create the lease
-        Lease4 lease(ADDRESS[i], HWADDR, sizeof(HWADDR),
+        Lease4 lease(ADDRESS[i], hwaddr_,
                      CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0,
                      current_time, SUBNET_ID, true, true,
                      "hostname.example.com.");
 
         EXPECT_EQ(ADDRESS[i], static_cast<uint32_t>(lease.addr_));
         EXPECT_EQ(0, lease.ext_);
-        EXPECT_TRUE(hwaddr == lease.hwaddr_);
+        EXPECT_TRUE(hwaddr_ == lease.hwaddr_);
         EXPECT_TRUE(clientid == *lease.client_id_);
         EXPECT_EQ(0, lease.t1_);
         EXPECT_EQ(0, lease.t2_);
@@ -103,11 +115,7 @@ TEST(Lease4, constructor) {
 }
 
 // This test verfies that copy constructor copies Lease4 fields correctly.
-TEST(Lease4, copyConstructor) {
-
-    // Random values for the tests
-    const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e};
-    std::vector<uint8_t> hwaddr(HWADDR, HWADDR + sizeof(HWADDR));
+TEST_F(Lease4Test, copyConstructor) {
 
     const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
     std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
@@ -121,7 +129,7 @@ TEST(Lease4, copyConstructor) {
     const uint32_t VALID_LIFETIME = 500;
 
     // Create the lease
-    Lease4 lease(0xffffffff, HWADDR, sizeof(HWADDR),
+    Lease4 lease(0xffffffff, hwaddr_,
                  CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time,
                  SUBNET_ID);
 
@@ -137,12 +145,9 @@ TEST(Lease4, copyConstructor) {
 
 // This test verfies that the assignment operator copies all Lease4 fields
 // correctly.
-TEST(Lease4, operatorAssign) {
+TEST_F(Lease4Test, operatorAssign) {
 
     // Random values for the tests
-    const uint8_t HWADDR[] = {0x08, 0x00, 0x2b, 0x02, 0x3f, 0x4e};
-    std::vector<uint8_t> hwaddr(HWADDR, HWADDR + sizeof(HWADDR));
-
     const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54};
     std::vector<uint8_t> clientid_vec(CLIENTID, CLIENTID + sizeof(CLIENTID));
     ClientId clientid(clientid_vec);
@@ -155,7 +160,7 @@ TEST(Lease4, operatorAssign) {
     const uint32_t VALID_LIFETIME = 500;
 
     // Create the lease
-    Lease4 lease(0xffffffff, HWADDR, sizeof(HWADDR),
+    Lease4 lease(0xffffffff, hwaddr_,
                  CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, 0, 0, current_time,
                  SUBNET_ID);
 
@@ -171,17 +176,23 @@ TEST(Lease4, operatorAssign) {
 
 // This test verifies that the matches() returns true if two leases differ
 // by values other than address, HW address, Client ID and ext_.
-TEST(Lease4, matches) {
+TEST_F(Lease4Test, matches) {
     // Create two leases which share the same address, HW address, client id
     // and ext_ value.
     const time_t current_time = time(NULL);
-    Lease4 lease1(IOAddress("192.0.2.3"), HWADDR, sizeof(HWADDR), CLIENTID,
+    Lease4 lease1(IOAddress("192.0.2.3"), hwaddr_, CLIENTID,
                   sizeof(CLIENTID), VALID_LIFETIME, current_time, 0, 0,
                   SUBNET_ID);
     lease1.hostname_ = "lease1.example.com.";
     lease1.fqdn_fwd_ = true;
     lease1.fqdn_rev_ = true;
-    Lease4 lease2(IOAddress("192.0.2.3"), HWADDR, sizeof(HWADDR), CLIENTID,
+
+    // We need to make an explicit copy. Otherwise the second lease will just
+    // store a pointer and we'll have two leases pointing to a single HWAddr.
+    // That would make modifications to only one impossible.
+    HWAddrPtr hwcopy(new HWAddr(*hwaddr_));
+
+    Lease4 lease2(IOAddress("192.0.2.3"), hwcopy, CLIENTID,
                   sizeof(CLIENTID), VALID_LIFETIME + 10, current_time - 10,
                   100, 200, SUBNET_ID);
     lease2.hostname_ = "lease2.example.com.";
@@ -198,7 +209,7 @@ TEST(Lease4, matches) {
     lease1.addr_ = lease2.addr_;
 
     // Change HW address, leases should not match.
-    lease1.hwaddr_[1] += 1;
+    lease1.hwaddr_->hwaddr_[1] += 1;
     EXPECT_FALSE(lease1.matches(lease2));
     lease1.hwaddr_ = lease2.hwaddr_;
 
@@ -220,7 +231,7 @@ TEST(Lease4, matches) {
 /// Checks that the operator==() correctly compares two leases for equality.
 /// As operator!=() is also defined for this class, every check on operator==()
 /// is followed by the reverse check on operator!=().
-TEST(Lease4, operatorEquals) {
+TEST_F(Lease4Test, operatorEquals) {
 
     // Random values for the tests
     const uint32_t ADDRESS = 0x01020304;
@@ -234,10 +245,16 @@ TEST(Lease4, operatorEquals) {
     const uint32_t VALID_LIFETIME = 500;
 
     // Check when the leases are equal.
-    Lease4 lease1(ADDRESS, HWADDR, sizeof(HWADDR),
+    Lease4 lease1(ADDRESS, hwaddr_,
                   CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, current_time, 0,
                   0, SUBNET_ID);
-    Lease4 lease2(ADDRESS, HWADDR, sizeof(HWADDR),
+
+    // We need to make an explicit copy. Otherwise the second lease will just
+    // store a pointer and we'll have two leases pointing to a single HWAddr.
+    // That would make modifications to only one impossible.
+    HWAddrPtr hwcopy(new HWAddr(*hwaddr_));
+
+    Lease4 lease2(ADDRESS, hwcopy,
                   CLIENTID, sizeof(CLIENTID), VALID_LIFETIME, current_time, 0, 0,
                   SUBNET_ID);
     EXPECT_TRUE(lease1 == lease2);
@@ -259,7 +276,7 @@ TEST(Lease4, operatorEquals) {
     EXPECT_TRUE(lease1 == lease2);  // Check that the reversion has made the
     EXPECT_FALSE(lease1 != lease2); // ... leases equal
 
-    ++lease1.hwaddr_[0];
+    ++lease1.hwaddr_->hwaddr_[0];
     EXPECT_FALSE(lease1 == lease2);
     EXPECT_TRUE(lease1 != lease2);
     lease1.hwaddr_ = lease2.hwaddr_;

+ 4 - 1
src/lib/dhcpsrv/tests/test_utils.cc

@@ -31,7 +31,10 @@ detailCompareLease(const Lease4Ptr& first, const Lease4Ptr& second) {
     // call the operator uint32_t() function, which causes an exception to be
     // thrown for IPv6 addresses.
     EXPECT_EQ(first->addr_, second->addr_);
-    EXPECT_TRUE(first->hwaddr_ == second->hwaddr_);
+
+    // We need to compare the actual HWAddr objects, not pointers
+    EXPECT_TRUE(*first->hwaddr_ == *second->hwaddr_);
+
     if (first->client_id_ && second->client_id_) {
         EXPECT_TRUE(*first->client_id_ == *second->client_id_);
     } else {