[3329] dhcpsrv::D2ClientMgr now registers IfaceMgr

D2ClientMgr now registers and unregisters its runReadyIO method and sender's
select-fd with IfaceMgr's external socket monitoring.

src/lib/dhcpsrv/d2_client_mgr.cc

@@ -12,10 +12,13 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <dhcp/iface_mgr.h>
 #include <dhcp_ddns/ncr_udp.h>
 #include <dhcpsrv/d2_client_mgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 
+#include <boost/bind.hpp>
+
 #include <string>
 
 using namespace std;
@@ -24,11 +27,13 @@ namespace isc {
 namespace dhcp {
 
 D2ClientMgr::D2ClientMgr() : d2_client_config_(new D2ClientConfig()),
-    name_change_sender_(), private_io_service_(), sender_io_service_(NULL) {
+    name_change_sender_(), private_io_service_(),
+    registered_select_fd_(dhcp_ddns::WatchSocket::INVALID_SOCKET) {
     // Default constructor initializes with a disabled configuration.
 }
 
 D2ClientMgr::~D2ClientMgr(){
+    stopSender();
 }
 
 void
@@ -209,14 +214,16 @@ D2ClientMgr::startSender(D2ClientErrorHandler error_handler,
     // Set the error handler.
     client_error_handler_ = error_handler;
 
-    // Remember the io service being used.
-    sender_io_service_ = &io_service;
-
     // Start the sender on the given service.
-    name_change_sender_->startSending(*sender_io_service_);
-
-    /// @todo need to register sender's select-fd with IfaceMgr once 3315 is
-    /// done.
+    name_change_sender_->startSending(io_service);
+
+    // Register sender's select-fd with IfaceMgr.
+    // We need to remember the fd that is registered so we can unregister later.
+    // IO error handling in the sender may alter its select-fd.
+    registered_select_fd_ = name_change_sender_->getSelectFd();
+    IfaceMgr::instance().addExternalSocket(registered_select_fd_,
+                                           boost::bind(&D2ClientMgr::runReadyIO,
+                                                       this));
 }
 
 bool
@@ -226,14 +233,16 @@ D2ClientMgr::amSending() const {
 
 void
 D2ClientMgr::stopSender() {
-    if (!name_change_sender_)  {
-        isc_throw(D2ClientError, "D2ClientMgr::stopSender sender is null");
+    /// Unregister sender's select-fd.
+    if (registered_select_fd_ != dhcp_ddns::WatchSocket::INVALID_SOCKET) {
+        IfaceMgr::instance().deleteExternalSocket(registered_select_fd_);
+        registered_select_fd_ = dhcp_ddns::WatchSocket::INVALID_SOCKET;
     }
 
-    /// @todo need to unregister sender's select-fd with IfaceMgr once 3315 is
-    /// done.
-
-    name_change_sender_->stopSending();
+    // If its not null, call stop.
+    if (name_change_sender_)  {
+        name_change_sender_->stopSending();
+    }
 }
 
 void
@@ -309,17 +318,13 @@ D2ClientMgr::getSelectFd() {
 
 void
 D2ClientMgr::runReadyIO() {
-    if (!sender_io_service_) {
+    if (!name_change_sender_) {
         // This should never happen.
         isc_throw(D2ClientError, "D2ClientMgr::runReadyIO"
-                  " sender io service is null");
+                  " name_change_sender is null");
     }
 
-    // We shouldn't be here if IO isn't ready to execute.
-    // By running poll we're gauranteed not to hang.
-    /// @todo Trac# 3325 requests that asiolink::IOService provide a
-    /// wrapper for poll().
-    sender_io_service_->get_io_service().poll();
+    name_change_sender_->runReadyIO();
 }
 
 };  // namespace dhcp

src/lib/dhcpsrv/d2_client_mgr.h

@@ -16,7 +16,7 @@
 #define D2_CLIENT_MGR_H
 
 /// @file d2_client_mgr.h Defines the D2ClientMgr class.
-/// This file defines the class Kea uses to act as a client of the 
+/// This file defines the class Kea uses to act as a client of the
 /// b10-dhcp-ddns module (aka D2).
 ///
 #include <asiolink/io_address.h>
@@ -62,13 +62,13 @@ boost::function<void(const dhcp_ddns::NameChangeSender::Result result,
 /// used by the NCR IPC with the select-driven IO used by Kea.  Senders expose
 /// a file descriptor, the "select-fd" that can monitored for read-readiness
 /// with the select() function (or variants).  D2ClientMgr provides a method,
-/// runReadyIO(), that will process all ready events on a sender's
-/// IOservice.  Track# 3315 is extending Kea's IfaceMgr to support the
-/// registration of multiple external sockets with callbacks that are then
-/// monitored with IO readiness via select().
-/// @todo D2ClientMgr will be modified to register the sender's select-fd and
-/// runReadyIO() with IfaceMgr when entering the send mode and will
-/// unregister when exiting send mode.
+/// runReadyIO(), that will instructs the sender to process the next ready
+/// ready IO handler on the sender's IOservice.  Track# 3315 extended
+/// Kea's IfaceMgr to support the registration of multiple external sockets
+/// with callbacks that are then monitored with IO readiness via select().
+/// D2ClientMgr registers the sender's select-fd and runReadyIO() with
+/// IfaceMgr when entering the send mode and unregisters it when exiting send
+/// mode.
 ///
 /// To place the manager in send mode, the calling layer must supply an error
 /// handler and optionally an IOService instance.  The error handler is invoked
@@ -161,7 +161,7 @@ public:
     /// Templated wrapper around the analyzeFqdn() allowing that method to
     /// be used for either IPv4 or IPv6 processing.  This methods resets all
     /// of the flags in the response to zero and then sets the S,N, and O
-    /// flags.  Any other flags are the responsiblity of the invoking layer.
+    /// flags.  Any other flags are the responsibility of the invoking layer.
     ///
     /// @param fqdn FQDN option from which to read client (inbound) flags
     /// @param fqdn_resp FQDN option to update with the server (outbound) flags
@@ -278,14 +278,17 @@ public:
 
     /// @brief Processes sender IO events
     ///
-    /// Runs all handlers ready for execution on the sender's IO service.
+    /// Serves as callback registered for the sender's select-fd with IfaceMgr.
+    /// It instructs the sender to execute the next ready IO handler.
+    /// It provides an instance method that can be bound via boost::bind, as
+    /// NameChangeSender is abstract.
     void runReadyIO();
 
 protected:
     /// @brief Function operator implementing the NCR sender callback.
     ///
     /// This method is invoked each time the NameChangeSender completes
-    /// an asychronous send.
+    /// an asynchronous send.
     ///
     /// @param result contains that send outcome status.
     /// @param ncr is a pointer to the NameChangeRequest that was
@@ -307,6 +310,14 @@ protected:
     /// mode.
     int getSelectFd();
 
+    /// @brief Fetches the select-fd that is currently registered.
+    ///
+    /// @return The currently registered select-fd or
+    /// dhcp_ddns::WatchSocket::INVALID_SOCKET.
+    ///
+    /// @note This is only exposed for testing purposes.
+    int getRegisteredSelectFd();
+
 private:
     /// @brief Container class for DHCP-DDNS configuration parameters.
     D2ClientConfigPtr d2_client_config_;
@@ -322,12 +333,8 @@ private:
     /// completes with a failed status.
     D2ClientErrorHandler client_error_handler_;
 
-    /// @brief Pointer to the IOService currently being used by the sender.
-    /// @note We need to remember the io_service given to the sender however
-    /// we may have received only a referenece to it from the calling layer.
-    /// Use a raw pointer to store it.  This value should never be exposed
-    /// and is only valid while in send mode.
-    asiolink::IOService* sender_io_service_;
+    /// @brief Remembers the select-fd registered with IfaceMgr.
+    int registered_select_fd_;
 };
 
 template <class T>

src/lib/dhcpsrv/tests/d2_udp_unittest.cc

@@ -19,6 +19,7 @@
 #include <asio.hpp>
 #include <asiolink/io_service.h>
 #include <config.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcpsrv/d2_client_mgr.h>
 #include <exceptions/exceptions.h>
 
@@ -376,4 +377,45 @@ TEST_F(D2ClientMgrTest, udpSendErrorHandler) {
     ASSERT_EQ(1, error_handler_count_);
 }
 
+TEST_F(D2ClientMgrTest, ifaceRegister) {
+    // Enable DDNS with server at 127.0.0.1/prot 53001 via UDP.
+    enableDdns("127.0.0.1", 530001, dhcp_ddns::NCR_UDP);
+
+    // Place sender in send mode.
+    ASSERT_NO_THROW(startSender(getErrorHandler()));
+
+    // Queue three messages.
+    for (int i = 0; i < 3; ++i) {
+        dhcp_ddns::NameChangeRequestPtr ncr = buildTestNcr();
+        ASSERT_NO_THROW(sendRequest(ncr));
+    }
+
+    EXPECT_EQ(3, getQueueSize());
+
+    // select_fd should evaluate to ready to read.
+    selectCheck(true);
+
+    // Calling receive should complete the first message and start the second.
+    IfaceMgr::instance().receive4(0, 0);
+
+    // Verify the callback hander was invoked, no errors counted.
+    EXPECT_EQ(2, getQueueSize());
+    ASSERT_EQ(1, callback_count_);
+    ASSERT_EQ(0, error_handler_count_);
+
+    // Stop the sender.  This should complete the second message but leave
+    // the third in the queue.
+    ASSERT_NO_THROW(stopSender());
+    EXPECT_EQ(1, getQueueSize());
+    ASSERT_EQ(2, callback_count_);
+    ASSERT_EQ(0, error_handler_count_);
+
+    // Calling recevie again should have no affect.
+    IfaceMgr::instance().receive4(0, 0);
+    EXPECT_EQ(1, getQueueSize());
+    ASSERT_EQ(2, callback_count_);
+    ASSERT_EQ(0, error_handler_count_);
+}
+
+
 } // end of anonymous namespace