From: Marcin Siodelski <marcin@isc.org>
Date: Thu, 14 Jun 2018 00:03:40 +0000 (+0200)
Subject: [5649] IfaceMgr's select reacts on sending data over control channel.
X-Git-Tag: Kea-1.4.0~1^2~3^2~12
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=83f252d8cd26066e181fe3842f3db20e87001a48;p=thirdparty%2Fkea.git

[5649] IfaceMgr's select reacts on sending data over control channel.
---

diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc
index 84cbf1511d..cfef610f16 100644
--- a/src/hooks/dhcp/high_availability/ha_service.cc
+++ b/src/hooks/dhcp/high_availability/ha_service.cc
@@ -33,6 +33,18 @@ using namespace isc::http;
 using namespace isc::log;
 using namespace isc::util;
 
+namespace {
+
+/// @brief Timeout for synchronization of leases with partner.
+///
+/// This timeout is very high because in some cases the number of
+/// gathered leases is huge. Syncing should not really take that
+/// long, but if the partner doesn't respond we can't do anything
+/// useful anyway. So, it doesn't really matter.
+const long HA_SYNC_TIMEOUT = 60000;
+
+}
+
 namespace isc {
 namespace ha {
 
@@ -1215,7 +1227,7 @@ HAService::asyncSyncLeases(http::HttpClient& http_client,
                 post_sync_action(error_message.empty(),
                                  error_message);
             }
-    });
+    }, HttpClient::RequestTimeout(HA_SYNC_TIMEOUT));
 }
 
 ConstElementPtr
diff --git a/src/hooks/dhcp/high_availability/ha_service.h b/src/hooks/dhcp/high_availability/ha_service.h
index bff04ca006..5d6325a120 100644
--- a/src/hooks/dhcp/high_availability/ha_service.h
+++ b/src/hooks/dhcp/high_availability/ha_service.h
@@ -540,7 +540,7 @@ protected:
     ///
     /// If there is an error while inserting or updating any of the leases
     /// a warning message is logged and the process continues for the
-    /// remaining leases.
+    /// remaining leases. The timeout for synchronization is set to 1 minute.
     ///
     /// @param http_client reference to the client to be used to communicate
     /// with the other server.
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 7d06326c0c..15877c1cb2 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -896,9 +896,11 @@ Pkt4Ptr IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     boost::scoped_ptr<SocketInfo> candidate;
     IfacePtr iface;
     fd_set sockets;
+    fd_set write_sockets;
     int maxfd = 0;
 
     FD_ZERO(&sockets);
+    FD_ZERO(&write_sockets);
 
     /// @todo: marginal performance optimization. We could create the set once
     /// and then use its copy for select(). Please note that select() modifies
@@ -922,6 +924,17 @@ Pkt4Ptr IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     if (!callbacks_.empty()) {
         BOOST_FOREACH(SocketCallbackInfo s, callbacks_) {
             FD_SET(s.socket_, &sockets);
+
+            // Sometimes we want to react to socket writes, not only to reads.
+            // However, current use cases are limited to CommandMgr which
+            // doesn't trigger any callbacks. For all use cases that install
+            // callbacks we don't react to socket reads being afraid what we
+            // can break.
+            /// @todo This whole solution is temporary and implemented for
+            /// #5649.
+            if (!s.callback_) {
+                FD_SET(s.socket_, &write_sockets);
+            }
             if (maxfd < s.socket_) {
                 maxfd = s.socket_;
             }
@@ -935,7 +948,7 @@ Pkt4Ptr IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     // zero out the errno to be safe
     errno = 0;
 
-    int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
+    int result = select(maxfd + 1, &sockets, &write_sockets, NULL, &select_timeout);
 
     if (result == 0) {
         // nothing received and timeout has been reached
@@ -1005,9 +1018,11 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
 
     boost::scoped_ptr<SocketInfo> candidate;
     fd_set sockets;
+    fd_set write_sockets;
     int maxfd = 0;
 
     FD_ZERO(&sockets);
+    FD_ZERO(&write_sockets);
 
     /// @todo: marginal performance optimization. We could create the set once
     /// and then use its copy for select(). Please note that select() modifies
@@ -1032,6 +1047,17 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
         BOOST_FOREACH(SocketCallbackInfo s, callbacks_) {
             // Add it to the set as well
             FD_SET(s.socket_, &sockets);
+
+            // Sometimes we want to react to socket writes, not only to reads.
+            // However, current use cases are limited to CommandMgr which
+            // doesn't trigger any callbacks. For all use cases that install
+            // callbacks we don't react to socket reads being afraid what we
+            // can break.
+            /// @todo This whole solution is temporary and implemented for
+            /// #5649.
+            if (!s.callback_) {
+                FD_SET(s.socket_, &write_sockets);
+            }
             if (maxfd < s.socket_) {
                 maxfd = s.socket_;
             }
@@ -1045,7 +1071,7 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     // zero out the errno to be safe
     errno = 0;
 
-    int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
+    int result = select(maxfd + 1, &sockets, &write_sockets, NULL, &select_timeout);
 
     if (result == 0) {
         // nothing received and timeout has been reached