From: Slawek Figiel Date: Thu, 24 Mar 2022 18:27:07 +0000 (+0100) Subject: [#1716] Refactor retry callback X-Git-Tag: Kea-2.1.5~91 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=83dae641659ff4927e25f46c4e00fc9bc861a273;p=thirdparty%2Fkea.git [#1716] Refactor retry callback --- diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 7e00a3608e..3ba8056cbb 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -586,43 +587,25 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast, continue; } - bool is_opened = false; - bool should_retry = false; - uint16_t attempt = 0; - do - { - try { - // We haven't open any broadcast sockets yet, so we can - // open at least one more or - // not broadcast capable, do not set broadcast flags. - openSocket(iface->getName(), addr.get(), port, is_open_as_broadcast, is_open_as_broadcast); - is_opened = true; - } catch (const Exception& ex) { - auto err_msg = (std::stringstream("failed to open socket on interface ") - << iface->getName() << ", reason: " - << ex.what() - ).str(); - - uint16_t wait_time = 0; - if (retry_callback != nullptr) { - // Callback produces a log message - const auto& pair = retry_callback(attempt++, err_msg); - should_retry = pair.first; - wait_time = pair.second; - } - if (!should_retry) { - IFACEMGR_ERROR(SocketConfigError, error_handler, err_msg); - } else { - // Wait before next attempt. The initialization cannot end before - // opening a socket so we can wait in the foreground. - std::this_thread::sleep_for(std::chrono::milliseconds(wait_time)); - } - } - } - while(!is_opened && should_retry); - - if (!is_opened) { - continue; + auto msg_stream = std::stringstream("failed to open socket on interface ") + << iface->getName(); + + try { + // We haven't open any broadcast sockets yet, so we can + // open at least one more or + // not broadcast capable, do not set broadcast flags. + callWithRetry( + std::bind(&IfaceMgr::openSocket, this, + iface->getName(), addr.get(), port, + is_open_as_broadcast, is_open_as_broadcast + ), + msg_stream.str(), retry_callback + ); + } catch (const Exception& ex) { + msg_stream << ", reason: " + << ex.what(); + IFACEMGR_ERROR(SocketConfigError, error_handler, msg_stream.str()); + continue; } if (is_open_as_broadcast) { @@ -688,38 +671,21 @@ IfaceMgr::openSockets6(const uint16_t port, // Open unicast sockets if there are any unicast addresses defined for (Iface::Address addr : iface->getUnicasts()) { - bool is_opened = false; - bool should_retry = false; - uint16_t attempt = 0; - - do { - try { - openSocket(iface->getName(), addr, port); - } catch (const Exception& ex) { - auto err_msg = (std::stringstream("failed to open unicast socket on interface ") - << iface->getName() << ", reason: " - << ex.what()).str(); - - uint16_t wait_time = 0; - if (retry_callback != nullptr) { - // Callback produces a log message - const auto& pair = retry_callback(attempt++, err_msg); - should_retry = pair.first; - wait_time = pair.second; - } - - if (!should_retry) { - IFACEMGR_ERROR(SocketConfigError, error_handler, err_msg); - } else { - // Wait before next attempt. The initialization cannot end before - // opening a socket so we can wait in the foreground. - std::this_thread::sleep_for(std::chrono::milliseconds(wait_time)); - } - } - } - while (!is_opened && should_retry); - - if (!is_opened) { + auto msg_stream = std::stringstream("failed to open unicast socket on interface ") + << iface->getName(); + + try { + callWithRetry( + std::bind(&IfaceMgr::openSocket, this, + iface->getName(), addr, port, false, false + ), + msg_stream.str(), + retry_callback + ); + } catch (const Exception& ex) { + msg_stream << ", reason: " + << ex.what(); + IFACEMGR_ERROR(SocketConfigError, error_handler, msg_stream.str()); continue; } @@ -745,10 +711,23 @@ IfaceMgr::openSockets6(const uint16_t port, // Run OS-specific function to open a socket capable of receiving // packets sent to All_DHCP_Relay_Agents_and_Servers multicast // address. - if (openMulticastSocket(*iface, addr, port, error_handler)) { + auto msg_stream = std::stringstream("failed to open multicast socket on interface ") + << iface->getName(); + + try { + callWithRetry( + std::bind(&IfaceMgr::openMulticastSocket, this, + // Pass a null pointer as an error handler to avoid + // suppressing an exception in a system-specific function. + std::ref(*iface), addr, port, nullptr), + msg_stream.str(), retry_callback + ); ++count; + } catch (const Exception& ex) { + msg_stream << ", reason: " + << ex.what(); + IFACEMGR_ERROR(SocketConfigError, error_handler, msg_stream.str()); } - } } diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 5abf5eff17..4e3f598adb 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -626,11 +626,11 @@ std::function IfaceMgrErrorMsgCallback; /// @brief This type describes the callback function invoked when an opening of /// a socket fails and can be retried. /// -/// @param attempt A number of an opening attempt +/// @param retries A number of an opening retries /// @return true if an opening should be retried, false otherwise, and a wait time /// from the last attempt typedef -std::function(uint16_t attempt, const std::string& msg)> IfaceMgrRetryCallback; +std::function(uint16_t retries, const std::string& msg)> IfaceMgrRetryCallback; /// @brief Handles network interfaces, transmission and reception. /// diff --git a/src/lib/dhcp/iface_mgr_retry_callback.h b/src/lib/dhcp/iface_mgr_retry_callback.h new file mode 100644 index 0000000000..36aed3c030 --- /dev/null +++ b/src/lib/dhcp/iface_mgr_retry_callback.h @@ -0,0 +1,73 @@ +// Copyright (C) 2011-2022 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef IFACE_MGR_RETRY_CALLBACK_H +#define IFACE_MGR_RETRY_CALLBACK_H + +#include +#include + +namespace isc { + +namespace dhcp { + +/// @brief An helper to call a function with retry. +/// +/// There are certain cases when IfaceMgr may hit an error caused by +/// temporary extarnal factors. A typical case is the function which opens +/// sockets on available interfaces for a DHCP server. If this function +/// fails to open a socket on a specific interface (for example, there is +/// another socket already open on this interface and bound to the same address +/// and port), it may be helpful to repeat an opening procedure. +/// It is allowed that the error handler function is not installed (is NULL). +/// In these cases it is expected that the function is just called without retrying. +/// +/// @param f A function to call; template type T is an output type of this function. +/// @param msg A message intended to log with a failed attempt. +/// @param retry_callback A retry callback that decides to continue retries and wait +/// time before next try. It should also log the info/warning message. +template +T callWithRetry(std::function f, + const std::string& msg, + IfaceMgrRetryCallback retry_callback) { + + // If the retry callback is NULL, just call the function and return. + if (retry_callback == nullptr) { + return f(); + } + + // Counter of the retries. + uint16_t retries = 0; + + // Leave the loop on success (return statement) + // or stop retrying (throw statement). + while(true) + { + try { + return f(); + } catch (const Exception& ex) { + auto retry_msg = (std::stringstream(msg) + << ", reason: " + << ex.what()).str(); + // Callback produces a log message + const auto [should_retry, wait_time] = retry_callback(retries++, retry_msg); + + if (!should_retry) { + throw; + } else { + // Wait before next attempt. The initialization cannot end before + // opening a socket so we can wait in the foreground. + std::this_thread::sleep_for(std::chrono::milliseconds(wait_time)); + } + } + } +} + +} + +} + +#endif // IFACE_MGR_RETRY_CALLBACK_H \ No newline at end of file diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc index feb77da57d..9fd457b759 100644 --- a/src/lib/dhcpsrv/cfg_iface.cc +++ b/src/lib/dhcpsrv/cfg_iface.cc @@ -232,11 +232,11 @@ CfgIface::socketOpenErrorHandler(const std::string& errmsg) { } std::pair -CfgIface::socketOpenRetryHandler(uint16_t attempt, const std::string& msg) const { - bool can_retry = attempt < service_sockets_max_retries_; +CfgIface::socketOpenRetryHandler(uint16_t retries, const std::string& msg) const { + bool can_retry = retries < service_sockets_max_retries_; if (can_retry) { std::stringstream msg_stream; - msg_stream << msg << "; retries left: " << service_sockets_max_retries_ - attempt; + msg_stream << msg << "; retries left: " << service_sockets_max_retries_ - retries; LOG_INFO(dhcpsrv_logger, DHCPSRV_OPEN_SOCKET_FAIL).arg(msg_stream.str()); } diff --git a/src/lib/dhcpsrv/cfg_iface.h b/src/lib/dhcpsrv/cfg_iface.h index 0b24de5078..83f7683b1b 100644 --- a/src/lib/dhcpsrv/cfg_iface.h +++ b/src/lib/dhcpsrv/cfg_iface.h @@ -392,10 +392,10 @@ private: /// wait from the last attempt. It allows extending the waiting time dynamically /// with the next tries. /// - /// @param attemp An index of opening attempt + /// @param retries An index of opening retries /// @param msg Message being logged by the function. /// @return true if the opening should be retried and milliseconds to wait from last attempt - std::pair socketOpenRetryHandler(uint16_t attempt, const std::string& msg) const; + std::pair socketOpenRetryHandler(uint16_t retries, const std::string& msg) const; /// @brief Represents a set of interface names. typedef std::set IfaceSet;