From 39f34be00f5fac7db52e49bc1d848abf49c0b667 Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Thu, 30 Jul 2020 13:45:07 +0200 Subject: [PATCH 1/7] bcl: remove timeout handling from event loop https://jira.prplfoundation.org/browse/PPM-18 This is a preparatory commit. As discussed in code reviews, associating a timeout to a read or write operation on a specific file descriptor is doubtedly useful and adds unnecessary complexity to the reactor design pattern. Moreover, current implementation of this feature has some flaws. This is not yet a problem because timeouts are not being used anywhere. As discussed in architecture meetings related to socket refactorings, operation timeouts and periodic actions will be handled by a new yet to be implemented Timer class (a wrapper around timerfd_create). Therefore current per-file-descriptor timeout handling will be removed. In order to use the current event loop with the new sockets API to implement the interface state monitor, some modifications are required in the event loop implementation. To simplify this task and avoid having to fix code that will be removed after all, this preparatory commit removes the timeout handling mechanism. Signed-off-by: Mario Maz --- .../bcl/include/bcl/beerocks_event_loop.h | 31 +---- .../include/bcl/beerocks_socket_event_loop.h | 22 +--- .../bcl/source/beerocks_socket_event_loop.cpp | 123 +++--------------- .../bcl/unit_tests/socket_event_loop_test.cpp | 85 +----------- .../ieee1905_transport/ieee1905_transport.cpp | 2 +- .../ieee1905_transport_broker.cpp | 6 +- .../ieee1905_transport_broker.h | 2 +- .../ieee1905_transport_network.cpp | 2 - 8 files changed, 33 insertions(+), 240 deletions(-) diff --git a/common/beerocks/bcl/include/bcl/beerocks_event_loop.h b/common/beerocks/bcl/include/bcl/beerocks_event_loop.h index 4e087147ae..ca290099e5 100644 --- a/common/beerocks/bcl/include/bcl/beerocks_event_loop.h +++ b/common/beerocks/bcl/include/bcl/beerocks_event_loop.h @@ -9,9 +9,6 @@ #ifndef _BEEROCKS_EVENT_LOOP_H_ #define _BEEROCKS_EVENT_LOOP_H_ -#include "beerocks_backport.h" - -#include #include namespace beerocks { @@ -33,10 +30,7 @@ namespace beerocks { * event loop is at the highest level of control within the program. */ -template class EventLoop { - // Fail the build if T (timeout type) is not derived from std::chrono::duration - static_assert(is_chrono_duration::value, "T must be derived from std::chrono::duration"); - +template class EventLoop { public: /** * The type of the event source (e.g. file descriptor). @@ -46,12 +40,7 @@ template class EventLoop { /** * The type of the event loop. */ - using EventLoopType = EventLoop; - - /** - * The type of the timeout units for the event loop. - */ - using TimeoutType = T; + using EventLoopType = EventLoop; /** * @brief Event handler function definition. @@ -91,15 +80,6 @@ template class EventLoop { */ EventHandler on_write; - /** - * Hook method that is called back by the event loop to handle timeout events. - * Timeout events are dispatched when timeout expires while waiting for the socket to - * be ready for a read or write operation. - * @param socket Socket the event was originated at. - * @param loop Event loop where the event was caught on. - */ - EventHandler on_timeout; - /** * Hook method that is called back by the event loop to handle disconnect events. * Disconnect events are dispatched when the remote socket is closed. @@ -127,17 +107,14 @@ template class EventLoop { * @brief Registers a set of event handlers for the given event source (e.g. socket). * * Event handler for the event that occurred will be called back when the event source is - * ready for a read/write operation, when a disconnect/error occurs or when given timeout expires. + * ready for a read/write operation, when a disconnect/error occurs. * * @param event Event source object. * @param handlers Set of event handlers: class with the methods to be called back when an * event occurs. - * @param timeout Time to wait in milliseconds (-1 to wait indefinitely) for a read/write - * event to occur. * @return True on success and false otherwise. */ - virtual bool add_event(EventType event, EventHandlers handlers, - TimeoutType timeout = TimeoutType::min()) = 0; + virtual bool add_event(EventType event, EventHandlers handlers) = 0; /** * @brief Removes previously registered event handlers for the given event source. diff --git a/common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h b/common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h index 6158d746fe..24867432b4 100644 --- a/common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h +++ b/common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h @@ -12,6 +12,7 @@ #include "beerocks_event_loop.h" #include "network/socket.h" +#include #include #include @@ -22,10 +23,8 @@ namespace beerocks { * @see EventLoop * * This class uses the Linux epoll APIs for monitoring the provided sockets for I/O operations. - * Timeout operations are achieved by using the timerfd mechanism, which delivers timer - * expiration notifications via a file descriptor. */ -class SocketEventLoop : public EventLoop, std::chrono::milliseconds> { +class SocketEventLoop : public EventLoop> { public: /** * @brief Class constructor. @@ -34,7 +33,7 @@ class SocketEventLoop : public EventLoop, std::chrono::m * * @param [in] timeout Sets the master timeout (in milliseconds) for the event loop. */ - explicit SocketEventLoop(TimeoutType timeout = TimeoutType::min()); + explicit SocketEventLoop(std::chrono::milliseconds timeout = std::chrono::milliseconds::min()); /** * @brief Class destructor. @@ -44,8 +43,7 @@ class SocketEventLoop : public EventLoop, std::chrono::m /** * @see EventPoll::add_event */ - virtual bool add_event(EventType socket, EventHandlers handlers, - TimeoutType timeout = TimeoutType::min()) override; + virtual bool add_event(EventType socket, EventHandlers handlers) override; /** * @see EventPoll::del_event @@ -69,7 +67,7 @@ class SocketEventLoop : public EventLoop, std::chrono::m /** * Event loop master timeout (used for the epoll_wait function). */ - TimeoutType m_timeout = TimeoutType::min(); + std::chrono::milliseconds m_timeout = std::chrono::milliseconds::min(); /** * @brief Data structure representing a socket added to the poll. @@ -85,16 +83,6 @@ class SocketEventLoop : public EventLoop, std::chrono::m * Shared pointer to the socket object. */ EventType socket = nullptr; - - /** - * timer file descriptor. - */ - int timerfd = -1; - - /** - * Socket timeout value in milliseconds. - */ - TimeoutType timeout_value = TimeoutType::min(); }; /** diff --git a/common/beerocks/bcl/source/beerocks_socket_event_loop.cpp b/common/beerocks/bcl/source/beerocks_socket_event_loop.cpp index 597cf152d0..2a5cf62184 100644 --- a/common/beerocks/bcl/source/beerocks_socket_event_loop.cpp +++ b/common/beerocks/bcl/source/beerocks_socket_event_loop.cpp @@ -27,7 +27,7 @@ static constexpr int MAX_POLL_EVENTS = 17; /////////////////////////////// Implementation /////////////////////////////// ////////////////////////////////////////////////////////////////////////////// -SocketEventLoop::SocketEventLoop(TimeoutType timeout) : m_timeout(timeout) +SocketEventLoop::SocketEventLoop(std::chrono::milliseconds timeout) : m_timeout(timeout) { m_epoll_fd = epoll_create1(0); LOG_IF(m_epoll_fd == -1, FATAL) << "Failed creating epoll: " << strerror(errno); @@ -49,8 +49,7 @@ SocketEventLoop::~SocketEventLoop() << "Failed closing epoll file descriptor: " << strerror(errno); } -bool SocketEventLoop::add_event(SocketEventLoop::EventType socket, - EventLoop::EventHandlers handlers, TimeoutType timeout) +bool SocketEventLoop::add_event(SocketEventLoop::EventType socket, EventHandlers handlers) { if (!socket) { LOG(ERROR) << "Invalid socket pointer!"; @@ -70,43 +69,6 @@ bool SocketEventLoop::add_event(SocketEventLoop::EventType socket, event_data->socket = socket; event_data->handlers = handlers; - // If timeout is set and a handler is defined, initialize a timerfd - if (timeout > TimeoutType::zero()) { - if (handlers.on_timeout) { - - // TODO: timerfd may not be supported on all kernels. - // Implement a check at creation, and in case it's not supported - // base the timeouts mechanism on the main timeout of the loop. - - // Create a new timerfd file descriptor and initialize the timeout values - event_data->timeout_value = timeout; - event_data->timerfd = timerfd_create(CLOCK_MONOTONIC, 0); - - if (event_data->timerfd == -1) { - LOG(ERROR) << "Failed creating timerfd: " << strerror(errno); - return false; - } - - // Convert the timeout into seconds and nano-seconds - auto timeout_sec = std::chrono::duration_cast(timeout); - auto timeout_nanosec = - std::chrono::duration_cast(timeout - timeout_sec); - - timespec timeout_spec({.tv_sec = static_cast(timeout_sec.count()), - .tv_nsec = static_cast(timeout_nanosec.count())}); - itimerspec timer_val({.it_interval = timeout_spec, .it_value = timeout_spec}); - - // Set the timerfd timeout value - if (timerfd_settime(event_data->timerfd, 0, &timer_val, nullptr) == -1) { - LOG(ERROR) << "Failed setting timerfd value: " << strerror(errno); - return false; - } - } else { - LOG(WARNING) << "Timeout was requested for '" << timeout.count() - << "' milliseconds, but not handler is provided. Ignoring."; - } - } - // Helper lambda function for adding a fd to the poll, and register for the following events: // EPOLLIN: The associated fd is available for read operations. // EPOLLOUT: The associated fd is available for write operations. @@ -118,8 +80,8 @@ bool SocketEventLoop::add_event(SocketEventLoop::EventType socket, event.data.fd = fd; event.events = EPOLLRDHUP | EPOLLERR | EPOLLHUP; - // If read or timeout handlers were set, also listen for POLL-IN events - if (event_data->handlers.on_read || event_data->handlers.on_timeout) { + // If read handler was set, also listen for POLL-IN events + if (event_data->handlers.on_read) { event.events |= EPOLLIN; } @@ -144,15 +106,6 @@ bool SocketEventLoop::add_event(SocketEventLoop::EventType socket, return false; } - // Add the timeout fd to the poll - if (event_data->timerfd != -1) { - if (!add_fd_to_epoll(event_data->timerfd)) { - // Remove the socket from the poll and fail - del_event(socket); - return false; - } - } - return true; } @@ -188,21 +141,6 @@ bool SocketEventLoop::del_event(SocketEventLoop::EventType socket) // Erase the fd from the map m_fd_to_event_data.erase(event_data->socket->getSocketFd()); - // Delete the timeout fd from the poll - if (event_data->timerfd != -1) { - if (epoll_ctl(m_epoll_fd, EPOLL_CTL_DEL, event_data->timerfd, nullptr) == -1) { - LOG(ERROR) << "Failed deleting timeout FD (" << event_data->timerfd - << ") from the poll: " << strerror(errno); - - error = true; - } - - // Remove the fd from the map. Since timeout sockets are created - // automatically by the event pool, also close the fd. - m_fd_to_event_data.erase(event_data->timerfd); - close(event_data->timerfd); - } - return !error; } @@ -213,7 +151,7 @@ int SocketEventLoop::run() // Convert the global event loop timeout (if set) to milliseconds int timeout_millis = - (m_timeout > TimeoutType::zero()) + (m_timeout > std::chrono::milliseconds::zero()) ? static_cast( std::chrono::duration_cast(m_timeout).count()) : -1; @@ -224,7 +162,9 @@ int SocketEventLoop::run() if (num_events == -1) { LOG(ERROR) << "Error during epoll_wait: " << strerror(errno); return -1; - } else if (num_events == 0) { + } + + if (num_events == 0) { // Timeout... Do nothing return 0; } @@ -277,47 +217,16 @@ int SocketEventLoop::run() } } - // Handle Data & Timeouts + // Handle incoming data } else if (events[i].events & EPOLLIN) { - // Handle incoming data - if (event_data->socket->getSocketFd() == fd) { - if (!event_data->handlers.on_read) { - LOG(WARNING) << "Incoming data on socket FD (" << fd - << ") without handler. Removing."; - del_event(socket); - } else { - if (!event_data->handlers.on_read(socket, *this)) { - return -1; - } - } - // Handle timeouts - } else if (event_data->timerfd == fd) { - if (!event_data->handlers.on_timeout) { - // This shouldn't happen as the timeout socket is created only if - // a handler was provided... If somehow if does happen, remove the - // associated socket and the timeout socket. - LOG(ERROR) << "Event on timeout FD (" << fd << ") without handler. Removing."; - del_event(socket); - } else { - // Read the number of expirations occurred on this timerfd - uint64_t num_exp; - if (read(event_data->timerfd, &num_exp, sizeof(num_exp)) != sizeof(num_exp)) { - LOG(ERROR) - << "Failed reading timerfd number of expirations: " << strerror(errno); - return -1; - } - - // If a timer has expired more than once, it means that there is some - // delay in processing events, so print a warning - if (num_exp > 1) { - LOG(WARNING) << "Timer on FD (" << fd << ") expired " << num_exp - << " times since it was previously handled"; - } - - if (!event_data->handlers.on_timeout(socket, *this)) { - return -1; - } + if (!event_data->handlers.on_read) { + LOG(WARNING) << "Incoming data on socket FD (" << fd + << ") without handler. Removing."; + del_event(socket); + } else { + if (!event_data->handlers.on_read(socket, *this)) { + return -1; } } diff --git a/common/beerocks/bcl/unit_tests/socket_event_loop_test.cpp b/common/beerocks/bcl/unit_tests/socket_event_loop_test.cpp index 66e07b4207..2565036ef2 100644 --- a/common/beerocks/bcl/unit_tests/socket_event_loop_test.cpp +++ b/common/beerocks/bcl/unit_tests/socket_event_loop_test.cpp @@ -26,7 +26,7 @@ static constexpr std::chrono::milliseconds s_test_timeout(100); // Define Event and Loop types for the use of the unit tests using EventType = std::shared_ptr; -using LoopType = EventLoop; +using LoopType = EventLoop; /** * @brief Mockable event handlers class @@ -36,11 +36,8 @@ class EventHandlersMock : public LoopType::EventHandlers { public: EventHandlersMock() { - on_read = [&](EventType socket, LoopType &loop) { return handle_read(socket, &loop); }; - on_write = [&](EventType socket, LoopType &loop) { return handle_write(socket, &loop); }; - on_timeout = [&](EventType socket, LoopType &loop) { - return handle_timeout(socket, &loop); - }; + on_read = [&](EventType socket, LoopType &loop) { return handle_read(socket, &loop); }; + on_write = [&](EventType socket, LoopType &loop) { return handle_write(socket, &loop); }; on_disconnect = [&](EventType socket, LoopType &loop) { return handle_disconnect(socket, &loop); }; @@ -49,7 +46,6 @@ class EventHandlersMock : public LoopType::EventHandlers { MOCK_METHOD(bool, handle_read, ((EventType &), LoopType *)); MOCK_METHOD(bool, handle_write, ((EventType &), LoopType *)); - MOCK_METHOD(bool, handle_timeout, ((EventType &), LoopType *)); MOCK_METHOD(bool, handle_disconnect, ((EventType &), LoopType *)); MOCK_METHOD(bool, handle_error, ((EventType &), LoopType *)); }; @@ -66,79 +62,6 @@ TEST(beerocks_socket_event_loop, setup) el::Loggers::reconfigureLogger("default", defaultConf); } -TEST(beerocks_socket_event_loop, simple_timeout) -{ - SocketEventLoop loop(s_test_timeout); - StrictMock timeout; - - // Disable the on_write handler (to prevent the loop from firing "write ready" events) - timeout.on_write = nullptr; - - // Create a dummy socket for checking the timeout mechanism - auto timeout_socket = std::make_shared(socket(AF_UNIX, SOCK_DGRAM, 0)); - ASSERT_TRUE(timeout_socket && timeout_socket->getSocketFd() != -1); - - { - InSequence sequence; - - // Socket timeout handler - EXPECT_CALL(timeout, handle_timeout(timeout_socket, &loop)) - .WillOnce(Invoke([](EventType socket, LoopType *loop) -> bool { return true; })); - }; - - // Add the dummy socket into the event loop - ASSERT_TRUE(loop.add_event(timeout_socket, timeout, std::chrono::milliseconds{1})); - - // Execute the event loop - ASSERT_GE(loop.run(), 0); - - // Remove the socket from the event loop - ASSERT_TRUE(loop.del_event(timeout_socket)); - close(timeout_socket->getSocketFd()); -} - -TEST(beerocks_socket_event_loop, repeated_timeout) -{ - SocketEventLoop loop(s_test_timeout); - StrictMock timeout; - - // Disable the on_write handler (to prevent the loop from firing "write ready" events) - timeout.on_write = nullptr; - - // Create a dummy socket for checking the timeout mechanism - auto timeout_socket = std::make_shared(socket(AF_UNIX, SOCK_DGRAM, 0)); - ASSERT_NE(timeout_socket, nullptr); - - // Timeout event counter - int timeout_seq = 0; - - { - InSequence sequence; - - // Expect the timeout handler to be executed exactly 3 times - EXPECT_CALL(timeout, handle_timeout(timeout_socket, &loop)).Times(3); - - ON_CALL(timeout, handle_timeout(timeout_socket, &loop)) - .WillByDefault([&](EventType socket, LoopType *loop) -> bool { - ++timeout_seq; - return true; - }); - }; - - // Add the dummy socket into the event loop - ASSERT_TRUE(loop.add_event(timeout_socket, timeout, std::chrono::milliseconds{1})); - - // Execute the event loop - ASSERT_EQ(1, loop.run()); - ASSERT_EQ(1, loop.run()); - ASSERT_EQ(1, loop.run()); - ASSERT_EQ(3, timeout_seq); - - // Remove the socket from the event loop - ASSERT_TRUE(loop.del_event(timeout_socket)); - close(timeout_socket->getSocketFd()); -} - TEST(beerocks_socket_event_loop, simple_read_write) { SocketEventLoop loop(s_test_timeout); @@ -179,7 +102,7 @@ TEST(beerocks_socket_event_loop, simple_read_write) }; // Add the reader/writer sockets and handlers to the loop - ASSERT_TRUE(loop.add_event(writer_socket, writer, std::chrono::milliseconds{1})); + ASSERT_TRUE(loop.add_event(writer_socket, writer)); ASSERT_TRUE(loop.add_event(reader_socket, reader)); // Run the loop diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.cpp b/framework/transport/ieee1905_transport/ieee1905_transport.cpp index 8bdc2d0989..dc22c32a12 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport.cpp @@ -6,6 +6,7 @@ * See LICENSE file for more details. */ +#include #include #include @@ -89,7 +90,6 @@ void Ieee1905Transport::run() // Not implemented .on_write = nullptr, - .on_timeout = nullptr, // Fail on server socket disconnections or errors .on_disconnect = diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp index 9ebf82f40f..7d6fbc5e2d 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp @@ -74,8 +74,7 @@ BrokerServer::BrokerServer(SocketServer &broker_server, BrokerEventLoop &event_l }, // Not implemented - .on_write = nullptr, - .on_timeout = nullptr, + .on_write = nullptr, // Fail on server socket disconnections or errors .on_disconnect = @@ -301,8 +300,7 @@ bool BrokerServer::socket_connected(std::shared_ptr sd) }, // Not implemented - .on_write = nullptr, - .on_timeout = nullptr, + .on_write = nullptr, // Remove the socket on disconnections or errors .on_disconnect = diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h index fc8384716d..a4826b8f31 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h @@ -36,7 +36,7 @@ class BrokerServer { /** * The type of the supported EventLoop. */ - using BrokerEventLoop = EventLoop, std::chrono::milliseconds>; + using BrokerEventLoop = EventLoop>; /** * @brief Transport messages (@see Message) handler function definition. diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp index be117709a4..548bad7ecc 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp @@ -147,7 +147,6 @@ void Ieee1905Transport::update_network_interfaces( // Not implemented .on_write = nullptr, - .on_timeout = nullptr, .on_disconnect = nullptr, // Handle interface errors @@ -310,7 +309,6 @@ void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bo // Not implemented .on_write = nullptr, - .on_timeout = nullptr, .on_disconnect = nullptr, // Handle interface errors From 05dee5394966c8769499b99f1487565f331e1049 Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Tue, 11 Aug 2020 17:55:23 +0200 Subject: [PATCH 2/7] bcl: remove template parameter from event loop https://jira.prplfoundation.org/browse/PPM-18 This is a preparatory commit. EventLoop interface and implementation are not intended to deal with sockets only (or any other specific class). Instead, EventLoop will deal with any OS resource implementing the file descriptor interface. So there is no need to couple EventLoop whith any specific class, even if using a template for that. In order to use the current event loop with the new sockets API to implement the interface state monitor, event loop interface and implementation have to be modified to use `int` type to refer to the source of events, being that integer the file descriptor of the OS resource where events occur. In the future, the file descriptor will be represented by a `FileDescriptor` class which, among other features, will close the file descriptor in its destructor. Current `Socket` class will be eventually deprecated in the future. Method to register handlers for events on a given file descriptor has been renamed from `add_event` to the more appropriate name `register_handlers`. For the same reason, the method to remove those handlers when they are not needed any more has been renamed from `del_event` to `remove_handlers`. `SocketEventLoop` class has been renamed to `EventLoopImpl` and the unit tests modified accordingly. While we are at it, a bug in the constructor of BrokerServer has been fixed (it was creating a shared_ptr from a reference). New `start()` and `stop()` methods have been added to BrokerServer to register and remove event handlers respectively. Registering handlers in `start()` instead of in the constructor prevents cppcheck from complaining about virtual methods being called from constructor. Signed-off-by: Mario Maz --- common/beerocks/bcl/CMakeLists.txt | 2 +- .../bcl/include/bcl/beerocks_event_loop.h | 54 ++-- .../include/bcl/beerocks_event_loop_impl.h | 81 ++++++ .../include/bcl/beerocks_socket_event_loop.h | 96 ------- .../bcl/source/beerocks_event_loop_impl.cpp | 215 +++++++++++++++ .../bcl/source/beerocks_socket_event_loop.cpp | 253 ------------------ .../bcl/unit_tests/event_loop_impl_test.cpp | 109 ++++++++ .../ieee1905_transport/ieee1905_transport.cpp | 69 ++--- .../ieee1905_transport_broker.cpp | 171 ++++++------ .../ieee1905_transport_broker.h | 50 ++-- .../ieee1905_transport_network.cpp | 110 ++++---- .../test/ieee1905_transport_broker_tests.cpp | 94 +++++-- 12 files changed, 687 insertions(+), 617 deletions(-) create mode 100644 common/beerocks/bcl/include/bcl/beerocks_event_loop_impl.h delete mode 100644 common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h create mode 100644 common/beerocks/bcl/source/beerocks_event_loop_impl.cpp delete mode 100644 common/beerocks/bcl/source/beerocks_socket_event_loop.cpp create mode 100644 common/beerocks/bcl/unit_tests/event_loop_impl_test.cpp diff --git a/common/beerocks/bcl/CMakeLists.txt b/common/beerocks/bcl/CMakeLists.txt index f5dcacf620..9048f458a8 100644 --- a/common/beerocks/bcl/CMakeLists.txt +++ b/common/beerocks/bcl/CMakeLists.txt @@ -46,7 +46,7 @@ if (BUILD_TESTS) set(unit_tests_sources ${bcl_sources} ${MODULE_PATH}/unit_tests/network_utils_test.cpp - ${MODULE_PATH}/unit_tests/socket_event_loop_test.cpp + ${MODULE_PATH}/unit_tests/event_loop_impl_test.cpp ${MODULE_PATH}/unit_tests/wireless_utils_test.cpp ) add_executable(${TEST_PROJECT_NAME} diff --git a/common/beerocks/bcl/include/bcl/beerocks_event_loop.h b/common/beerocks/bcl/include/bcl/beerocks_event_loop.h index ca290099e5..3439665e1a 100644 --- a/common/beerocks/bcl/include/bcl/beerocks_event_loop.h +++ b/common/beerocks/bcl/include/bcl/beerocks_event_loop.h @@ -30,29 +30,19 @@ namespace beerocks { * event loop is at the highest level of control within the program. */ -template class EventLoop { +class EventLoop { public: - /** - * The type of the event source (e.g. file descriptor). - */ - using EventType = E; - - /** - * The type of the event loop. - */ - using EventLoopType = EventLoop; - /** * @brief Event handler function definition. * * Parameters to the event handler function are: - * @param[in] event The resource where the event was originated at. + * @param[in] fd The file descriptor of the OS resource where the event was originated at. * @param[in] loop The event loop where the event was caught. Event handler can install new handlers, * remove existing handlers and even ask for loop termination. * * @returns True on success or false otherwise */ - using EventHandler = std::function; + using EventHandler = std::function; /** * Set of event handler functions, one function to handle each possible event happened. @@ -64,36 +54,28 @@ template class EventLoop { struct EventHandlers { /** * Hook method that is called back by the event loop to handle read events. - * Read events are dispatched when the socket is ready for a read operation (a read - * operation will not block). - * @param socket Socket the event was originated at. - * @param loop Event loop where the event was caught on. + * Read events are dispatched for example when a socket is ready for a read operation (a + * read operation will not block). */ EventHandler on_read; /** * Hook method that is called back by the event loop to handle write events. - * Write events are dispatched when the socket is ready for a write operation (a write - * operation will not block). - * @param socket Socket the event was originated at. - * @param loop Event loop where the event was caught on. + * Write events are dispatched for example when a socket is ready for a write operation (a + * write operation will not block). */ EventHandler on_write; /** * Hook method that is called back by the event loop to handle disconnect events. - * Disconnect events are dispatched when the remote socket is closed. - * @param socket Socket the event was originated at. - * @param loop Event loop where the event was caught on. + * Disconnect events are dispatched for example when the remote socket is closed. */ EventHandler on_disconnect; /** * Hook method that is called back by the event loop to handle error events. - * Error events are dispatched when an error occurs while waiting for the socket to - * be ready for a read or write operation. - * @param socket Socket the event was originated at. - * @param loop Event loop where the event was caught on. + * Error events are dispatched for example when an error occurs while waiting for a socket + * to be ready for a read or write operation. */ EventHandler on_error; }; @@ -109,30 +91,30 @@ template class EventLoop { * Event handler for the event that occurred will be called back when the event source is * ready for a read/write operation, when a disconnect/error occurs. * - * @param event Event source object. + * @param fd File descriptor of the event source object. * @param handlers Set of event handlers: class with the methods to be called back when an * event occurs. * @return True on success and false otherwise. */ - virtual bool add_event(EventType event, EventHandlers handlers) = 0; + virtual bool register_handlers(int fd, const EventHandlers &handlers) = 0; /** * @brief Removes previously registered event handlers for the given event source. * - * @param event Event source object. + * @param fd File descriptor of the event source object. * @return True on success and false otherwise. */ - virtual bool del_event(EventType socket) = 0; + virtual bool remove_handlers(int fd) = 0; /** * @brief Runs message loop. * - * Performs a single loop iteration and returns after processing IO events - * or when an error or timeout occurs. + * Performs a single loop iteration and returns after processing IO events or when an error + * or timeout occurs. * * @return -1 on critical errors - * @return 0 on timeout without any socket events - * @return >0 number of socket events processed during the class to this method. + * @return 0 on timeout waiting for events on all file descriptors. + * @return >0 number of events processed during the call to this method. */ virtual int run() = 0; }; diff --git a/common/beerocks/bcl/include/bcl/beerocks_event_loop_impl.h b/common/beerocks/bcl/include/bcl/beerocks_event_loop_impl.h new file mode 100644 index 0000000000..a08a3f16ea --- /dev/null +++ b/common/beerocks/bcl/include/bcl/beerocks_event_loop_impl.h @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2016-2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef _BEEROCKS_EVENT_LOOP_IMPL_H_ +#define _BEEROCKS_EVENT_LOOP_IMPL_H_ + +#include "beerocks_event_loop.h" + +#include +#include + +namespace beerocks { + +/** + * @brief ePoll based implementation of the EventLoop interface. + * @see EventLoop + * + * This class uses the Linux epoll APIs for monitoring the provided file descriptors for I/O + * operations. + */ +class EventLoopImpl : public EventLoop { +public: + /** + * @brief Class constructor. + * + * Initializes an epoll file descriptor. + * + * @param [in] timeout Sets the master timeout (in milliseconds) for the event loop. + */ + explicit EventLoopImpl(std::chrono::milliseconds timeout = std::chrono::milliseconds::min()); + + /** + * @brief Class destructor. + */ + ~EventLoopImpl() override; + + /** + * @see EventLoop::register_handlers + */ + bool register_handlers(int fd, const EventHandlers &handlers) override; + + /** + * @see EventLoop::remove_handlers + */ + bool remove_handlers(int fd) override; + + /** + * @brief Main event loop method. + * @see EventLoop::run + * + * Executes the epoll_wait() function and processes occurred events. + */ + int run() override; + +private: + /** + * epoll file descriptor. + */ + int m_epoll_fd = -1; + + /** + * Event loop master timeout (used for the epoll_wait function). + */ + std::chrono::milliseconds m_timeout = std::chrono::milliseconds::min(); + + /** + * Map of registered event handlers. + * Key value is the file descriptor and value is the EventHandlers structure containing the + * event handlers to deall with events occurred on that file descriptor. + */ + std::unordered_map m_fd_to_event_handlers; +}; + +} // namespace beerocks + +#endif // _BEEROCKS_EVENT_LOOP_IMPL_H_ diff --git a/common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h b/common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h deleted file mode 100644 index 24867432b4..0000000000 --- a/common/beerocks/bcl/include/bcl/beerocks_socket_event_loop.h +++ /dev/null @@ -1,96 +0,0 @@ -/* SPDX-License-Identifier: BSD-2-Clause-Patent - * - * SPDX-FileCopyrightText: 2016-2020 the prplMesh contributors (see AUTHORS.md) - * - * This code is subject to the terms of the BSD+Patent license. - * See LICENSE file for more details. - */ - -#ifndef _BEEROCKS_SOCKET_EVENT_LOOP_H_ -#define _BEEROCKS_SOCKET_EVENT_LOOP_H_ - -#include "beerocks_event_loop.h" -#include "network/socket.h" - -#include -#include -#include - -namespace beerocks { - -/** - * @brief ePoll based implementation of the EventLoop interface. - * @see EventLoop - * - * This class uses the Linux epoll APIs for monitoring the provided sockets for I/O operations. - */ -class SocketEventLoop : public EventLoop> { -public: - /** - * @brief Class constructor. - * - * Initializes an epoll file descriptor. - * - * @param [in] timeout Sets the master timeout (in milliseconds) for the event loop. - */ - explicit SocketEventLoop(std::chrono::milliseconds timeout = std::chrono::milliseconds::min()); - - /** - * @brief Class destructor. - */ - virtual ~SocketEventLoop(); - - /** - * @see EventPoll::add_event - */ - virtual bool add_event(EventType socket, EventHandlers handlers) override; - - /** - * @see EventPoll::del_event - */ - virtual bool del_event(EventType socket) override; - - /** - * @brief Main event loop method. - * @see EventPoll::run - * - * Executes the epoll_wait() function and processes ocurred events. - */ - virtual int run() override; - -private: - /** - * epoll file descriptor. - */ - int m_epoll_fd = -1; - - /** - * Event loop master timeout (used for the epoll_wait function). - */ - std::chrono::milliseconds m_timeout = std::chrono::milliseconds::min(); - - /** - * @brief Data structure representing a socket added to the poll. - * This structure groups all the information required for processing socket events. - */ - struct EventData { - /** - * Socket event handler functions structure. - */ - EventHandlers handlers; - - /** - * Shared pointer to the socket object. - */ - EventType socket = nullptr; - }; - - /** - * Map file descriptors to EventData structure instances. - */ - std::unordered_map> m_fd_to_event_data; -}; - -} // namespace beerocks - -#endif // _BEEROCKS_SOCKET_EVENT_LOOP_H_ diff --git a/common/beerocks/bcl/source/beerocks_event_loop_impl.cpp b/common/beerocks/bcl/source/beerocks_event_loop_impl.cpp new file mode 100644 index 0000000000..f2ccd24910 --- /dev/null +++ b/common/beerocks/bcl/source/beerocks_event_loop_impl.cpp @@ -0,0 +1,215 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2016-2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#include + +#include +#include +#include + +#include + +namespace beerocks { + +////////////////////////////////////////////////////////////////////////////// +////////////////////////// Local module definitions ////////////////////////// +////////////////////////////////////////////////////////////////////////////// + +// Maximal number of events to process in a single epoll_wait call +static constexpr int MAX_POLL_EVENTS = 17; + +////////////////////////////////////////////////////////////////////////////// +/////////////////////////////// Implementation /////////////////////////////// +////////////////////////////////////////////////////////////////////////////// + +EventLoopImpl::EventLoopImpl(std::chrono::milliseconds timeout) : m_timeout(timeout) +{ + m_epoll_fd = epoll_create1(0); + LOG_IF(m_epoll_fd == -1, FATAL) << "Failed creating epoll: " << strerror(errno); +} + +EventLoopImpl::~EventLoopImpl() +{ + // Delete all the file descriptors in the poll + LOG(DEBUG) << "Removing " << m_fd_to_event_handlers.size() << " FDs from the event loop"; + + while (!m_fd_to_event_handlers.empty()) { + int fd = m_fd_to_event_handlers.begin()->first; + EventLoopImpl::remove_handlers(fd); + } + + // Close the poll fd + LOG_IF(close(m_epoll_fd) == -1, ERROR) + << "Failed closing epoll file descriptor: " << strerror(errno); +} + +bool EventLoopImpl::register_handlers(int fd, const EventLoop::EventHandlers &handlers) +{ + if (-1 == fd) { + LOG(ERROR) << "Invalid file descriptor!"; + return false; + } + + // Make sure that the file descriptor is not already part of the poll + if (m_fd_to_event_handlers.find(fd) != m_fd_to_event_handlers.end()) { + LOG(WARNING) << "Requested to add FD (" << fd << ") to the poll, but it's already there..."; + + return false; + } + + // Helper lambda function for adding a fd to the poll, and register for the following events: + // EPOLLIN: The associated fd is available for read operations. + // EPOLLOUT: The associated fd is available for write operations. + // EPOLLRDHUP: Socket peer closed connection, or shut down writing half of connection. + // EPOLLERR: Error condition happened on the associated fd. + // EPOLLHUP: Hang up happened on the associated fd. + auto add_fd_to_epoll = [&](int fd) -> bool { + epoll_event event = {}; + event.data.fd = fd; + event.events = EPOLLRDHUP | EPOLLERR | EPOLLHUP; + + // If read handler was set, also listen for POLL-IN events + if (handlers.on_read) { + event.events |= EPOLLIN; + } + + // If write handler was set, also listen for POLL-OUT events + if (handlers.on_write) { + event.events |= EPOLLOUT; + } + + if (epoll_ctl(m_epoll_fd, EPOLL_CTL_ADD, fd, &event) == -1) { + LOG(ERROR) << "Failed adding FD (" << fd << ") to the poll: " << strerror(errno); + return false; + } + + // Map the file descriptor to the event handlers structure + m_fd_to_event_handlers[fd] = handlers; + + return true; + }; + + // Add the file descriptor to the poll + if (!add_fd_to_epoll(fd)) { + return false; + } + + return true; +} + +bool EventLoopImpl::remove_handlers(int fd) +{ + if (-1 == fd) { + LOG(ERROR) << "Invalid file descriptor!"; + return false; + } + + // Make sure that the file descriptor was previously added to the poll + const auto &it = m_fd_to_event_handlers.find(fd); + if (it == m_fd_to_event_handlers.end()) { + LOG(WARNING) << "Requested to delete FD (" << fd + << ") from the poll, but it wasn't previously added."; + + return false; + } + + // Delete the file descriptor from the poll + auto error = false; + if (epoll_ctl(m_epoll_fd, EPOLL_CTL_DEL, fd, nullptr) == -1) { + LOG(ERROR) << "Failed deleting FD (" << fd << ") from the poll: " << strerror(errno); + + error = true; + } + + // Erase the file descriptor from the map + m_fd_to_event_handlers.erase(fd); + + return !error; +} + +int EventLoopImpl::run() +{ + // Poll events + epoll_event events[MAX_POLL_EVENTS]{}; + + // Convert the global event loop timeout (if set) to milliseconds + int timeout_millis = + (m_timeout > std::chrono::milliseconds::zero()) + ? static_cast( + std::chrono::duration_cast(m_timeout).count()) + : -1; + + // Poll the file descriptors + auto num_events = epoll_wait(m_epoll_fd, events, sizeof(events), timeout_millis); + + if (num_events == -1) { + LOG(ERROR) << "Error during epoll_wait: " << strerror(errno); + return -1; + } + + if (num_events == 0) { + // Timeout... Do nothing + return 0; + } + + // Trigger event handlers + for (int i = 0; i < num_events; i++) { + int fd = events[i].data.fd; + + const auto &it = m_fd_to_event_handlers.find(fd); + if (it == m_fd_to_event_handlers.end()) { + LOG(ERROR) << "Event on unknown FD: " << fd; + continue; + } + + const auto &handlers = it->second; + + // Handle errors + if (events[i].events & EPOLLERR) { + + // Remove the file descriptor from the poll + remove_handlers(fd); + + // Call the on_error handler of this file descriptor + if (handlers.on_error && (!handlers.on_error(fd, *this))) { + return -1; + } + + // Handle disconnected sockets (stream socket peer closed connection) + } else if ((events[i].events & EPOLLRDHUP) || (events[i].events & EPOLLHUP)) { + LOG(DEBUG) << "Socket with FD (" << fd << ") disconnected"; + + // Remove the file descriptor from the poll + remove_handlers(fd); + + // Call the on_disconnect handler of this file descriptor + if (handlers.on_disconnect && (!handlers.on_disconnect(fd, *this))) { + return -1; + } + + // Handle incoming data + } else if (events[i].events & EPOLLIN) { + if (handlers.on_read && (!handlers.on_read(fd, *this))) { + return -1; + } + + // Handle write operations + } else if (events[i].events & EPOLLOUT) { + if (handlers.on_write && (!handlers.on_write(fd, *this))) { + return -1; + } + + } else { + LOG(ERROR) << "FD (" << fd << ") generated unknown event: " << events[i].events; + } + } + + return num_events; +} + +} // namespace beerocks diff --git a/common/beerocks/bcl/source/beerocks_socket_event_loop.cpp b/common/beerocks/bcl/source/beerocks_socket_event_loop.cpp deleted file mode 100644 index 2a5cf62184..0000000000 --- a/common/beerocks/bcl/source/beerocks_socket_event_loop.cpp +++ /dev/null @@ -1,253 +0,0 @@ -/* SPDX-License-Identifier: BSD-2-Clause-Patent - * - * SPDX-FileCopyrightText: 2016-2020 the prplMesh contributors (see AUTHORS.md) - * - * This code is subject to the terms of the BSD+Patent license. - * See LICENSE file for more details. - */ - -#include - -#include -#include -#include - -#include - -namespace beerocks { - -////////////////////////////////////////////////////////////////////////////// -////////////////////////// Local module definitions ////////////////////////// -////////////////////////////////////////////////////////////////////////////// - -// Maximal number of events to process in a single epoll_wait call -static constexpr int MAX_POLL_EVENTS = 17; - -////////////////////////////////////////////////////////////////////////////// -/////////////////////////////// Implementation /////////////////////////////// -////////////////////////////////////////////////////////////////////////////// - -SocketEventLoop::SocketEventLoop(std::chrono::milliseconds timeout) : m_timeout(timeout) -{ - m_epoll_fd = epoll_create1(0); - LOG_IF(m_epoll_fd == -1, FATAL) << "Failed creating epoll: " << strerror(errno); -} - -SocketEventLoop::~SocketEventLoop() -{ - // Delete all the sockets in the poll - LOG(DEBUG) << "Removing " << m_fd_to_event_data.size() - << " FDs from the event loop: " << m_fd_to_event_data; - - while (!m_fd_to_event_data.empty()) { - auto event_data = m_fd_to_event_data.begin()->second; - del_event(event_data->socket); - } - - // Close the poll fd - LOG_IF(close(m_epoll_fd) == -1, ERROR) - << "Failed closing epoll file descriptor: " << strerror(errno); -} - -bool SocketEventLoop::add_event(SocketEventLoop::EventType socket, EventHandlers handlers) -{ - if (!socket) { - LOG(ERROR) << "Invalid socket pointer!"; - return false; - } - - // Make sure that the FD is not already part of the poll - if (m_fd_to_event_data.find(socket->getSocketFd()) != m_fd_to_event_data.end()) { - LOG(WARNING) << "Requested to add FD (" << socket->getSocketFd() - << ") to the poll, but it's already there..."; - - return false; - } - - // Build EventData structure - auto event_data = std::make_shared(); - event_data->socket = socket; - event_data->handlers = handlers; - - // Helper lambda function for adding a fd to the poll, and register for the following events: - // EPOLLIN: The associated fd is available for read operations. - // EPOLLOUT: The associated fd is available for write operations. - // EPOLLRDHUP: Socket peer closed connection, or shut down writing half of connection. - // EPOLLERR: Error condition happened on the associated fd. - // EPOLLHUP: Hang up happened on the associated fd. - auto add_fd_to_epoll = [&](int fd) -> bool { - epoll_event event = {}; - event.data.fd = fd; - event.events = EPOLLRDHUP | EPOLLERR | EPOLLHUP; - - // If read handler was set, also listen for POLL-IN events - if (event_data->handlers.on_read) { - event.events |= EPOLLIN; - } - - // If write handler was set, also listen for POLL-OUT events - if (event_data->handlers.on_write) { - event.events |= EPOLLOUT; - } - - if (epoll_ctl(m_epoll_fd, EPOLL_CTL_ADD, fd, &event) == -1) { - LOG(ERROR) << "Failed adding FD (" << fd << ") to the poll: " << strerror(errno); - return false; - } - - // Map the fd to the event data structure - m_fd_to_event_data[fd] = event_data; - - return true; - }; - - // Add the socket fd to the poll - if (!add_fd_to_epoll(event_data->socket->getSocketFd())) { - return false; - } - - return true; -} - -bool SocketEventLoop::del_event(SocketEventLoop::EventType socket) -{ - if (!socket) { - LOG(ERROR) << "Invalid socket pointer!"; - return false; - } - - // Make sure that the fd was previously added to the poll - auto event_data_itr = m_fd_to_event_data.find(socket->getSocketFd()); - if (event_data_itr == m_fd_to_event_data.end()) { - LOG(WARNING) << "Requested to delete FD (" << socket->getSocketFd() - << ") from the poll, but it wasn't previously added."; - - return false; - } - - // Store a copy of the shared_ptr to prevent loosing the reference - // when removing the instance from the map - auto event_data = event_data_itr->second; - - // Delete the socket fd from the poll - auto error = false; - if (epoll_ctl(m_epoll_fd, EPOLL_CTL_DEL, event_data->socket->getSocketFd(), nullptr) == -1) { - LOG(ERROR) << "Failed deleting socket FD (" << event_data->socket->getSocketFd() - << ") from the poll: " << strerror(errno); - - error = true; - } - - // Erase the fd from the map - m_fd_to_event_data.erase(event_data->socket->getSocketFd()); - - return !error; -} - -int SocketEventLoop::run() -{ - // Poll events - epoll_event events[MAX_POLL_EVENTS] = {0}; - - // Convert the global event loop timeout (if set) to milliseconds - int timeout_millis = - (m_timeout > std::chrono::milliseconds::zero()) - ? static_cast( - std::chrono::duration_cast(m_timeout).count()) - : -1; - - // Poll the sockets - auto num_events = epoll_wait(m_epoll_fd, events, sizeof(events), timeout_millis); - - if (num_events == -1) { - LOG(ERROR) << "Error during epoll_wait: " << strerror(errno); - return -1; - } - - if (num_events == 0) { - // Timeout... Do nothing - return 0; - } - - // Trigger event handlers - for (int i = 0; i < num_events; i++) { - int fd = events[i].data.fd; - auto event_data_itr = m_fd_to_event_data.find(fd); - - if (event_data_itr == m_fd_to_event_data.end()) { - LOG(ERROR) << "Event on unknown FD: " << fd; - continue; - } - - // Store a copy of the shared_ptr to prevent loosing the reference - // when removing the instance from the map - auto event_data = event_data_itr->second; - auto &socket = event_data->socket; - - // Handle errors - if (events[i].events & EPOLLERR) { - - // Read the error from the socket - int error = 0; - socklen_t errlen = sizeof(error); - getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&error, &errlen); - LOG(ERROR) << "Error on FD (" << fd << "): " << strerror(error); - - // Remove the socket from the poll - del_event(socket); - - // Call the on_error handler of this socket - if (event_data->handlers.on_error) { - if (!event_data->handlers.on_error(socket, *this)) { - return -1; - } - } - - // Handle Disconnected Sockets - } else if ((events[i].events & EPOLLRDHUP) || (events[i].events & EPOLLHUP)) { - LOG(DEBUG) << "Socket with FD (" << fd << ") disconnected"; - - // Remove the socket from the poll - del_event(socket); - - // Call the on_disconnect handler of this socket - if (event_data->handlers.on_disconnect) { - if (!event_data->handlers.on_disconnect(socket, *this)) { - return -1; - } - } - - // Handle incoming data - } else if (events[i].events & EPOLLIN) { - - if (!event_data->handlers.on_read) { - LOG(WARNING) << "Incoming data on socket FD (" << fd - << ") without handler. Removing."; - del_event(socket); - } else { - if (!event_data->handlers.on_read(socket, *this)) { - return -1; - } - } - - // Handle socket is ready for write operations - } else if (events[i].events & EPOLLOUT) { - if (!event_data->handlers.on_write) { - LOG(WARNING) << "Socket FD (" << fd - << ") is ready for write, but there's no handler. Removing."; - del_event(socket); - } else { - if (!event_data->handlers.on_write(socket, *this)) { - return -1; - } - } - - } else { - LOG(ERROR) << "FD (" << fd << ") generated unknown event: " << events[i].events; - } - } - - return num_events; -} - -} // namespace beerocks diff --git a/common/beerocks/bcl/unit_tests/event_loop_impl_test.cpp b/common/beerocks/bcl/unit_tests/event_loop_impl_test.cpp new file mode 100644 index 0000000000..8c295e41a8 --- /dev/null +++ b/common/beerocks/bcl/unit_tests/event_loop_impl_test.cpp @@ -0,0 +1,109 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#include + +#include +#include + +#include + +#include + +// Initialize easylogging++ +INITIALIZE_EASYLOGGINGPP + +using namespace beerocks; + +using ::testing::InSequence; +using ::testing::Invoke; +using ::testing::StrictMock; + +/** + * @brief Mockable event handlers class + */ +class EventHandlersMock : public EventLoop::EventHandlers { + +public: + EventHandlersMock() + { + on_read = [&](int fd, EventLoop &loop) { return handle_read(fd, &loop); }; + on_write = [&](int fd, EventLoop &loop) { return handle_write(fd, &loop); }; + on_disconnect = [&](int fd, EventLoop &loop) { return handle_disconnect(fd, &loop); }; + on_error = [&](int fd, EventLoop &loop) { return handle_error(fd, &loop); }; + } + + MOCK_METHOD(bool, handle_read, (int fd, EventLoop *)); + MOCK_METHOD(bool, handle_write, (int fd, EventLoop *)); + MOCK_METHOD(bool, handle_disconnect, (int fd, EventLoop *)); + MOCK_METHOD(bool, handle_error, (int fd, EventLoop *)); +}; + +TEST(beerocks_event_loop_impl, setup) +{ + // Configure easylogging++ formatting + el::Configurations defaultConf; + defaultConf.setToDefault(); + defaultConf.setGlobally(el::ConfigurationType::Format, + "%level %datetime{%H:%m:%s:%g} <%thread> %fbase[%line] --> %msg"); + + // el::Loggers::addFlag(el::LoggingFlag::ColoredTerminalOutput); + el::Loggers::reconfigureLogger("default", defaultConf); +} + +TEST(beerocks_event_loop_impl, simple_read_write) +{ + EventLoopImpl loop; + StrictMock reader; + StrictMock writer; + + // Disable the on_write handler (to prevent the loop from firing "write ready" events) + reader.on_write = nullptr; + + // Create a socketpair for reader/writer simulation + int sv[2]; + int rc = socketpair(AF_UNIX, SOCK_STREAM, 0, sv); + ASSERT_NE(-1, rc); + + int writer_fd = sv[0]; + int reader_fd = sv[1]; + + { + InSequence sequence; + + EXPECT_CALL(writer, handle_write(writer_fd, &loop)) + .WillOnce(Invoke([](int fd, EventLoop *loop) -> bool { + // Send a dummy byte and remove the socket from the loop to prevent additional events + EXPECT_EQ(1, send(fd, "X", 1, 0)); + EXPECT_TRUE(loop->remove_handlers(fd)); + return true; + })); + + EXPECT_CALL(reader, handle_read(reader_fd, &loop)) + .WillOnce(Invoke([](int fd, EventLoop *loop) -> bool { + char dummy; + EXPECT_EQ(1, read(fd, &dummy, 1)); + return true; + })); + }; + + // Add the reader/writer sockets and handlers to the loop + ASSERT_TRUE(loop.register_handlers(writer_fd, writer)); + ASSERT_TRUE(loop.register_handlers(reader_fd, reader)); + + // Run the loop + ASSERT_EQ(1, loop.run()); + ASSERT_EQ(1, loop.run()); + + // Delete the reader socket + ASSERT_TRUE(loop.remove_handlers(reader_fd)); + + // Close the file descriptors + close(writer_fd); + close(reader_fd); +} diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.cpp b/framework/transport/ieee1905_transport/ieee1905_transport.cpp index dc22c32a12..8f94a4dacf 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport.cpp @@ -8,7 +8,7 @@ #include #include -#include +#include #include @@ -35,16 +35,16 @@ static constexpr int listen_buffer_size = 10; void Ieee1905Transport::run() { // Broker server UDS socket - SocketServer broker_server_socket(std::string(TMP_PATH "/" BEEROCKS_BROKER_UDS), - listen_buffer_size); + auto server_socket = std::make_shared( + std::string(TMP_PATH "/" BEEROCKS_BROKER_UDS), listen_buffer_size); // Broker socket based event loop - SocketEventLoop broker_event_loop; + auto event_loop = std::make_shared(); LOG(INFO) << "Starting 1905 transport..."; // Create the broker server - m_broker = std::make_unique(broker_server_socket, broker_event_loop); + m_broker = std::make_unique(server_socket, event_loop); LOG_IF(!m_broker, FATAL) << "Failed creating broker server!"; // Register broker handlers for internal and external messages @@ -62,6 +62,11 @@ void Ieee1905Transport::run() return true; }); + if (!m_broker->start()) { + MAPF_ERR("cannot open start broker."); + return; + } + // init netlink socket if (!open_netlink_socket()) { MAPF_ERR("cannot open netlink socket."); @@ -77,34 +82,32 @@ void Ieee1905Transport::run() }); // Add the netlink socket into the broker's event loop - broker_event_loop.add_event(netlink_socket, - { - // Accept incoming connections - .on_read = - [&](BrokerServer::BrokerEventLoop::EventType socket, - BrokerServer::BrokerEventLoop::EventLoopType &loop) { - LOG(DEBUG) << "incoming message on the netlink socket"; - handle_netlink_pollin_event(); - return true; - }, - - // Not implemented - .on_write = nullptr, - - // Fail on server socket disconnections or errors - .on_disconnect = - [&](BrokerServer::BrokerEventLoop::EventType socket, - BrokerServer::BrokerEventLoop::EventLoopType &loop) { - LOG(ERROR) << "netlink socket disconnected"; - return false; - }, - .on_error = - [&](BrokerServer::BrokerEventLoop::EventType socket, - BrokerServer::BrokerEventLoop::EventLoopType &loop) { - LOG(ERROR) << "netlink socket error"; - return false; - }, - }); + event_loop->register_handlers(netlink_socket->getSocketFd(), + { + // Accept incoming connections + .on_read = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) + << "incoming message on the netlink socket"; + handle_netlink_pollin_event(); + return true; + }, + + // Not implemented + .on_write = nullptr, + + // Fail on server socket disconnections or errors + .on_disconnect = + [&](int fd, EventLoop &loop) { + LOG(ERROR) << "netlink socket disconnected"; + return false; + }, + .on_error = + [&](int fd, EventLoop &loop) { + LOG(ERROR) << "netlink socket error"; + return false; + }, + }); // Run the broker event loop for (;;) { diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp index 7d6fbc5e2d..10e42d9268 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp @@ -41,71 +41,67 @@ static bool is_restricted_type(uint32_t type) /////////////////////////// Broker Implementation //////////////////////////// ////////////////////////////////////////////////////////////////////////////// -BrokerServer::BrokerServer(SocketServer &broker_server, BrokerEventLoop &event_loop) - : m_broker_server(std::make_shared(broker_server)), - m_broker_event_loop(event_loop) +BrokerServer::BrokerServer(const std::shared_ptr &server_socket, + const std::shared_ptr &event_loop) + : m_server_socket(server_socket), m_event_loop(event_loop) { - LOG_IF(!m_broker_server, FATAL) << "Failed allocating memory!"; + LOG_IF(!m_server_socket, FATAL) << "Server socket is a null pointer!"; + LOG_IF(!m_event_loop, FATAL) << "Event loop is a null pointer!"; // Check for errors - LOG_IF(!m_broker_server->getError().empty(), FATAL) - << "Failed opening server socket: " << m_broker_server - << " [ERROR: " << m_broker_server->getError() << "]"; - - // Add the socket to the poll - LOG_IF( - !m_broker_event_loop.add_event( - m_broker_server, - { - // Accept incoming connections - .on_read = - [&](BrokerEventLoop::EventType socket, BrokerEventLoop::EventLoopType &loop) { - // cppcheck falsely detects the call to socket_connected() as a call to a - // virtual method from whithin the constructor, so suppress this warning, - // by explicitly calling BrokerServer::socket_connected(). - - // Handle connections to one of the server sockets - if (!BrokerServer::socket_connected( - std::dynamic_pointer_cast(socket))) { - // NOTE: Do NOT stop the broker on connection errors... - } - - return true; - }, - - // Not implemented - .on_write = nullptr, - - // Fail on server socket disconnections or errors - .on_disconnect = - [&](BrokerEventLoop::EventType socket, BrokerEventLoop::EventLoopType &loop) { - LOG(ERROR) << "Broker socket disconnected!"; - return false; - }, - .on_error = - [&](BrokerEventLoop::EventType socket, BrokerEventLoop::EventLoopType &loop) { - LOG(ERROR) << "Broker socket error!"; - return false; - }, - }), - FATAL) - << "Failed adding the broker socket into the poll"; - - LOG(INFO) << "Broker server is running!"; + LOG_IF(!m_server_socket->getError().empty(), FATAL) + << "Failed opening server socket: " << m_server_socket + << " [ERROR: " << m_server_socket->getError() << "]"; } -bool BrokerServer::add_event(BrokerEventLoop::EventType event, - BrokerEventLoop::EventHandlers handlers) +bool BrokerServer::start() { - return m_broker_event_loop.add_event(event, handlers); + EventLoop::EventHandlers handlers{ + // Accept incoming connections + .on_read = + [&](int fd, EventLoop &loop) { + // Handle connections to one of the server sockets + if (!socket_connected()) { + // NOTE: Do NOT stop the broker on connection errors... + } + + return true; + }, + + // Not implemented + .on_write = nullptr, + + // Fail on server socket disconnections or errors + .on_disconnect = + [&](int fd, EventLoop &loop) { + LOG(ERROR) << "Broker socket disconnected!"; + return false; + }, + .on_error = + [&](int fd, EventLoop &loop) { + LOG(ERROR) << "Broker socket error!"; + return false; + }, + }; + + if (!m_event_loop->register_handlers(m_server_socket->getSocketFd(), handlers)) { + LOG(ERROR) << "Failed adding the broker server socket into the poll"; + return false; + } + + return true; } -bool BrokerServer::del_event(BrokerEventLoop::EventType event) +bool BrokerServer::stop() { return m_event_loop->remove_handlers(m_server_socket->getSocketFd()); } + +bool BrokerServer::add_event(int fd, const EventLoop::EventHandlers &handlers) { - return m_broker_event_loop.del_event(event); + return m_event_loop->register_handlers(fd, handlers); } -int BrokerServer::run() { return m_broker_event_loop.run(); } +bool BrokerServer::del_event(int fd) { return m_event_loop->remove_handlers(fd); } + +int BrokerServer::run() { return m_event_loop->run(); } bool BrokerServer::publish(const messages::Message &msg) { @@ -167,7 +163,7 @@ void BrokerServer::register_external_message_handler(MessageHandler handler) m_external_message_handler = handler; } -bool BrokerServer::handle_msg(std::shared_ptr &sd) +bool BrokerServer::handle_msg(const std::shared_ptr &sd) { // Check if the socket contains enough bytes for the header if (sd->getBytesReady() < static_cast(sizeof(messages::Message::Header))) { @@ -213,7 +209,7 @@ bool BrokerServer::handle_msg(std::shared_ptr &sd) return true; } -bool BrokerServer::handle_subscribe(std::shared_ptr &sd, +bool BrokerServer::handle_subscribe(const std::shared_ptr &sd, const messages::SubscribeMessage &msg) { // Validate the message type @@ -273,13 +269,13 @@ bool BrokerServer::handle_subscribe(std::shared_ptr &sd, return true; } -bool BrokerServer::socket_connected(std::shared_ptr sd) +bool BrokerServer::socket_connected() { // Accept the connection - auto new_socket = std::shared_ptr(sd->acceptConnections()); + auto new_socket = std::shared_ptr(m_server_socket->acceptConnections()); // Check for errors - const auto error_msg = sd->getError(); + const auto error_msg = m_server_socket->getError(); if ((!new_socket) || (!error_msg.empty())) { LOG(ERROR) << "Socket error: " << error_msg; return false; @@ -288,34 +284,33 @@ bool BrokerServer::socket_connected(std::shared_ptr sd) LOG(DEBUG) << "Accepted new connection, fd = " << new_socket->getSocketFd(); // Add the newly accepted socket into the poll - if (!m_broker_event_loop.add_event( - new_socket, - { - // Handle incoming data - .on_read = - [&](BrokerEventLoop::EventType socket, BrokerEventLoop::EventLoopType &loop) { - // NOTE: Do NOT stop the broker on parsing errors... - handle_msg(socket); - return true; - }, - - // Not implemented - .on_write = nullptr, - - // Remove the socket on disconnections or errors - .on_disconnect = - [&](BrokerEventLoop::EventType socket, BrokerEventLoop::EventLoopType &loop) { - // NOTE: Do NOT stop the broker on errors... - socket_disconnected(socket); - return true; - }, - .on_error = - [&](BrokerEventLoop::EventType socket, BrokerEventLoop::EventLoopType &loop) { - // NOTE: Do NOT stop the broker on errors... - socket_disconnected(socket); - return true; - }, - })) { + EventLoop::EventHandlers handlers{ + // Handle incoming data + .on_read = + [new_socket, this](int fd, EventLoop &loop) { + // NOTE: Do NOT stop the broker on parsing errors... + handle_msg(new_socket); + return true; + }, + + // Not implemented + .on_write = nullptr, + + // Remove the socket on disconnections or errors + .on_disconnect = + [new_socket, this](int fd, EventLoop &loop) { + // NOTE: Do NOT stop the broker on errors... + socket_disconnected(new_socket); + return true; + }, + .on_error = + [new_socket, this](int fd, EventLoop &loop) { + // NOTE: Do NOT stop the broker on errors... + socket_disconnected(new_socket); + return true; + }, + }; + if (!m_event_loop->register_handlers(new_socket->getSocketFd(), handlers)) { LOG(ERROR) << "Failed adding new socket into the poll!"; return false; } @@ -323,7 +318,7 @@ bool BrokerServer::socket_connected(std::shared_ptr sd) return true; } -bool BrokerServer::socket_disconnected(std::shared_ptr sd) +bool BrokerServer::socket_disconnected(const std::shared_ptr &sd) { LOG(DEBUG) << "Socket disconnected: FD(" << sd->getSocketFd() << ")"; diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h index a4826b8f31..0c6fb36bc5 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h @@ -33,11 +33,6 @@ namespace broker { */ class BrokerServer { public: - /** - * The type of the supported EventLoop. - */ - using BrokerEventLoop = EventLoop>; - /** * @brief Transport messages (@see Message) handler function definition. * @@ -53,27 +48,38 @@ class BrokerServer { /** * Constructor. * - * @param [in] broker_uds_path The path and file name to the server UDS file. + * @param [in] server_socket Server socket listening for incoming connections. + * @param [in] Application event loop. */ - explicit BrokerServer(SocketServer &broker_server, BrokerEventLoop &event_loop); + BrokerServer(const std::shared_ptr &server_socket, + const std::shared_ptr &event_loop); /** * Destructor. */ virtual ~BrokerServer() = default; + /** + * @brief Start the Broker's server socket. + */ + bool start(); + + /** + * @brief Stop the Broker's server socket. + */ + bool stop(); + /** * @brief Add an event to the Broker's event loop. * @see EventLoop::add_event */ - virtual bool add_event(BrokerEventLoop::EventType event, - BrokerEventLoop::EventHandlers handlers); + virtual bool add_event(int fd, const EventLoop::EventHandlers &handlers); /** * @brief Delete an event from the Broker's event loop. * @see EventLoop::del_event */ - virtual bool del_event(BrokerEventLoop::EventType event); + virtual bool del_event(int fd); /** * @brief Run the Broker's event loop. @@ -117,9 +123,9 @@ class BrokerServer { * * @param [in] sd The socket interface on which the incoming data event originated. * - * @return true on success of false otherwise. + * @return true on success and false otherwise. */ - virtual bool handle_msg(std::shared_ptr &sd); + virtual bool handle_msg(const std::shared_ptr &sd); private: /** @@ -128,38 +134,36 @@ class BrokerServer { * @param [in] sd The socket interface on which the incoming data event originated. * @param [in] msg The internal SubscribeMessage message. * - * @return true on success of false otherwise. + * @return true on success and false otherwise. */ - bool handle_subscribe(std::shared_ptr &sd, const messages::SubscribeMessage &msg); + bool handle_subscribe(const std::shared_ptr &sd, const messages::SubscribeMessage &msg); /** - * @brief Handler method for socket connections. - * - * @param [in] sd The socket interface on which the connection event originated. + * @brief Handler method to accept incoming socket connections. * - * @return true on success of false otherwise. + * @return true on success and false otherwise. */ - bool socket_connected(std::shared_ptr sd); + bool socket_connected(); /** * @brief Handler method for socket disconnections. * * @param [in] sd The socket interface on which the disconnection event originated. * - * @return true on success of false otherwise. + * @return true on success and false otherwise. */ - bool socket_disconnected(std::shared_ptr sd); + bool socket_disconnected(const std::shared_ptr &sd); private: /** * Shared pointer to the broker server socket. */ - std::shared_ptr m_broker_server = nullptr; + std::shared_ptr m_server_socket; /** * Reference to the event loop that should be used by the broker. */ - BrokerEventLoop &m_broker_event_loop; + std::shared_ptr m_event_loop; /** * Map for storing Socket->CMDU Type subscriptions. diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp index 548bad7ecc..5318169b55 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp @@ -95,7 +95,7 @@ void Ieee1905Transport::update_network_interfaces( if (updated_network_interfaces.count(ifname) == 0) { MAPF_INFO("interface " << ifname << " is no longer used."); if (network_interface.fd) { - m_broker->del_event(network_interface.fd); + m_broker->del_event(network_interface.fd->getSocketFd()); close(network_interface.fd->getSocketFd()); network_interface.fd = nullptr; } @@ -132,34 +132,31 @@ void Ieee1905Transport::update_network_interfaces( // add interface raw socket fd to poller loop (unless it's a bridge interface) if (!network_interfaces_[ifname].is_bridge && network_interfaces_[ifname].fd) { // TODO: Move to a separate method - m_broker->add_event( - network_interfaces_[ifname].fd, - { - // Accept incoming connections - .on_read = - [&](broker::BrokerServer::BrokerEventLoop::EventType socket, - broker::BrokerServer::BrokerEventLoop::EventLoopType &loop) { - LOG(DEBUG) << "Incoming message on interface fd: " - << socket->getSocketFd(); - handle_interface_pollin_event(socket->getSocketFd()); - return true; - }, - - // Not implemented - .on_write = nullptr, - .on_disconnect = nullptr, - - // Handle interface errors - .on_error = - [&](broker::BrokerServer::BrokerEventLoop::EventType socket, - broker::BrokerServer::BrokerEventLoop::EventLoopType &loop) { - LOG(DEBUG) << "Error on interface fd: " << socket->getSocketFd() - << " (disabling it)."; - - handle_interface_status_change(socket->getSocketFd(), false); - return true; - }, - }); + m_broker->add_event(network_interfaces_[ifname].fd->getSocketFd(), + { + // Accept incoming connections + .on_read = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) + << "Incoming message on interface fd: " << fd; + handle_interface_pollin_event(fd); + return true; + }, + + // Not implemented + .on_write = nullptr, + .on_disconnect = nullptr, + + // Handle interface errors + .on_error = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) << "Error on interface fd: " << fd + << " (disabling it)."; + + handle_interface_status_change(fd, false); + return true; + }, + }); } } @@ -283,7 +280,7 @@ void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bo MAPF_INFO("interface " << ifname << " is now " << (is_active ? "active" : "inactive") << "."); if (!is_active && network_interfaces_[ifname].fd) { - m_broker->del_event(network_interfaces_[ifname].fd); + m_broker->del_event(network_interfaces_[ifname].fd->getSocketFd()); close(network_interfaces_[ifname].fd->getSocketFd()); network_interfaces_[ifname].fd = nullptr; } @@ -294,34 +291,31 @@ void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bo } if (network_interfaces_[ifname].fd) // Handle network events - m_broker->add_event( - network_interfaces_[ifname].fd, - { - // Accept incoming connections - .on_read = - [&](broker::BrokerServer::BrokerEventLoop::EventType socket, - broker::BrokerServer::BrokerEventLoop::EventLoopType &loop) { - LOG(DEBUG) - << "Incoming message on interface fd: " << socket->getSocketFd(); - handle_interface_pollin_event(socket->getSocketFd()); - return true; - }, - - // Not implemented - .on_write = nullptr, - .on_disconnect = nullptr, - - // Handle interface errors - .on_error = - [&](broker::BrokerServer::BrokerEventLoop::EventType socket, - broker::BrokerServer::BrokerEventLoop::EventLoopType &loop) { - LOG(DEBUG) << "Error on interface fd: " << socket->getSocketFd() - << " (disabling it)."; - - handle_interface_status_change(socket->getSocketFd(), false); - return true; - }, - }); + m_broker->add_event(network_interfaces_[ifname].fd->getSocketFd(), + { + // Accept incoming connections + .on_read = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) + << "Incoming message on interface fd: " << fd; + handle_interface_pollin_event(fd); + return true; + }, + + // Not implemented + .on_write = nullptr, + .on_disconnect = nullptr, + + // Handle interface errors + .on_error = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) << "Error on interface fd: " << fd + << " (disabling it)."; + + handle_interface_status_change(fd, false); + return true; + }, + }); } } diff --git a/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp b/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp index 26b56bdb61..95fa796aad 100644 --- a/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp +++ b/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp @@ -12,8 +12,8 @@ #include +#include #include -#include #include #include @@ -65,7 +65,7 @@ class BrokerServerWrapper : public BrokerServer { protected: bool m_error_occurred = false; - virtual bool handle_msg(std::shared_ptr &sd) override + bool handle_msg(const std::shared_ptr &sd) override { if (BrokerServer::handle_msg(sd) == false) { m_error_occurred = true; @@ -97,9 +97,11 @@ TEST(broker_server, setup) TEST(broker_server, invalid_message_magic) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Register a dummy internal message handler broker_wrapper.register_internal_message_handler( @@ -124,13 +126,17 @@ TEST(broker_server, invalid_message_magic) ASSERT_TRUE(messages::send_transport_message(sock1, dummy)); ASSERT_EQ(1, broker_wrapper.run()); ASSERT_FALSE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, subscribe_empty_message) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Create a subscribe message SubscribeMessage subscribe; @@ -144,13 +150,17 @@ TEST(broker_server, subscribe_empty_message) ASSERT_EQ(1, broker_wrapper.run()); // Process ASSERT_TRUE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, subscribe_single_type) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Create a subscribe message SubscribeMessage subscribe; @@ -166,13 +176,17 @@ TEST(broker_server, subscribe_single_type) ASSERT_EQ(1, broker_wrapper.run()); // Process ASSERT_FALSE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, subscribe_multiple_types) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Create a subscribe message SubscribeMessage subscribe; @@ -190,13 +204,17 @@ TEST(broker_server, subscribe_multiple_types) ASSERT_EQ(1, broker_wrapper.run()); // Process ASSERT_FALSE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, unsubscribe_empty_message) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Create a subscribe message SubscribeMessage subscribe; @@ -210,13 +228,17 @@ TEST(broker_server, unsubscribe_empty_message) ASSERT_EQ(1, broker_wrapper.run()); // Process ASSERT_TRUE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, unsubscribe_single_type) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Create a subscribe message SubscribeMessage subscribe; @@ -232,13 +254,17 @@ TEST(broker_server, unsubscribe_single_type) ASSERT_EQ(1, broker_wrapper.run()); // Process ASSERT_FALSE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, unsubscribe_multiple_types) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Create a subscribe message SubscribeMessage subscribe; @@ -256,13 +282,17 @@ TEST(broker_server, unsubscribe_multiple_types) ASSERT_EQ(1, broker_wrapper.run()); // Process ASSERT_FALSE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, subscribe_unsubscribe) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Create a subscribe message SubscribeMessage subscribe; @@ -286,13 +316,17 @@ TEST(broker_server, subscribe_unsubscribe) ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); ASSERT_EQ(1, broker_wrapper.run()); // Process ASSERT_FALSE(broker_wrapper.error()); + + ASSERT_TRUE(broker_wrapper.stop()); } TEST(broker_server, publish_internal_message) { - SocketServer broker_server_socket(broker_uds_file, broker_listen_buffer); - SocketEventLoop broker_event_loop(broker_timeout); - BrokerServerWrapper broker_wrapper(broker_server_socket, broker_event_loop); + auto server_socket = std::make_shared(broker_uds_file, broker_listen_buffer); + auto event_loop = std::make_shared(broker_timeout); + BrokerServerWrapper broker_wrapper(server_socket, event_loop); + + ASSERT_TRUE(broker_wrapper.start()); // Connect to the broker SocketClient sock1(broker_uds_file); @@ -335,6 +369,8 @@ TEST(broker_server, publish_internal_message) // Validate the number of interfaces matches the sent message ASSERT_TRUE(iface_indication_msg_rx.metadata()->numInterfaces == NUM_OF_IFACES); + + ASSERT_TRUE(broker_wrapper.stop()); } } // namespace tests From 648c47f847bff1d241ae78d5fcdc9887ce7f80ca Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Tue, 11 Aug 2020 18:44:22 +0200 Subject: [PATCH 3/7] transport: move event loop to main function https://jira.prplfoundation.org/browse/PPM-18 This is a preparatory commit. Application event loop is currently defined in Ieee1905Transport::run(). Since event loop is going to be a dependency for the interface state monitor as well as for the transport class, move it to the main function so it can later be reused. The event loop must be then injected as a parameter in constructor to all classes that depend on it like, for example, the BrokerServer class. Since event loop is now a member variable of the Ieee1905Transport class, remove the methods add_event, del_event and run which are not necessary any more and update unit tests accordingly. Signed-off-by: Mario Maz --- .../ieee1905_transport/ieee1905_transport.cpp | 72 ++++++------- .../ieee1905_transport/ieee1905_transport.h | 13 +++ .../ieee1905_transport_broker.cpp | 9 -- .../ieee1905_transport_broker.h | 20 +--- .../ieee1905_transport_network.cpp | 102 +++++++++--------- .../transport/ieee1905_transport/main.cpp | 17 ++- .../test/ieee1905_transport_broker_tests.cpp | 40 +++---- 7 files changed, 132 insertions(+), 141 deletions(-) diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.cpp b/framework/transport/ieee1905_transport/ieee1905_transport.cpp index 8f94a4dacf..48332d0b0d 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport.cpp @@ -8,7 +8,6 @@ #include #include -#include #include @@ -32,19 +31,22 @@ static constexpr int listen_buffer_size = 10; /////////////////////////////// Implementation /////////////////////////////// ////////////////////////////////////////////////////////////////////////////// +Ieee1905Transport::Ieee1905Transport(const std::shared_ptr &event_loop) + : m_event_loop(event_loop) +{ + LOG_IF(!m_event_loop, FATAL) << "Event loop is a null pointer!"; +} + void Ieee1905Transport::run() { // Broker server UDS socket auto server_socket = std::make_shared( std::string(TMP_PATH "/" BEEROCKS_BROKER_UDS), listen_buffer_size); - // Broker socket based event loop - auto event_loop = std::make_shared(); - LOG(INFO) << "Starting 1905 transport..."; // Create the broker server - m_broker = std::make_unique(server_socket, event_loop); + m_broker = std::make_unique(server_socket, m_event_loop); LOG_IF(!m_broker, FATAL) << "Failed creating broker server!"; // Register broker handlers for internal and external messages @@ -82,40 +84,32 @@ void Ieee1905Transport::run() }); // Add the netlink socket into the broker's event loop - event_loop->register_handlers(netlink_socket->getSocketFd(), - { - // Accept incoming connections - .on_read = - [&](int fd, EventLoop &loop) { - LOG(DEBUG) - << "incoming message on the netlink socket"; - handle_netlink_pollin_event(); - return true; - }, - - // Not implemented - .on_write = nullptr, - - // Fail on server socket disconnections or errors - .on_disconnect = - [&](int fd, EventLoop &loop) { - LOG(ERROR) << "netlink socket disconnected"; - return false; - }, - .on_error = - [&](int fd, EventLoop &loop) { - LOG(ERROR) << "netlink socket error"; - return false; - }, - }); - - // Run the broker event loop - for (;;) { - if (m_broker->run() < 0) { - LOG(ERROR) << "Broker event loop failure!"; - return; - } - } + m_event_loop->register_handlers(netlink_socket->getSocketFd(), + { + // Accept incoming connections + .on_read = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) + << "incoming message on the netlink socket"; + handle_netlink_pollin_event(); + return true; + }, + + // Not implemented + .on_write = nullptr, + + // Fail on server socket disconnections or errors + .on_disconnect = + [&](int fd, EventLoop &loop) { + LOG(ERROR) << "netlink socket disconnected"; + return false; + }, + .on_error = + [&](int fd, EventLoop &loop) { + LOG(ERROR) << "netlink socket error"; + return false; + }, + }); } } // namespace transport diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.h b/framework/transport/ieee1905_transport/ieee1905_transport.h index 0eb12d39c0..a7e04cd764 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport.h @@ -12,6 +12,8 @@ #include #include +#include + #include "ieee1905_transport_broker.h" #include @@ -53,9 +55,20 @@ namespace transport { class Ieee1905Transport { public: + /** + * Class constructor + * + * @param event_loop Event loop to wait for I/O events. + */ + explicit Ieee1905Transport(const std::shared_ptr &event_loop); void run(); private: + /** + * Application event loop used by the process to wait for I/O events. + */ + std::shared_ptr m_event_loop; + std::string if_index2name(unsigned int index) { char ifname[IF_NAMESIZE] = {0}; diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp index 10e42d9268..1c43f98b49 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp @@ -94,15 +94,6 @@ bool BrokerServer::start() bool BrokerServer::stop() { return m_event_loop->remove_handlers(m_server_socket->getSocketFd()); } -bool BrokerServer::add_event(int fd, const EventLoop::EventHandlers &handlers) -{ - return m_event_loop->register_handlers(fd, handlers); -} - -bool BrokerServer::del_event(int fd) { return m_event_loop->remove_handlers(fd); } - -int BrokerServer::run() { return m_event_loop->run(); } - bool BrokerServer::publish(const messages::Message &msg) { messages::SubscribeMessage::MsgType msg_opcode; diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h index 0c6fb36bc5..d7cf950dfa 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h @@ -69,24 +69,6 @@ class BrokerServer { */ bool stop(); - /** - * @brief Add an event to the Broker's event loop. - * @see EventLoop::add_event - */ - virtual bool add_event(int fd, const EventLoop::EventHandlers &handlers); - - /** - * @brief Delete an event from the Broker's event loop. - * @see EventLoop::del_event - */ - virtual bool del_event(int fd); - - /** - * @brief Run the Broker's event loop. - * @see EventLoop::run - */ - virtual int run(); - /** * @brief Publishes the message with the broker subscribers. * Sends the message to all the clients that subscribed for the type of the message. @@ -161,7 +143,7 @@ class BrokerServer { std::shared_ptr m_server_socket; /** - * Reference to the event loop that should be used by the broker. + * Application event loop to use by the broker to wait for I/O events. */ std::shared_ptr m_event_loop; diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp index 5318169b55..15851317bb 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp @@ -95,7 +95,7 @@ void Ieee1905Transport::update_network_interfaces( if (updated_network_interfaces.count(ifname) == 0) { MAPF_INFO("interface " << ifname << " is no longer used."); if (network_interface.fd) { - m_broker->del_event(network_interface.fd->getSocketFd()); + m_event_loop->remove_handlers(network_interface.fd->getSocketFd()); close(network_interface.fd->getSocketFd()); network_interface.fd = nullptr; } @@ -132,31 +132,30 @@ void Ieee1905Transport::update_network_interfaces( // add interface raw socket fd to poller loop (unless it's a bridge interface) if (!network_interfaces_[ifname].is_bridge && network_interfaces_[ifname].fd) { // TODO: Move to a separate method - m_broker->add_event(network_interfaces_[ifname].fd->getSocketFd(), - { - // Accept incoming connections - .on_read = - [&](int fd, EventLoop &loop) { - LOG(DEBUG) - << "Incoming message on interface fd: " << fd; - handle_interface_pollin_event(fd); - return true; - }, - - // Not implemented - .on_write = nullptr, - .on_disconnect = nullptr, - - // Handle interface errors - .on_error = - [&](int fd, EventLoop &loop) { - LOG(DEBUG) << "Error on interface fd: " << fd - << " (disabling it)."; - - handle_interface_status_change(fd, false); - return true; - }, - }); + m_event_loop->register_handlers( + network_interfaces_[ifname].fd->getSocketFd(), + { + // Accept incoming connections + .on_read = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) << "Incoming message on interface fd: " << fd; + handle_interface_pollin_event(fd); + return true; + }, + + // Not implemented + .on_write = nullptr, + .on_disconnect = nullptr, + + // Handle interface errors + .on_error = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) << "Error on interface fd: " << fd << " (disabling it)."; + + handle_interface_status_change(fd, false); + return true; + }, + }); } } @@ -280,7 +279,7 @@ void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bo MAPF_INFO("interface " << ifname << " is now " << (is_active ? "active" : "inactive") << "."); if (!is_active && network_interfaces_[ifname].fd) { - m_broker->del_event(network_interfaces_[ifname].fd->getSocketFd()); + m_event_loop->remove_handlers(network_interfaces_[ifname].fd->getSocketFd()); close(network_interfaces_[ifname].fd->getSocketFd()); network_interfaces_[ifname].fd = nullptr; } @@ -291,31 +290,30 @@ void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bo } if (network_interfaces_[ifname].fd) // Handle network events - m_broker->add_event(network_interfaces_[ifname].fd->getSocketFd(), - { - // Accept incoming connections - .on_read = - [&](int fd, EventLoop &loop) { - LOG(DEBUG) - << "Incoming message on interface fd: " << fd; - handle_interface_pollin_event(fd); - return true; - }, - - // Not implemented - .on_write = nullptr, - .on_disconnect = nullptr, - - // Handle interface errors - .on_error = - [&](int fd, EventLoop &loop) { - LOG(DEBUG) << "Error on interface fd: " << fd - << " (disabling it)."; - - handle_interface_status_change(fd, false); - return true; - }, - }); + m_event_loop->register_handlers( + network_interfaces_[ifname].fd->getSocketFd(), + { + // Accept incoming connections + .on_read = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) << "Incoming message on interface fd: " << fd; + handle_interface_pollin_event(fd); + return true; + }, + + // Not implemented + .on_write = nullptr, + .on_disconnect = nullptr, + + // Handle interface errors + .on_error = + [&](int fd, EventLoop &loop) { + LOG(DEBUG) << "Error on interface fd: " << fd << " (disabling it)."; + + handle_interface_status_change(fd, false); + return true; + }, + }); } } diff --git a/framework/transport/ieee1905_transport/main.cpp b/framework/transport/ieee1905_transport/main.cpp index 6c0b357017..5dbdec1d91 100644 --- a/framework/transport/ieee1905_transport/main.cpp +++ b/framework/transport/ieee1905_transport/main.cpp @@ -8,19 +8,32 @@ #include "ieee1905_transport.h" +#include + #include #include +using namespace beerocks; using namespace beerocks::transport; int main(int argc, char *argv[]) { mapf::Logger::Instance().LoggerInit("transport"); - Ieee1905Transport ieee1905_transport; - MAPF_INFO("starting main loop..."); + auto event_loop = std::make_shared(); + + Ieee1905Transport ieee1905_transport(event_loop); + ieee1905_transport.run(); + // Run the application event loop + MAPF_INFO("starting main loop..."); + for (;;) { + if (event_loop->run() < 0) { + LOG(ERROR) << "Broker event loop failure!"; + return -1; + } + } MAPF_INFO("done"); return 0; diff --git a/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp b/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp index 95fa796aad..ea0a30131e 100644 --- a/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp +++ b/framework/transport/ieee1905_transport/test/ieee1905_transport_broker_tests.cpp @@ -115,16 +115,16 @@ TEST(broker_server, invalid_message_magic) header.magic = INVALID_MAGIC; SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection LOG(DEBUG) << "Sending INVALID magic..."; ASSERT_TRUE(messages::send_transport_message(sock1, dummy, &header)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_TRUE(broker_wrapper.error()); LOG(DEBUG) << "Sending VALID magic..."; ASSERT_TRUE(messages::send_transport_message(sock1, dummy)); - ASSERT_EQ(1, broker_wrapper.run()); + ASSERT_EQ(1, event_loop->run()); ASSERT_FALSE(broker_wrapper.error()); ASSERT_TRUE(broker_wrapper.stop()); @@ -144,10 +144,10 @@ TEST(broker_server, subscribe_empty_message) // Connect to the broker and send the message SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_TRUE(broker_wrapper.error()); @@ -170,10 +170,10 @@ TEST(broker_server, subscribe_single_type) // Connect to the broker and send the message SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_FALSE(broker_wrapper.error()); @@ -198,10 +198,10 @@ TEST(broker_server, subscribe_multiple_types) // Connect to the broker and send the message SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_FALSE(broker_wrapper.error()); @@ -222,10 +222,10 @@ TEST(broker_server, unsubscribe_empty_message) // Connect to the broker and send the message SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_TRUE(broker_wrapper.error()); @@ -248,10 +248,10 @@ TEST(broker_server, unsubscribe_single_type) // Connect to the broker and send the message SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_FALSE(broker_wrapper.error()); @@ -276,10 +276,10 @@ TEST(broker_server, unsubscribe_multiple_types) // Connect to the broker and send the message SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_FALSE(broker_wrapper.error()); @@ -304,17 +304,17 @@ TEST(broker_server, subscribe_unsubscribe) // Connect to the broker and send the message SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_FALSE(broker_wrapper.error()); ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_FALSE(broker_wrapper.error()); // Unsubscribe subscribe.metadata()->type = SubscribeMessage::ReqType::UNSUBSCRIBE; ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_FALSE(broker_wrapper.error()); ASSERT_TRUE(broker_wrapper.stop()); @@ -330,7 +330,7 @@ TEST(broker_server, publish_internal_message) // Connect to the broker SocketClient sock1(broker_uds_file); - ASSERT_EQ(1, broker_wrapper.run()); // Accept the connection + ASSERT_EQ(1, event_loop->run()); // Accept the connection ASSERT_FALSE(broker_wrapper.error()); // Build a subscribe message @@ -346,7 +346,7 @@ TEST(broker_server, publish_internal_message) // Subscribe to the InterfaceConfigurationIndicationMessage message ASSERT_TRUE(messages::send_transport_message(sock1, subscribe)); - ASSERT_EQ(1, broker_wrapper.run()); // Process + ASSERT_EQ(1, event_loop->run()); // Process ASSERT_FALSE(broker_wrapper.error()); // Publish an InterfaceConfigurationIndicationMessage message to subscribers From cf17ec8e3b5b979ff38433d9caab51eca4c260d2 Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Thu, 13 Aug 2020 13:35:32 +0200 Subject: [PATCH 4/7] transport: move message broker to main function https://jira.prplfoundation.org/browse/PPM-18 Message broker is currently defined in Ieee1905Transport::run(). As well as the event loop, the message broker is a dependency of the class. In order to have testable code, move it to the main function so it can later be replaced with a mock while unit testing (in a future PR). Strictly speaking, these changes are not required for this PR but produce clearer code as all dependencies are create and injected the same way. While we are at it, optimize parameter passing in methods to register a message handler (by const-ref instead of by value). Also call the stop() method of the message broker on application exit, which was not being called. Signed-off-by: Mario Maz --- .../ieee1905_transport/ieee1905_transport.cpp | 43 ++--------- .../ieee1905_transport/ieee1905_transport.h | 11 ++- .../ieee1905_transport_broker.cpp | 4 +- .../ieee1905_transport_broker.h | 6 +- .../transport/ieee1905_transport/main.cpp | 77 ++++++++++++++++--- 5 files changed, 85 insertions(+), 56 deletions(-) diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.cpp b/framework/transport/ieee1905_transport/ieee1905_transport.cpp index 48332d0b0d..ddf43e5dc3 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport.cpp @@ -6,69 +6,38 @@ * See LICENSE file for more details. */ -#include -#include - -#include - #include "ieee1905_transport.h" -#include - namespace beerocks { namespace transport { -using broker::BrokerServer; - -////////////////////////////////////////////////////////////////////////////// -////////////////////////// Local Module Definitions ////////////////////////// -////////////////////////////////////////////////////////////////////////////// - -// Number of concurrent connections on the server socket -static constexpr int listen_buffer_size = 10; - -////////////////////////////////////////////////////////////////////////////// -/////////////////////////////// Implementation /////////////////////////////// -////////////////////////////////////////////////////////////////////////////// - -Ieee1905Transport::Ieee1905Transport(const std::shared_ptr &event_loop) - : m_event_loop(event_loop) +Ieee1905Transport::Ieee1905Transport(const std::shared_ptr &broker, + const std::shared_ptr &event_loop) + : m_broker(broker), m_event_loop(event_loop) { + LOG_IF(!m_broker, FATAL) << "Broker server is a null pointer!"; LOG_IF(!m_event_loop, FATAL) << "Event loop is a null pointer!"; } void Ieee1905Transport::run() { - // Broker server UDS socket - auto server_socket = std::make_shared( - std::string(TMP_PATH "/" BEEROCKS_BROKER_UDS), listen_buffer_size); - LOG(INFO) << "Starting 1905 transport..."; - // Create the broker server - m_broker = std::make_unique(server_socket, m_event_loop); - LOG_IF(!m_broker, FATAL) << "Failed creating broker server!"; - // Register broker handlers for internal and external messages m_broker->register_external_message_handler( - [&](std::unique_ptr &msg, BrokerServer &broker) -> bool { + [&](std::unique_ptr &msg, broker::BrokerServer &broker) -> bool { LOG(DEBUG) << "Processing external message: " << uint32_t(msg->type()); handle_broker_pollin_event(msg); return true; }); m_broker->register_internal_message_handler( - [&](std::unique_ptr &msg, BrokerServer &broker) -> bool { + [&](std::unique_ptr &msg, broker::BrokerServer &broker) -> bool { LOG(DEBUG) << "Processing internal message: " << uint32_t(msg->type()); handle_broker_pollin_event(msg); return true; }); - if (!m_broker->start()) { - MAPF_ERR("cannot open start broker."); - return; - } - // init netlink socket if (!open_netlink_socket()) { MAPF_ERR("cannot open netlink socket."); diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.h b/framework/transport/ieee1905_transport/ieee1905_transport.h index a7e04cd764..851ea9cf30 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport.h @@ -58,12 +58,19 @@ class Ieee1905Transport { /** * Class constructor * + * @param broker Message broker. * @param event_loop Event loop to wait for I/O events. */ - explicit Ieee1905Transport(const std::shared_ptr &event_loop); + Ieee1905Transport(const std::shared_ptr &broker, + const std::shared_ptr &event_loop); void run(); private: + /** + * Message broker implementing the publish/subscribe design pattern. + */ + std::shared_ptr m_broker; + /** * Application event loop used by the process to wait for I/O events. */ @@ -100,8 +107,6 @@ class Ieee1905Transport { // netlink socket file descriptor (used to track network interface status) int netlink_fd_ = -1; - std::unique_ptr m_broker; - uint16_t message_id_ = 0; uint8_t al_mac_addr_[ETH_ALEN] = {0}; diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp index 1c43f98b49..cdfd4b13b1 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.cpp @@ -142,13 +142,13 @@ bool BrokerServer::publish(const messages::Message &msg) return true; } -void BrokerServer::register_internal_message_handler(MessageHandler handler) +void BrokerServer::register_internal_message_handler(const MessageHandler &handler) { LOG_IF(m_internal_message_handler, WARNING) << "Overriding previously registered handler"; m_internal_message_handler = handler; } -void BrokerServer::register_external_message_handler(MessageHandler handler) +void BrokerServer::register_external_message_handler(const MessageHandler &handler) { LOG_IF(m_internal_message_handler, WARNING) << "Overriding previously registered handler"; m_external_message_handler = handler; diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h index d7cf950dfa..6f95b79c1c 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_broker.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport_broker.h @@ -29,7 +29,7 @@ namespace broker { * The broker accepts connection over a SocketServer. * Once connected to the server, a client can subscribe to CMDU types. * Message filtering is implemented inside the server, so that clients receive only - * the message types they subscribed to. + * the message types they are subscribed to. */ class BrokerServer { public: @@ -88,7 +88,7 @@ class BrokerServer { * * @param [in] handler Handler function. */ - virtual void register_internal_message_handler(MessageHandler handler); + virtual void register_internal_message_handler(const MessageHandler &handler); /** * @brief Register a handler function for external (CMDU_TX/CMDU_RX) messages @@ -97,7 +97,7 @@ class BrokerServer { * * @param [in] handler Handler function. */ - virtual void register_external_message_handler(MessageHandler handler); + virtual void register_external_message_handler(const MessageHandler &handler); protected: /** diff --git a/framework/transport/ieee1905_transport/main.cpp b/framework/transport/ieee1905_transport/main.cpp index 5dbdec1d91..c3e96aacde 100644 --- a/framework/transport/ieee1905_transport/main.cpp +++ b/framework/transport/ieee1905_transport/main.cpp @@ -8,6 +8,8 @@ #include "ieee1905_transport.h" +#include +#include #include #include @@ -16,25 +18,78 @@ using namespace beerocks; using namespace beerocks::transport; +static std::shared_ptr create_event_loop() +{ + // Create application event loop to wait for blocking I/O operations. + return std::make_shared(); +} + +static std::shared_ptr +create_broker_server(const std::shared_ptr &event_loop) +{ + // UDS path for broker server socker; + constexpr const char *broker_uds_path = TMP_PATH "/" BEEROCKS_BROKER_UDS; + + // Number of concurrent connections on the server socket + constexpr int listen_buffer_size = 10; + + // Create the server UDS socket for the message broker + auto server_socket = std::make_shared(broker_uds_path, listen_buffer_size); + + // Create the broker server + return std::make_shared(server_socket, event_loop); +} + int main(int argc, char *argv[]) { mapf::Logger::Instance().LoggerInit("transport"); - auto event_loop = std::make_shared(); + /** + * Create required objects in the order defined by the dependency tree. + */ + auto event_loop = create_event_loop(); + auto broker = create_broker_server(event_loop); + + /** + * Application exit code: 0 on success and -1 on failure. + * From this point on, there's a single exit point to allow for start/stop methods to be + * cleanly called in pairs. + */ + int exit_code = 0; - Ieee1905Transport ieee1905_transport(event_loop); + /** + * Create the IEEE1905 transport process. + */ + Ieee1905Transport ieee1905_transport(broker, event_loop); - ieee1905_transport.run(); + /** + * Start the message broker + */ + if (broker->start()) { - // Run the application event loop - MAPF_INFO("starting main loop..."); - for (;;) { - if (event_loop->run() < 0) { - LOG(ERROR) << "Broker event loop failure!"; - return -1; + /** + * Start the IEEE1905 transport process + */ + ieee1905_transport.run(); + + /** + * Run the application event loop + */ + MAPF_INFO("starting main loop..."); + while (0 == exit_code) { + if (event_loop->run() < 0) { + LOG(ERROR) << "Broker event loop failure!"; + exit_code = -1; + } } + MAPF_INFO("done"); + + broker->stop(); + + } else { + LOG(ERROR) << "Unable to start message broker!"; + exit_code = -1; } - MAPF_INFO("done"); - return 0; + return exit_code; } From 2736c4d93e34552c5fdf6a9023e3d97905a6a316 Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Fri, 31 Jul 2020 19:14:33 +0200 Subject: [PATCH 5/7] bcl: add InterfaceStateManager and its dependencies https://jira.prplfoundation.org/browse/PPM-18 Create InterfaceStateManager class and all its dependencies. Follow-up commits will use this class in the ieee1905_transport process as a new mechanism to detect changes in the state of network interfaces, based on the new sockets API. Signed-off-by: Mario Maz --- common/beerocks/bcl/CMakeLists.txt | 1 + .../bcl/include/bcl/network/file_descriptor.h | 37 ++ .../bcl/network/file_descriptor_impl.h | 143 +++++++ .../bcl/network/interface_flags_reader.h | 38 ++ .../bcl/network/interface_flags_reader_impl.h | 32 ++ .../bcl/network/interface_state_manager.h | 28 ++ .../network/interface_state_manager_impl.h | 120 ++++++ .../bcl/network/interface_state_monitor.h | 94 +++++ .../network/interface_state_monitor_impl.h | 90 +++++ .../network/interface_state_monitor_mock.h | 35 ++ .../bcl/network/interface_state_reader.h | 37 ++ .../bcl/network/interface_state_reader_impl.h | 49 +++ .../bcl/network/interface_state_reader_mock.h | 27 ++ .../bcl/include/bcl/network/sockets.h | 184 +++++++++ .../bcl/include/bcl/network/sockets_impl.h | 348 ++++++++++++++++++ .../network/interface_flags_reader_impl.cpp | 37 ++ .../network/interface_state_manager_impl.cpp | 79 ++++ .../network/interface_state_monitor_impl.cpp | 75 ++++ .../network/interface_state_reader_impl.cpp | 36 ++ .../interface_state_manager_impl_test.cpp | 150 ++++++++ 20 files changed, 1640 insertions(+) create mode 100644 common/beerocks/bcl/include/bcl/network/file_descriptor.h create mode 100644 common/beerocks/bcl/include/bcl/network/file_descriptor_impl.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_flags_reader.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_flags_reader_impl.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_manager.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_manager_impl.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_monitor.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_monitor_impl.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_monitor_mock.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_reader.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_reader_impl.h create mode 100644 common/beerocks/bcl/include/bcl/network/interface_state_reader_mock.h create mode 100644 common/beerocks/bcl/include/bcl/network/sockets.h create mode 100644 common/beerocks/bcl/include/bcl/network/sockets_impl.h create mode 100644 common/beerocks/bcl/source/network/interface_flags_reader_impl.cpp create mode 100644 common/beerocks/bcl/source/network/interface_state_manager_impl.cpp create mode 100644 common/beerocks/bcl/source/network/interface_state_monitor_impl.cpp create mode 100644 common/beerocks/bcl/source/network/interface_state_reader_impl.cpp create mode 100644 common/beerocks/bcl/unit_tests/interface_state_manager_impl_test.cpp diff --git a/common/beerocks/bcl/CMakeLists.txt b/common/beerocks/bcl/CMakeLists.txt index 9048f458a8..5859173d51 100644 --- a/common/beerocks/bcl/CMakeLists.txt +++ b/common/beerocks/bcl/CMakeLists.txt @@ -47,6 +47,7 @@ if (BUILD_TESTS) ${bcl_sources} ${MODULE_PATH}/unit_tests/network_utils_test.cpp ${MODULE_PATH}/unit_tests/event_loop_impl_test.cpp + ${MODULE_PATH}/unit_tests/interface_state_manager_impl_test.cpp ${MODULE_PATH}/unit_tests/wireless_utils_test.cpp ) add_executable(${TEST_PROJECT_NAME} diff --git a/common/beerocks/bcl/include/bcl/network/file_descriptor.h b/common/beerocks/bcl/include/bcl/network/file_descriptor.h new file mode 100644 index 0000000000..25b16241bd --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/file_descriptor.h @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_FILE_DESCRIPTOR_H_ +#define BCL_NETWORK_FILE_DESCRIPTOR_H_ + +namespace beerocks { +namespace net { + +/** + * This interface models OS resources implementing the file descriptor interface. + */ +class FileDescriptor { +public: + static constexpr int invalid_descriptor = -1; + + virtual ~FileDescriptor() = default; + + /** + * @brief Returns the file descriptor. + * + * A file descriptor is a number that uniquely identifies an open file in the OS. + * + * @return File descriptor. + */ + virtual int fd() = 0; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_FILE_DESCRIPTOR_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/file_descriptor_impl.h b/common/beerocks/bcl/include/bcl/network/file_descriptor_impl.h new file mode 100644 index 0000000000..6e5b31f3da --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/file_descriptor_impl.h @@ -0,0 +1,143 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_FILE_DESCRIPTOR_IMPL_H_ +#define BCL_NETWORK_FILE_DESCRIPTOR_IMPL_H_ + +#include "file_descriptor.h" + +#include + +#include + +namespace beerocks { +namespace net { + +/** + * File descriptor implementation. + * This class is basically a wrapper around an `int fd` that automatically closes the descriptor + * on destructor and prevents from having multiple copies of the file descriptor to avoid + * programming errors. + * + * This class will be aggregated by all classes modeling OS resources implementing the file + * descriptor interface. + */ +class FileDescriptorImpl : public FileDescriptor { +public: + /** + * @brief Class constructor. + * + * @param fd File descriptor value + */ + explicit FileDescriptorImpl(int fd) : m_fd(fd) + { + if (invalid_descriptor == fd) { + LOG(ERROR) << "Invalid file descriptor: " << strerror(errno); + } + } + + /** + * @brief Copy constructor + * + * Delete copy constructor to avoid having multiple copies of the file descriptor + */ + FileDescriptorImpl(const FileDescriptorImpl &) = delete; + + /** + * @brief Move constructor. + * + * A move constructor allows the resources owned by an rvalue object to be moved into an + * lvalue without creating its copy. + */ + FileDescriptorImpl(FileDescriptorImpl &&obj) + { + this->m_fd = obj.m_fd; + obj.m_fd = FileDescriptor::invalid_descriptor; + } + + /** + * @brief Assignment operator + * + * Delete assignment operator to avoid having multiple copies of the file descriptor + */ + FileDescriptorImpl &operator=(const FileDescriptorImpl &) = delete; + + /** + * @brief Move assignment operator + * + * The move assignment operator is used to transfer ownership of a file descriptor + */ + FileDescriptorImpl &operator=(FileDescriptorImpl &&obj) + { + // Self-assignment detection + if (&obj == this) { + return *this; + } + + // Release any resource we're holding + close(); + + // Transfer ownership of obj.m_fd to m_fd + this->m_fd = obj.m_fd; + + return *this; + } + + /** + * @brief Class destructor. + * + * Close socket on destructor (if it is still open) + */ + ~FileDescriptorImpl() override { close(); } + + /** + * @brief Gets file descriptor. + * + * File descriptor can be used for example in: + * - select(), poll() or epoll() to wait for events on this descriptor. + * - send() family of functions to send data through the socket connection. + * - recv() family of functions to receive data from the socket connection. + * + * @return File descriptor value. + */ + int fd() override { return m_fd; } + +private: + /** + * File descriptor value + */ + int m_fd; + + /** + * @brief Closes file descriptor. + * + * If valid, closes file descriptor and then invalidates it. + * + * @return True on success and false otherwise (for example, if it was already closed) + */ + bool close() + { + if (FileDescriptor::invalid_descriptor == m_fd) { + return false; + } + + int rc = ::close(m_fd); + if (0 != rc) { + LOG(ERROR) << "Unable to close descriptor: " << strerror(errno); + } + + m_fd = FileDescriptor::invalid_descriptor; + + return (0 == rc); + } +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_FILE_DESCRIPTOR_IMPL_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_flags_reader.h b/common/beerocks/bcl/include/bcl/network/interface_flags_reader.h new file mode 100644 index 0000000000..fcb2a9a747 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_flags_reader.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_FLAGS_READER_H_ +#define BCL_NETWORK_INTERFACE_FLAGS_READER_H_ + +#include + +#include + +namespace beerocks { +namespace net { + +class InterfaceFlagsReader { +public: + virtual ~InterfaceFlagsReader() = default; + + /** + * @brief Reads interface flags. + * + * Reads the active flag word of the network interface with given index. + * + * @param[in] iface_name Interface name. + * @param[out] iface_flags Device flags (bitmask) + * @return True on success and false otherwise. + */ + virtual bool read_flags(const std::string &iface_name, uint16_t &iface_flags) = 0; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_FLAGS_READER_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_flags_reader_impl.h b/common/beerocks/bcl/include/bcl/network/interface_flags_reader_impl.h new file mode 100644 index 0000000000..71a7842039 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_flags_reader_impl.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_FLAGS_READER_IMPL_H_ +#define BCL_NETWORK_INTERFACE_FLAGS_READER_IMPL_H_ + +#include "interface_flags_reader.h" + +namespace beerocks { +namespace net { + +class InterfaceFlagsReaderImpl : public InterfaceFlagsReader { +public: + /** + * @brief Reads interface flags. + * + * @see InterfaceFlagsReader::read_flags + * + * This implementation uses ioctl() with SIOCGIFFLAGS to read device flags. + */ + bool read_flags(const std::string &iface_name, uint16_t &iface_flags) override; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_FLAGS_READER_IMPL_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_manager.h b/common/beerocks/bcl/include/bcl/network/interface_state_manager.h new file mode 100644 index 0000000000..4340fa059a --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_manager.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_MANAGER_H_ +#define BCL_NETWORK_INTERFACE_STATE_MANAGER_H_ + +#include "interface_state_monitor.h" +#include "interface_state_reader.h" + +namespace beerocks { +namespace net { + +/** + * The InterfaceStateManager is a facade interface for both the InterfaceStateMonitor and + * InterfaceStateReader interfaces together. + */ +class InterfaceStateManager : public InterfaceStateMonitor, public InterfaceStateReader { +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_MANAGER_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_manager_impl.h b/common/beerocks/bcl/include/bcl/network/interface_state_manager_impl.h new file mode 100644 index 0000000000..34c541782b --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_manager_impl.h @@ -0,0 +1,120 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_MANAGER_IMPL_H_ +#define BCL_NETWORK_INTERFACE_STATE_MANAGER_IMPL_H_ + +#include "interface_state_manager.h" + +#include +#include + +namespace beerocks { +namespace net { + +/** + * This class implements the InterfaceStateManager facade interface in terms of (by delegating to) + * the InterfaceStateMonitor and InterfaceStateReader interfaces and performs additional + * functionality before/after forwarding requests. + */ +class InterfaceStateManagerImpl : public InterfaceStateManager { +public: + /** + * @brief Class constructor + * + * This implementation delegates InterfaceStateMonitor and InterfaceStateReader requests to + * given reader and monitor instances respectively. + * + * The interface state monitor is used to monitor changes in the state of the network + * interfaces in an event-driven way, that is, without polling (which is very CPU expensive). + * + * The interface state reader is used to read the state of an interface when it is not already + * known (i.e. the first time it is queried and no state-changed event has occurred yet). + * + * @param interface_state_monitor Interface state monitor. + * @param interface_state_reader Interface state reader. + */ + InterfaceStateManagerImpl(std::unique_ptr interface_state_monitor, + std::unique_ptr interface_state_reader); + + /** + * @brief Starts the interface state manager. + * + * Installs a state-changed event handler on the monitor and then delegates to + * InterfaceStateMonitor::start. + * + * The handler function stores the interface state into the list of current states for each + * known interface. This way, when read_state() is called, the cached state can be quickly + * returned instead of having to query the network interface with the reader. + * + * @see InterfaceStateMonitor::start + */ + bool start() override; + + /** + * @brief Stops the interface state manager. + * + * Removes the state-changed event handler on the monitor and then delegates to + * InterfaceStateMonitor::stop. + * + * @see InterfaceStateMonitor::stop + */ + bool stop() override; + + /** + * @brief Reads interface up-and-running state. + * + * If the interface state is known (either because a state-changed event has occurred or + * because it has been explicitly read), the cached state is returned. Otherwise delegates to + * InterfaceStateReader::read_state and stores obtained state. + * + * @see InterfaceStateReader::read_state + */ + bool read_state(const std::string &iface_name, bool &iface_state) override; + +private: + /** + * Interface state monitor used to monitor changes in the state of the network interfaces. + */ + std::unique_ptr m_interface_state_monitor; + + /** + * Interface state reader used to read the state of an interface when it is not already known + * (i.e. the first time it is queried and no state-changed event has been occurred yet). + */ + std::unique_ptr m_interface_state_reader; + + /** + * Map containing the current state of each known interface. + * The map key is the interface name and the map value is the interface state (true if + * up-and-running and false otherwise). + */ + std::unordered_map m_interface_states; + + /** + * @brief Gets last known interface state. + * + * @param[in] iface_name Interface name. + * @param[out] iface_state Interface state (true if it is up-and-running). + * @return True on success and false otherwise (i.e.: interface state is not known yet). + */ + bool get_state(const std::string &iface_name, bool &iface_state); + + /** + * @brief Sets last known interface state. + * + * @param[in] iface_name Interface name. + * @param[in] iface_state Interface state (true if it is up and running). + */ + void set_state(const std::string &iface_name, bool iface_state); +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_MANAGER_IMPL_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_monitor.h b/common/beerocks/bcl/include/bcl/network/interface_state_monitor.h new file mode 100644 index 0000000000..6c5de6f349 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_monitor.h @@ -0,0 +1,94 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_MONITOR_H_ +#define BCL_NETWORK_INTERFACE_STATE_MONITOR_H_ + +#include +#include + +namespace beerocks { +namespace net { + +class InterfaceStateMonitor { +public: + /** + * Network interface state-change handler function. + * + * @param iface_name Interface name. + * @param iface_state Interface state (true if it is up and running). + */ + using StateChangeHandler = std::function; + + /** + * @brief Class destructor + */ + virtual ~InterfaceStateMonitor() = default; + + /** + * @brief Starts the interface state monitor. + * + * Starts monitoring the state of all the network interfaces and calls back the installed + * handler (if any) whenever any of the interfaces changes its state to or from the + * up-and-running value. + * + * @return True on success and false otherwise. + */ + virtual bool start() = 0; + + /** + * @brief Stops the interface state monitor. + * + * @return True on success and false otherwise. + */ + virtual bool stop() = 0; + + /** + * @brief Sets the state-changed event handler function. + * + * Sets the callback function to handle network interface state changes. + * Use nullptr to remove previously installed callback function. + * + * @param handler State change handler function (or nullptr). + */ + void set_handler(const StateChangeHandler &handler) { m_handler = handler; } + + /** + * @brief Clears previously set state-changed event handler function. + * + * Clears callback function previously set. + * Behaves like set_handler(nullptr) + */ + void clear_handler() { m_handler = nullptr; } + +protected: + /** + * @brief Notifies a network interface state-changed event. + * + * @param iface_name Name of the network interface that changed state. + * @param iface_state New state of the network interface (true means up-and-running). + */ + void notify_state_changed(const std::string &iface_name, bool iface_state) const + { + if (m_handler) { + m_handler(iface_name, iface_state); + } + } + +private: + /** + * Network interface state-change handler function that is called back whenever any network + * interface changes its state. + */ + StateChangeHandler m_handler; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_MONITOR_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_monitor_impl.h b/common/beerocks/bcl/include/bcl/network/interface_state_monitor_impl.h new file mode 100644 index 0000000000..427efe409d --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_monitor_impl.h @@ -0,0 +1,90 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_MONITOR_IMPL_H_ +#define BCL_NETWORK_INTERFACE_STATE_MONITOR_IMPL_H_ + +#include "interface_state_monitor.h" +#include "sockets_impl.h" + +#include + +namespace beerocks { + +class EventLoop; + +namespace net { + +class InterfaceStateMonitorImpl : public InterfaceStateMonitor { + static constexpr size_t netlink_buffer_size = 8192; + +public: + /** + * @brief Class constructor + * + * @param connection Netlink socket connection for kernel/user-space communication. + * @param event_loop Event loop to wait for I/O events. + */ + InterfaceStateMonitorImpl(const std::shared_ptr &connection, + const std::shared_ptr &event_loop); + + /** + * @brief Starts the interface state monitor. + * + * @see InterfaceStateMonitor::start + */ + bool start() override; + + /** + * @brief Stops the interface state monitor. + * + * @see InterfaceStateMonitor::stop + */ + bool stop() override; + +private: + /** + * Buffer to hold data received through socket connection + */ + BufferImpl m_buffer; + + /** + * Socket connection through which interface state information is received. + */ + std::shared_ptr m_connection; + + /** + * Application event loop used by the monitor to wait for I/O events. + */ + std::shared_ptr m_event_loop; + + /** + * @brief Parses data received through the Netlink socket connection. + * + * The array of bytes contains a list of Netlink messages. + * + * @param data Pointer to array of bytes to parse. + * @param length Number of bytes to parse. + */ + void parse(const uint8_t *data, size_t length) const; + + /** + * @brief Parses message received through the Netlink socket connection. + * + * If the type of the Netlink message is RTM_NEWLINK or RTM_DELLINK then reads the interface + * index and state and notifies a change in the interface state. + * + * @param msg_hdr Netlink message to parse. + */ + void parse(const nlmsghdr *msg_hdr) const; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_MONITOR_IMPL_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_monitor_mock.h b/common/beerocks/bcl/include/bcl/network/interface_state_monitor_mock.h new file mode 100644 index 0000000000..63198905d7 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_monitor_mock.h @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_MONITOR_MOCK_H_ +#define BCL_NETWORK_INTERFACE_STATE_MONITOR_MOCK_H_ + +#include "interface_state_monitor.h" + +#include + +namespace beerocks { +namespace net { + +class InterfaceStateMonitorMock : public InterfaceStateMonitor { +public: + MOCK_METHOD(bool, start, (), (override)); + MOCK_METHOD(bool, stop, (), (override)); + + /** + * This method was inherited as protected but we're changing it to public via a using + * declaration so we can invoke it from unit tests to emulate that a state-changed event has + * occurred. + */ + using InterfaceStateMonitor::notify_state_changed; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_MONITOR_MOCK_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_reader.h b/common/beerocks/bcl/include/bcl/network/interface_state_reader.h new file mode 100644 index 0000000000..2e9a876a22 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_reader.h @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_READER_H_ +#define BCL_NETWORK_INTERFACE_STATE_READER_H_ + +#include + +namespace beerocks { +namespace net { + +class InterfaceStateReader { +public: + virtual ~InterfaceStateReader() = default; + + /** + * @brief Reads interface up-and-running state. + * + * Reads the state of the network interface with given name. State is true if the interface is + * up and running and false otherwise. + * + * @param[in] iface_name Interface name. + * @param[out] iface_state Interface state (true if it is up and running). + * @return True on success and false otherwise. + */ + virtual bool read_state(const std::string &iface_name, bool &iface_state) = 0; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_READER_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_reader_impl.h b/common/beerocks/bcl/include/bcl/network/interface_state_reader_impl.h new file mode 100644 index 0000000000..aca49c043f --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_reader_impl.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_READER_IMPL_H_ +#define BCL_NETWORK_INTERFACE_STATE_READER_IMPL_H_ + +#include "interface_state_reader.h" + +#include + +namespace beerocks { +namespace net { + +class InterfaceFlagsReader; + +class InterfaceStateReaderImpl : public InterfaceStateReader { +public: + /** + * @brief Class constructor + */ + explicit InterfaceStateReaderImpl( + const std::shared_ptr &interface_flags_reader); + + /** + * @brief Reads interface up-and-running state. + * + * @see InterfaceStateReader::read_state + * + * This implementation uses an interface flags reader to read device flags and compares the + * bitmask against bits IFF_UP & IFF_RUNNING. + */ + bool read_state(const std::string &iface_name, bool &iface_state) override; + +private: + /** + * Interface flags reader used to read device flags. + */ + std::shared_ptr m_interface_flags_reader; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_READER_IMPL_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/interface_state_reader_mock.h b/common/beerocks/bcl/include/bcl/network/interface_state_reader_mock.h new file mode 100644 index 0000000000..2098585590 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/interface_state_reader_mock.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_INTERFACE_STATE_READER_MOCK_H_ +#define BCL_NETWORK_INTERFACE_STATE_READER_MOCK_H_ + +#include "interface_state_reader.h" + +#include + +namespace beerocks { +namespace net { + +class InterfaceStateReaderMock : public InterfaceStateReader { +public: + MOCK_METHOD(bool, read_state, (const std::string &iface_name, bool &iface_state), (override)); +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_INTERFACE_STATE_READER_MOCK_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/sockets.h b/common/beerocks/bcl/include/bcl/network/sockets.h new file mode 100644 index 0000000000..1669bf3599 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/sockets.h @@ -0,0 +1,184 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_SOCKETS_H_ +#define BCL_NETWORK_SOCKETS_H_ + +#include "file_descriptor_impl.h" + +#include + +#include + +namespace beerocks { +namespace net { + +/** + * Array of bytes used to hold data received through a socket. + * Code is programmed to interfaces so it does not care about which implementation is used. + * Unit tests can use a mock and set different expectations per test (pretend that different data + * has been received through the socket). + */ +class Buffer { +public: + virtual ~Buffer() = default; + virtual const uint8_t *data() const = 0; + virtual size_t size() const = 0; + virtual void clear() = 0; + + uint8_t *data() { return const_cast(const_cast(this)->data()); } +}; + +/** + * Sockets are OS resources implementing the file descriptor interface. The way this fact is + * modeled is by extending the FileDescriptor interface. + */ +class Socket : public FileDescriptor { +public: + /** + * Wrapper class around sockaddr (structure describing a generic socket address) + */ + class Address { + public: + /** + * @brief Class destructor + */ + virtual ~Address() = default; + + /** + * @brief Returns address of sockaddr structure. + * + * @return address of sockaddr. + */ + virtual const struct sockaddr *sockaddr() const = 0; + + /** + * @brief Returns the length of the sockaddr structure. + * + * @return length of sockaddr + */ + virtual socklen_t length() const = 0; + + /** + * @brief Returns address of sockaddr structure. + * + * This is the non-const version of the method with the same name. + * + * @return address of sockaddr. + */ + struct sockaddr *sockaddr() + { + /** + * This is a way to "Avoid Duplication in const and Non-const Member Function" as + * described in "Effective C++, 3rd ed" by Scott Meyers. + * The two casts and function call may be ugly but they're correct and the method is + * implemented in the interface class, so available to all implementation classes for free. + */ + return const_cast(const_cast(this)->sockaddr()); + } + }; + + /** + * Classes implementing this interface model either the socket connection established at the + * server side when accept() system call is called or at the client side when connect() is called. + * + * The interface defines the methods to send data over a socket and to receive data from a socket. + */ + class Connection { + public: + /** + * @brief Class destructor + */ + virtual ~Connection() = default; + + /** + * @brief Returns the underlying socket used by this connection. + * + * Access to the underlying socket is required to obtain the socket file descriptor with which + * wait for read or write events using select() or epoll() functions. + * + * @return Socket used by the connection + */ + virtual std::shared_ptr socket() = 0; + + /** + * @brief Receives data through the socket connection. + * + * @param[out] buffer Buffer to hold received data. + * @param[in] offset Position into the buffer to start receiving data. + * @return Number of bytes received, -1 on failure. + */ + virtual int receive(Buffer &buffer, size_t offset = 0) = 0; + + /** + * @brief Receives data through the socket connection. + * + * @param[out] buffer Buffer to hold received data. + * @param[out] address Address where the data came from. + * @return Number of bytes received, -1 on failure. + */ + virtual int receive_from(Buffer &buffer, Address &address) = 0; + + /** + * @brief Sends data through the socket connection. + * + * @param[in] buffer Buffer holding data to send. + * @param[in] length Number of bytes to send. + * @return Number of bytes transmitted, -1 on failure. + */ + virtual int send(const Buffer &buffer, size_t length) = 0; + + /** + * @brief Sends data through the socket connection. + * + * @param[in] buffer Buffer holding data to send. + * @param[in] length Number of bytes to be transmitted. + * @param[in] address Destination address. + * @return Number of bytes transmitted, -1 on failure. + */ + virtual int send_to(const Buffer &buffer, size_t length, const Address &address) = 0; + }; +}; + +class ServerSocket { +public: + /** + * @brief Class destructor + */ + virtual ~ServerSocket() = default; + + /** + * @brief Accepts a connection request. + * + * @param address Address of the peer socket. + * @return First connection request on the queue of pending connections for the listening + * socket. + */ + virtual std::unique_ptr accept(Socket::Address &address) = 0; +}; + +class ClientSocket { +public: + /** + * @brief Class destructor + */ + virtual ~ClientSocket() = default; + + /** + * @brief Connects the socket to the address specified. + * + * @param address Destination address. + * @return Connection established with peer socket. + */ + virtual std::unique_ptr connect(const Socket::Address &address) = 0; +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_SOCKETS_H_ */ diff --git a/common/beerocks/bcl/include/bcl/network/sockets_impl.h b/common/beerocks/bcl/include/bcl/network/sockets_impl.h new file mode 100644 index 0000000000..8760a4dee5 --- /dev/null +++ b/common/beerocks/bcl/include/bcl/network/sockets_impl.h @@ -0,0 +1,348 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#ifndef BCL_NETWORK_SOCKETS_IMPL_H_ +#define BCL_NETWORK_SOCKETS_IMPL_H_ + +#include "sockets.h" + +#include +#include + +#include + +#include +#include +#include +#include +#include +#include + +namespace beerocks { +namespace net { + +/** + * One possible Buffer implementation + */ +template class BufferImpl : public Buffer { +public: + const uint8_t *data() const override { return m_data; } + size_t size() const override { return sizeof(m_data); } + void clear() override { memset(m_data, 0, size()); } + +private: + uint8_t m_data[Size]{}; +}; + +/** + * Abstract base class for all types of sockets: Raw, UDP, TCP, UDS, ... + * This implementation class aggregates a FileDescriptor implementation so it has a file + * descriptor. Methods overridden from FileDescriptor interface delegate on the aggregated + * implementation. + * Derived classes provide the file descriptor obtained with a call to socket(), using different + * family, type and protocol parameters. + * This class aggregates a FileDescriptor instead of inheriting from one of its implementations to + * follow the principle of "Favor Aggregation over Inheritance". + * See https://wiki.c2.com/?UseCompositionAndInterfacesWithoutClassInheritance + */ +class SocketAbstractImpl : public Socket { +public: + /** + * @brief Returns the socket file descriptor. + * + * @return Socket file descriptor. + */ + int fd() override { return m_descriptor.fd(); } + +protected: + /** + * @brief Class constructor. + * + * Constructor is protected so only derived classes can call it. + */ + explicit SocketAbstractImpl(int fd) : m_descriptor(fd) {} + +private: + /** + * File descriptor (i.e.: wrapper to `int fd` that closes descriptor on destructor) + */ + FileDescriptorImpl m_descriptor; +}; + +class UdsAddress : public Socket::Address { +public: + UdsAddress(const std::string &path = "") + { + m_address.sun_family = AF_UNIX; + string_utils::copy_string(m_address.sun_path, path.c_str(), sizeof(m_address.sun_path)); + } + + std::string path() const { return m_address.sun_path; } + + const struct sockaddr *sockaddr() const override + { + return reinterpret_cast(&m_address); + } + socklen_t length() const override { return m_length; } + +private: + sockaddr_un m_address = {}; + socklen_t m_length = sizeof(m_address); +}; + +class InternetAddress : public Socket::Address { +public: + explicit InternetAddress(uint16_t port, uint32_t address = INADDR_ANY) + { + m_address.sin_family = AF_INET; + m_address.sin_addr.s_addr = address; + m_address.sin_port = htons(port); + } + + const struct sockaddr *sockaddr() const override + { + return reinterpret_cast(&m_address); + } + socklen_t length() const override { return m_length; } + +private: + sockaddr_in m_address = {}; + socklen_t m_length = sizeof(m_address); +}; + +class LinkLevelAddress : public Socket::Address { +public: + LinkLevelAddress() {} + + LinkLevelAddress(uint32_t iface_index, const sMacAddr &mac) + { + m_address.sll_family = AF_PACKET; + m_address.sll_ifindex = iface_index; + m_address.sll_halen = sizeof(sMacAddr); + std::copy_n(mac.oct, sizeof(sMacAddr), m_address.sll_addr); + } + + const struct sockaddr *sockaddr() const override + { + return reinterpret_cast(&m_address); + } + socklen_t length() const override { return m_length; } + +private: + sockaddr_ll m_address = {}; + socklen_t m_length = sizeof(m_address); +}; + +class NetlinkAddress : public Socket::Address { +public: + explicit NetlinkAddress(uint32_t groups = 0) + { + m_address.nl_family = AF_NETLINK; + m_address.nl_groups = groups; + } + + const struct sockaddr *sockaddr() const override + { + return reinterpret_cast(&m_address); + } + socklen_t length() const override { return m_length; } + +private: + sockaddr_nl m_address = {}; + socklen_t m_length = sizeof(m_address); +}; + +/** + * This class is a wrapper for the socket file descriptor obtained with the accept() system call. + */ +class ConnectedSocket : public SocketAbstractImpl { +public: + explicit ConnectedSocket(int fd) : SocketAbstractImpl(fd) {} +}; + +class RawSocket : public SocketAbstractImpl { +public: + explicit RawSocket(uint16_t protocol = ETH_P_ALL) + : SocketAbstractImpl(socket(AF_PACKET, SOCK_RAW, htons(protocol))) + { + } +}; + +class UdpSocket : public SocketAbstractImpl { +public: + UdpSocket() : SocketAbstractImpl(socket(AF_INET, SOCK_DGRAM, IPPROTO_IP)) {} +}; + +class TcpSocket : public SocketAbstractImpl { +public: + TcpSocket() : SocketAbstractImpl(socket(AF_INET, SOCK_STREAM, IPPROTO_IP)) {} +}; + +class UdsSocket : public SocketAbstractImpl { +public: + UdsSocket() : SocketAbstractImpl(socket(AF_UNIX, SOCK_STREAM, 0)) {} +}; + +class AbstractNetlinkSocket : public SocketAbstractImpl { +protected: + explicit AbstractNetlinkSocket(uint16_t protocol) + : SocketAbstractImpl(socket(AF_NETLINK, SOCK_RAW, protocol)) + { + } +}; + +class NetlinkRouteSocket : public AbstractNetlinkSocket { +public: + NetlinkRouteSocket() : AbstractNetlinkSocket(NETLINK_ROUTE) {} +}; + +/** + * This class implements the Socket::Connection interface with methods that wrap the system calls + * to send and receive both bytes and packets in stream-oriented and packet-oriented sockets + * respectively. + */ +class SocketConnectionImpl : public Socket::Connection { +public: + explicit SocketConnectionImpl(const std::shared_ptr &socket) : m_socket(socket) {} + + /** + * @brief Returns the underlying socket used by this connection. + * + * @see Connection::socket + */ + std::shared_ptr socket() override { return m_socket; } + + /** + * @brief Receives data through the socket connection. + * + * @see Connection::receive + * + * This implementation uses the recv() system calll. + */ + int receive(Buffer &buffer, size_t offset = 0) override + { + if (offset >= buffer.size()) { + return -1; + } + return ::recv(m_socket->fd(), buffer.data() + offset, buffer.size() - offset, MSG_DONTWAIT); + } + + /** + * @brief Receives data through the socket connection. + * + * @see Connection::receive_from + * + * This implementation uses the recvfrom() system calll. + */ + int receive_from(Buffer &buffer, Socket::Address &address) override + { + socklen_t address_length = address.length(); + return ::recvfrom(m_socket->fd(), buffer.data(), buffer.size(), MSG_DONTWAIT, + address.sockaddr(), &address_length); + } + + /** + * @brief Sends data through the socket connection. + * + * @see Connection::send + * + * This implementation uses the send() system calll. + */ + int send(const Buffer &buffer, size_t length) override + { + if (length > buffer.size()) { + return -1; + } + return ::send(m_socket->fd(), buffer.data(), length, MSG_NOSIGNAL); + } + + /** + * @brief Sends data through the socket connection. + * + * @see Connection::send_to + * + * This implementation uses the sendto() system calll. + */ + int send_to(const Buffer &buffer, size_t length, const Socket::Address &address) override + { + return ::sendto(m_socket->fd(), buffer.data(), buffer.size(), 0, address.sockaddr(), + address.length()); + } + +private: + /** + * Connected socket used by this connection object. + */ + std::shared_ptr m_socket; +}; + +class ServerSocketAbstractImpl : public ServerSocket { +public: + int bind(const Socket::Address &address) + { + return ::bind(m_socket->fd(), address.sockaddr(), address.length()); + } + + int listen(int backlog) { return ::listen(m_socket->fd(), backlog); } + + std::unique_ptr accept(Socket::Address &address) override + { + socklen_t address_length = address.length(); + int s = ::accept(m_socket->fd(), address.sockaddr(), &address_length); + if (FileDescriptor::invalid_descriptor == s) { + return nullptr; + } + + return std::make_unique(std::make_shared(s)); + } + +protected: + explicit ServerSocketAbstractImpl(const std::shared_ptr &socket) : m_socket(socket) {} + std::shared_ptr m_socket; +}; + +class ClientSocketAbstractImpl : public ClientSocket { +public: + int bind(const Socket::Address &address) + { + return ::bind(m_socket->fd(), address.sockaddr(), address.length()); + } + + std::unique_ptr connect(const Socket::Address &address) override + { + if (0 != ::connect(m_socket->fd(), address.sockaddr(), address.length())) { + return nullptr; + } + return std::make_unique(m_socket); + } + +protected: + explicit ClientSocketAbstractImpl(const std::shared_ptr &socket) : m_socket(socket) {} + std::shared_ptr m_socket; +}; + +template class ServerSocketImpl : public ServerSocketAbstractImpl { +public: + explicit ServerSocketImpl(const std::shared_ptr &socket) + : ServerSocketAbstractImpl(socket) + { + } +}; + +template class ClientSocketImpl : public ClientSocketAbstractImpl { +public: + explicit ClientSocketImpl(const std::shared_ptr &socket) + : ClientSocketAbstractImpl(socket) + { + } +}; + +} // namespace net +} // namespace beerocks + +#endif /* BCL_NETWORK_SOCKETS_IMPL_H_ */ diff --git a/common/beerocks/bcl/source/network/interface_flags_reader_impl.cpp b/common/beerocks/bcl/source/network/interface_flags_reader_impl.cpp new file mode 100644 index 0000000000..e1e293ef4a --- /dev/null +++ b/common/beerocks/bcl/source/network/interface_flags_reader_impl.cpp @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#include +#include +#include + +#include +#include + +namespace beerocks { +namespace net { + +bool InterfaceFlagsReaderImpl::read_flags(const std::string &iface_name, uint16_t &iface_flags) +{ + UdpSocket socket; + + ifreq if_req; + string_utils::copy_string(if_req.ifr_name, iface_name.c_str(), IFNAMSIZ); + + int rc = ioctl(socket.fd(), SIOCGIFFLAGS, &if_req); + if (rc == -1) { + return false; + } + + iface_flags = if_req.ifr_flags; + + return true; +} + +} // namespace net +} // namespace beerocks diff --git a/common/beerocks/bcl/source/network/interface_state_manager_impl.cpp b/common/beerocks/bcl/source/network/interface_state_manager_impl.cpp new file mode 100644 index 0000000000..6b1c003cfb --- /dev/null +++ b/common/beerocks/bcl/source/network/interface_state_manager_impl.cpp @@ -0,0 +1,79 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#include + +namespace beerocks { +namespace net { + +InterfaceStateManagerImpl::InterfaceStateManagerImpl( + std::unique_ptr interface_state_monitor, + std::unique_ptr interface_state_reader) + : m_interface_state_monitor(std::move(interface_state_monitor)), + m_interface_state_reader(std::move(interface_state_reader)) +{ +} + +bool InterfaceStateManagerImpl::start() +{ + m_interface_state_monitor->set_handler([&](const std::string &iface_name, bool iface_state) { + bool last_iface_state; + if (get_state(iface_name, last_iface_state)) { + if (last_iface_state == iface_state) { + return; + } + } + + set_state(iface_name, iface_state); + notify_state_changed(iface_name, iface_state); + }); + + return m_interface_state_monitor->start(); +} + +bool InterfaceStateManagerImpl::stop() +{ + m_interface_state_monitor->clear_handler(); + + return m_interface_state_monitor->stop(); +} + +bool InterfaceStateManagerImpl::read_state(const std::string &iface_name, bool &iface_state) +{ + if (get_state(iface_name, iface_state)) { + return true; + } + + if (!m_interface_state_reader->read_state(iface_name, iface_state)) { + return false; + } + + set_state(iface_name, iface_state); + + return true; +} + +bool InterfaceStateManagerImpl::get_state(const std::string &iface_name, bool &iface_state) +{ + const auto &it = m_interface_states.find(iface_name); + if (m_interface_states.end() == it) { + return false; + } + + iface_state = it->second; + + return true; +} + +void InterfaceStateManagerImpl::set_state(const std::string &iface_name, bool iface_state) +{ + m_interface_states[iface_name] = iface_state; +} + +} // namespace net +} // namespace beerocks diff --git a/common/beerocks/bcl/source/network/interface_state_monitor_impl.cpp b/common/beerocks/bcl/source/network/interface_state_monitor_impl.cpp new file mode 100644 index 0000000000..1172bee9d9 --- /dev/null +++ b/common/beerocks/bcl/source/network/interface_state_monitor_impl.cpp @@ -0,0 +1,75 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#include +#include +#include + +#include + +using namespace beerocks; + +namespace beerocks { +namespace net { + +InterfaceStateMonitorImpl::InterfaceStateMonitorImpl( + const std::shared_ptr &connection, + const std::shared_ptr &event_loop) + : m_connection(connection), m_event_loop(event_loop) +{ +} + +bool InterfaceStateMonitorImpl::start() +{ + EventLoop::EventHandlers handlers; + handlers.on_read = [&](int fd, EventLoop &loop) -> bool { + int length = m_connection->receive(m_buffer); + if (length > 0) { + parse(m_buffer.data(), length); + } + + return true; + }; + + return m_event_loop->register_handlers(m_connection->socket()->fd(), handlers); +} + +bool InterfaceStateMonitorImpl::stop() +{ + return m_event_loop->remove_handlers(m_connection->socket()->fd()); +} + +void InterfaceStateMonitorImpl::parse(const uint8_t *data, size_t length) const +{ + for (const nlmsghdr *msg_hdr = reinterpret_cast(data); + NLMSG_OK(msg_hdr, length); msg_hdr = NLMSG_NEXT(msg_hdr, length)) { + parse(msg_hdr); + } +} + +void InterfaceStateMonitorImpl::parse(const nlmsghdr *msg_hdr) const +{ + switch (msg_hdr->nlmsg_type) { + case RTM_NEWLINK: + case RTM_DELLINK: + const ifinfomsg *ifi = static_cast(NLMSG_DATA(msg_hdr)); + + uint32_t iface_index = ifi->ifi_index; + bool iface_state = (ifi->ifi_flags & IFF_UP) && (ifi->ifi_flags & IFF_RUNNING); + + char iface_name[IFNAMSIZ]{}; + if (0 != if_indextoname(iface_index, iface_name)) { + notify_state_changed(iface_name, iface_state); + } + + break; + } +} + +} // namespace net +} // namespace beerocks diff --git a/common/beerocks/bcl/source/network/interface_state_reader_impl.cpp b/common/beerocks/bcl/source/network/interface_state_reader_impl.cpp new file mode 100644 index 0000000000..e899cdcdf8 --- /dev/null +++ b/common/beerocks/bcl/source/network/interface_state_reader_impl.cpp @@ -0,0 +1,36 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#include +#include + +#include + +namespace beerocks { +namespace net { + +InterfaceStateReaderImpl::InterfaceStateReaderImpl( + const std::shared_ptr &interface_flags_reader) + : m_interface_flags_reader(interface_flags_reader) +{ +} + +bool InterfaceStateReaderImpl::read_state(const std::string &iface_name, bool &iface_state) +{ + uint16_t flags; + if (!m_interface_flags_reader->read_flags(iface_name, flags)) { + return false; + } + + iface_state = (flags & IFF_UP) && (flags & IFF_RUNNING); + + return true; +} + +} // namespace net +} // namespace beerocks diff --git a/common/beerocks/bcl/unit_tests/interface_state_manager_impl_test.cpp b/common/beerocks/bcl/unit_tests/interface_state_manager_impl_test.cpp new file mode 100644 index 0000000000..5b60677ba7 --- /dev/null +++ b/common/beerocks/bcl/unit_tests/interface_state_manager_impl_test.cpp @@ -0,0 +1,150 @@ +/* SPDX-License-Identifier: BSD-2-Clause-Patent + * + * SPDX-FileCopyrightText: 2020 the prplMesh contributors (see AUTHORS.md) + * + * This code is subject to the terms of the BSD+Patent license. + * See LICENSE file for more details. + */ + +#include +#include +#include + +#include + +#include +#include + +using ::testing::_; +using ::testing::InSequence; +using ::testing::Invoke; +using ::testing::Return; + +TEST(interface_state_manager_impl, start_stop_should_succeed) +{ + auto monitor = std::make_unique(); + auto reader = std::make_unique(); + + { + InSequence sequence; + + EXPECT_CALL(*monitor, start()).WillOnce(Return(true)); + EXPECT_CALL(*monitor, stop()).WillOnce(Return(true)); + }; + + beerocks::net::InterfaceStateManagerImpl interface_state_manager(std::move(monitor), + std::move(reader)); + + ASSERT_TRUE(interface_state_manager.start()); + ASSERT_TRUE(interface_state_manager.stop()); +} + +/** + * In this test, no state-changed event has occurred yet so interface state is obtained with an + * explicit read through the interface state reader. + */ +TEST(interface_state_manager_impl, read_state_should_succeed_before_event) +{ + auto monitor = std::make_unique(); + auto reader = std::make_unique(); + + const char *iface_name = "eth0"; + const bool expected_iface_state = true; + bool actual_iface_state = false; + + { + InSequence sequence; + + EXPECT_CALL(*monitor, start()).WillOnce(Return(true)); + EXPECT_CALL(*reader, read_state(iface_name, _)) + .WillOnce(Invoke([&](const std::string &iface_name, bool &iface_state) -> bool { + iface_state = expected_iface_state; + return true; + })); + EXPECT_CALL(*monitor, stop()).WillOnce(Return(true)); + }; + + beerocks::net::InterfaceStateManagerImpl interface_state_manager(std::move(monitor), + std::move(reader)); + + ASSERT_TRUE(interface_state_manager.start()); + ASSERT_TRUE(interface_state_manager.read_state(iface_name, actual_iface_state)); + ASSERT_EQ(actual_iface_state, expected_iface_state); + ASSERT_TRUE(interface_state_manager.stop()); +} + +/** + * In this test, the interface state is obtained after a state-changed event (no explicit read + * operation is required nor performed) + */ +TEST(interface_state_manager_impl, read_state_should_succeed_after_event) +{ + auto monitor = std::make_unique(); + auto reader = std::make_unique(); + + const char *iface_name = "eth0"; + const bool expected_iface_state = true; + bool actual_iface_state = false; + + // The monitor mock is used in the expectation to emulate that a state-changed event has + // occurred. + // Since the unique_ptr to the monitor mock is moved into the interface state manager, it + // is not available inside the expectation. To overcome this problem, we use the raw pointer + // instead. + auto monitor_raw_ptr = monitor.get(); + + { + InSequence sequence; + + EXPECT_CALL(*monitor, start()).WillOnce(Invoke([&]() -> bool { + monitor_raw_ptr->notify_state_changed(iface_name, expected_iface_state); + return true; + })); + EXPECT_CALL(*monitor, stop()).WillOnce(Return(true)); + }; + + beerocks::net::InterfaceStateManagerImpl interface_state_manager(std::move(monitor), + std::move(reader)); + + ASSERT_TRUE(interface_state_manager.start()); + ASSERT_TRUE(interface_state_manager.read_state(iface_name, actual_iface_state)); + ASSERT_EQ(actual_iface_state, expected_iface_state); + ASSERT_TRUE(interface_state_manager.stop()); +} + +TEST(interface_state_manager_impl, notify_state_changed_should_succeed) +{ + auto monitor = std::make_unique(); + auto reader = std::make_unique(); + + const char *iface_name = "eth0"; + const bool expected_iface_state = true; + bool actual_iface_state = false; + + // The monitor mock is used in the expectation to emulate that a state-changed event has + // occurred. + // Since the unique_ptr to the monitor mock is moved into the interface state manager, it + // is not available inside the expectation. To overcome this problem, we use the raw pointer + // instead. + auto monitor_raw_ptr = monitor.get(); + + { + InSequence sequence; + + EXPECT_CALL(*monitor, start()).WillOnce(Invoke([&]() -> bool { + monitor_raw_ptr->notify_state_changed(iface_name, expected_iface_state); + return true; + })); + EXPECT_CALL(*monitor, stop()).WillOnce(Return(true)); + }; + + beerocks::net::InterfaceStateManagerImpl interface_state_manager(std::move(monitor), + std::move(reader)); + + interface_state_manager.set_handler( + [&](const std::string &iface_name, bool iface_state) { actual_iface_state = iface_state; }); + + ASSERT_TRUE(interface_state_manager.start()); + ASSERT_EQ(actual_iface_state, expected_iface_state); + ASSERT_TRUE(interface_state_manager.stop()); +} From 5140b3c21977a7df83bf7ea8fc714714261318c9 Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Fri, 31 Jul 2020 19:15:07 +0200 Subject: [PATCH 6/7] transport: use new InterfaceStateManager https://jira.prplfoundation.org/browse/PPM-18 Use the InterfaceStateManager class in the ieee1905_transport process as a new mechanism to read and detect changes in the state of the network interfaces. Existing code in `ieee1905_transport_netlink.cpp` is deprecated and will be removed in next commit. InterfaceStateManager uses interface names. Since we should use interface names everywhere rather than interface indexes (because interfaces are not renamed while prplMesh is running and conversely, indexes can change, and because names are easier to debug than indexes), modify existing code to favor names over indexes. BTW, this change also fixes a bug in calls to `handle_interface_status_change` that use a file descriptor when they shall be using an interface index. Signed-off-by: Mario Maz --- .../ieee1905_transport/ieee1905_transport.cpp | 56 +++-------- .../ieee1905_transport/ieee1905_transport.h | 35 +++++-- .../ieee1905_transport_netlink.cpp | 2 +- .../ieee1905_transport_network.cpp | 47 +++------- .../transport/ieee1905_transport/main.cpp | 94 +++++++++++++++---- 5 files changed, 135 insertions(+), 99 deletions(-) diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.cpp b/framework/transport/ieee1905_transport/ieee1905_transport.cpp index ddf43e5dc3..6d0bf7f69c 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport.cpp @@ -11,15 +11,18 @@ namespace beerocks { namespace transport { -Ieee1905Transport::Ieee1905Transport(const std::shared_ptr &broker, - const std::shared_ptr &event_loop) - : m_broker(broker), m_event_loop(event_loop) +Ieee1905Transport::Ieee1905Transport( + const std::shared_ptr &interface_state_manager, + const std::shared_ptr &broker, + const std::shared_ptr &event_loop) + : m_interface_state_manager(interface_state_manager), m_broker(broker), m_event_loop(event_loop) { + LOG_IF(!m_interface_state_manager, FATAL) << "Interface state manager is a null pointer!"; LOG_IF(!m_broker, FATAL) << "Broker server is a null pointer!"; LOG_IF(!m_event_loop, FATAL) << "Event loop is a null pointer!"; } -void Ieee1905Transport::run() +bool Ieee1905Transport::start() { LOG(INFO) << "Starting 1905 transport..."; @@ -38,47 +41,18 @@ void Ieee1905Transport::run() return true; }); - // init netlink socket - if (!open_netlink_socket()) { - MAPF_ERR("cannot open netlink socket."); - return; - } - - // Create a shared_ptr socket wrapper for the netlink socket - auto netlink_socket = std::shared_ptr(new Socket(netlink_fd_), [](Socket *socket) { - // Close the socket file descriptor - if (socket) { - close(socket->getSocketFd()); - } + m_interface_state_manager->set_handler([&](const std::string &iface_name, bool iface_state) { + handle_interface_status_change(iface_name, iface_state); }); - // Add the netlink socket into the broker's event loop - m_event_loop->register_handlers(netlink_socket->getSocketFd(), - { - // Accept incoming connections - .on_read = - [&](int fd, EventLoop &loop) { - LOG(DEBUG) - << "incoming message on the netlink socket"; - handle_netlink_pollin_event(); - return true; - }, + return true; +} - // Not implemented - .on_write = nullptr, +bool Ieee1905Transport::stop() +{ + m_interface_state_manager->clear_handler(); - // Fail on server socket disconnections or errors - .on_disconnect = - [&](int fd, EventLoop &loop) { - LOG(ERROR) << "netlink socket disconnected"; - return false; - }, - .on_error = - [&](int fd, EventLoop &loop) { - LOG(ERROR) << "netlink socket error"; - return false; - }, - }); + return true; } } // namespace transport diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.h b/framework/transport/ieee1905_transport/ieee1905_transport.h index 851ea9cf30..2c4aca24c8 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport.h @@ -13,6 +13,7 @@ #include #include +#include #include "ieee1905_transport_broker.h" @@ -58,14 +59,36 @@ class Ieee1905Transport { /** * Class constructor * + * @param interface_state_manager Interface state manager. * @param broker Message broker. * @param event_loop Event loop to wait for I/O events. */ - Ieee1905Transport(const std::shared_ptr &broker, - const std::shared_ptr &event_loop); - void run(); + Ieee1905Transport( + const std::shared_ptr &interface_state_manager, + const std::shared_ptr &broker, + const std::shared_ptr &event_loop); + + /** + * @brief Starts the transport process. + * + * @return True on success and false otherwise. + */ + bool start(); + + /** + * @brief Stops the transport process. + * + * @return True on success and false otherwise. + */ + bool stop(); private: + /** + * Interface state manager to read and detect changes (transitions to and from the + * up-and-running state) in the state of the network interfaces. + */ + std::shared_ptr m_interface_state_manager; + /** * Message broker implementing the publish/subscribe design pattern. */ @@ -284,9 +307,9 @@ class Ieee1905Transport { // void update_network_interfaces(std::map updated_network_interfaces); - bool open_interface_socket(unsigned int if_index); - bool attach_interface_socket_filter(unsigned int if_index); - void handle_interface_status_change(unsigned int if_index, bool is_active); + bool open_interface_socket(const std::string &iface_name); + bool attach_interface_socket_filter(const std::string &iface_name); + void handle_interface_status_change(const std::string &iface_name, bool is_active); void handle_interface_pollin_event(int fd); bool get_interface_mac_addr(unsigned int if_index, uint8_t *addr); bool send_packet_to_network_interface(unsigned int if_index, Packet &packet); diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp index 059058351a..a93b71adbb 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp @@ -80,7 +80,7 @@ int Ieee1905Transport::handle_netlink_message(struct nlmsghdr *msghdr) << ifname << " is " << (is_active ? "active" : "inactive") << ")."); if (ifi->ifi_index > 0 && network_interfaces_.count(ifname) > 0) { - handle_interface_status_change((unsigned)ifi->ifi_index, is_active); + handle_interface_status_change(ifname, is_active); } else if (ifi->ifi_index < 0) { MAPF_WARN("bad interface index (" << ifi->ifi_index << ") in netlink message."); } diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp index 15851317bb..8fc2ece052 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp +++ b/framework/transport/ieee1905_transport/ieee1905_transport_network.cpp @@ -125,8 +125,8 @@ void Ieee1905Transport::update_network_interfaces( if (!network_interfaces_[ifname].fd) { // if the interface is not already open, try to open it and add it to the poller loop - if (!open_interface_socket(if_index)) { - MAPF_WARN("cannot open interface " << if_index << "."); + if (!open_interface_socket(ifname)) { + MAPF_WARN("cannot open interface " << ifname << "."); } // add interface raw socket fd to poller loop (unless it's a bridge interface) @@ -152,7 +152,7 @@ void Ieee1905Transport::update_network_interfaces( [&](int fd, EventLoop &loop) { LOG(DEBUG) << "Error on interface fd: " << fd << " (disabling it)."; - handle_interface_status_change(fd, false); + handle_interface_status_change(ifname, false); return true; }, }); @@ -167,13 +167,8 @@ void Ieee1905Transport::update_network_interfaces( } } -bool Ieee1905Transport::open_interface_socket(unsigned int if_index) +bool Ieee1905Transport::open_interface_socket(const std::string &ifname) { - std::string ifname = if_index2name(if_index); - if (ifname.empty()) { - MAPF_ERR("Failed to get interface name for index " << if_index); - return false; - } MAPF_DBG("opening raw socket on interface " << ifname << "."); if (network_interfaces_[ifname].fd) { @@ -206,7 +201,7 @@ bool Ieee1905Transport::open_interface_socket(unsigned int if_index) memset(&sockaddr, 0, sizeof(struct sockaddr_ll)); sockaddr.sll_family = AF_PACKET; sockaddr.sll_protocol = htons(ETH_P_ALL); - sockaddr.sll_ifindex = if_index; + sockaddr.sll_ifindex = if_nametoindex(ifname.c_str()); if (bind(sockfd, (struct sockaddr *)&sockaddr, sizeof(sockaddr)) < 0) { MAPF_ERR("cannot bind socket to interface \"" << strerror(errno) << "\" (" << errno << ")."); @@ -217,21 +212,15 @@ bool Ieee1905Transport::open_interface_socket(unsigned int if_index) network_interfaces_[ifname].fd = std::make_shared(sockfd); LOG_IF(!sockfd, FATAL) << "Failed creating new Socket for fd: " << sockfd; - attach_interface_socket_filter(if_index); + attach_interface_socket_filter(ifname); return true; } -bool Ieee1905Transport::attach_interface_socket_filter(unsigned int if_index) +bool Ieee1905Transport::attach_interface_socket_filter(const std::string &ifname) { - std::string ifname = if_index2name(if_index); - if (ifname.empty()) { - MAPF_ERR("Failed to get interface name for index " << if_index); - return false; - } - if (!network_interfaces_.count(ifname)) { - MAPF_ERR("un-tracked interface " << if_index << "."); + MAPF_ERR("un-tracked interface " << ifname << "."); return false; } @@ -240,7 +229,7 @@ bool Ieee1905Transport::attach_interface_socket_filter(unsigned int if_index) // the AL MAC address (which is different the the interfaces HW address) // struct packet_mreq mr = {0}; - mr.mr_ifindex = if_index; + mr.mr_ifindex = if_nametoindex(ifname.c_str()); mr.mr_type = PACKET_MR_PROMISC; if (setsockopt(network_interfaces_[ifname].fd->getSocketFd(), SOL_PACKET, PACKET_ADD_MEMBERSHIP, &mr, sizeof(mr)) == -1) { @@ -263,14 +252,8 @@ bool Ieee1905Transport::attach_interface_socket_filter(unsigned int if_index) return true; } -void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bool is_active) +void Ieee1905Transport::handle_interface_status_change(const std::string &ifname, bool is_active) { - std::string ifname = if_index2name(if_index); - if (ifname.empty()) { - MAPF_ERR("Failed to get interface name for index " << if_index); - return; - } - if (!network_interfaces_.count(ifname)) { MAPF_ERR("un-tracked interface " << ifname << "."); return; @@ -285,8 +268,8 @@ void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bo } if (is_active && !network_interfaces_[ifname].fd) { - if (!open_interface_socket(if_index)) { - MAPF_ERR("cannot open network interface " << if_index << "."); + if (!open_interface_socket(ifname)) { + MAPF_ERR("cannot open network interface " << ifname << "."); } if (network_interfaces_[ifname].fd) // Handle network events @@ -310,7 +293,7 @@ void Ieee1905Transport::handle_interface_status_change(unsigned int if_index, bo [&](int fd, EventLoop &loop) { LOG(DEBUG) << "Error on interface fd: " << fd << " (disabling it)."; - handle_interface_status_change(fd, false); + handle_interface_status_change(ifname, false); return true; }, }); @@ -456,11 +439,9 @@ void Ieee1905Transport::set_al_mac_addr(const uint8_t *addr) // refresh packet filtering on all active interfaces to use the new AL MAC address for (auto it = network_interfaces_.begin(); it != network_interfaces_.end(); ++it) { auto &network_interface = it->second; - auto &ifname = network_interface.ifname; - unsigned int if_index = if_nametoindex(ifname.c_str()); if (network_interface.fd) { - attach_interface_socket_filter(if_index); + attach_interface_socket_filter(network_interface.ifname); } } } diff --git a/framework/transport/ieee1905_transport/main.cpp b/framework/transport/ieee1905_transport/main.cpp index c3e96aacde..32cb4546c2 100644 --- a/framework/transport/ieee1905_transport/main.cpp +++ b/framework/transport/ieee1905_transport/main.cpp @@ -11,11 +11,17 @@ #include #include #include +#include +#include +#include +#include +#include #include #include using namespace beerocks; +using namespace beerocks::net; using namespace beerocks::transport; static std::shared_ptr create_event_loop() @@ -40,6 +46,38 @@ create_broker_server(const std::shared_ptr &event_loop) return std::make_shared(server_socket, event_loop); } +static std::shared_ptr +create_interface_state_manager(const std::shared_ptr &event_loop) +{ + // Create NETLINK_ROUTE netlink socket for kernel/user-space communication + auto socket = std::make_shared(); + + // Create client socket + ClientSocketImpl client(socket); + + // Bind client socket to "route netlink" multicast group to listen for multicast packets sent + // from the kernel containing network interface create/delete/up/down events + client.bind(NetlinkAddress(RTMGRP_LINK)); + + // Create connection to send/receive data using this socket + auto connection = std::make_shared(socket); + + // Create the interface state monitor + auto interface_state_monitor = + std::make_unique(connection, event_loop); + + // Create the interface flags reader + auto interface_flags_reader = std::make_shared(); + + // Create the interface state reader + auto interface_state_reader = + std::make_unique(interface_flags_reader); + + // Create the interface state manager + return std::make_shared(std::move(interface_state_monitor), + std::move(interface_state_reader)); +} + int main(int argc, char *argv[]) { mapf::Logger::Instance().LoggerInit("transport"); @@ -47,8 +85,9 @@ int main(int argc, char *argv[]) /** * Create required objects in the order defined by the dependency tree. */ - auto event_loop = create_event_loop(); - auto broker = create_broker_server(event_loop); + auto event_loop = create_event_loop(); + auto broker = create_broker_server(event_loop); + auto interface_state_manager = create_interface_state_manager(event_loop); /** * Application exit code: 0 on success and -1 on failure. @@ -60,34 +99,53 @@ int main(int argc, char *argv[]) /** * Create the IEEE1905 transport process. */ - Ieee1905Transport ieee1905_transport(broker, event_loop); + Ieee1905Transport ieee1905_transport(interface_state_manager, broker, event_loop); /** - * Start the message broker + * Start the interface state monitor */ - if (broker->start()) { + if (interface_state_manager->start()) { /** - * Start the IEEE1905 transport process + * Start the message broker */ - ieee1905_transport.run(); - - /** - * Run the application event loop - */ - MAPF_INFO("starting main loop..."); - while (0 == exit_code) { - if (event_loop->run() < 0) { - LOG(ERROR) << "Broker event loop failure!"; + if (broker->start()) { + + /** + * Start the IEEE1905 transport process + */ + if (ieee1905_transport.start()) { + + /** + * Run the application event loop + */ + MAPF_INFO("starting main loop..."); + while (0 == exit_code) { + if (event_loop->run() < 0) { + LOG(ERROR) << "Broker event loop failure!"; + exit_code = -1; + } + } + MAPF_INFO("done"); + + ieee1905_transport.stop(); + + } else { + LOG(ERROR) << "Unable to start transport process!"; exit_code = -1; } + + broker->stop(); + + } else { + LOG(ERROR) << "Unable to start message broker!"; + exit_code = -1; } - MAPF_INFO("done"); - broker->stop(); + interface_state_manager->stop(); } else { - LOG(ERROR) << "Unable to start message broker!"; + LOG(ERROR) << "Unable to start interface state manager!"; exit_code = -1; } From 5016b5b788f9805ed78d9f9700fad6e6abf8de36 Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Fri, 31 Jul 2020 19:57:43 +0200 Subject: [PATCH 7/7] transport: remove unused code https://jira.prplfoundation.org/browse/PPM-18 Remove dead code after adding the InterfaceStateManager class. Signed-off-by: Mario Maz --- .../ieee1905_transport/CMakeLists.txt | 1 - .../ieee1905_transport/ieee1905_transport.h | 11 -- .../ieee1905_transport_netlink.cpp | 122 ------------------ 3 files changed, 134 deletions(-) delete mode 100644 framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp diff --git a/framework/transport/ieee1905_transport/CMakeLists.txt b/framework/transport/ieee1905_transport/CMakeLists.txt index 89ff9eb7cf..d492e5f162 100644 --- a/framework/transport/ieee1905_transport/CMakeLists.txt +++ b/framework/transport/ieee1905_transport/CMakeLists.txt @@ -2,7 +2,6 @@ add_library(ieee1905_transport_lib ieee1905_transport.cpp ieee1905_transport_broker.cpp ieee1905_transport_network.cpp - ieee1905_transport_netlink.cpp ieee1905_transport_local_bus.cpp ieee1905_transport_packet_processing.cpp) diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.h b/framework/transport/ieee1905_transport/ieee1905_transport.h index 2c4aca24c8..b08de024b8 100644 --- a/framework/transport/ieee1905_transport/ieee1905_transport.h +++ b/framework/transport/ieee1905_transport/ieee1905_transport.h @@ -21,7 +21,6 @@ #include #include -#include #include #include @@ -127,9 +126,6 @@ class Ieee1905Transport { // interface name (ifname) is used as Key to the table std::map network_interfaces_; - // netlink socket file descriptor (used to track network interface status) - int netlink_fd_ = -1; - uint16_t message_id_ = 0; uint8_t al_mac_addr_[ETH_ALEN] = {0}; @@ -315,13 +311,6 @@ class Ieee1905Transport { bool send_packet_to_network_interface(unsigned int if_index, Packet &packet); void set_al_mac_addr(const uint8_t *addr); - // - // NETLINK STUFF - // - bool open_netlink_socket(); - int handle_netlink_message(struct nlmsghdr *msg); - void handle_netlink_pollin_event(); - // // BROKER STUFF // diff --git a/framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp b/framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp deleted file mode 100644 index a93b71adbb..0000000000 --- a/framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp +++ /dev/null @@ -1,122 +0,0 @@ -/* SPDX-License-Identifier: BSD-2-Clause-Patent - * - * SPDX-FileCopyrightText: 2016-2020 the prplMesh contributors (see AUTHORS.md) - * - * This code is subject to the terms of the BSD+Patent license. - * See LICENSE file for more details. - */ - -#include "ieee1905_transport.h" - -#include -#include -#include -#include -#include - -// Why is this netlink interface tracking required? -// ------------------------------------------------ -// When an interface is down or comes down - a poll on the interface's raw socket returns an error. When this happens -// we have no choice but to close the socket for that interface. At a later time the interface may become again active -// and we should then re-open the interface's raw socket, and add it to the poll list. To do that we need an event -// to tell us that the interface is up and running - This is what we use netlink for. - -namespace beerocks { -namespace transport { - -bool Ieee1905Transport::open_netlink_socket() -{ - struct sockaddr_nl addr; - - // open a netlink socket of NETLINK_ROUTE family to get link updates - int sockfd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE); - if (sockfd < 0) { - MAPF_ERR("cannot open netlink socket."); - return false; - } - - memset((void *)&addr, 0, sizeof(addr)); - - addr.nl_family = AF_NETLINK; - addr.nl_pid = 0; // let the kernel assign nl_pid - addr.nl_groups = RTMGRP_LINK; // subscribe for link updates - - if (bind(sockfd, (struct sockaddr *)&addr, sizeof(addr)) < 0) { - MAPF_ERR("cannot bind netlink socket."); - close(sockfd); - return false; - } - - netlink_fd_ = sockfd; - - return true; -} - -// handle a single netlink message (each netlink received buffer can contain many messages) -// return 1 if successful, or zero if last message or -1 on error. -int Ieee1905Transport::handle_netlink_message(struct nlmsghdr *msghdr) -{ - // done - if (msghdr->nlmsg_type == NLMSG_DONE) { - return 0; - } - - // error - if (msghdr->nlmsg_type == NLMSG_ERROR) { - MAPF_ERR("received netlink NLMSG_ERROR message."); - return -1; - } - - // only consider interface state update messages (silently ignore other messages) - if (msghdr->nlmsg_type == RTM_NEWLINK || msghdr->nlmsg_type == RTM_DELLINK) { - struct ifinfomsg *ifi = (struct ifinfomsg *)NLMSG_DATA(msghdr); - bool is_active = (ifi->ifi_flags & IFF_RUNNING) && (ifi->ifi_flags & IFF_UP); - std::string ifname = if_index2name(ifi->ifi_index); - if (ifname.empty()) { - MAPF_DBG("Failed to get interface name for index " << ifi->ifi_index); - return false; - } - MAPF_DBG("received netlink RTM_NEWLINK/RTM_DELLINK message (interface " - << ifname << " is " << (is_active ? "active" : "inactive") << ")."); - - if (ifi->ifi_index > 0 && network_interfaces_.count(ifname) > 0) { - handle_interface_status_change(ifname, is_active); - } else if (ifi->ifi_index < 0) { - MAPF_WARN("bad interface index (" << ifi->ifi_index << ") in netlink message."); - } - } - - return 1; -} - -void Ieee1905Transport::handle_netlink_pollin_event() -{ - if (netlink_fd_ < 0) { - MAPF_ERR("invalid netlink socket file descriptor."); - return; - } - - char buf[4096]; - ssize_t len = recvfrom(netlink_fd_, buf, sizeof(buf), 0, NULL, 0); - if (len < 0) { - if (errno == EWOULDBLOCK || errno == EAGAIN) - return; - - MAPF_ERR("cannot read from netlink socket \"" << strerror(errno) << "\" (" << errno - << ")."); - return; - } - - // the stream can contain multiple messages - for (struct nlmsghdr *msghdr = (struct nlmsghdr *)buf; NLMSG_OK(msghdr, len); - msghdr = NLMSG_NEXT(msghdr, len)) { - if (handle_netlink_message(msghdr) <= 0) { - break; - } - } - - return; -} - -} // namespace transport -} // namespace beerocks