From 362df32ed8ef709ab604b92426da8bc1ae1ec821 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Wed, 29 Oct 2025 22:01:25 +0200 Subject: [PATCH] [#4141] added unittests --- .../libloadtests/close_unittests.cc | 39 ++--- src/lib/dhcp/iface_mgr.cc | 6 + src/lib/dhcp/select_event_handler.cc | 21 +-- src/lib/dhcp/select_event_handler.h | 6 +- .../fd_event_handler_factory_unittests.cc | 28 ++++ .../dhcp/tests/fd_event_handler_unittests.h | 135 ++++++++++++++++++ src/lib/dhcp/tests/meson.build | 2 + .../tests/select_event_handler_unittests.cc | 12 ++ src/lib/util/ready_check.cc | 2 +- 9 files changed, 204 insertions(+), 47 deletions(-) create mode 100644 src/lib/dhcp/tests/fd_event_handler_factory_unittests.cc create mode 100644 src/lib/dhcp/tests/fd_event_handler_unittests.h create mode 100644 src/lib/dhcp/tests/select_event_handler_unittests.cc diff --git a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc index cfed042a37..741fc9c271 100644 --- a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc +++ b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc @@ -228,44 +228,29 @@ CloseHATest::runPartners(bool const backup /* = true */) { } wthread_->markReady(WatchedThread::READY); + FDEventHandlerPtr ev_handler_ = FDEventHandlerFactory::factoryFDEventHandler(); for (;;) { - int nfd; - fd_set fds; - FD_ZERO(&fds); - FD_SET(wthread_->getWatchFd(WatchedThread::TERMINATE), &fds); - nfd = wthread_->getWatchFd(WatchedThread::TERMINATE); - FD_SET(accept_partner1, &fds); - if (accept_partner1 > nfd) { - nfd = accept_partner1; - } - FD_SET(accept_partner2, &fds); - if (accept_partner2 > nfd) { - nfd = accept_partner2; - } + ev_handler_->clear(); + ev_handler_->add(wthread_->getWatchFd(WatchedThread::TERMINATE)); + ev_handler_->add(accept_partner1); + ev_handler_->add(accept_partner2); for (auto const& reader : readers) { if (!reader.second) { continue; } - int fd = reader.first; - FD_SET(fd, &fds); - if (fd > nfd) { - nfd = fd; - } + ev_handler_->add(reader.first); } - struct timeval tm; - tm.tv_sec = tm.tv_usec = 0; - // @todo implement this using SelectEventHandler - // @todo: should move to epoll/kqueue? - int n = select(nfd + 1, &fds, 0, 0, &tm); + + int n = ev_handler_->waitEvent(0, 0); if ((n < 0) && (errno == EINTR)) { cerr << "interrupted" << endl; continue; } - if (FD_ISSET(wthread_->getWatchFd(WatchedThread::TERMINATE), &fds)) { + if (ev_handler_->readReady(wthread_->getWatchFd(WatchedThread::TERMINATE))) { break; } - if (FD_ISSET(accept_partner1, &fds)) { + if (ev_handler_->readReady(accept_partner1)) { int fd = accept(accept_partner1, 0, 0); if (fd < 0) { cerr << "accept1 failed " << strerror(errno) << endl; @@ -276,7 +261,7 @@ CloseHATest::runPartners(bool const backup /* = true */) { readers[fd] = true; } } - if (FD_ISSET(accept_partner2, &fds)) { + if (ev_handler_->readReady(accept_partner2)) { int fd = accept(accept_partner2, 0, 0); if (fd < 0) { cerr << "accept2 failed " << strerror(errno) << endl; @@ -292,7 +277,7 @@ CloseHATest::runPartners(bool const backup /* = true */) { continue; } int fd = reader.first; - if (FD_ISSET(fd, &fds)) { + if (ev_handler_->readReady(fd)) { char buf[128]; int cc = read(fd, buf, 128); if (cc < 0) { diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 351c29229d..cec5c896e9 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -1258,6 +1258,8 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* /// @todo: marginal performance optimization. We could create the set once /// and then use its copy for select(). Please note that select() modifies /// provided set to indicated which sockets have something to read. + /// @note: this can be achieved with FDEventHandler (initialize only if max_fd_ is 0) + /// and do not clear the state. for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { // Only deal with IPv4 addresses. @@ -1340,6 +1342,7 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* } // Let's find out which interface/socket has the data + // @todo: fix iface starvation IfacePtr recv_if; for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { @@ -1387,6 +1390,8 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) /// @todo: marginal performance optimization. We could create the set once /// and then use its copy for select(). Please note that select() modifies /// provided set to indicated which sockets have something to read. + /// @note: this can be achieved with FDEventHandler (initialize only if max_fd_ is 0) + /// and do not clear the state. for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { // Only deal with IPv6 addresses. @@ -1469,6 +1474,7 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) } // Let's find out which interface/socket has the data + // @todo: fix iface starvation for (const IfacePtr& iface : ifaces_) { for (const SocketInfo& s : iface->getSockets()) { if (fd_event_handler_->readReady(s.sockfd_)) { diff --git a/src/lib/dhcp/select_event_handler.cc b/src/lib/dhcp/select_event_handler.cc index 1810834e84..0d63f6e579 100644 --- a/src/lib/dhcp/select_event_handler.cc +++ b/src/lib/dhcp/select_event_handler.cc @@ -43,7 +43,6 @@ void SelectEventHandler::add(int fd, bool read /* = true */, bool write /* = fal } } -// @brief Wait for events on registered file descriptors. int SelectEventHandler::waitEvent(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */) { // Sanity check for microsecond timeout. if (timeout_usec >= 1000000) { @@ -54,28 +53,18 @@ int SelectEventHandler::waitEvent(uint32_t timeout_sec, uint32_t timeout_usec /* select_timeout.tv_sec = timeout_sec; select_timeout.tv_usec = timeout_usec; - FD_COPY(&read_fd_set_, &ready_read_fd_set_); - FD_COPY(&write_fd_set_, &ready_write_fd_set_); + FD_COPY(&read_fd_set_, &read_fd_set_data_); + FD_COPY(&write_fd_set_, &write_fd_set_data_); - return (select(max_fd_ + 1, &ready_read_fd_set_, &ready_write_fd_set_, 0, &select_timeout)); + return (select(max_fd_ + 1, &read_fd_set_data_, &write_fd_set_data_, 0, &select_timeout)); } -// @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 SelectEventHandler::readReady(int fd) { - return (FD_ISSET(fd, &ready_read_fd_set_)); + return (FD_ISSET(fd, &read_fd_set_data_)); } -// @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 SelectEventHandler::writeReady(int fd) { - return (FD_ISSET(fd, &ready_write_fd_set_)); + return (FD_ISSET(fd, &write_fd_set_data_)); } void SelectEventHandler::clear() { diff --git a/src/lib/dhcp/select_event_handler.h b/src/lib/dhcp/select_event_handler.h index ab51510d43..4dd8a38a49 100644 --- a/src/lib/dhcp/select_event_handler.h +++ b/src/lib/dhcp/select_event_handler.h @@ -7,7 +7,7 @@ #ifndef SELECT_EVENT_HANDLER_H #define SELECT_EVENT_HANDLER_H -#include +#include #include @@ -69,10 +69,10 @@ private: fd_set write_fd_set_; /// @brief The read event FD set. - fd_set ready_read_fd_set_; + fd_set read_fd_set_data_; /// @brief The write event FD set. - fd_set ready_write_fd_set_; + fd_set write_fd_set_data_; }; } // namespace isc::dhcp diff --git a/src/lib/dhcp/tests/fd_event_handler_factory_unittests.cc b/src/lib/dhcp/tests/fd_event_handler_factory_unittests.cc new file mode 100644 index 0000000000..4539ad418c --- /dev/null +++ b/src/lib/dhcp/tests/fd_event_handler_factory_unittests.cc @@ -0,0 +1,28 @@ +// 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 + +#include +#include + +using namespace isc; +using namespace isc::dhcp; + +namespace { + +// This test verifies if the constructors are working as expected +// and process passed parameters. +TEST(FDEventHandlerFactory, factory) { + FDEventHandlerPtr handler = FDEventHandlerFactory::factoryFDEventHandler(); + EXPECT_EQ(handler->type(), FDEventHandler::TYPE_SELECT); + EXPECT_THROW(handler->add(FD_SETSIZE), BadValue); +} + +} // end of anonymous namespace diff --git a/src/lib/dhcp/tests/fd_event_handler_unittests.h b/src/lib/dhcp/tests/fd_event_handler_unittests.h new file mode 100644 index 0000000000..e41523eabc --- /dev/null +++ b/src/lib/dhcp/tests/fd_event_handler_unittests.h @@ -0,0 +1,135 @@ +// Copyright (C) 2012-2015 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 + +#include + +using namespace isc; +using namespace isc::dhcp; + +const unsigned char MARKER = 0; + +namespace { + +// Just a NOP function to ignore a signal but let it interrupt function. +void no_handler(int) { } + +/// @brief Shared test fixture for testing code using fd event handlers. +class FDEventHandlerTest : public ::testing::Test { +public: + /// @brief Constructor. + FDEventHandlerTest() { + handler_.reset(new isc::dhcp::FDEventHandlerType); + pipe(pipefd); + } + + /// @brief Destructor. + ~FDEventHandlerTest() { + close(pipefd[1]); + close(pipefd[0]); + } + + /// @brief The tested fd event handler. + FDEventHandlerPtr handler_; + + /// @brief The pipe used tor testing read and write operations. + int pipefd[2]; +}; + +TEST_F(FDEventHandlerTest, events) { + EXPECT_NO_THROW(handler_->clear()); + + EXPECT_EQ(0, handler_->waitEvent(0, 1000)); + + handler_->add(pipefd[0], true, false); + handler_->add(pipefd[1], false, true); + + EXPECT_FALSE(handler_->readReady(pipefd[0])); + EXPECT_FALSE(handler_->writeReady(pipefd[0])); + EXPECT_FALSE(handler_->readReady(pipefd[1])); + EXPECT_FALSE(handler_->writeReady(pipefd[1])); + + EXPECT_EQ(1, handler_->waitEvent(0, 1000)); + + EXPECT_FALSE(handler_->readReady(pipefd[0])); + EXPECT_FALSE(handler_->writeReady(pipefd[0])); + EXPECT_FALSE(handler_->readReady(pipefd[1])); + EXPECT_TRUE(handler_->writeReady(pipefd[1])); + + EXPECT_EQ(1, write(pipefd[1], &MARKER, sizeof(MARKER))); + + EXPECT_EQ(2, handler_->waitEvent(0, 1000)); + + EXPECT_TRUE(handler_->readReady(pipefd[0])); + EXPECT_FALSE(handler_->writeReady(pipefd[0])); + EXPECT_FALSE(handler_->readReady(pipefd[1])); + EXPECT_TRUE(handler_->writeReady(pipefd[1])); + + EXPECT_EQ(1, write(pipefd[1], &MARKER, sizeof(MARKER))); + + EXPECT_EQ(2, handler_->waitEvent(0, 1000)); + + EXPECT_TRUE(handler_->readReady(pipefd[0])); + EXPECT_FALSE(handler_->writeReady(pipefd[0])); + EXPECT_FALSE(handler_->readReady(pipefd[1])); + EXPECT_TRUE(handler_->writeReady(pipefd[1])); + + unsigned char data; + + EXPECT_EQ(1, read(pipefd[0], &data, sizeof(data))); + + EXPECT_EQ(2, handler_->waitEvent(0, 1000)); + + EXPECT_TRUE(handler_->readReady(pipefd[0])); + EXPECT_FALSE(handler_->writeReady(pipefd[0])); + EXPECT_FALSE(handler_->readReady(pipefd[1])); + EXPECT_TRUE(handler_->writeReady(pipefd[1])); + + EXPECT_EQ(1, read(pipefd[0], &data, sizeof(data))); + + EXPECT_EQ(1, handler_->waitEvent(0, 1000)); + + EXPECT_FALSE(handler_->readReady(pipefd[0])); + EXPECT_FALSE(handler_->writeReady(pipefd[0])); + EXPECT_FALSE(handler_->readReady(pipefd[1])); + EXPECT_TRUE(handler_->writeReady(pipefd[1])); + + EXPECT_NO_THROW(handler_->clear()); + + EXPECT_EQ(0, handler_->waitEvent(0, 1000)); + + struct sigaction ignored, original; + memset(&ignored, 0, sizeof ignored); + ignored.sa_handler = no_handler; + sigaction(SIGINT, &ignored, &original); + + auto tid = pthread_self(); + sigset_t sset; + sigset_t osset; + sigemptyset(&sset); + sigaddset(&sset, SIGINT); + pthread_sigmask(SIG_BLOCK, &sset, &osset); + std::thread tr([&tid]() { + usleep(500); + pthread_kill(tid, SIGINT); + }); + + pthread_sigmask(SIG_SETMASK, &osset, 0); + + EXPECT_EQ(-1, handler_->waitEvent(1, 0)); + + EXPECT_EQ(errno, EINTR); + + tr.join(); + + sigaction(SIGINT, &original, NULL); +} + +} // end of anonymous namespace diff --git a/src/lib/dhcp/tests/meson.build b/src/lib/dhcp/tests/meson.build index b08f6ca9ac..9a66ab0734 100644 --- a/src/lib/dhcp/tests/meson.build +++ b/src/lib/dhcp/tests/meson.build @@ -17,6 +17,7 @@ kea_dhcp_tests = executable( 'classify_unittest.cc', 'duid_factory_unittest.cc', 'duid_unittest.cc', + 'fd_event_handler_factory_unittests.cc', 'hwaddr_unittest.cc', 'iface_mgr_unittest.cc', 'libdhcp++_unittest.cc', @@ -61,6 +62,7 @@ kea_dhcp_tests = executable( 'protocol_util_unittest.cc', 'run_unittests.cc', pkt_filter_xpf_unittest_cc, + 'select_event_handler_unittests.cc', dependencies: [GTEST_DEP, CRYPTO_DEP], cpp_args: [f'-DTEST_DATA_BUILDDIR="@current_build_dir@"'], include_directories: [include_directories('.')] + INCLUDES, diff --git a/src/lib/dhcp/tests/select_event_handler_unittests.cc b/src/lib/dhcp/tests/select_event_handler_unittests.cc new file mode 100644 index 0000000000..3a2f2eedd0 --- /dev/null +++ b/src/lib/dhcp/tests/select_event_handler_unittests.cc @@ -0,0 +1,12 @@ +// 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 + +#define FDEventHandlerType SelectEventHandler +#define FDEventHandlerTest SelectEventHandlerTest + +#include diff --git a/src/lib/util/ready_check.cc b/src/lib/util/ready_check.cc index 1e7aeca8b2..29d835606e 100644 --- a/src/lib/util/ready_check.cc +++ b/src/lib/util/ready_check.cc @@ -13,7 +13,7 @@ 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: implement this using SelectEventHandler // @todo: should move to epoll/kqueue? if (fd_to_check < 0) { return (-1); -- 2.47.3