diff --git a/src/hooks/dhcp/ping_check/ping_channel.cc b/src/hooks/dhcp/ping_check/ping_channel.cc index 6a6a88c038..11cef80dde 100644 --- a/src/hooks/dhcp/ping_check/ping_channel.cc +++ b/src/hooks/dhcp/ping_check/ping_channel.cc @@ -36,11 +36,13 @@ PingChannel::nextEchoInstanceNum() { PingChannel::PingChannel(IOServicePtr& io_service, NextToSendCallback next_to_send_cb, + UpdateToSendCallback update_to_send_cb, EchoSentCallback echo_sent_cb, ReplyReceivedCallback reply_received_cb, ShutdownCallback shutdown_cb) : io_service_(io_service), next_to_send_cb_(next_to_send_cb), + update_to_send_cb_(update_to_send_cb), echo_sent_cb_(echo_sent_cb), reply_received_cb_(reply_received_cb), shutdown_cb_(shutdown_cb), @@ -310,19 +312,25 @@ PingChannel::startRead() { void PingChannel::sendNext() { try { + // Fetch the next one to send (outside the mutex). + IOAddress target("0.0.0.0"); + PingContextPtr context = ((next_to_send_cb_)(target)); + if (!context) { + // Nothing to send. + return; + } + MultiThreadingLock lock(*mutex_); if (!canSend()) { // Can't send right now, get out. return; } - - // Fetch the next one to send. - IOAddress target("0.0.0.0"); - if (!((next_to_send_cb_)(target))) { - // Nothing to send. - return; + + // Update context to SENDING inside the mutex. + if (update_to_send_cb_) { + (update_to_send_cb_)(context); } - + // Have an target IP, build an ECHO REQUEST for it. sending_ = true; ICMPMsgPtr next_echo(new ICMPMsg()); diff --git a/src/hooks/dhcp/ping_check/ping_channel.h b/src/hooks/dhcp/ping_check/ping_channel.h index 064e7a2a82..d6fbedcd44 100644 --- a/src/hooks/dhcp/ping_check/ping_channel.h +++ b/src/hooks/dhcp/ping_check/ping_channel.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -66,8 +67,12 @@ typedef ICMPSocket PingSocket; /// @brief Defines a pointer to PingSocket. typedef boost::shared_ptr PingSocketPtr; -/// @brief Function type for callback that fetches next IOAddress to ping. -typedef std::function NextToSendCallback; +/// @brief Function type for callback that fetches next target to ping. +/// Returns the context selected to send, and outputs its target address. +typedef std::function NextToSendCallback; + +/// @brief Function type for callback to update a context to SENDING state. +typedef std::function UpdateToSendCallback; /// @brief Function type for callback to invoke upon ECHO send completion. typedef std::function EchoSentCallback; @@ -108,8 +113,10 @@ class PingChannel : public boost::enable_shared_from_this { /// /// @param io_service pointer to the IOService instance that will manage /// the channel's IO. Must not be empty - /// @param next_to_send_cb callback to invoke to fetch the next IOAddress - /// to ping + /// @param next_to_send_cb callback to invoke to fetch the next context and + /// its target address to ping (called outside the mutex) + /// @param update_to_send_cb callback to invoke to update the selected + /// context to SENDING and persist it (called inside the mutex) /// @param echo_sent_cb callback to invoke when an ECHO send has completed /// @param reply_received_cb callback to invoke when an ICMP reply has been /// received. This callback is passed all inbound ICMP messages (e.g. ECHO @@ -120,6 +127,7 @@ class PingChannel : public boost::enable_shared_from_this { /// @throw BadValue if io_service is empty. PingChannel(asiolink::IOServicePtr& io_service, NextToSendCallback next_to_send_cb, + UpdateToSendCallback update_to_send_cb, EchoSentCallback echo_sent_cb, ReplyReceivedCallback reply_received_cb, ShutdownCallback shutdown_cb = ShutdownCallback()); @@ -315,9 +323,12 @@ class PingChannel : public boost::enable_shared_from_this { /// @brief IOService instance the drives socket IO asiolink::IOServicePtr io_service_; - /// @brief Callback to invoke to fetch the next address to ping. + /// @brief Callback to invoke to fetch the next context/address to ping. NextToSendCallback next_to_send_cb_; + /// @brief Callback to invoke to update selected context to SENDING state. + UpdateToSendCallback update_to_send_cb_; + /// @brief Callback to invoke when an ECHO write has completed. EchoSentCallback echo_sent_cb_; diff --git a/src/hooks/dhcp/ping_check/ping_check_mgr.cc b/src/hooks/dhcp/ping_check/ping_check_mgr.cc index aadea02934..949b9974eb 100644 --- a/src/hooks/dhcp/ping_check/ping_check_mgr.cc +++ b/src/hooks/dhcp/ping_check/ping_check_mgr.cc @@ -178,23 +178,26 @@ PingCheckMgr::startPing(dhcp::Lease4Ptr& lease, dhcp::Pkt4Ptr& query, hooks::Par startPing(lease, query, parking_lot, getGlobalConfig()); } -bool +PingContextPtr PingCheckMgr::nextToSend(IOAddress& next) { if (checkSuspended()) { - return (false); + return (PingContextPtr()); } PingContextPtr context = store_->getNextToSend(); if (!context) { - return (false); + return (PingContextPtr()); } next = context->getTarget(); + return (context); +} + +void +PingCheckMgr::updateContextToSend(PingContextPtr context) { // Transition to sending. context->setState(PingContext::SENDING); store_->updateContext(context); - - return (true); } void @@ -632,6 +635,8 @@ PingCheckMgr::createChannel(IOServicePtr io_service) { return (PingChannelPtr(new PingChannel(io_service, std::bind(&PingCheckMgr::nextToSend, this, ph::_1), + std::bind(&PingCheckMgr::updateContextToSend, + this, ph::_1), std::bind(&PingCheckMgr::sendCompleted, this, ph::_1, ph::_2), std::bind(&PingCheckMgr::replyReceived, diff --git a/src/hooks/dhcp/ping_check/ping_check_mgr.h b/src/hooks/dhcp/ping_check/ping_check_mgr.h index 5f1b83e9fb..e759d37378 100644 --- a/src/hooks/dhcp/ping_check/ping_check_mgr.h +++ b/src/hooks/dhcp/ping_check/ping_check_mgr.h @@ -118,16 +118,24 @@ class PingCheckMgr : public boost::enable_shared_from_this { hooks::ParkingLotHandlePtr& parking_lot); /// @brief Callback passed to PingChannel to use to retrieve the next - /// address to check. + /// context/address to check. /// /// Fetches the context which has been in the WAITING_TO_SEND state the - /// longest and returns its lease address. + /// longest and returns it, also outputs its lease address. /// - /// @param[out] next upon return it will contain the next target address. - /// Contents are only meaningful if the function returns true. + /// @param[out] next upon return it will contain the next target address + /// if a context is available. /// - /// @return True another target address exists, false otherwise. - virtual bool nextToSend(asiolink::IOAddress& next); + /// @return The context selected to send, or empty if none available. + virtual PingContextPtr nextToSend(asiolink::IOAddress& next); + + /// @brief Callback passed to PingChannel to update a context to SENDING + /// just before sending. + /// + /// This is invoked inside the channel mutex. + /// + /// @param context context to update. + virtual void updateContextToSend(PingContextPtr context); /// @brief Callback passed to PingChannel to invoke when an ECHO REQUEST /// send has completed.