From 23b9923eeba13bdba51b601f2c5c7e4f20b8710a Mon Sep 17 00:00:00 2001 From: Anthony PERARD Date: Sun, 30 May 2021 15:56:56 +0100 Subject: [PATCH 1/4] network: Parse whole RTM_NEWROUTE msg before interpreting it The check to figure out if we have the default route should be after the for loop that parses the route attributes, to avoid acting on incomplete information. We are going to use more fields from the message. --- src/modules/network.cpp | 76 ++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/src/modules/network.cpp b/src/modules/network.cpp index 583daaea..3edc4dcc 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -623,49 +623,47 @@ int waybar::modules::Network::handleEvents(struct nl_msg *msg, void *data) { default: break; } + } - /* If this is the default route, and we know the interface index, - * we can stop parsing this message. - */ - if (has_gateway && !has_destination && temp_idx != -1) { - if (!is_del_event) { - net->ifid_ = temp_idx; + // Check if we have a default route. + if (has_gateway && !has_destination && temp_idx != -1) { + if (!is_del_event) { + net->ifid_ = temp_idx; - spdlog::debug("network: new default route via if{}", temp_idx); + spdlog::debug("network: new default route via if{}", temp_idx); - /* Ask ifname associated with temp_idx as well as carrier status */ - struct ifinfomsg ifinfo_hdr = { - .ifi_family = AF_UNSPEC, - .ifi_index = temp_idx, - }; - int err; - err = nl_send_simple(net->ev_sock_, RTM_GETLINK, NLM_F_REQUEST, - &ifinfo_hdr, sizeof (ifinfo_hdr)); - if (err < 0) { - spdlog::error("network: failed to ask link info: {}", err); - /* Ask for a dump of all links instead */ - net->want_link_dump_ = true; - } - - /* Also ask for the address. Asking for a addresses of a specific - * interface doesn't seems to work so ask for a dump of all - * addresses. */ - net->want_addr_dump_ = true; - net->askForStateDump(); - net->thread_timer_.wake_up(); - } else if (is_del_event && temp_idx == net->ifid_) { - spdlog::debug("network: default route deleted {}/if{}", - net->ifname_, temp_idx); - - net->ifname_.clear(); - net->clearIface(); - net->dp.emit(); - /* Ask for a dump of all routes in case another one is already - * setup. If there's none, there'll be an event with new one - * later. */ - net->want_route_dump_ = true; - net->askForStateDump(); + /* Ask ifname associated with temp_idx as well as carrier status */ + struct ifinfomsg ifinfo_hdr = { + .ifi_family = AF_UNSPEC, + .ifi_index = temp_idx, + }; + int err; + err = nl_send_simple(net->ev_sock_, RTM_GETLINK, NLM_F_REQUEST, + &ifinfo_hdr, sizeof (ifinfo_hdr)); + if (err < 0) { + spdlog::error("network: failed to ask link info: {}", err); + /* Ask for a dump of all links instead */ + net->want_link_dump_ = true; } + + /* Also ask for the address. Asking for a addresses of a specific + * interface doesn't seems to work so ask for a dump of all + * addresses. */ + net->want_addr_dump_ = true; + net->askForStateDump(); + net->thread_timer_.wake_up(); + } else if (is_del_event && temp_idx == net->ifid_) { + spdlog::debug("network: default route deleted {}/if{}", + net->ifname_, temp_idx); + + net->ifname_.clear(); + net->clearIface(); + net->dp.emit(); + /* Ask for a dump of all routes in case another one is already + * setup. If there's none, there'll be an event with new one + * later. */ + net->want_route_dump_ = true; + net->askForStateDump(); } } break; From ce97df34e69689d97adbf481ef65997beb4f3f98 Mon Sep 17 00:00:00 2001 From: Anthony PERARD Date: Sat, 5 Jun 2021 15:44:32 +0100 Subject: [PATCH 2/4] network: Also clear ifname in clearIface() Since we reset `ifid_`, clear `ifname_` as well. --- src/modules/network.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/network.cpp b/src/modules/network.cpp index 3edc4dcc..e895569a 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -400,6 +400,7 @@ bool waybar::modules::Network::checkInterface(std::string name) { void waybar::modules::Network::clearIface() { ifid_ = -1; + ifname_.clear(); essid_.clear(); ipaddr_.clear(); netmask_.clear(); @@ -656,7 +657,6 @@ int waybar::modules::Network::handleEvents(struct nl_msg *msg, void *data) { spdlog::debug("network: default route deleted {}/if{}", net->ifname_, temp_idx); - net->ifname_.clear(); net->clearIface(); net->dp.emit(); /* Ask for a dump of all routes in case another one is already From efaac20d8200b2249668773fa96ef40ea66fe4fe Mon Sep 17 00:00:00 2001 From: Anthony PERARD Date: Wed, 2 Jun 2021 22:15:34 +0100 Subject: [PATCH 3/4] network: Handle ip route priority When there's a new default route with higher priority, switch to it. --- include/modules/network.hpp | 1 + src/modules/network.cpp | 26 ++++++++++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/include/modules/network.hpp b/include/modules/network.hpp index 2523dc22..009ae5a3 100644 --- a/include/modules/network.hpp +++ b/include/modules/network.hpp @@ -72,6 +72,7 @@ class Network : public ALabel { int32_t signal_strength_dbm_; uint8_t signal_strength_; uint32_t frequency_; + uint32_t route_priority; util::SleeperThread thread_; util::SleeperThread thread_timer_; diff --git a/src/modules/network.cpp b/src/modules/network.cpp index e895569a..de023ef4 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -573,11 +573,7 @@ int waybar::modules::Network::handleEvents(struct nl_msg *msg, void *data) { bool has_gateway = false; bool has_destination = false; int temp_idx = -1; - - /* If we found the correct answer, skip parsing the attributes. */ - if (!is_del_event && net->ifid_ != -1) { - return NL_OK; - } + uint32_t priority = 0; /* Find the message(s) concerting the main routing table, each message * corresponds to a single routing table entry. @@ -621,6 +617,9 @@ int waybar::modules::Network::handleEvents(struct nl_msg *msg, void *data) { /* The output interface index. */ temp_idx = *static_cast(RTA_DATA(attr)); break; + case RTA_PRIORITY: + priority = *(uint32_t*)RTA_DATA(attr); + break; default: break; } @@ -628,10 +627,16 @@ int waybar::modules::Network::handleEvents(struct nl_msg *msg, void *data) { // Check if we have a default route. if (has_gateway && !has_destination && temp_idx != -1) { - if (!is_del_event) { + // Check if this is the first default route we see, or if this new + // route have a higher priority. + if (!is_del_event && ((net->ifid_ == -1) || (priority < net->route_priority))) { + // Clear if's state for the case were there is a higher priority + // route on a different interface. + net->clearIface(); net->ifid_ = temp_idx; + net->route_priority = priority; - spdlog::debug("network: new default route via if{}", temp_idx); + spdlog::debug("network: new default route via if{} metric {}", temp_idx, priority); /* Ask ifname associated with temp_idx as well as carrier status */ struct ifinfomsg ifinfo_hdr = { @@ -653,9 +658,10 @@ int waybar::modules::Network::handleEvents(struct nl_msg *msg, void *data) { net->want_addr_dump_ = true; net->askForStateDump(); net->thread_timer_.wake_up(); - } else if (is_del_event && temp_idx == net->ifid_) { - spdlog::debug("network: default route deleted {}/if{}", - net->ifname_, temp_idx); + } else if (is_del_event && temp_idx == net->ifid_ + && net->route_priority == priority) { + spdlog::debug("network: default route deleted {}/if{} metric {}", + net->ifname_, temp_idx, priority); net->clearIface(); net->dp.emit(); From 33617b67f0c342f15a72da413c4e6b0b46f9e196 Mon Sep 17 00:00:00 2001 From: Anthony PERARD Date: Wed, 2 Jun 2021 22:16:28 +0100 Subject: [PATCH 4/4] network: Fix one case where default route is deleted without notification When an interface's state is change to "down", all the route associated with it are deleted without an RTM_DELROUTE event. So when this happen, reset the module to start looking for a new external interface / default route. Fixes #1117 --- src/modules/network.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/modules/network.cpp b/src/modules/network.cpp index de023ef4..ec221a2f 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -1,6 +1,7 @@ #include "modules/network.hpp" #include #include +#include #include #include #include @@ -432,6 +433,20 @@ int waybar::modules::Network::handleEvents(struct nl_msg *msg, void *data) { return NL_OK; } + // Check if the interface goes "down" and if we want to detect the + // external interface. + if (net->ifid_ != -1 && !(ifi->ifi_flags & IFF_UP) + && !net->config_["interface"].isString()) { + // The current interface is now down, all the routes associated with + // it have been deleted, so start looking for a new default route. + spdlog::debug("network: if{} down", net->ifid_); + net->clearIface(); + net->dp.emit(); + net->want_route_dump_ = true; + net->askForStateDump(); + return NL_OK; + } + for (; RTA_OK(ifla, attrlen); ifla = RTA_NEXT(ifla, attrlen)) { switch (ifla->rta_type) { case IFLA_IFNAME: