diff options
26 files changed, 335 insertions, 192 deletions
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 7372750205..c02fd9f5ad 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/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()) diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc index 2a787e2b0f..a21970a5fc 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.cc +++ b/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; } diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index bb328307b0..395972367a 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/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())); diff --git a/src/bin/dhcp4/tests/direct_client_unittest.cc b/src/bin/dhcp4/tests/direct_client_unittest.cc index 4904d84dd6..ee439905a9 100644 --- a/src/bin/dhcp4/tests/direct_client_unittest.cc +++ b/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), diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc index 203e3b7e88..f649efc78d 100644 --- a/src/bin/dhcp4/tests/fqdn_unittest.cc +++ b/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())); diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 20abb08ff6..cbaf674de2 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/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()) { diff --git a/src/bin/dhcp6/tests/dhcp6_client.cc b/src/bin/dhcp6/tests/dhcp6_client.cc index da836ce9d6..37baf67571 100644 --- a/src/bin/dhcp6/tests/dhcp6_client.cc +++ b/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); } diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index c3440788ff..35a8932b28 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/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 diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc index 56f2eca9f7..78f65f5db2 100644 --- a/src/bin/dhcp6/tests/fqdn_unittest.cc +++ b/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(); } diff --git a/src/bin/dhcp6/tests/hooks_unittest.cc b/src/bin/dhcp6/tests/hooks_unittest.cc index b943df82cf..28fe75a6ee 100644 --- a/src/bin/dhcp6/tests/hooks_unittest.cc +++ b/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)); diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 11b0700e3a..5009cfcf78 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/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; diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index ed2a767266..68492d4852 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/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); diff --git a/src/lib/dhcpsrv/csv_lease_file4.cc b/src/lib/dhcpsrv/csv_lease_file4.cc index 49760c7ad3..e48d47e0b4 100644 --- a/src/lib/dhcpsrv/csv_lease_file4.cc +++ b/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), diff --git a/src/lib/dhcpsrv/csv_lease_file6.cc b/src/lib/dhcpsrv/csv_lease_file6.cc index 4965df546e..d9e68a3732 100644 --- a/src/lib/dhcpsrv/csv_lease_file6.cc +++ b/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 diff --git a/src/lib/dhcpsrv/csv_lease_file6.h b/src/lib/dhcpsrv/csv_lease_file6.h index 3cc7bee44a..33e9ba3264 100644 --- a/src/lib/dhcpsrv/csv_lease_file6.h +++ b/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); //@} }; diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index 17813e44fd..3fe6bcbd9e 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/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. diff --git a/src/lib/dhcpsrv/lease.cc b/src/lib/dhcpsrv/lease.cc index 7553ab486e..7ae7459974 100644 --- a/src/lib/dhcpsrv/lease.cc +++ b/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>& diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index 90e73d164c..220e73634b 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/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 /// diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 5fc4644e3d..431e68e476 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/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)); } } diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index a25474cd93..44b093b28c 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/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_> > diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc index 47c4e4736a..e808f64c57 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc +++ b/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); diff --git a/src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc b/src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc index f34110e40c..b2ecdc20b9 100644 --- a/src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc +++ b/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)); diff --git a/src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc b/src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc index 93589706a5..31f40e4d05 100644 --- a/src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc +++ b/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()); } diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 4925e3cce4..ca57e11293 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/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()); diff --git a/src/lib/dhcpsrv/tests/lease_unittest.cc b/src/lib/dhcpsrv/tests/lease_unittest.cc index 4e7451f2d1..fcb1b108d2 100644 --- a/src/lib/dhcpsrv/tests/lease_unittest.cc +++ b/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_; diff --git a/src/lib/dhcpsrv/tests/test_utils.cc b/src/lib/dhcpsrv/tests/test_utils.cc index b9bd0cce21..e794ded64b 100644 --- a/src/lib/dhcpsrv/tests/test_utils.cc +++ b/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 { |