From fbbebc21230b707551b0bb7dc920368059644cdc Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 16 Aug 2016 15:55:04 +0200
Subject: [PATCH] device: always expose device statistics information

Instead of updating the device-statistic counters only periodically as
we refresh the link, update them on every link-changed event from
platform.

That means, also for devices that have RefreshRateMs at zero, the values
will be updated at random times when the link information changes.
The difference is, that previously the counters would be zero unless
RefreshRateMs was set. Now, they have some (probably stale) values
which however are not guaranteed to be kept up-to-date.

Also, now 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                | 55 +++++++++++++-------------
 2 files changed, 31 insertions(+), 32 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 @@
     <!--
         RefreshRateMs:
 
-        Rate of change of the rest of properties of this interface. If zero, the
-        properties do not change. Otherwise, the properties are refreshed each
-        RefreshRateMs milliseconds in case the underlying counter has changed
-        too.
+        Refresh rate of the rest of properties of this interface. The properties
+        are guaranteed to be refreshed each RefreshRateMs milliseconds in case
+        the underlying counter has changed too.
+        If zero, there is no guaranteed refresh rate of the properties.
     -->
     <property name="RefreshRateMs" type="u" access="readwrite"/>
 
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index de7ec70db4..7e41e9e743 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -803,30 +803,24 @@ _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;
+
+	ifindex = nm_device_get_ip_ifindex (self);
+
+	_LOGT (LOGD_DEVICE, "stats: refresh %d", ifindex);
+
+	if (ifindex > 0)
+		nm_platform_link_refresh (NM_PLATFORM_GET, ifindex);
 
-	_LOGT (LOGD_DEVICE, "stats: refresh");
-	_stats_refresh (self);
 	return G_SOURCE_CONTINUE;
 }
 
@@ -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,17 @@ _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);
+	if (!refresh_rate_ms)
+		return;
+
+	/* 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 +1826,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 +1957,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 +2252,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 +2323,8 @@ 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);
+	if (real_rate)
 		priv->stats.timeout_id = g_timeout_add (real_rate, _stats_timeout_cb, self);
-	}
 
 	klass->realize_start_notify (self, plink);