From 8f112316e617151dc722a2c549e3995ed85283c1 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Fri, 17 Oct 2014 18:06:40 +0200 Subject: [3555] Lease4,Lease6 now uses HWAddr structure. --- src/lib/dhcpsrv/alloc_engine.cc | 20 +++---- src/lib/dhcpsrv/alloc_engine.h | 6 +- src/lib/dhcpsrv/csv_lease_file4.cc | 8 ++- src/lib/dhcpsrv/csv_lease_file6.cc | 37 ++++++++++++ src/lib/dhcpsrv/csv_lease_file6.h | 6 ++ src/lib/dhcpsrv/dhcpsrv_messages.mes | 6 ++ src/lib/dhcpsrv/lease.cc | 45 ++++++++++----- src/lib/dhcpsrv/lease.h | 39 ++++++++----- src/lib/dhcpsrv/memfile_lease_mgr.cc | 2 +- src/lib/dhcpsrv/memfile_lease_mgr.h | 18 +++--- src/lib/dhcpsrv/tests/alloc_engine_unittest.cc | 66 ++++++++++++--------- src/lib/dhcpsrv/tests/csv_lease_file4_unittest.cc | 16 ++++-- src/lib/dhcpsrv/tests/csv_lease_file6_unittest.cc | 16 +++--- .../dhcpsrv/tests/generic_lease_mgr_unittest.cc | 66 ++++++++++----------- src/lib/dhcpsrv/tests/lease_unittest.cc | 67 ++++++++++++++-------- src/lib/dhcpsrv/tests/test_utils.cc | 5 +- 16 files changed, 272 insertions(+), 151 deletions(-) (limited to 'src/lib') 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 #include 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& +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& 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 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& 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& 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::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::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::getClientIdVector>, - // The hardware address is held in the hwaddr_ member of the - // Lease4 object. - boost::multi_index::member, - &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::getRawHWAddr>, // The subnet id is accessed through the subnet_id_ member. boost::multi_index::member > 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(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(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(6, 0x08); + lease->hwaddr_.reset(new HWAddr(vector(6, 0x08), HTYPE_ETHER)); lease->client_id_ = ClientIdPtr(new ClientId(vector(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(6, 0x19); + lease->hwaddr_.reset(new HWAddr(vector(6, 0x19), HTYPE_ETHER)); lease->client_id_ = ClientIdPtr( new ClientId(vector(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(6, 0x2a); + lease->hwaddr_.reset(new HWAddr(vector(6, 0x2a), HTYPE_ETHER)); lease->client_id_ = ClientIdPtr(new ClientId(vector(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(6, 0x19); // Same as lease 1 + // Hardware address same as lease 1. + lease->hwaddr_.reset(new HWAddr(vector(6, 0x19), HTYPE_ETHER)); lease->client_id_ = ClientIdPtr( new ClientId(vector(8, 0x75))); @@ -133,7 +134,7 @@ GenericLeaseMgrTest::initializeLease4(std::string address) { lease->hostname_ = "otherhost.example.com."; } else if (address == straddress4_[4]) { - lease->hwaddr_ = vector(6, 0x4c); + lease->hwaddr_.reset(new HWAddr(vector(6, 0x4c), HTYPE_ETHER)); // Same ClientId as straddr4_[1] lease->client_id_ = ClientIdPtr( new ClientId(vector(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(6, 0x19); // Same as lease 1 + // Same as lease 1 + lease->hwaddr_.reset(new HWAddr(vector(6, 0x19), HTYPE_ETHER)); // Same ClientId and IAID as straddress4_1 lease->client_id_ = ClientIdPtr( new ClientId(vector(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(6, 0x6e); + lease->hwaddr_.reset(new HWAddr(vector(6, 0x6e), HTYPE_ETHER)); // Same ClientId as straddress4_1 lease->client_id_ = ClientIdPtr( new ClientId(vector(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(); // Empty + lease->hwaddr_.reset(new HWAddr(vector(), 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(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 hwaddr(HWADDR, HWADDR + sizeof(HWADDR)); - const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54}; std::vector 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(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 hwaddr(HWADDR, HWADDR + sizeof(HWADDR)); +TEST_F(Lease4Test, copyConstructor) { const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54}; std::vector 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 hwaddr(HWADDR, HWADDR + sizeof(HWADDR)); - const uint8_t CLIENTID[] = {0x17, 0x34, 0xe2, 0xff, 0x09, 0x92, 0x54}; std::vector 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 { -- cgit v1.2.3