[master] Merge branch 'trac3478'

src/bin/dhcp4/dhcp4_srv.cc

@@ -157,7 +157,15 @@ Dhcpv4Srv::run() {
 
         try {
             query = receivePacket(timeout);
+
+        } catch (const SignalInterruptOnSelect) {
+            // Packet reception interrupted because a signal has been received.
+            // This is not an error because we might have received a SIGTERM,
+            // SIGINT or SIGHUP which are handled by the server. For signals
+            // that are not handled by the server we rely on the default
+            // behavior of the system, but there is nothing we should log here.
         } catch (const std::exception& e) {
+            // Log all other errors.
             LOG_ERROR(dhcp4_logger, DHCP4_PACKET_RECEIVE_FAIL).arg(e.what());
         }
 

src/bin/dhcp4/tests/dhcp4_process_tests.sh.in

@@ -191,6 +191,13 @@ dynamic_reconfiguration_test() {
         clean_exit 1
     fi
 
+    # When the server receives a signal the call to select() function is
+    # interrupted. This should not be logged as an error.
+    get_log_messages "DHCP4_PACKET_RECEIVE_FAIL"
+    assert_eq 0 ${_GET_LOG_MESSAGES} \
+        "Expected get_log_messages DHCP4_PACKET_RECEIVE_FAIL return %d, \
+returned %d."
+
     # All ok. Shut down Kea and exit.
     test_finish 0
 }
@@ -252,6 +259,12 @@ shutdown_test() {
     assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
         "Expected wait_for_server_down return %d, returned %d"
 
+    # When the server receives a signal the call to select() function is
+    # interrupted. This should not be logged as an error.
+    get_log_messages "DHCP4_PACKET_RECEIVE_FAIL"
+    assert_eq 0 ${_GET_LOG_MESSAGES} \
+        "Expected get_log_messages return %d, returned %d."
+
     test_finish 0
 }
 

src/bin/dhcp6/dhcp6_srv.cc

@@ -242,6 +242,13 @@ bool Dhcpv6Srv::run() {
 
         try {
             query = receivePacket(timeout);
+
+        } catch (const SignalInterruptOnSelect) {
+            // Packet reception interrupted because a signal has been received.
+            // This is not an error because we might have received a SIGTERM,
+            // SIGINT or SIGHUP which are handled by the server. For signals
+            // that are not handled by the server we rely on the default
+            // behavior of the system, but there is nothing we should log here.
         } catch (const std::exception& e) {
             LOG_ERROR(dhcp6_logger, DHCP6_PACKET_RECEIVE_FAIL).arg(e.what());
         }

src/bin/dhcp6/tests/dhcp6_process_tests.sh.in

@@ -193,6 +193,13 @@ dynamic_reconfiguration_test() {
         clean_exit 1
     fi
 
+    # When the server receives a signal the call to select() function is
+    # interrupted. This should not be logged as an error.
+    get_log_messages "DHCP6_PACKET_RECEIVE_FAIL"
+    assert_eq 0 ${_GET_LOG_MESSAGES} \
+        "Expected get_log_messages DHCP6_PACKET_RECEIVE_FAIL return %d, \
+returned %d."
+
     # All ok. Shut down Kea and exit.
     test_finish 0
 }
@@ -255,6 +262,13 @@ shutdown_test() {
     assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
         "Expected wait_for_server_down return %d, returned %d"
 
+    # When the server receives a signal the call to select() function is
+    # interrupted. This should not be logged as an error.
+    get_log_messages "DHCP6_PACKET_RECEIVE_FAIL"
+    assert_eq 0 ${_GET_LOG_MESSAGES} \
+        "Expected get_log_messages DHCP6_PACKET_RECEIVE_FAIL return %d, \
+returned %d."
+
     test_finish 0
 }
 

src/lib/dhcp/iface_mgr.cc

@@ -938,8 +938,20 @@ IfaceMgr::receive4(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) {
     if (result == 0) {
         // nothing received and timeout has been reached
         return (Pkt4Ptr()); // NULL
+
     } else if (result < 0) {
-        isc_throw(SocketReadError, strerror(errno));
+        // In most cases we would like to know whether select() returned
+        // an error because of a signal being received  or for some other
+        // reasaon. This is because DHCP servers use signals to trigger
+        // certain actions, like reconfiguration or graceful shutdown.
+        // By catching a dedicated exception the caller will know if the
+        // error returned by the function is due to the reception of the
+        // signal or for some other reason.
+        if (errno == EINTR) {
+            isc_throw(SignalInterruptOnSelect, strerror(errno));
+        } else {
+            isc_throw(SocketReadError, strerror(errno));
+        }
     }
 
     // Let's find out which socket has the data
@@ -1041,8 +1053,20 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     if (result == 0) {
         // nothing received and timeout has been reached
         return (Pkt6Ptr()); // NULL
+
     } else if (result < 0) {
-        isc_throw(SocketReadError, strerror(errno));
+        // In most cases we would like to know whether select() returned
+        // an error because of a signal being received  or for some other
+        // reasaon. This is because DHCP servers use signals to trigger
+        // certain actions, like reconfiguration or graceful shutdown.
+        // By catching a dedicated exception the caller will know if the
+        // error returned by the function is due to the reception of the
+        // signal or for some other reason.
+        if (errno == EINTR) {
+            isc_throw(SignalInterruptOnSelect, strerror(errno));
+        } else {
+            isc_throw(SocketReadError, strerror(errno));
+        }
     }
 
     // Let's find out which socket has the data

src/lib/dhcp/iface_mgr.h

@@ -49,6 +49,13 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+/// @brief Exception thrown when a call to select is interrupted by a signal.
+class SignalInterruptOnSelect : public Exception {
+public:
+    SignalInterruptOnSelect(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 /// @brief IfaceMgr exception thrown thrown when socket opening
 /// or configuration failed.
 class SocketConfigError : public Exception {
@@ -73,6 +80,7 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+
 /// Holds information about socket.
 struct SocketInfo {
 
@@ -598,37 +606,47 @@ public:
     /// @return true if sending was successful
     bool send(const Pkt4Ptr& pkt);
 
-    /// @brief Tries to receive IPv6 packet over open IPv6 sockets.
+    /// @brief Tries to receive DHCPv6 message over open IPv6 sockets.
     ///
-    /// Attempts to receive a single IPv6 packet of any of the open IPv6 sockets.
-    /// If reception is successful and all information about its sender
-    /// are obtained, Pkt6 object is created and returned.
+    /// Attempts to receive a single DHCPv6 message over any of the open IPv6
+    /// sockets. If reception is successful and all information about its
+    /// sender is obtained, Pkt6 object is created and returned.
     ///
-    /// TODO Start using select() and add timeout to be able
-    /// to not wait infinitely, but rather do something useful
-    /// (e.g. remove expired leases)
+    /// This method also checks if data arrived over registered external socket.
+    /// This data may be of a different protocol family than AF_INET6.
     ///
     /// @param timeout_sec specifies integral part of the timeout (in seconds)
     /// @param timeout_usec specifies fractional part of the timeout
     /// (in microseconds)
     ///
     /// @throw isc::BadValue if timeout_usec is greater than one million
-    /// @throw isc::dhcp::SocketReadError if error occured when receiving a packet.
+    /// @throw isc::dhcp::SocketReadError if error occured when receiving a
+    /// packet.
+    /// @throw isc::dhcp::SignalInterruptOnSelect when a call to select() is
+    /// interrupted by a signal.
+    ///
     /// @return Pkt6 object representing received packet (or NULL)
     Pkt6Ptr receive6(uint32_t timeout_sec, uint32_t timeout_usec = 0);
 
     /// @brief Tries to receive IPv4 packet over open IPv4 sockets.
     ///
-    /// Attempts to receive a single IPv4 packet of any of the open IPv4 sockets.
-    /// If reception is successful and all information about its sender
-    /// are obtained, Pkt4 object is created and returned.
+    /// Attempts to receive a single DHCPv4 message over any of the open
+    /// IPv4 sockets. If reception is successful and all information about
+    /// its sender is obtained, Pkt4 object is created and returned.
+    ///
+    /// This method also checks if data arrived over registered external socket.
+    /// This data may be of a different protocol family than AF_INET.
     ///
     /// @param timeout_sec specifies integral part of the timeout (in seconds)
     /// @param timeout_usec specifies fractional part of the timeout
     /// (in microseconds)
     ///
     /// @throw isc::BadValue if timeout_usec is greater than one million
-    /// @throw isc::dhcp::SocketReadError if error occured when receiving a packet.
+    /// @throw isc::dhcp::SocketReadError if error occured when receiving a
+    /// packet.
+    /// @throw isc::dhcp::SignalInterruptOnSelect when a call to select() is
+    /// interrupted by a signal.
+    ///
     /// @return Pkt4 object representing received packet (or NULL)
     Pkt4Ptr receive4(uint32_t timeout_sec, uint32_t timeout_usec = 0);