From 6c019d93aa3b35675414b12322357e5972f809bf Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Wed, 29 Oct 2025 16:16:49 +0200 Subject: [PATCH] [#4141] implemented select event handler --- .../dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 2 - .../tests/http_control_socket_unittest.cc | 2 - .../libloadtests/close_unittests.cc | 3 +- src/lib/dhcp/fd_event_handler.cc | 2 +- src/lib/dhcp/fd_event_handler.h | 72 ++++---- src/lib/dhcp/fd_event_handler_factory.cc | 21 +++ src/lib/dhcp/fd_event_handler_factory.h | 34 ++++ src/lib/dhcp/iface_mgr.cc | 155 ++++++------------ src/lib/dhcp/iface_mgr.h | 19 ++- src/lib/dhcp/meson.build | 2 + src/lib/dhcp/pkt4.h | 2 +- src/lib/dhcp/select_event_handler.cc | 21 ++- src/lib/dhcp/select_event_handler.h | 66 ++++---- src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc | 2 - src/lib/dhcpsrv/cfg_iface.cc | 2 + src/lib/dhcpsrv/tests/d2_udp_unittest.cc | 1 - src/lib/testutils/log_utils.h | 4 - src/lib/util/ready_check.cc | 3 +- src/lib/util/watch_socket.cc | 1 - 19 files changed, 214 insertions(+), 200 deletions(-) create mode 100644 src/lib/dhcp/fd_event_handler_factory.cc create mode 100644 src/lib/dhcp/fd_event_handler_factory.h diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 1334052140..59922cad0b 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -41,9 +41,7 @@ #include #include -#include #include -#include #include #include diff --git a/src/bin/dhcp6/tests/http_control_socket_unittest.cc b/src/bin/dhcp6/tests/http_control_socket_unittest.cc index 7124a04e13..7684a69469 100644 --- a/src/bin/dhcp6/tests/http_control_socket_unittest.cc +++ b/src/bin/dhcp6/tests/http_control_socket_unittest.cc @@ -43,9 +43,7 @@ #include #include -#include #include -#include #include #include diff --git a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc index 8db3352151..cfed042a37 100644 --- a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc +++ b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc @@ -255,7 +255,8 @@ CloseHATest::runPartners(bool const backup /* = true */) { } struct timeval tm; tm.tv_sec = tm.tv_usec = 0; - // r4z: should move to epoll/kqueue? + // @todo implement this using SelectEventHandler + // @todo: should move to epoll/kqueue? int n = select(nfd + 1, &fds, 0, 0, &tm); if ((n < 0) && (errno == EINTR)) { cerr << "interrupted" << endl; diff --git a/src/lib/dhcp/fd_event_handler.cc b/src/lib/dhcp/fd_event_handler.cc index 11ad33588d..ddd324e777 100644 --- a/src/lib/dhcp/fd_event_handler.cc +++ b/src/lib/dhcp/fd_event_handler.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2025 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2025 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 diff --git a/src/lib/dhcp/fd_event_handler.h b/src/lib/dhcp/fd_event_handler.h index dd54240cc2..a5f5ed85f9 100644 --- a/src/lib/dhcp/fd_event_handler.h +++ b/src/lib/dhcp/fd_event_handler.h @@ -1,4 +1,4 @@ -// Copyright (C) 2010-2024 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2025 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 @@ -21,56 +21,56 @@ public: TYPE_UNKNOWN = 0, TYPE_SELECT = 1, TYPE_EPOLL = 2, // Linux OS (Linux like OS) only - TYPE_KQUEUE = 3, // BSD (BSD like OS) only + TYPE_KQUEUE = 3, // BSD OS (BSD like OS) only }; - // @brief Constructor. - // - // @param type The file descriptor event handler type. + /// @brief Constructor. + /// + /// @param type The file descriptor event handler type. FDEventHandler(HandlerType type = TYPE_UNKNOWN); - // @brief Destructor. + /// @brief Destructor. virtual ~FDEventHandler() = default; - // @brief Add file descriptor to watch for events. - // - // @param fd The file descriptor. - // @param read The flag indicating if the file descriptor should be - // registered for read ready events. - // @param write The flag indicating if the file descriptor should be - // registered for write ready events. - virtual void addFD(int fd, bool read = true, bool write = false) = 0; - - // @brief Wait for events on registered file descriptors. - // - // @param timeout_sec The wait timeout in seconds. - // @param timeout_usec The wait timeout in micro seconds - // @return -1 on error, 0 if no data is available (timeout expired), - // 1 if data is ready. + /// @brief Add file descriptor to watch for events. + /// + /// @param fd The file descriptor. + /// @param read The flag indicating if the file descriptor should be + /// registered for read ready events. + /// @param write The flag indicating if the file descriptor should be + /// registered for write ready events. + virtual void add(int fd, bool read = true, bool write = false) = 0; + + /// @brief Wait for events on registered file descriptors. + /// + /// @param timeout_sec The wait timeout in seconds. + /// @param timeout_usec The wait timeout in micro seconds + /// @return -1 on error, 0 if no data is available (timeout expired), + /// 1 if data is ready. virtual int waitEvent(uint32_t timeout_sec, uint32_t timeout_usec = 0) = 0; - // @brief Check if file descriptor is ready for read operation. - // - // @param fd The file descriptor. - // - // @return True if file descriptor is ready for reading. - virtual bool readReadyFD(int fd) = 0; - - // @brief Check if file descriptor is ready for write operation. - // - // @param fd The file descriptor. - // - // @return True if file descriptor is ready for writing. + /// @brief Check if file descriptor is ready for read operation. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor is ready for reading. + virtual bool readReady(int fd) = 0; + + /// @brief Check if file descriptor is ready for write operation. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor is ready for writing. virtual bool writeReady(int fd) = 0; - // @brief Clear registered file descriptors. + /// @brief Clear registered file descriptors. virtual void clear() = 0; - // @brief Return the event handler type. + /// @brief Return the event handler type. HandlerType type(); private: - // @brief The event handler type. + /// @brief The event handler type. HandlerType type_; }; diff --git a/src/lib/dhcp/fd_event_handler_factory.cc b/src/lib/dhcp/fd_event_handler_factory.cc new file mode 100644 index 0000000000..ec99519f62 --- /dev/null +++ b/src/lib/dhcp/fd_event_handler_factory.cc @@ -0,0 +1,21 @@ +// Copyright (C) 2025 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/. + +#include + +#include +#include + +namespace isc { +namespace dhcp { + +FDEventHandlerPtr FDEventHandlerFactory::factoryFDEventHandler() { + // todo: use configuration to initialize the FDEventHandler. + return (FDEventHandlerPtr(new SelectEventHandler())); +} + +} // end of namespace isc::dhcp +} // end of namespace isc diff --git a/src/lib/dhcp/fd_event_handler_factory.h b/src/lib/dhcp/fd_event_handler_factory.h new file mode 100644 index 0000000000..567cbbbd68 --- /dev/null +++ b/src/lib/dhcp/fd_event_handler_factory.h @@ -0,0 +1,34 @@ +// Copyright (C) 2025 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 FD_EVENT_HANDLER_FACTORY_H +#define FD_EVENT_HANDLER_FACTORY_H + +#include + +namespace isc { +namespace dhcp { + +/// @brief File descriptor event handler factory class handles the creation of +/// the FDEventHangler instance according to configuration and OS supported +/// syscalls. +class FDEventHandlerFactory { +public: + + /// @brief Constructor. + FDEventHandlerFactory() = default; + + /// @brief Destructor. + virtual ~FDEventHandlerFactory() = default; + + /// @brief Return an FDEventhandler. + static FDEventHandlerPtr factoryFDEventHandler(); +}; + +} // namespace isc::dhcp +} // namespace isc + +#endif // FD_EVENT_HANDLER_FACTORY_H diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 5c9432ff9e..351c29229d 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -30,15 +31,6 @@ #include #include #include -#include -#include - -#ifndef FD_COPY -#define FD_COPY(orig, copy) \ - do { \ - memmove(copy, orig, sizeof(fd_set)); \ - } while (0) -#endif using namespace std; using namespace isc::asiolink; @@ -208,6 +200,13 @@ IfaceMgr::IfaceMgr() } catch (const std::exception& ex) { isc_throw(IfaceDetectError, ex.what()); } + + initializeFDEventHandler(); +} + +void IfaceMgr::initializeFDEventHandler() { + fd_event_handler_ = FDEventHandlerFactory::factoryFDEventHandler(); + receiver_fd_event_handler_ = FDEventHandlerFactory::factoryFDEventHandler(); } void Iface::addUnicast(const isc::asiolink::IOAddress& addr) { @@ -1137,10 +1136,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / " one million microseconds"); } - fd_set sockets; - int maxfd = 0; - - FD_ZERO(&sockets); + fd_event_handler_->clear(); // if there are any callbacks for external sockets registered... { @@ -1148,16 +1144,16 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / if (!callbacks_.empty()) { for (const SocketCallbackInfo& s : callbacks_) { // Add this socket to listening set - addFDtoSet(s.socket_, maxfd, &sockets); + fd_event_handler_->add(s.socket_); } } } // Add Receiver ready watch socket - addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::READY), maxfd, &sockets); + fd_event_handler_->add(dhcp_receiver_->getWatchFd(WatchedThread::READY)); // Add Receiver error watch socket - addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::ERROR), maxfd, &sockets); + fd_event_handler_->add(dhcp_receiver_->getWatchFd(WatchedThread::ERROR)); // Set timeout for our next select() call. If there are // no DHCP packets to read, then we'll wait for a finite @@ -1165,19 +1161,15 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / // poll (timeout = 0 secs). We need to poll, even if // DHCP packets are waiting so we don't starve external // sockets under heavy DHCP load. - struct timeval select_timeout; - if (getPacketQueue4()->empty()) { - select_timeout.tv_sec = timeout_sec; - select_timeout.tv_usec = timeout_usec; - } else { - select_timeout.tv_sec = 0; - select_timeout.tv_usec = 0; + if (!getPacketQueue4()->empty()) { + timeout_sec = 0; + timeout_usec = 0; } // zero out the errno to be safe errno = 0; - int result = select(maxfd + 1, &sockets, 0, 0, &select_timeout); + int result = fd_event_handler_->waitEvent(timeout_sec, timeout_usec); if ((result == 0) && getPacketQueue4()->empty()) { // nothing received and timeout has been reached @@ -1217,7 +1209,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / { std::lock_guard lock(callbacks_mutex_); for (const SocketCallbackInfo& s : callbacks_) { - if (!FD_ISSET(s.socket_, &sockets)) { + if (!fd_event_handler_->readReady(s.socket_)) { continue; } found = true; @@ -1258,11 +1250,10 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* isc_throw(BadValue, "fractional timeout must be shorter than" " one million microseconds"); } + boost::scoped_ptr candidate; - fd_set sockets; - int maxfd = 0; - FD_ZERO(&sockets); + fd_event_handler_->clear(); /// @todo: marginal performance optimization. We could create the set once /// and then use its copy for select(). Please note that select() modifies @@ -1272,7 +1263,7 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* // Only deal with IPv4 addresses. if (s.addr_.isV4()) { // Add this socket to listening set - addFDtoSet(s.sockfd_, maxfd, &sockets); + fd_event_handler_->add(s.sockfd_); } } } @@ -1283,19 +1274,15 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* if (!callbacks_.empty()) { for (const SocketCallbackInfo& s : callbacks_) { // Add this socket to listening set - addFDtoSet(s.socket_, maxfd, &sockets); + fd_event_handler_->add(s.socket_); } } } - struct timeval select_timeout; - select_timeout.tv_sec = timeout_sec; - select_timeout.tv_usec = timeout_usec; - // zero out the errno to be safe errno = 0; - int result = select(maxfd + 1, &sockets, 0, 0, &select_timeout); + int result = fd_event_handler_->waitEvent(timeout_sec, timeout_usec); if (result == 0) { // nothing received and timeout has been reached @@ -1327,7 +1314,7 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* { std::lock_guard lock(callbacks_mutex_); for (const SocketCallbackInfo& s : callbacks_) { - if (!FD_ISSET(s.socket_, &sockets)) { + if (!fd_event_handler_->readReady(s.socket_)) { continue; } found = true; @@ -1356,7 +1343,7 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* IfacePtr recv_if; for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { - if (FD_ISSET(s.sockfd_, &sockets)) { + if (fd_event_handler_->readReady(s.sockfd_)) { candidate.reset(new SocketInfo(s)); break; } @@ -1385,22 +1372,6 @@ IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) { return (receive6Direct(timeout_sec, timeout_usec)); } -void -IfaceMgr::addFDtoSet(int fd, int& maxfd, fd_set* sockets) { - if (!sockets) { - isc_throw(BadValue, "addFDtoSet: sockets can't be null"); - } - - if (fd >= FD_SETSIZE) { - isc_throw(BadValue, "addFDtoSet: sockets fd too large: " << fd << " >= " << FD_SETSIZE); - } - - FD_SET(fd, sockets); - if (maxfd < fd) { - maxfd = fd; - } -} - Pkt6Ptr IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) { // Sanity check for microsecond timeout. @@ -1410,10 +1381,8 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) } boost::scoped_ptr candidate; - fd_set sockets; - int maxfd = 0; - FD_ZERO(&sockets); + fd_event_handler_->clear(); /// @todo: marginal performance optimization. We could create the set once /// and then use its copy for select(). Please note that select() modifies @@ -1423,7 +1392,7 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) // Only deal with IPv6 addresses. if (s.addr_.isV6()) { // Add this socket to listening set - addFDtoSet(s.sockfd_, maxfd, &sockets); + fd_event_handler_->add(s.sockfd_); } } } @@ -1434,19 +1403,15 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) if (!callbacks_.empty()) { for (const SocketCallbackInfo& s : callbacks_) { // Add this socket to listening set - addFDtoSet(s.socket_, maxfd, &sockets); + fd_event_handler_->add(s.socket_); } } } - struct timeval select_timeout; - select_timeout.tv_sec = timeout_sec; - select_timeout.tv_usec = timeout_usec; - // zero out the errno to be safe errno = 0; - int result = select(maxfd + 1, &sockets, 0, 0, &select_timeout); + int result = fd_event_handler_->waitEvent(timeout_sec, timeout_usec); if (result == 0) { // nothing received and timeout has been reached @@ -1478,7 +1443,7 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) { std::lock_guard lock(callbacks_mutex_); for (const SocketCallbackInfo& s : callbacks_) { - if (!FD_ISSET(s.socket_, &sockets)) { + if (!fd_event_handler_->readReady(s.socket_)) { continue; } found = true; @@ -1506,7 +1471,7 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) // Let's find out which interface/socket has the data for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { - if (FD_ISSET(s.sockfd_, &sockets)) { + if (fd_event_handler_->readReady(s.sockfd_)) { candidate.reset(new SocketInfo(s)); break; } @@ -1531,10 +1496,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ " one million microseconds"); } - fd_set sockets; - int maxfd = 0; - - FD_ZERO(&sockets); + fd_event_handler_->clear(); // if there are any callbacks for external sockets registered... { @@ -1542,16 +1504,16 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ if (!callbacks_.empty()) { for (const SocketCallbackInfo& s : callbacks_) { // Add this socket to listening set - addFDtoSet(s.socket_, maxfd, &sockets); + fd_event_handler_->add(s.socket_); } } } // Add Receiver ready watch socket - addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::READY), maxfd, &sockets); + fd_event_handler_->add(dhcp_receiver_->getWatchFd(WatchedThread::READY)); // Add Receiver error watch socket - addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::ERROR), maxfd, &sockets); + fd_event_handler_->add(dhcp_receiver_->getWatchFd(WatchedThread::ERROR)); // Set timeout for our next select() call. If there are // no DHCP packets to read, then we'll wait for a finite @@ -1559,19 +1521,15 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ // poll (timeout = 0 secs). We need to poll, even if // DHCP packets are waiting so we don't starve external // sockets under heavy DHCP load. - struct timeval select_timeout; - if (getPacketQueue6()->empty()) { - select_timeout.tv_sec = timeout_sec; - select_timeout.tv_usec = timeout_usec; - } else { - select_timeout.tv_sec = 0; - select_timeout.tv_usec = 0; + if (!getPacketQueue6()->empty()) { + timeout_sec = 0; + timeout_usec = 0; } // zero out the errno to be safe errno = 0; - int result = select(maxfd + 1, &sockets, 0, 0, &select_timeout); + int result = fd_event_handler_->waitEvent(timeout_sec, timeout_usec); if ((result == 0) && getPacketQueue6()->empty()) { // nothing received and timeout has been reached @@ -1611,7 +1569,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ { std::lock_guard lock(callbacks_mutex_); for (const SocketCallbackInfo& s : callbacks_) { - if (!FD_ISSET(s.socket_, &sockets)) { + if (!fd_event_handler_->readReady(s.socket_)) { continue; } found = true; @@ -1648,13 +1606,10 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ void IfaceMgr::receiveDHCP4Packets() { - fd_set sockets; - int maxfd = 0; - - FD_ZERO(&sockets); + receiver_fd_event_handler_->clear(); // Add terminate watch socket. - addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::TERMINATE), maxfd, &sockets); + receiver_fd_event_handler_->add(dhcp_receiver_->getWatchFd(WatchedThread::TERMINATE)); // Add Interface sockets. for (const IfacePtr& iface : ifaces_) { @@ -1662,7 +1617,7 @@ IfaceMgr::receiveDHCP4Packets() { // Only deal with IPv4 addresses. if (s.addr_.isV4()) { // Add this socket to listening set. - addFDtoSet(s.sockfd_, maxfd, &sockets); + receiver_fd_event_handler_->add(s.sockfd_); } } } @@ -1673,14 +1628,11 @@ IfaceMgr::receiveDHCP4Packets() { return; } - fd_set rd_set; - FD_COPY(&sockets, &rd_set); - // zero out the errno to be safe. errno = 0; // Select with null timeouts to wait indefinitely an event - int result = select(maxfd + 1, &rd_set, 0, 0, 0); + int result = receiver_fd_event_handler_->waitEvent(0, 0); // Re-check the watch socket. if (dhcp_receiver_->shouldTerminate()) { @@ -1707,7 +1659,7 @@ IfaceMgr::receiveDHCP4Packets() { // Let's find out which interface/socket has data. for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { - if (FD_ISSET(s.sockfd_, &sockets)) { + if (receiver_fd_event_handler_->readReady(s.sockfd_)) { receiveDHCP4Packet(*iface, s); // Can take time so check one more time the watch socket. if (dhcp_receiver_->shouldTerminate()) { @@ -1717,18 +1669,14 @@ IfaceMgr::receiveDHCP4Packets() { } } } - } void IfaceMgr::receiveDHCP6Packets() { - fd_set sockets; - int maxfd = 0; - - FD_ZERO(&sockets); + receiver_fd_event_handler_->clear(); // Add terminate watch socket. - addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::TERMINATE), maxfd, &sockets); + receiver_fd_event_handler_->add(dhcp_receiver_->getWatchFd(WatchedThread::TERMINATE)); // Add Interface sockets. for (const IfacePtr& iface : ifaces_) { @@ -1736,7 +1684,7 @@ IfaceMgr::receiveDHCP6Packets() { // Only deal with IPv6 addresses. if (s.addr_.isV6()) { // Add this socket to listening set. - addFDtoSet(s.sockfd_ , maxfd, &sockets); + receiver_fd_event_handler_->add(s.sockfd_); } } } @@ -1747,14 +1695,11 @@ IfaceMgr::receiveDHCP6Packets() { return; } - fd_set rd_set; - FD_COPY(&sockets, &rd_set); - // zero out the errno to be safe. errno = 0; - // Note we wait until something happen. - int result = select(maxfd + 1, &rd_set, 0, 0, 0); + // Select with null timeouts to wait indefinitely an event + int result = receiver_fd_event_handler_->waitEvent(0, 0); // Re-check the watch socket. if (dhcp_receiver_->shouldTerminate()) { @@ -1780,7 +1725,7 @@ IfaceMgr::receiveDHCP6Packets() { // Let's find out which interface/socket has data. for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { - if (FD_ISSET(s.sockfd_, &sockets)) { + if (receiver_fd_event_handler_->readReady(s.sockfd_)) { receiveDHCP6Packet(s); // Can take time so check one more time the watch socket. if (dhcp_receiver_->shouldTerminate()) { diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 6d043d343c..4db709a721 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -713,6 +714,9 @@ public: /// Closes open sockets. virtual ~IfaceMgr(); + /// @brief Initialize the FD event handler; + void initializeFDEventHandler(); + /// @brief Sets or clears the test mode for @c IfaceMgr. /// /// Various unit test may set this flag to true, to indicate that the @@ -1343,15 +1347,6 @@ public: bool configureDHCPPacketQueue(const uint16_t family, data::ConstElementPtr queue_control); - /// @brief Convenience method for adding an descriptor to a set - /// - /// @param fd descriptor to add - /// @param[out] maxfd maximum fd value in the set. If the new fd is - /// larger than it's current value, it will be updated to new fd value - /// @param sockets pointer to the set of sockets - /// @throw BadValue if sockets is null - static void addFDtoSet(int fd, int& maxfd, fd_set* sockets); - // don't use private, we need derived classes in tests protected: @@ -1633,6 +1628,12 @@ private: /// @brief DHCP packet receiver. isc::util::WatchedThreadPtr dhcp_receiver_; + + /// @brief The FDEventHandler instance. + FDEventHandlerPtr fd_event_handler_; + + /// @brief The receiver FDEventHandler instance. + FDEventHandlerPtr receiver_fd_event_handler_; }; } // namespace isc::dhcp diff --git a/src/lib/dhcp/meson.build b/src/lib/dhcp/meson.build index 7133b3f6e3..306cac9a52 100644 --- a/src/lib/dhcp/meson.build +++ b/src/lib/dhcp/meson.build @@ -16,6 +16,7 @@ kea_dhcp_lib = shared_library( 'duid.cc', 'duid_factory.cc', 'fd_event_handler.cc', + 'fd_event_handler_factory.cc', 'hwaddr.cc', 'iface_mgr.cc', iface_mgr, @@ -75,6 +76,7 @@ kea_dhcp_headers = [ 'duid.h', 'duid_factory.h', 'fd_event_handler.h', + 'fd_event_handler_factory.h', 'hwaddr.h', 'iface_mgr.h', 'iface_mgr_error_handler.h', diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h index 0b555706e1..3f2ab84962 100644 --- a/src/lib/dhcp/pkt4.h +++ b/src/lib/dhcp/pkt4.h @@ -504,7 +504,7 @@ protected: /// @brief local HW address (dst if receiving packet, src if sending packet) HWAddrPtr local_hwaddr_; - // @brief List of deferred option codes + /// @brief List of deferred option codes std::list deferred_options_; /// @brief message operation code diff --git a/src/lib/dhcp/select_event_handler.cc b/src/lib/dhcp/select_event_handler.cc index 41b8234c76..1810834e84 100644 --- a/src/lib/dhcp/select_event_handler.cc +++ b/src/lib/dhcp/select_event_handler.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2025 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2025 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 @@ -9,6 +9,13 @@ #include #include +#ifndef FD_COPY +#define FD_COPY(orig, copy) \ + do { \ + memcpy(copy, orig, sizeof(fd_set)); \ + } while (0) +#endif + namespace isc { namespace dhcp { @@ -31,6 +38,9 @@ void SelectEventHandler::add(int fd, bool read /* = true */, bool write /* = fal // Add this socket to write set FD_SET(fd, &write_fd_set_); } + if (fd > max_fd_) { + max_fd_ = fd; + } } // @brief Wait for events on registered file descriptors. @@ -44,7 +54,10 @@ int SelectEventHandler::waitEvent(uint32_t timeout_sec, uint32_t timeout_usec /* select_timeout.tv_sec = timeout_sec; select_timeout.tv_usec = timeout_usec; - return (select(max_fd_ + 1, &read_fd_set_, &write_fd_set_, 0, &select_timeout)); + FD_COPY(&read_fd_set_, &ready_read_fd_set_); + FD_COPY(&write_fd_set_, &ready_write_fd_set_); + + return (select(max_fd_ + 1, &ready_read_fd_set_, &ready_write_fd_set_, 0, &select_timeout)); } // @brief Check if file descriptor is ready for read operation. @@ -53,7 +66,7 @@ int SelectEventHandler::waitEvent(uint32_t timeout_sec, uint32_t timeout_usec /* // // @return True if file descriptor is ready for reading. bool SelectEventHandler::readReady(int fd) { - return (FD_ISSET(fd, &read_fd_set_)); + return (FD_ISSET(fd, &ready_read_fd_set_)); } // @brief Check if file descriptor is ready for write operation. @@ -62,7 +75,7 @@ bool SelectEventHandler::readReady(int fd) { // // @return True if file descriptor is ready for writing. bool SelectEventHandler::writeReady(int fd) { - return (FD_ISSET(fd, &write_fd_set_)); + return (FD_ISSET(fd, &ready_write_fd_set_)); } void SelectEventHandler::clear() { diff --git a/src/lib/dhcp/select_event_handler.h b/src/lib/dhcp/select_event_handler.h index 2a5cd064df..ab51510d43 100644 --- a/src/lib/dhcp/select_event_handler.h +++ b/src/lib/dhcp/select_event_handler.h @@ -1,4 +1,4 @@ -// Copyright (C) 2010-2024 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2025 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 @@ -18,55 +18,61 @@ namespace dhcp { /// file descriptors. This class uses the OS select syscall for event handling. class SelectEventHandler : public FDEventHandler { public: - // @brief Constructor. + /// @brief Constructor. SelectEventHandler(); - // @brief Destructor. + /// @brief Destructor. virtual ~SelectEventHandler() = default; - // @brief Add file descriptor to watch for events. - // - // @param fd The file descriptor. - // @param read The flag indicating if the file descriptor should be - // registered for read ready events. - // @param write The flag indicating if the file descriptor should be - // registered for write ready events. + /// @brief Add file descriptor to watch for events. + /// + /// @param fd The file descriptor. + /// @param read The flag indicating if the file descriptor should be + /// registered for read ready events. + /// @param write The flag indicating if the file descriptor should be + /// registered for write ready events. void add(int fd, bool read = true, bool write = false); - // @brief Wait for events on registered file descriptors. - // - // @param timeout_sec The wait timeout in seconds. - // @param timeout_usec The wait timeout in micro seconds - // @return -1 on error, 0 if no data is available (timeout expired), - // 1 if data is ready. + /// @brief Wait for events on registered file descriptors. + /// + /// @param timeout_sec The wait timeout in seconds. + /// @param timeout_usec The wait timeout in micro seconds + /// @return -1 on error, 0 if no data is available (timeout expired), + /// 1 if data is ready. int waitEvent(uint32_t timeout_sec, uint32_t timeout_usec = 0); - // @brief Check if file descriptor is ready for read operation. - // - // @param fd The file descriptor. - // - // @return True if file descriptor is ready for reading. + /// @brief Check if file descriptor is ready for read operation. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor is ready for reading. bool readReady(int fd); - // @brief Check if file descriptor is ready for write operation. - // - // @param fd The file descriptor. - // - // @return True if file descriptor is ready for writing. + /// @brief Check if file descriptor is ready for write operation. + /// + /// @param fd The file descriptor. + /// + /// @return True if file descriptor is ready for writing. bool writeReady(int fd); - // @brief Clear registered file descriptors. + /// @brief Clear registered file descriptors. void clear(); private: - // @brief The maximum value of registered file descriptors. + /// @brief The maximum value of registered file descriptors. int max_fd_; - // @brief The read event FD set. + /// @brief The read event FD set. fd_set read_fd_set_; - // @brief The write event FD set. + /// @brief The write event FD set. fd_set write_fd_set_; + + /// @brief The read event FD set. + fd_set ready_read_fd_set_; + + /// @brief The write event FD set. + fd_set ready_write_fd_set_; }; } // namespace isc::dhcp diff --git a/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc b/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc index a1d2b54266..927c651004 100644 --- a/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc +++ b/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc @@ -19,8 +19,6 @@ #include #include -#include - using namespace std; using namespace isc; using namespace isc::asiolink; diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc index 99c40c882b..e9a77d69cd 100644 --- a/src/lib/dhcpsrv/cfg_iface.cc +++ b/src/lib/dhcpsrv/cfg_iface.cc @@ -92,6 +92,8 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port, IfaceMgr& iface_mgr = IfaceMgr::instance(); // Remove selection of unicast addresses from all interfaces. iface_mgr.clearUnicasts(); + // Initialize IfaceMgr FDEventHandler. + iface_mgr.initializeFDEventHandler(); // Allow the loopback interface when required. iface_mgr.setAllowLoopBack(loopback_used_); // For the DHCPv4 server, if the user has selected that raw sockets diff --git a/src/lib/dhcpsrv/tests/d2_udp_unittest.cc b/src/lib/dhcpsrv/tests/d2_udp_unittest.cc index 5a32279842..a3982493dd 100644 --- a/src/lib/dhcpsrv/tests/d2_udp_unittest.cc +++ b/src/lib/dhcpsrv/tests/d2_udp_unittest.cc @@ -19,7 +19,6 @@ #include #include -#include using namespace std; using namespace isc::asiolink; diff --git a/src/lib/testutils/log_utils.h b/src/lib/testutils/log_utils.h index 4aeb59d13a..9bc430b0f1 100644 --- a/src/lib/testutils/log_utils.h +++ b/src/lib/testutils/log_utils.h @@ -10,14 +10,10 @@ #include #include -//#include - #include #include #include -//#include -//#include #include using namespace std; diff --git a/src/lib/util/ready_check.cc b/src/lib/util/ready_check.cc index fd3c08c530..1e7aeca8b2 100644 --- a/src/lib/util/ready_check.cc +++ b/src/lib/util/ready_check.cc @@ -7,13 +7,14 @@ #include #include -#include namespace isc { namespace util { int selectCheck(const int fd_to_check, const unsigned int timeout_sec, bool read_check, bool write_check) { + // @todo implement this using SelectEventHandler + // @todo: should move to epoll/kqueue? if (fd_to_check < 0) { return (-1); } diff --git a/src/lib/util/watch_socket.cc b/src/lib/util/watch_socket.cc index 6ffb396083..6d69e54f3c 100644 --- a/src/lib/util/watch_socket.cc +++ b/src/lib/util/watch_socket.cc @@ -8,7 +8,6 @@ #include -//#include #include #include -- 2.47.3