Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Commit

Permalink
transport: use new InterfaceStateManager
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@mariomaz
mariomaz committed Aug 13, 2020
1 parent de2a052 commit 79c5857
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 98 deletions.
56 changes: 15 additions & 41 deletions framework/transport/ieee1905_transport/ieee1905_transport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@
namespace beerocks {
namespace transport {

Ieee1905Transport::Ieee1905Transport(const std::shared_ptr<broker::BrokerServer> &broker,
const std::shared_ptr<EventLoop> &event_loop)
: m_broker(broker), m_event_loop(event_loop)
Ieee1905Transport::Ieee1905Transport(
const std::shared_ptr<beerocks::net::InterfaceStateManager> &interface_state_manager,
const std::shared_ptr<broker::BrokerServer> &broker,
const std::shared_ptr<EventLoop> &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...";

Expand All @@ -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<Socket>(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
Expand Down
35 changes: 29 additions & 6 deletions framework/transport/ieee1905_transport/ieee1905_transport.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <mapf/transport/ieee1905_transport_messages.h>

#include <bcl/beerocks_event_loop.h>
#include <bcl/network/interface_state_manager.h>

#include "ieee1905_transport_broker.h"

Expand Down Expand Up @@ -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::BrokerServer> &broker,
const std::shared_ptr<EventLoop> &event_loop);
void run();
Ieee1905Transport(
const std::shared_ptr<beerocks::net::InterfaceStateManager> &interface_state_manager,
const std::shared_ptr<broker::BrokerServer> &broker,
const std::shared_ptr<EventLoop> &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<beerocks::net::InterfaceStateManager> m_interface_state_manager;

/**
* Message broker implementing the publish/subscribe design pattern.
*/
Expand Down Expand Up @@ -284,9 +307,9 @@ class Ieee1905Transport {
//
void
update_network_interfaces(std::map<std::string, NetworkInterface> 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);
Expand Down
2 changes: 1 addition & 1 deletion framework/transport/ieee1905_transport/ieee1905_transport_netlink.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand Down
46 changes: 14 additions & 32 deletions framework/transport/ieee1905_transport/ieee1905_transport_network.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;
},
});
Expand All @@ -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) {
Expand Down Expand Up @@ -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
<< ").");
Expand All @@ -217,21 +212,15 @@ bool Ieee1905Transport::open_interface_socket(unsigned int if_index)
network_interfaces_[ifname].fd = std::make_shared<Socket>(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;
}

Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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;
},
});
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Loading

0 comments on commit 79c5857

Please sign in to comment.