From 79c5857d5ae2c245231a201f7ee4af63c94efed1 Mon Sep 17 00:00:00 2001 From: Mario Maz Date: Fri, 31 Jul 2020 19:15:07 +0200 Subject: [PATCH] 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 | 46 +++------ .../transport/ieee1905_transport/main.cpp | 94 +++++++++++++++---- 5 files changed, 135 insertions(+), 98 deletions(-) diff --git a/framework/transport/ieee1905_transport/ieee1905_transport.cpp b/framework/transport/ieee1905_transport/ieee1905_transport.cpp index ddf43e5dc3..b9e533c380 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->set_handler(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; - }, - }); + 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..2bd612e6b7 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; }, }); @@ -457,10 +440,9 @@ void Ieee1905Transport::set_al_mac_addr(const uint8_t *addr) 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(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; }