[3315] Support for multiple external sockets in dhcp::IfaceMgr

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -226,7 +226,7 @@ void ControlledDhcpv4Srv::establishSession() {
     int ctrl_socket = cc_session_->getSocketDesc();
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_CCSESSION_STARTED)
               .arg(ctrl_socket);
-    IfaceMgr::instance().set_session_socket(ctrl_socket, sessionReader);
+    IfaceMgr::instance().addExternalSocket(ctrl_socket, sessionReader);
 }
 
 void ControlledDhcpv4Srv::disconnectSession() {
@@ -235,13 +235,14 @@ void ControlledDhcpv4Srv::disconnectSession() {
         config_session_ = NULL;
     }
     if (cc_session_) {
+
+        int ctrl_socket = cc_session_->getSocketDesc();
         cc_session_->disconnect();
+
+        IfaceMgr::instance().deleteExternalSocket(ctrl_socket);
         delete cc_session_;
         cc_session_ = NULL;
     }
-
-    // deregister session socket
-    IfaceMgr::instance().set_session_socket(IfaceMgr::INVALID_SOCKET, NULL);
 }
 
 ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)

src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -227,7 +227,7 @@ void ControlledDhcpv6Srv::establishSession() {
     int ctrl_socket = cc_session_->getSocketDesc();
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_CCSESSION_STARTED)
               .arg(ctrl_socket);
-    IfaceMgr::instance().set_session_socket(ctrl_socket, sessionReader);
+    IfaceMgr::instance().addExternalSocket(ctrl_socket, sessionReader);
 }
 
 void ControlledDhcpv6Srv::disconnectSession() {
@@ -236,13 +236,16 @@ void ControlledDhcpv6Srv::disconnectSession() {
         config_session_ = NULL;
     }
     if (cc_session_) {
+
+        int ctrl_socket = cc_session_->getSocketDesc();
         cc_session_->disconnect();
+
+        // deregister session socket
+        IfaceMgr::instance().deleteExternalSocket(ctrl_socket);
+
         delete cc_session_;
         cc_session_ = NULL;
     }
-
-    // deregister session socket
-    IfaceMgr::instance().set_session_socket(IfaceMgr::INVALID_SOCKET, NULL);
 }
 
 ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)

src/lib/dhcp/iface_mgr.cc

@@ -170,7 +170,6 @@ bool Iface::delSocket(const uint16_t sockfd) {
 IfaceMgr::IfaceMgr()
     :control_buf_len_(CMSG_SPACE(sizeof(struct in6_pktinfo))),
      control_buf_(new char[control_buf_len_]),
-     session_socket_(INVALID_SOCKET), session_callback_(NULL),
      packet_filter_(new PktFilterInet()),
      packet_filter6_(new PktFilterInet6())
 {
@@ -228,6 +227,37 @@ IfaceMgr::isDirectResponseSupported() const {
 }
 
 void
+IfaceMgr::addExternalSocket(int socketfd, SessionCallback callback) {
+    for (SocketCallbackContainer::iterator s = callbacks_.begin();
+         s != callbacks_.end(); ++s) {
+
+        // There's such a socket description there already.
+        // Update the callback and we're done
+        if (s->socket_ == socketfd) {
+            s->callback_ = callback;
+            return;
+        }
+    }
+
+    // Add a new entry to the callbacks vector
+    SocketCallback x;
+    x.socket_ = socketfd;
+    x.callback_ = callback;
+    callbacks_.push_back(x);
+}
+
+void
+IfaceMgr::deleteExternalSocket(int socketfd) {
+    for (SocketCallbackContainer::iterator s = callbacks_.begin();
+         s != callbacks_.end(); ++s) {
+        if (s->socket_ == socketfd) {
+            callbacks_.erase(s);
+            return;
+        }
+    }
+}
+
+void
 IfaceMgr::setPacketFilter(const PktFilterPtr& packet_filter) {
     // Do not allow NULL pointer.
     if (!packet_filter) {
@@ -815,13 +845,16 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
         }
     }
 
-    // if there is session socket registered...
-    if (session_socket_ != INVALID_SOCKET) {
-        // at it to the set as well
-        FD_SET(session_socket_, &sockets);
-        if (maxfd < session_socket_)
-            maxfd = session_socket_;
-        names << session_socket_ << "(session)";
+    // if there are any session sockets registered...
+    if (!callbacks_.empty()) {
+        for (SocketCallbackContainer::const_iterator s = callbacks_.begin();
+             s != callbacks_.end(); ++s) {
+            FD_SET(s->socket_, &sockets);
+            if (maxfd < s->socket_) {
+                maxfd = s->socket_;
+            }
+            names << s->socket_ << "(session)";
+        }
     }
 
     struct timeval select_timeout;
@@ -838,18 +871,24 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
     }
 
     // Let's find out which socket has the data
-    if ((session_socket_ != INVALID_SOCKET) && (FD_ISSET(session_socket_, &sockets))) {
+    for (SocketCallbackContainer::iterator s = callbacks_.begin();
+         s != callbacks_.end(); ++s) {
+        if (!FD_ISSET(s->socket_, &sockets)) {
+            continue;
+        }
+
         // something received over session socket
-        if (session_callback_) {
-            // in theory we could call io_service.run_one() here, instead of
-            // implementing callback mechanism, but that would introduce
-            // asiolink dependency to libdhcp++ and that is something we want
-            // to avoid (see CPE market and out long term plans for minimalistic
-            // implementations.
-            session_callback_();
+
+        // in theory we could call io_service.run_one() here, instead of
+        // implementing callback mechanism, but that would introduce
+        // asiolink dependency to libdhcp++ and that is something we want
+        // to avoid (see CPE market and out long term plans for minimalistic
+        // implementations.
+        if (s->callback_) {
+            s->callback_();
         }
 
-        return (Pkt4Ptr()); // NULL
+        return (Pkt4Ptr());
     }
 
     // Let's find out which interface/socket has the data
@@ -912,13 +951,18 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
         }
     }
 
-    // if there is session socket registered...
-    if (session_socket_ != INVALID_SOCKET) {
-        // at it to the set as well
-        FD_SET(session_socket_, &sockets);
-        if (maxfd < session_socket_)
-            maxfd = session_socket_;
-        names << session_socket_ << "(session)";
+    // if there are any session sockets registered...
+    if (!callbacks_.empty()) {
+        for (SocketCallbackContainer::const_iterator s = callbacks_.begin();
+             s != callbacks_.end(); ++s) {
+
+            // Add it to the set as well
+            FD_SET(s->socket_, &sockets);
+            if (maxfd < s->socket_) {
+                maxfd = s->socket_;
+            }
+            names << s->socket_ << "(session)";
+        }
     }
 
     struct timeval select_timeout;
@@ -935,18 +979,21 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     }
 
     // Let's find out which socket has the data
-    if ((session_socket_ != INVALID_SOCKET) && (FD_ISSET(session_socket_, &sockets))) {
-        // something received over session socket
-        if (session_callback_) {
-            // in theory we could call io_service.run_one() here, instead of
-            // implementing callback mechanism, but that would introduce
-            // asiolink dependency to libdhcp++ and that is something we want
-            // to avoid (see CPE market and out long term plans for minimalistic
-            // implementations.
-            session_callback_();
+    for (SocketCallbackContainer::iterator s = callbacks_.begin();
+         s != callbacks_.end(); ++s) {
+        if (FD_ISSET(s->socket_, &sockets)) {
+            // something received over session socket
+            if (s->callback_) {
+                // in theory we could call io_service.run_one() here, instead of
+                // implementing callback mechanism, but that would introduce
+                // asiolink dependency to libdhcp++ and that is something we want
+                // to avoid (see CPE market and out long term plans for minimalistic
+                // implementations.
+                s->callback_();
+            }
         }
 
-        return (Pkt6Ptr()); // NULL
+        return (Pkt6Ptr());
     }
 
     // Let's find out which interface/socket has the data

src/lib/dhcp/iface_mgr.h

@@ -397,6 +397,18 @@ public:
     /// Defines callback used when commands are received over control session.
     typedef void (*SessionCallback) (void);
 
+    /// Keeps callback information for external sockets.
+    struct SocketCallback {
+        /// Socket descriptor of the external socket.
+        int socket_;
+
+        /// A callback that will be called when data arrives over socket_.
+        SessionCallback callback_;
+    };
+
+    /// Defines storage container for callbacks for external sockets
+    typedef std::list<SocketCallback> SocketCallbackContainer;
+
     /// @brief Packet reception buffer size
     ///
     /// RFC3315 states that server responses may be
@@ -785,17 +797,18 @@ public:
     /// @return number of detected interfaces
     uint16_t countIfaces() { return ifaces_.size(); }
 
-    /// @brief Sets session socket and a callback
+    /// @brief Sets external socket and a callback
     ///
     /// Specifies session socket and a callback that will be called
     /// when data will be received over that socket.
     ///
     /// @param socketfd socket descriptor
     /// @param callback callback function
-    void set_session_socket(int socketfd, SessionCallback callback) {
-        session_socket_ = socketfd;
-        session_callback_ = callback;
-    }
+    void addExternalSocket(int socketfd, SessionCallback callback);
+
+    /// @brief Deletes external socket
+
+    void deleteExternalSocket(int socketfd);
 
     /// @brief Set packet filter object to handle sending and receiving DHCPv4
     /// messages.
@@ -881,9 +894,6 @@ public:
     /// @return true if there is a socket bound to the specified address.
     bool hasOpenSocket(const isc::asiolink::IOAddress& addr) const;
 
-    /// A value of socket descriptor representing "not specified" state.
-    static const int INVALID_SOCKET = -1;
-
     // don't use private, we need derived classes in tests
 protected:
 
@@ -977,13 +987,7 @@ protected:
     /// @return true if successful, false otherwise
     bool os_receive4(struct msghdr& m, Pkt4Ptr& pkt);
 
-    /// Socket descriptor of the session socket.
-    int session_socket_;
-
-    /// A callback that will be called when data arrives over session_socket_.
-    SessionCallback session_callback_;
 private:
-
     /// @brief Identifies local network address to be used to
     /// connect to remote address.
     ///
@@ -1041,6 +1045,9 @@ private:
     /// messages. It is possible to supply a custom object using
     /// setPacketFilter method.
     PktFilter6Ptr packet_filter6_;
+
+    /// @brief Contains list of callbacks for external sockets
+    SocketCallbackContainer callbacks_;
 };
 
 }; // namespace isc::dhcp

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -2590,15 +2590,20 @@ TEST_F(IfaceMgrTest, detectIfaces) {
 }
 
 volatile bool callback_ok;
+volatile bool callback2_ok;
 
 void my_callback(void) {
     cout << "Callback triggered." << endl;
     callback_ok = true;
 }
 
-TEST_F(IfaceMgrTest, controlSession) {
-    // Tests if extra control socket and its callback can be passed and
-    // it is supported properly by receive4() method.
+void my_callback2(void) {
+    callback2_ok = true;
+}
+
+// Tests if a signle external socket and its callback can be passed and
+// it is supported properly by receive4() method.
+TEST_F(IfaceMgrTest, SingleExternalSession) {
 
     callback_ok = false;
 
@@ -2607,7 +2612,7 @@ TEST_F(IfaceMgrTest, controlSession) {
     // Create pipe and register it as extra socket
     int pipefd[2];
     EXPECT_TRUE(pipe(pipefd) == 0);
-    EXPECT_NO_THROW(ifacemgr->set_session_socket(pipefd[0], my_callback));
+    EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0], my_callback));
 
     Pkt4Ptr pkt4;
     ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
@@ -2635,6 +2640,123 @@ TEST_F(IfaceMgrTest, controlSession) {
     close(pipefd[0]);
 }
 
+// Tests if multiple external sockets and their callbacks can be passed and
+// it is supported properly by receive4() method.
+TEST_F(IfaceMgrTest, MiltipleControlSessions) {
+
+    callback_ok = false;
+    callback2_ok = false;
+
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+
+    // Create first pipe and register it as extra socket
+    int pipefd[2];
+    EXPECT_TRUE(pipe(pipefd) == 0);
+    EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0], my_callback));
+
+    // Let's create a second pipe and register it as well
+    int secondpipe[2];
+    EXPECT_TRUE(pipe(secondpipe) == 0);
+    EXPECT_NO_THROW(ifacemgr->addExternalSocket(secondpipe[0], my_callback2));
+
+    Pkt4Ptr pkt4;
+    ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
+
+    // Our callbacks should not be called this time (there was no data)
+    EXPECT_FALSE(callback_ok);
+    EXPECT_FALSE(callback2_ok);
+
+    // IfaceMgr should not process control socket data as incoming packets
+    EXPECT_FALSE(pkt4);
+
+    // Now, send some data over the first pipe (38 bytes)
+    EXPECT_EQ(38, write(pipefd[1], "Hi, this is a message sent over a pipe", 38));
+
+    // ... and repeat
+    ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
+
+    // IfaceMgr should not process control socket data as incoming packets
+    EXPECT_FALSE(pkt4);
+
+    // There was some data, so this time callback should be called
+    EXPECT_TRUE(callback_ok);
+    EXPECT_FALSE(callback2_ok);
+
+    // Read the data sent, because our test callbacks are too dumb to actually
+    // do it.
+    char buf[80];
+    read(pipefd[0], buf, 80);
+
+    // Clear the status...
+    callback_ok = false;
+    callback2_ok = false;
+
+    // And try again, using the second pipe
+    EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
+
+    // ... and repeat
+    ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
+
+    // IfaceMgr should not process control socket data as incoming packets
+    EXPECT_FALSE(pkt4);
+
+    // There was some data, so this time callback should be called
+    EXPECT_FALSE(callback_ok);
+    EXPECT_TRUE(callback2_ok);
+
+    // close both pipe ends
+    close(pipefd[1]);
+    close(pipefd[0]);
+
+    close(secondpipe[1]);
+    close(secondpipe[0]);
+}
+
+// Tests if existing external socket can be deleted and that such deletion does
+// not affect any other existing sockets.
+TEST_F(IfaceMgrTest, DeleteControlSessions) {
+
+    callback_ok = false;
+    callback2_ok = false;
+
+    scoped_ptr<NakedIfaceMgr> ifacemgr(new NakedIfaceMgr());
+
+    // Create first pipe and register it as extra socket
+    int pipefd[2];
+    EXPECT_TRUE(pipe(pipefd) == 0);
+    EXPECT_NO_THROW(ifacemgr->addExternalSocket(pipefd[0], my_callback));
+
+    // Let's create a second pipe and register it as well
+    int secondpipe[2];
+    EXPECT_TRUE(pipe(secondpipe) == 0);
+    EXPECT_NO_THROW(ifacemgr->addExternalSocket(secondpipe[0], my_callback2));
+
+    // Now delete the first session socket
+    EXPECT_NO_THROW(ifacemgr->deleteExternalSocket(pipefd[0]));
+
+    // Now check whether the second callback is still functional
+    EXPECT_EQ(38, write(secondpipe[1], "Hi, this is a message sent over a pipe", 38));
+
+    // ... and repeat
+    Pkt4Ptr pkt4;
+    ASSERT_NO_THROW(pkt4 = ifacemgr->receive4(1));
+
+    // IfaceMgr should not process control socket data as incoming packets
+    EXPECT_FALSE(pkt4);
+
+    // There was some data, so this time callback should be called
+    EXPECT_FALSE(callback_ok);
+    EXPECT_TRUE(callback2_ok);
+
+    // close both pipe ends
+    close(pipefd[1]);
+    close(pipefd[0]);
+
+    close(secondpipe[1]);
+    close(secondpipe[0]);
+}
+
+
 // Test checks if the unicast sockets can be opened.
 // This test is now disabled, because there is no reliable way to test it. We
 // can't even use loopback, beacuse openSockets() skips loopback interface