diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index c1e52c5104..f1d578a7c3 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -34,18 +34,20 @@ #include #include -#include #include -#include +#include #include #include #include +#include + using namespace isc::asiolink; using namespace isc::dhcp; using namespace isc::dhcp_ddns; using namespace isc::hooks; using namespace isc::stats; +using namespace isc::util; namespace { @@ -87,7 +89,7 @@ namespace isc { namespace dhcp { AllocEngine::IterativeAllocator::IterativeAllocator(Lease::Type lease_type) - :Allocator(lease_type) { + : Allocator(lease_type) { } isc::asiolink::IOAddress @@ -106,17 +108,14 @@ AllocEngine::IterativeAllocator::increasePrefix(const isc::asiolink::IOAddress& << prefix_len); } - // Brief explanation what happens here: - // http://www.youtube.com/watch?v=NFQCYpIHLNQ - uint8_t n_bytes = (prefix_len - 1)/8; uint8_t n_bits = 8 - (prefix_len - n_bytes*8); uint8_t mask = 1 << n_bits; - // Longer explanation: n_bytes specifies number of full bytes that are - // in-prefix. They can also be used as an offset for the first byte that - // is not in prefix. n_bits specifies number of bits on the last byte that - // is (often partially) in prefix. For example for a /125 prefix, the values + // Explanation: n_bytes specifies number of full bytes that are in-prefix. + // They can also be used as an offset for the first byte that is not in + // prefix. n_bits specifies number of bits on the last byte that is + // (often partially) in prefix. For example for a /125 prefix, the values // are 15 and 3, respectively. Mask is a bitmask that has the least // significant bit from the prefix set. @@ -158,11 +157,10 @@ AllocEngine::IterativeAllocator::increaseAddress(const isc::asiolink::IOAddress& } isc::asiolink::IOAddress -AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet, - const ClientClasses& client_classes, - const DuidPtr&, - const IOAddress&) { - +AllocEngine::IterativeAllocator::pickAddressInternal(const SubnetPtr& subnet, + const ClientClasses& client_classes, + const DuidPtr&, + const IOAddress&) { // Is this prefix allocation? bool prefix = pool_type_ == Lease::TYPE_PD; uint8_t prefix_len = 0; @@ -287,28 +285,28 @@ AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet, } AllocEngine::HashedAllocator::HashedAllocator(Lease::Type lease_type) - :Allocator(lease_type) { + : Allocator(lease_type) { isc_throw(NotImplemented, "Hashed allocator is not implemented"); } isc::asiolink::IOAddress -AllocEngine::HashedAllocator::pickAddress(const SubnetPtr&, - const ClientClasses&, - const DuidPtr&, - const IOAddress&) { +AllocEngine::HashedAllocator::pickAddressInternal(const SubnetPtr&, + const ClientClasses&, + const DuidPtr&, + const IOAddress&) { isc_throw(NotImplemented, "Hashed allocator is not implemented"); } AllocEngine::RandomAllocator::RandomAllocator(Lease::Type lease_type) - :Allocator(lease_type) { + : Allocator(lease_type) { isc_throw(NotImplemented, "Random allocator is not implemented"); } isc::asiolink::IOAddress -AllocEngine::RandomAllocator::pickAddress(const SubnetPtr&, - const ClientClasses&, - const DuidPtr&, - const IOAddress&) { +AllocEngine::RandomAllocator::pickAddressInternal(const SubnetPtr&, + const ClientClasses&, + const DuidPtr&, + const IOAddress&) { isc_throw(NotImplemented, "Random allocator is not implemented"); } @@ -1058,7 +1056,7 @@ AllocEngine::allocateUnreservedLeases6(ClientContext6& ctx) { } Lease6Ptr existing = LeaseMgrFactory::instance().getLease6(ctx.currentIA().type_, - candidate); + candidate); if (!existing) { // there's no existing lease for selected candidate, so it is @@ -3375,9 +3373,6 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { Lease4Ptr client_lease; findClientLease(ctx, client_lease); - // Obtain the sole instance of the LeaseMgr. - LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); - // When the client sends the DHCPREQUEST, it should always specify the // address which it is requesting or renewing. That is, the client should // either use the requested IP address option or set the ciaddr. However, @@ -3539,7 +3534,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { .arg(ctx.query_->getLabel()) .arg(client_lease->addr_.toText()); - if (lease_mgr.deleteLease(client_lease)) { + if (LeaseMgrFactory::instance().deleteLease(client_lease)) { // Need to decrease statistic for assigned addresses. StatsMgr::instance().addValue( StatsMgr::generateName("subnet", client_lease->subnet_id_, @@ -3583,14 +3578,12 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr, time_t now = time(NULL); - // @todo: remove this kludge? - std::vector local_copy; - if (ctx.clientid_ && ctx.subnet_->getMatchClientId()) { - local_copy = ctx.clientid_->getDuid(); + ClientIdPtr client_id; + if (ctx.subnet_->getMatchClientId()) { + client_id = ctx.clientid_; } - const uint8_t* local_copy0 = local_copy.empty() ? 0 : &local_copy[0]; - Lease4Ptr lease(new Lease4(addr, ctx.hwaddr_, local_copy0, local_copy.size(), + Lease4Ptr lease(new Lease4(addr, ctx.hwaddr_, client_id, valid_lft, now, ctx.subnet_->getID())); // Set FQDN specific lease parameters. diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index ed5ba6a01b..3fed7e0564 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -29,6 +30,7 @@ #include #include +#include #include #include @@ -88,30 +90,44 @@ protected: /// @param subnet next address will be returned from pool of that subnet /// @param client_classes list of classes client belongs to /// @param duid Client's DUID - /// @param hint client's hint + /// @param hint Client's hint /// /// @return the next address virtual isc::asiolink::IOAddress pickAddress(const SubnetPtr& subnet, const ClientClasses& client_classes, const DuidPtr& duid, - const isc::asiolink::IOAddress& hint) = 0; + const isc::asiolink::IOAddress& hint) { + return util::MultiThreadingMgr::call( + mutex_, [&]() {return pickAddressInternal(subnet, client_classes, duid, hint);}); + } /// @brief Default constructor. /// /// Specifies which type of leases this allocator will assign /// @param pool_type specifies pool type (addresses, temp. addr or prefixes) - Allocator(Lease::Type pool_type) - :pool_type_(pool_type) { + Allocator(Lease::Type pool_type) : pool_type_(pool_type) { } /// @brief virtual destructor virtual ~Allocator() { } + + private: + virtual isc::asiolink::IOAddress + pickAddressInternal(const SubnetPtr& subnet, + const ClientClasses& client_classes, + const DuidPtr& duid, + const isc::asiolink::IOAddress& hint) = 0; + protected: /// @brief defines pool type allocation Lease::Type pool_type_; + + private: + + std::mutex mutex_; }; /// defines a pointer to allocator @@ -132,18 +148,22 @@ protected: /// @param type - specifies allocation type IterativeAllocator(Lease::Type type); + private: + /// @brief returns the next address from pools in a subnet /// /// @param subnet next address will be returned from pool of that subnet /// @param client_classes list of classes client belongs to /// @param duid Client's DUID (ignored) - /// @param hint client's hint (ignored) + /// @param hint Client's hint (ignored) + /// /// @return the next address virtual isc::asiolink::IOAddress - pickAddress(const SubnetPtr& subnet, - const ClientClasses& client_classes, - const DuidPtr& duid, - const isc::asiolink::IOAddress& hint); + pickAddressInternal(const SubnetPtr& subnet, + const ClientClasses& client_classes, + const DuidPtr& duid, + const isc::asiolink::IOAddress& hint); + protected: /// @brief Returns the next prefix @@ -155,6 +175,7 @@ protected: /// /// @param prefix prefix to be increased /// @param prefix_len length of the prefix to be increased + /// /// @return result prefix static isc::asiolink::IOAddress increasePrefix(const isc::asiolink::IOAddress& prefix, @@ -168,11 +189,11 @@ protected: /// @param address address or prefix to be increased /// @param prefix true when the previous argument is a prefix /// @param prefix_len length of the prefix + /// /// @return result address or prefix static isc::asiolink::IOAddress increaseAddress(const isc::asiolink::IOAddress& address, bool prefix, const uint8_t prefix_len); - }; /// @brief Address/prefix allocator that gets an address based on a hash @@ -185,6 +206,8 @@ protected: /// @param type - specifies allocation type HashedAllocator(Lease::Type type); + private: + /// @brief returns an address based on hash calculated from client's DUID. /// /// @todo: Implement this method @@ -193,12 +216,13 @@ protected: /// @param client_classes list of classes client belongs to /// @param duid Client's DUID /// @param hint a hint (last address that was picked) + /// /// @return selected address virtual isc::asiolink::IOAddress - pickAddress(const SubnetPtr& subnet, - const ClientClasses& client_classes, - const DuidPtr& duid, - const isc::asiolink::IOAddress& hint); + pickAddressInternal(const SubnetPtr& subnet, + const ClientClasses& client_classes, + const DuidPtr& duid, + const isc::asiolink::IOAddress& hint); }; /// @brief Random allocator that picks address randomly @@ -211,6 +235,8 @@ protected: /// @param type - specifies allocation type RandomAllocator(Lease::Type type); + private: + /// @brief returns a random address from pool of specified subnet /// /// @todo: Implement this method @@ -219,12 +245,13 @@ protected: /// @param client_classes list of classes client belongs to /// @param duid Client's DUID (ignored) /// @param hint the last address that was picked (ignored) + /// /// @return a random address from the pool virtual isc::asiolink::IOAddress - pickAddress(const SubnetPtr& subnet, - const ClientClasses& client_classes, - const DuidPtr& duid, - const isc::asiolink::IOAddress& hint); + pickAddressInternal(const SubnetPtr& subnet, + const ClientClasses& client_classes, + const DuidPtr& duid, + const isc::asiolink::IOAddress& hint); }; public: @@ -255,7 +282,9 @@ public: /// @brief Returns allocator for a given pool type /// /// @param type type of pool (V4, IA, TA or PD) + /// /// @throw BadValue if allocator for a given type is missing + /// /// @return pointer to allocator handling a given resource types AllocatorPtr getAllocator(Lease::Type type); @@ -334,6 +363,7 @@ public: /// @brief Compares two @c AllocEngine::Resource objects for equality. /// /// @param other object to be compared with this object + /// /// @return true if objects are equal, false otherwise. bool equals(const Resource& other) const { return (address_ == other.address_ && @@ -343,6 +373,7 @@ public: /// @brief Equality operator. /// /// @param other object to be compared with this object + /// /// @return true if objects are equal, false otherwise. bool operator==(const Resource& other) const { return (equals(other)); @@ -370,8 +401,9 @@ public: /// @brief Compare operator /// /// @note Only the address/prefix part of resources is used. - /// @param lhr Left hand resource objet - /// @param rhr Right hand resource objet + /// @param lhr Left hand resource object + /// @param rhr Right hand resource object + /// /// @return true if lhr is less than rhr, false otherwise bool operator() (const Resource& lhr, const Resource& rhr) const { if (lhr.getAddress() == rhr.getAddress()) { @@ -541,12 +573,14 @@ public: /// @brief Convenience method adding new hint from IAADDR option. /// /// @param iaaddr Pointer to IAADDR. + /// /// @throw BadValue if iaaddr is null. void addHint(const Option6IAAddrPtr& iaaddr); /// @brief Convenience method adding new hint from IAPREFIX option. /// /// @param iaprefix Pointer to IAPREFIX. + /// /// @throw BadValue if iaprefix is null. void addHint(const Option6IAPrefixPtr& iaprefix); }; @@ -908,12 +942,14 @@ public: /// pointer if no matches are found. /// /// @param ctx Client context holding various information about the client. + /// /// @return Pointer to the reservation found, or an empty pointer. static ConstHostPtr findGlobalReservation(ClientContext6& ctx); /// @brief Creates an IPv6Resrv instance from a Lease6 /// /// @param lease Reference to the Lease6 + /// /// @return The newly formed IPv6Resrv instance static IPv6Resrv makeIPv6Resrv(const Lease6& lease) { if (lease.type_ == Lease::TYPE_NA) { @@ -941,10 +977,10 @@ private: /// @param [out] callout_status callout returned by the lease6_select /// /// The following fields of the ctx structure are used: - /// @ref ClientContext6::subnet_ subnet the lease is allocated from - /// @ref ClientContext6::duid_ client's DUID + /// @ref ClientContext6::subnet_ Subnet the lease is allocated from + /// @ref ClientContext6::duid_ Client's DUID /// @ref ClientContext6::iaid_ IAID from the IA_NA container the client sent to us - /// @ref ClientContext6::type_ lease type (IA, TA or PD) + /// @ref ClientContext6::type_ Lease type (IA, TA or PD) /// @ref ClientContext6::fwd_dns_update_ A boolean value which indicates that server takes /// responsibility for the forward DNS Update for this lease /// (if true). @@ -958,6 +994,7 @@ private: /// registered) /// @ref ClientContext6::fake_allocation_ is this real i.e. REQUEST (false) or just picking /// an address for SOLICIT that is not really allocated (true) + /// /// @return allocated lease (or NULL in the unlikely case of the lease just /// became unavailable) Lease6Ptr createLease6(ClientContext6& ctx, @@ -973,6 +1010,7 @@ private: /// This may change in the future. /// /// @param ctx client context that contains all details (subnet, client-id, etc.) + /// /// @return collection of newly allocated leases Lease6Collection allocateUnreservedLeases6(ClientContext6& ctx); @@ -1061,8 +1099,8 @@ private: /// @param [out] callout_status callout returned by the lease6_select /// /// The following parameters are used from the ctx structure: - /// @ref ClientContext6::subnet_ subnet the lease is allocated from - /// @ref ClientContext6::duid_ client's DUID + /// @ref ClientContext6::subnet_ Subnet the lease is allocated from + /// @ref ClientContext6::duid_ Client's DUID /// @ref ClientContext6::iaid_ IAID from the IA_NA container the client sent to us /// @ref ClientContext6::fwd_dns_update_ A boolean value which indicates that server takes /// responsibility for the forward DNS Update for this lease @@ -1078,6 +1116,7 @@ private: /// allocated (true) /// /// @return refreshed lease + /// /// @throw BadValue if trying to recycle lease that is still valid Lease6Ptr reuseExpiredLease(Lease6Ptr& expired, @@ -1109,6 +1148,7 @@ private: /// @brief Utility function that removes all leases with a specified address /// @param container A collection of Lease6 pointers /// @param addr address to be removed + /// /// @return true if removed (false otherwise) static bool removeLeases(Lease6Collection& container, @@ -1232,6 +1272,7 @@ private: /// - call lease4_recover hook /// /// @param lease Lease to be reclaimed from Declined state + /// /// @return true if it's ok to remove the lease (false = hooks status says /// to keep it) bool reclaimDeclined(const Lease4Ptr& lease); @@ -1245,6 +1286,7 @@ private: /// - call lease6_recover hook /// /// @param lease Lease to be reclaimed from Declined state + /// /// @return true if it's ok to remove the lease (false = hooks status says /// to keep it) bool reclaimDeclined(const Lease6Ptr& lease); @@ -1518,6 +1560,7 @@ public: /// matches are found. /// /// @param ctx Client context holding various information about the client. + /// /// @return Pointer to the reservation found, or an empty pointer. static ConstHostPtr findGlobalReservation(ClientContext4& ctx); @@ -1585,7 +1628,7 @@ private: /// -# If the client is not requesting any specific address, allocate /// the address from the dynamic pool. /// - /// @throws various exceptions if the allocation goes wrong. + /// @throw various exceptions if the allocation goes wrong. /// /// @param ctx Client context holding the data extracted from the /// client's message. @@ -1620,6 +1663,7 @@ private: /// - @ref ClientContext4::fake_allocation_ Is this real i.e. REQUEST (false) /// or just picking an address for DISCOVER that is not really /// allocated (true) + /// /// @return allocated lease (or NULL in the unlikely case of the lease just /// become unavailable) Lease4Ptr createLease4(const ClientContext4& ctx, @@ -1654,6 +1698,7 @@ private: /// @param [out] callout_status callout returned by the lease4_select /// /// @return Updated lease instance. + /// /// @throw BadValue if trying to reuse a lease which is still valid or /// when the provided parameters are invalid. Lease4Ptr diff --git a/src/lib/util/multi_threading_mgr.h b/src/lib/util/multi_threading_mgr.h index abaa7ae667..1b2f768110 100644 --- a/src/lib/util/multi_threading_mgr.h +++ b/src/lib/util/multi_threading_mgr.h @@ -8,6 +8,7 @@ #define MULTI_THREADING_MGR_H #include +#include namespace isc { namespace util { @@ -28,18 +29,14 @@ namespace util { /// For instance for a class protected by its mutex: /// @code /// namespace locked { -/// void foo() { ... } +/// int foo() { ... } /// } // end of locked namespace /// -/// void foo() { -/// if (MultiThreadingMgr::instance().getMode()) { -/// lock_guard lock(mutex_); -/// locked::foo(); -/// } else { -/// locked::foo(); -/// } +/// int foo() { +/// return MultiThreadingMgr::call(mutex_, []() {return locked::foo()}); /// } /// @endcode + class MultiThreadingMgr : public boost::noncopyable { public: @@ -62,6 +59,23 @@ public: /// @param enabled The new mode. void setMode(bool enabled); + /// @brief Call a Functor in MT or ST mode + /// + /// @tparam Lockable a lock which is used to create a thread safe context + /// @tparam Callable a functor which will be called in MT or ST mode + /// @param lk the lock object to perform lock in MT mode + /// @param f the functor to call + /// @result the result of the functor call + template + static auto call(Lockable& lk, const Callable& f) -> decltype(f()) { + if (MultiThreadingMgr::instance().getMode()) { + std::lock_guard lock(lk); + return f(); + } else { + return f(); + } + } + protected: /// @brief Constructor.