From 9d9f95976926d5817837d03344b9506d700cee16 Mon Sep 17 00:00:00 2001 From: Ciaran Downey Date: Wed, 25 Aug 2021 14:47:51 -0700 Subject: [PATCH 1/2] Switch network module to read /proc/net/dev This fixes issue #610 by reading bandwidth usage per-interface from /proc/net/dev instead of globally via /proc/net/netstat. It supports the same matching logic as elsewhere, so setting interface to '*' should display the same sum-total bandwidth usage as the previous implementation. --- include/modules/network.hpp | 1 + src/modules/network.cpp | 168 +++++++++++++++++------------------- 2 files changed, 79 insertions(+), 90 deletions(-) diff --git a/include/modules/network.hpp b/include/modules/network.hpp index 009ae5a3..183c9e2c 100644 --- a/include/modules/network.hpp +++ b/include/modules/network.hpp @@ -43,6 +43,7 @@ class Network : public ALabel { const std::string getNetworkState() const; void clearIface(); bool wildcardMatch(const std::string& pattern, const std::string& text) const; + std::optional> readBandwidthUsage(); int ifid_; sa_family_t family_; diff --git a/src/modules/network.cpp b/src/modules/network.cpp index 7d0f6382..22b9f713 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -1,87 +1,79 @@ -#include "modules/network.hpp" +#include #include #include -#include -#include + #include +#include +#include #include + +#include "modules/network.hpp" #include "util/format.hpp" #ifdef WANT_RFKILL #include "util/rfkill.hpp" #endif namespace { - using namespace waybar::util; - -constexpr const char *NETSTAT_FILE = - "/proc/net/netstat"; // std::ifstream does not take std::string_view as param -constexpr std::string_view BANDWIDTH_CATEGORY = "IpExt"; -constexpr std::string_view BANDWIDTH_DOWN_TOTAL_KEY = "InOctets"; -constexpr std::string_view BANDWIDTH_UP_TOTAL_KEY = "OutOctets"; constexpr const char *DEFAULT_FORMAT = "{ifname}"; - -std::ifstream netstat(NETSTAT_FILE); -std::optional read_netstat(std::string_view category, std::string_view key) { - if (!netstat) { - spdlog::warn("Failed to open netstat file {}", NETSTAT_FILE); - return {}; - } - netstat.seekg(std::ios_base::beg); - - // finding corresponding line (category) - // looks into the file for the first line starting by the 'category' string - auto starts_with = [](const std::string &str, std::string_view start) { - return start == std::string_view{str.data(), std::min(str.size(), start.size())}; - }; - - std::string read; - while (std::getline(netstat, read) && !starts_with(read, category)) - ; - if (!starts_with(read, category)) { - spdlog::warn("Category '{}' not found in netstat file {}", category, NETSTAT_FILE); - return {}; - } - - // finding corresponding column (key) - // looks into the fetched line for the first word (space separated) equal to 'key' - int index = 0; - auto r_it = read.begin(); - auto k_it = key.begin(); - while (k_it != key.end() && r_it != read.end()) { - if (*r_it != *k_it) { - r_it = std::find(r_it, read.end(), ' '); - if (r_it != read.end()) { - ++r_it; - } - k_it = key.begin(); - ++index; - } else { - ++r_it; - ++k_it; - } - } - - if (r_it == read.end() && k_it != key.end()) { - spdlog::warn( - "Key '{}' not found in category '{}' of netstat file {}", key, category, NETSTAT_FILE); - return {}; - } - - // finally accessing value - // accesses the line right under the fetched one - std::getline(netstat, read); - assert(starts_with(read, category)); - std::istringstream iss(read); - while (index--) { - std::getline(iss, read, ' '); - } - unsigned long long value; - iss >> value; - return value; -} } // namespace +constexpr const char *NETDEV_FILE = + "/proc/net/dev"; // std::ifstream does not take std::string_view as param +std::optional> +waybar::modules::Network::readBandwidthUsage() { + std::ifstream netdev(NETDEV_FILE); + if (!netdev) { + spdlog::warn("Failed to open netdev file {}", NETDEV_FILE); + return {}; + } + + // skip the headers + std::string line; + std::getline(netdev, line); + std::getline(netdev, line); + if (!netdev) { + spdlog::warn("Unexpectedly short netdev file {}", NETDEV_FILE); + return {}; + } + + unsigned long long receivedBytes = 0ull; + unsigned long long transmittedBytes = 0ull; + for (std::getline(netdev, line); netdev; std::getline(netdev, line)) { + std::istringstream iss(line); + + std::string ifacename; + iss >> ifacename; // ifacename contains "eth0:" + ifacename.pop_back(); // remove trailing ':' + if (!checkInterface(ifacename)) { + continue; + } + + // The rest of the line consists of whitespace separated counts divided + // into two groups (receive and transmit). The first column in each group + // is bytes, which is the only one we care about. + unsigned long long r = 0ull; + unsigned long long t = 0ull; + // Read received bytes + iss >> r; + // Skip all the other columns in the received group + for (int colsToSkip = 7; colsToSkip > 0; colsToSkip--) { + // skip whitespace between columns + while (iss.peek() == ' ') { iss.ignore(); } + // skip the irrelevant column + while (iss.peek() != ' ') { iss.ignore(); } + } + // Read transmit bytes + iss >> t; + spdlog::trace("read r={}, t={}, iface={}", r, t, ifacename); + + receivedBytes += r; + transmittedBytes += t; + } + + return {{receivedBytes, transmittedBytes}}; +} + waybar::modules::Network::Network(const std::string &id, const Json::Value &config) : ALabel(config, "network", id, DEFAULT_FORMAT, 60), ifid_(-1), @@ -106,17 +98,12 @@ waybar::modules::Network::Network(const std::string &id, const Json::Value &conf // the module start with no text, but the the event_box_ is shown. label_.set_markup(""); - auto down_octets = read_netstat(BANDWIDTH_CATEGORY, BANDWIDTH_DOWN_TOTAL_KEY); - auto up_octets = read_netstat(BANDWIDTH_CATEGORY, BANDWIDTH_UP_TOTAL_KEY); - if (down_octets) { - bandwidth_down_total_ = *down_octets; + auto bandwidth = readBandwidthUsage(); + if (bandwidth.has_value()) { + bandwidth_down_total_ = (*bandwidth).first; + bandwidth_up_total_ = (*bandwidth).second; } else { bandwidth_down_total_ = 0; - } - - if (up_octets) { - bandwidth_up_total_ = *up_octets; - } else { bandwidth_up_total_ = 0; } @@ -303,20 +290,21 @@ const std::string waybar::modules::Network::getNetworkState() const { auto waybar::modules::Network::update() -> void { std::lock_guard lock(mutex_); std::string tooltip_format; - auto down_octets = read_netstat(BANDWIDTH_CATEGORY, BANDWIDTH_DOWN_TOTAL_KEY); - auto up_octets = read_netstat(BANDWIDTH_CATEGORY, BANDWIDTH_UP_TOTAL_KEY); - unsigned long long bandwidth_down = 0; - if (down_octets) { - bandwidth_down = *down_octets - bandwidth_down_total_; - bandwidth_down_total_ = *down_octets; + auto bandwidth = readBandwidthUsage(); + auto bandwidth_down = 0ull; + auto bandwidth_up = 0ull; + if (bandwidth.has_value()) { + auto down_octets = (*bandwidth).first; + auto up_octets = (*bandwidth).second; + + bandwidth_down = down_octets - bandwidth_down_total_; + bandwidth_down_total_ = down_octets; + + bandwidth_up = up_octets - bandwidth_up_total_; + bandwidth_up_total_ = up_octets; } - unsigned long long bandwidth_up = 0; - if (up_octets) { - bandwidth_up = *up_octets - bandwidth_up_total_; - bandwidth_up_total_ = *up_octets; - } if (!alt_) { auto state = getNetworkState(); if (!state_.empty() && label_.get_style_context()->has_class(state_)) { From 5186dd27e64b00b19736a2fa3bd6b325c4fcd555 Mon Sep 17 00:00:00 2001 From: Ciaran Downey Date: Thu, 26 Aug 2021 11:30:06 -0700 Subject: [PATCH 2/2] Use while (getline) instead of a for loop Also make the comments surrounding the /proc/net/dev parsing clearer and remove the apparently redundant "is the netdev file still good?" check. --- src/modules/network.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/modules/network.cpp b/src/modules/network.cpp index 22b9f713..5818f025 100644 --- a/src/modules/network.cpp +++ b/src/modules/network.cpp @@ -28,18 +28,14 @@ waybar::modules::Network::readBandwidthUsage() { return {}; } - // skip the headers std::string line; + // skip the headers (first two lines) std::getline(netdev, line); std::getline(netdev, line); - if (!netdev) { - spdlog::warn("Unexpectedly short netdev file {}", NETDEV_FILE); - return {}; - } unsigned long long receivedBytes = 0ull; unsigned long long transmittedBytes = 0ull; - for (std::getline(netdev, line); netdev; std::getline(netdev, line)) { + while (std::getline(netdev, line)) { std::istringstream iss(line); std::string ifacename; @@ -50,8 +46,11 @@ waybar::modules::Network::readBandwidthUsage() { } // The rest of the line consists of whitespace separated counts divided - // into two groups (receive and transmit). The first column in each group - // is bytes, which is the only one we care about. + // into two groups (receive and transmit). Each group has the following + // columns: bytes, packets, errs, drop, fifo, frame, compressed, multicast + // + // We only care about the bytes count, so we'll just ignore the 7 other + // columns. unsigned long long r = 0ull; unsigned long long t = 0ull; // Read received bytes @@ -65,7 +64,6 @@ waybar::modules::Network::readBandwidthUsage() { } // Read transmit bytes iss >> t; - spdlog::trace("read r={}, t={}, iface={}", r, t, ifacename); receivedBytes += r; transmittedBytes += t;