From: Francis Dupont Date: Wed, 27 May 2020 09:22:22 +0000 (+0200) Subject: [#1147] Reorganized v6 client handler X-Git-Tag: Kea-1.7.9~117 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fbaf8134b326e5bcee66ea81e5f5cb85d284bb76;p=thirdparty%2Fkea.git [#1147] Reorganized v6 client handler --- diff --git a/src/bin/dhcp6/client_handler.cc b/src/bin/dhcp6/client_handler.cc index f4c9085def..ba821e06c2 100644 --- a/src/bin/dhcp6/client_handler.cc +++ b/src/bin/dhcp6/client_handler.cc @@ -18,20 +18,6 @@ using namespace isc::util; namespace isc { namespace dhcp { -mutex ClientHandler::mutex_; - -ClientHandler::ClientContainer ClientHandler::clients_; - -ClientHandler::ClientHandler() : client_(), locked_() { -} - -ClientHandler::~ClientHandler() { - if (locked_) { - lock_guard lock_(mutex_); - unLock(); - } -} - ClientHandler::Client::Client(Pkt6Ptr query, DuidPtr client_id) : query_(query), thread_(this_thread::get_id()) { // Sanity checks. @@ -45,6 +31,10 @@ ClientHandler::Client::Client(Pkt6Ptr query, DuidPtr client_id) duid_ = client_id->getDuid(); } +mutex ClientHandler::mutex_; + +ClientHandler::ClientContainer ClientHandler::clients_; + ClientHandler::ClientPtr ClientHandler::lookup(const DuidPtr& duid) { // Sanity check. @@ -60,37 +50,34 @@ ClientHandler::lookup(const DuidPtr& duid) { } void -ClientHandler::lock() { +ClientHandler::add(const ClientPtr& client) { // Sanity check. - if (!locked_) { - isc_throw(Unexpected, "nothing to lock in ClientHandler::lock"); + if (!client) { + isc_throw(InvalidParameter, "client is null in ClientHandler::add"); } // Assume insert will never fail so not checking its result. - clients_.insert(client_); + clients_.insert(client); } void -ClientHandler::unLock() { +ClientHandler::del(const DuidPtr& duid) { // Sanity check. - if (!locked_) { - isc_throw(Unexpected, "nothing to unlock in ClientHandler::unLock"); + if (!duid) { + isc_throw(InvalidParameter, "duid is null in ClientHandler::del"); } // Assume erase will never fail so not checking its result. - clients_.erase(locked_->getDuid()); - locked_.reset(); - if (!client_ || !client_->cont_) { - return; - } + clients_.erase(duid->getDuid()); +} - // Try to process next query. As the caller holds the mutex of - // the handler class the continuation will be resumed after. - MultiThreadingMgr& mt_mgr = MultiThreadingMgr::instance(); - if (mt_mgr.getMode()) { - if (!mt_mgr.getThreadPool().addFront(client_->cont_)) { - LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_PACKET_QUEUE_FULL); - } +ClientHandler::ClientHandler() : client_(), locked_() { +} + +ClientHandler::~ClientHandler() { + if (locked_) { + lock_guard lock_(mutex_); + unLock(); } } @@ -158,5 +145,37 @@ ClientHandler::tryLock(Pkt6Ptr query, ContinuationPtr cont) { return (false); } +void +ClientHandler::lock() { + // Sanity check. + if (!locked_) { + isc_throw(Unexpected, "nothing to lock in ClientHandler::lock"); + } + add(client_); +} + +void +ClientHandler::unLock() { + // Sanity check. + if (!locked_) { + isc_throw(Unexpected, "nothing to unlock in ClientHandler::unLock"); + } + + del(locked_); + locked_.reset(); + if (!client_ || !client_->cont_) { + return; + } + + // Try to process next query. As the caller holds the mutex of + // the handler class the continuation will be resumed after. + MultiThreadingMgr& mt_mgr = MultiThreadingMgr::instance(); + if (mt_mgr.getMode()) { + if (!mt_mgr.getThreadPool().addFront(client_->cont_)) { + LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_PACKET_QUEUE_FULL); + } + } +} + } // namespace dhcp } // namespace isc diff --git a/src/bin/dhcp6/client_handler.h b/src/bin/dhcp6/client_handler.h index d7dd8a015d..9ad5a7b719 100644 --- a/src/bin/dhcp6/client_handler.h +++ b/src/bin/dhcp6/client_handler.h @@ -35,31 +35,10 @@ inline ContinuationPtr makeContinuation(Continuation&& cont) { /// @brief Client race avoidance RAII handler. class ClientHandler : public boost::noncopyable { -public: - - /// @brief Constructor. - ClientHandler(); - - /// @brief Destructor. - /// - /// Releases the client if it was acquired. - virtual ~ClientHandler(); - - /// @brief Tries to acquires a client. - /// - /// Lookup the client: - /// - if not found insert the client in the clients map and return true - /// - if found, if has a continuation put it in the holder, - /// and return false - /// - /// @param query The query from the client. - /// @param cont The continuation in the case the client was held. - /// @return true if the client was acquired, false if there is already - /// a query from the same client. - bool tryLock(Pkt6Ptr query, ContinuationPtr cont = ContinuationPtr()); - private: + /// Class (aka static) types, methods and members. + /// @brief Structure representing a client. struct Client { @@ -95,25 +74,85 @@ private: /// @brief The type of shared pointers to clients. typedef boost::shared_ptr ClientPtr; - /// @brief Local client. - ClientPtr client_; + /// @brief The type of the client container. + typedef boost::multi_index_container< - /// @brief Client ID locked by this handler. - DuidPtr locked_; + // This container stores pointers to client objects. + ClientPtr, - /// @brief Mutex to protect the client container. - /// - /// The mutex is used only by public methods for guards. - static std::mutex mutex_; + // Start specification of indexes here. + boost::multi_index::indexed_by< + + // First index is used to search by Duid. + boost::multi_index::hashed_unique< + + // Client ID binary content as a member of the Client object. + boost::multi_index::member< + Client, std::vector, &Client::duid_ + > + > + > + > ClientContainer; /// @brief Lookup a client. /// /// The mutex must be held by the caller. /// /// @param duid The duid of the query from the client. - /// @return The held client or null. + /// @return The client found in the container or null. static ClientPtr lookup(const DuidPtr& duid); + /// @brief Add a client. + /// + /// The mutex must be held by the caller. + /// + /// @param client The client to insert into the client container. + static void add(const ClientPtr& client); + + /// @brief Delete a client. + /// + /// The mutex must be held by the caller. + /// + /// @param duid The duid to delete from the client container. + static void del(const DuidPtr& duid); + + /// @brief Mutex to protect the client container. + /// + /// The mutex is used only by public methods for guards. + static std::mutex mutex_; + + /// @brief The client container. + static ClientContainer clients_; + +public: + + /// Public interface. + + /// @brief Constructor. + ClientHandler(); + + /// @brief Destructor. + /// + /// Releases the client if it was acquired. + virtual ~ClientHandler(); + + /// @brief Tries to acquires a client. + /// + /// Lookup the client: + /// - if not found insert the client in the clients map and return true + /// - if found, if has a continuation put it in the holder, + /// and return false + /// + /// @param query The query from the client. + /// @param cont The continuation in the case the client was held. + /// @return true if the client was acquired, false if there is already + /// a query from the same client. + bool tryLock(Pkt6Ptr query, ContinuationPtr cont = ContinuationPtr()); + +private: + + /// Instance methods and members. + /// @brief Acquire a client. /// /// The mutex must be held by the caller. @@ -124,31 +163,15 @@ private: /// If the client has a continuation, push it at front of the thread /// packet queue. /// - /// The mutex must be held by the caller. + /// The mutex must be held by the only caller: the destructor. void unLock(); - /// @brief The type of the client container. - typedef boost::multi_index_container< - - // This container stores pointers to client objects. - ClientPtr, - - // Start specification of indexes here. - boost::multi_index::indexed_by< - - // First index is used to search by Duid. - boost::multi_index::hashed_unique< + /// @brief Local client. + ClientPtr client_; - // Client ID binary content as a member of the Client object. - boost::multi_index::member< - Client, std::vector, &Client::duid_ - > - > - > - > ClientContainer; + /// @brief Client ID locked by this handler. + DuidPtr locked_; - /// @brief The client container. - static ClientContainer clients_; }; } // namespace isc