From a11c20a25d9af8107d1791e5435b0e9b36efd1b2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 15:55:04 +0200 Subject: device: always expose device statistics information An unpreviledged user cannot enable the device statistics. That limits the use of the device-statistics counter, also as this is not private information and can be obtained otherwise via `ip -stat -stat link show`. Now, always internally enable a minimum RefreshRateMs 30 seconds. Yes, there is a certain overhad associated with it, as we now refresh every link at least every 60 seconds. Also note, that now the new counters are read during device_link_changed(). That means, they update whenever a netlink message indicates a change on the link, which in turn causes a D-Bus signal with the new values. This means, that we possibly get updates much more often then RefreshRateMs suggests. After all, it's RefreshRateMs, not RateLimitMs. That is, it configures a minimal refresh-rate of events, not a maximum limit on the events. I think that is not a real performance issue, as we don't expect to get flodded with netlink messages. If that happens, we can add proper rate-limiting later. Also, how we refresh more often then promised by RefreshRateMs. But the API technically doesn't specify that, so if we find there is a problem with this, we may revert it later. --- introspection/nm-device-statistics.xml | 8 ++--- src/devices/nm-device.c | 57 ++++++++++++++++------------------ 2 files changed, 30 insertions(+), 35 deletions(-) diff --git a/introspection/nm-device-statistics.xml b/introspection/nm-device-statistics.xml index 5d23bf3062..bdb19c89ce 100644 --- a/introspection/nm-device-statistics.xml +++ b/introspection/nm-device-statistics.xml @@ -5,10 +5,10 @@ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index ff4ced9d49..3337d2d5a9 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -803,44 +803,38 @@ _stats_update_counters (NMDevice *self, } static void -_stats_refresh (NMDevice *self) +_stats_update_counters_from_pllink (NMDevice *self, const NMPlatformLink *pllink) { - const NMPlatformLink *pllink; - int ifindex; - - ifindex = nm_device_get_ip_ifindex (self); - if (ifindex > 0) { - nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); - pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); - if (pllink) { - _stats_update_counters (self, pllink->tx_bytes, pllink->rx_bytes); - return; - } - } - _stats_update_counters (self, 0, 0); + _stats_update_counters (self, pllink->tx_bytes, pllink->rx_bytes); } static gboolean _stats_timeout_cb (gpointer user_data) { NMDevice *self = user_data; + int ifindex; _LOGT (LOGD_DEVICE, "stats: refresh"); - _stats_refresh (self); + + ifindex = nm_device_get_ip_ifindex (self); + if (ifindex > 0) + nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); + return G_SOURCE_CONTINUE; } static guint _stats_refresh_rate_real (guint refresh_rate_ms) { + const guint STATS_REFRESH_RATE_MS_MAX = 30000; const guint STATS_REFRESH_RATE_MS_MIN = 200; - if (refresh_rate_ms == 0) - return 0; + if (refresh_rate_ms == 0 || refresh_rate_ms > STATS_REFRESH_RATE_MS_MAX) + return STATS_REFRESH_RATE_MS_MAX; if (refresh_rate_ms < STATS_REFRESH_RATE_MS_MIN) { /* you cannot set the refresh-rate arbitrarly small. E.g. - * setting to 1ms is just killing. */ + * setting to 1ms is just killing. Have a lowest number. */ return STATS_REFRESH_RATE_MS_MIN; } @@ -851,6 +845,7 @@ static void _stats_set_refresh_rate (NMDevice *self, guint refresh_rate_ms) { NMDevicePrivate *priv; + int ifindex; guint old_rate; priv = NM_DEVICE_GET_PRIVATE (self); @@ -871,16 +866,14 @@ _stats_set_refresh_rate (NMDevice *self, guint refresh_rate_ms) if (_stats_refresh_rate_real (old_rate) == refresh_rate_ms) return; - if (!refresh_rate_ms) { - nm_clear_g_source (&priv->stats.timeout_id); - _stats_update_counters (self, 0, 0); - return; - } - nm_clear_g_source (&priv->stats.timeout_id); - /* trigger an inital refresh of the data when the refresh-rate changes */ - _stats_refresh (self); + /* trigger an inital refresh of the data whenever the refresh-rate changes. + * As we process the result in an idle handler with device_link_changed(), + * we don't get the result right away. */ + ifindex = nm_device_get_ip_ifindex (self); + if (ifindex > 0) + nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); priv->stats.timeout_id = g_timeout_add (refresh_rate_ms, _stats_timeout_cb, self); } @@ -1830,6 +1823,9 @@ device_link_changed (NMDevice *self) _notify (self, PROP_DRIVER); } + if (ifindex == nm_device_get_ip_ifindex (self)) + _stats_update_counters_from_pllink (self, &info); + had_hw_addr = (priv->hw_addr != NULL); nm_device_update_hw_address (self); got_hw_addr = (!had_hw_addr && priv->hw_addr); @@ -1958,6 +1954,8 @@ device_ip_link_changed (NMDevice *self) if (!pllink) return G_SOURCE_REMOVE; + _stats_update_counters_from_pllink (self, pllink); + if (pllink->name[0] && g_strcmp0 (priv->ip_iface, pllink->name)) { _LOGI (LOGD_DEVICE, "interface index %d renamed ip_iface (%d) from '%s' to '%s'", priv->ifindex, nm_device_get_ip_ifindex (self), @@ -2251,6 +2249,7 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) if (plink) { g_return_if_fail (link_type_compatible (self, plink->type, NULL, NULL)); update_device_from_platform_link (self, plink); + _stats_update_counters_from_pllink (self, plink); } if (priv->ifindex > 0) { @@ -2321,11 +2320,7 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) nm_assert (!priv->stats.timeout_id); real_rate = _stats_refresh_rate_real (priv->stats.refresh_rate_ms); - if (real_rate) { - if (plink) - _stats_update_counters (self, plink->tx_bytes, plink->rx_bytes); - priv->stats.timeout_id = g_timeout_add (real_rate, _stats_timeout_cb, self); - } + priv->stats.timeout_id = g_timeout_add (real_rate, _stats_timeout_cb, self); klass->realize_start_notify (self, plink); -- cgit v1.2.1