From f04baa63c0a6828b4b6256c495d23408ebacec77 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 15:40:08 +0200 Subject: [PATCH 01/18] device: copy the plink instance before realize_start_setup() To make sure, we don't end up with a dangling pointer due to an intermediate platform access which may invalidate the pointer. --- src/devices/nm-device.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 177080d9c5..9c7319f03d 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1955,6 +1955,8 @@ nm_device_realize_start (NMDevice *self, gboolean *out_compatible, GError **error) { + NMPlatformLink plink_copy; + NM_SET_OUT (out_compatible, TRUE); if (plink) { @@ -1969,6 +1971,10 @@ nm_device_realize_start (NMDevice *self, return FALSE; } + if (plink) { + plink_copy = *plink; + plink = &plink_copy; + } realize_start_setup (self, plink); return TRUE; From 3bc5c7dbc1d33a13be753ca9469d25eab569c862 Mon Sep 17 00:00:00 2001 From: Alfonso Sanchez-Beato Date: Wed, 10 Aug 2016 11:54:29 +0200 Subject: [PATCH 02/18] exported-object: allow exporting multiple ifaces Allow exporting more than one interface per object class. --- src/nm-exported-object.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/nm-exported-object.c b/src/nm-exported-object.c index b313ff6c07..c4dbab8715 100644 --- a/src/nm-exported-object.c +++ b/src/nm-exported-object.c @@ -257,12 +257,16 @@ nm_exported_object_class_add_interface (NMExportedObjectClass *object_class, g_return_if_fail (NM_IS_EXPORTED_OBJECT_CLASS (object_class)); g_return_if_fail (g_type_is_a (dbus_skeleton_type, G_TYPE_DBUS_INTERFACE_SKELETON)); - classinfo = g_slice_new (NMExportedObjectClassInfo); - classinfo->skeleton_types = NULL; - classinfo->methods = g_array_new (FALSE, FALSE, sizeof (NMExportedObjectDBusMethodImpl)); - classinfo->properties = g_hash_table_new (g_str_hash, g_str_equal); - g_type_set_qdata (G_TYPE_FROM_CLASS (object_class), - nm_exported_object_class_info_quark (), classinfo); + classinfo = g_type_get_qdata (G_TYPE_FROM_CLASS (object_class), + nm_exported_object_class_info_quark ()); + if (!classinfo) { + classinfo = g_slice_new (NMExportedObjectClassInfo); + classinfo->skeleton_types = NULL; + classinfo->methods = g_array_new (FALSE, FALSE, sizeof (NMExportedObjectDBusMethodImpl)); + classinfo->properties = g_hash_table_new (g_str_hash, g_str_equal); + g_type_set_qdata (G_TYPE_FROM_CLASS (object_class), + nm_exported_object_class_info_quark (), classinfo); + } classinfo->skeleton_types = g_slist_prepend (classinfo->skeleton_types, GSIZE_TO_POINTER (dbus_skeleton_type)); @@ -342,8 +346,6 @@ nm_exported_object_class_add_interface (NMExportedObjectClass *object_class, } } - g_assert_cmpint (n_method_signals, ==, classinfo->methods->len); - g_type_class_unref (dbus_object_class); } From 6ed939e84176413dab4762d63d57bc848d190b54 Mon Sep 17 00:00:00 2001 From: Alfonso Sanchez-Beato Date: Wed, 10 Aug 2016 11:54:30 +0200 Subject: [PATCH 03/18] platform: add network statistics Make network traffic statistics data available through the platform. --- src/platform/nm-linux-platform.c | 32 ++++++++++++++++++++++++++++++++ src/platform/nm-platform.c | 19 +++++++++++++++++++ src/platform/nm-platform.h | 12 ++++++++++++ 3 files changed, 63 insertions(+) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index deb6c5f977..d185f96f12 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1476,6 +1476,15 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr nl_info_data = li[IFLA_INFO_DATA]; } + if (tb[IFLA_STATS64]) { + struct rtnl_link_stats64 *stats = nla_data (tb[IFLA_STATS64]); + + obj->link.rx_packets = stats->rx_packets; + obj->link.rx_bytes = stats->rx_bytes; + obj->link.tx_packets = stats->tx_packets; + obj->link.tx_bytes = stats->tx_bytes; + } + obj->link.n_ifi_flags = ifi->ifi_flags; obj->link.connected = NM_FLAGS_HAS (obj->link.n_ifi_flags, IFF_LOWER_UP); obj->link.arptype = ifi->ifi_type; @@ -3732,6 +3741,7 @@ event_valid_msg (NMPlatform *platform, struct nl_msg *msg, gboolean handle_event case RTM_NEWLINK: case RTM_NEWADDR: case RTM_NEWROUTE: + case RTM_GETLINK: cache_op = nmp_cache_update_netlink (priv->cache, obj, &obj_cache, &was_visible, cache_pre_hook, platform); cache_post (platform, msghdr, cache_op, obj, obj_cache); @@ -4517,6 +4527,27 @@ link_get_dev_id (NMPlatform *platform, int ifindex) return errno ? 0 : (int) int_val; } +static gboolean +link_get_stats (NMPlatform *platform, int ifindex, + guint64 *rx_packets, guint64 *rx_bytes, + guint64 *tx_packets, guint64 *tx_bytes) +{ + nm_auto_pop_netns NMPNetns *netns = NULL; + const NMPObject *obj; + + obj = cache_lookup_link (platform, ifindex); + + if (!obj) + return FALSE; + + *rx_packets = obj->link.rx_packets; + *rx_bytes = obj->link.rx_bytes; + *tx_packets = obj->link.tx_packets; + *tx_bytes = obj->link.tx_bytes; + + return TRUE; +} + static int vlan_add (NMPlatform *platform, const char *name, @@ -6509,6 +6540,7 @@ nm_linux_platform_class_init (NMLinuxPlatformClass *klass) platform_class->link_get_physical_port_id = link_get_physical_port_id; platform_class->link_get_dev_id = link_get_dev_id; + platform_class->link_get_stats = link_get_stats; platform_class->link_get_wake_on_lan = link_get_wake_on_lan; platform_class->link_get_driver_info = link_get_driver_info; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 75c85b9255..99dc00c812 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1265,6 +1265,21 @@ nm_platform_link_get_dev_id (NMPlatform *self, int ifindex) return 0; } +gboolean nm_platform_link_get_stats (NMPlatform *self, int ifindex, + guint64 *rx_packets, guint64 *rx_bytes, + guint64 *tx_packets, guint64 *tx_bytes) +{ + _CHECK_SELF (self, klass, 0); + + g_return_val_if_fail (ifindex >= 0, 0); + + if (klass->link_get_stats) + return klass->link_get_stats (self, ifindex, + rx_packets, rx_bytes, + tx_packets, tx_bytes); + return FALSE; +} + /** * nm_platform_link_get_wake_onlan: * @self: platform instance @@ -3777,6 +3792,10 @@ int nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) { _CMP_SELF (a, b); + _CMP_FIELD (a, b, rx_packets); + _CMP_FIELD (a, b, rx_bytes); + _CMP_FIELD (a, b, tx_packets); + _CMP_FIELD (a, b, tx_bytes); _CMP_FIELD (a, b, ifindex); _CMP_FIELD (a, b, type); _CMP_FIELD_STR (a, b, name); diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index c419855a98..19148deb4f 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -156,6 +156,12 @@ struct _NMPlatformLink { * initialized with memset(0) has and unset value.*/ guint8 inet6_addr_gen_mode_inv; + /* Statistics */ + guint64 rx_packets; + guint64 rx_bytes; + guint64 tx_packets; + guint64 tx_bytes; + /* @connected is mostly identical to (@n_ifi_flags & IFF_UP). Except for bridge/bond masters, * where we coerce the link as disconnect if it has no slaves. */ bool connected:1; @@ -532,6 +538,9 @@ typedef struct { char * (*link_get_physical_port_id) (NMPlatform *, int ifindex); guint (*link_get_dev_id) (NMPlatform *, int ifindex); + gboolean (*link_get_stats) (NMPlatform *platform, int ifindex, + guint64 *rx_packets, guint64 *rx_bytes, + guint64 *tx_packets, guint64 *tx_bytes); gboolean (*link_get_wake_on_lan) (NMPlatform *, int ifindex); gboolean (*link_get_driver_info) (NMPlatform *, int ifindex, @@ -763,6 +772,9 @@ gboolean nm_platform_link_set_mtu (NMPlatform *self, int ifindex, guint32 mtu); char *nm_platform_link_get_physical_port_id (NMPlatform *self, int ifindex); guint nm_platform_link_get_dev_id (NMPlatform *self, int ifindex); +gboolean nm_platform_link_get_stats (NMPlatform *self, int ifindex, + guint64 *rx_packets, guint64 *rx_bytes, + guint64 *tx_packets, guint64 *tx_bytes); gboolean nm_platform_link_get_wake_on_lan (NMPlatform *self, int ifindex); gboolean nm_platform_link_get_driver_info (NMPlatform *self, int ifindex, From 85834a66758aee6cd26ab79d169cad8a50eac3b7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 16:44:06 +0200 Subject: [PATCH 04/18] platform/tests: relax condition in platform test With device-statistics counters in NMPlatformLink we may get an additional link-changed event. Relax the assertion in the test. --- src/platform/tests/test-link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index e0870df561..9bf8aca831 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -567,7 +567,7 @@ test_internal (void) g_assert (nm_platform_link_set_up (NM_PLATFORM_GET, ifindex, NULL)); g_assert (nm_platform_link_is_up (NM_PLATFORM_GET, ifindex)); g_assert (nm_platform_link_is_connected (NM_PLATFORM_GET, ifindex)); - accept_signal (link_changed); + accept_signals (link_changed, 1, 2); g_assert (nm_platform_link_set_down (NM_PLATFORM_GET, ifindex)); g_assert (!nm_platform_link_is_up (NM_PLATFORM_GET, ifindex)); g_assert (!nm_platform_link_is_connected (NM_PLATFORM_GET, ifindex)); From 24b193ab641f63f3f8de770a40f0c7ec30ea04b5 Mon Sep 17 00:00:00 2001 From: Alfonso Sanchez-Beato Date: Wed, 10 Aug 2016 11:54:31 +0200 Subject: [PATCH 05/18] device: add statistics interface Add statistics interface to all device instances. When active, the properties of this interface are refreshed whenever there is network activity for the device. Activation is performed by changing RefreshRateMs property. If set to zero, the interface is deactivated. If set to other value, the rest of the interface properties are refreshed whenever the related network metric changes, being RefreshRateMs the minimum time between property changes, in milliseconds. --- introspection/Makefile.am | 6 +- introspection/nm-device-statistics.xml | 37 ++++++++ libnm-core/nm-dbus-interface.h | 1 + src/Makefile.am | 2 + src/devices/nm-device-private.h | 3 + src/devices/nm-device-statistics.c | 99 +++++++++++++++++++++ src/devices/nm-device-statistics.h | 31 +++++++ src/devices/nm-device.c | 117 +++++++++++++++++++++++++ src/devices/nm-device.h | 4 + 9 files changed, 299 insertions(+), 1 deletion(-) create mode 100644 introspection/nm-device-statistics.xml create mode 100644 src/devices/nm-device-statistics.c create mode 100644 src/devices/nm-device-statistics.h diff --git a/introspection/Makefile.am b/introspection/Makefile.am index d4762637a0..27b54e5d04 100644 --- a/introspection/Makefile.am +++ b/introspection/Makefile.am @@ -43,6 +43,8 @@ nodist_libnmdbus_la_SOURCES = \ nmdbus-device-modem.h \ nmdbus-device-olpc-mesh.c \ nmdbus-device-olpc-mesh.h \ + nmdbus-device-statistics.c \ + nmdbus-device-statistics.h \ nmdbus-device-team.c \ nmdbus-device-team.h \ nmdbus-device-tun.c \ @@ -114,7 +116,8 @@ DBUS_INTERFACE_DOCS = \ nmdbus-device-veth-org.freedesktop.NetworkManager.Device.Veth.xml \ nmdbus-settings-org.freedesktop.NetworkManager.Settings.xml \ nmdbus-device-ethernet-org.freedesktop.NetworkManager.Device.Wired.xml \ - nmdbus-ip4-config-org.freedesktop.NetworkManager.IP4Config.xml + nmdbus-ip4-config-org.freedesktop.NetworkManager.IP4Config.xml \ + nmdbus-device-statistics-org.freedesktop.NetworkManager.Device.Statistics.xml define _make_nmdbus_rule $(1): $(patsubst nmdbus-%.c,nm-%.xml,$(1)) @@ -154,6 +157,7 @@ EXTRA_DIST = \ nm-device-macvlan.xml \ nm-device-modem.xml \ nm-device-olpc-mesh.xml \ + nm-device-statistics.xml \ nm-device-team.xml \ nm-device-tun.xml \ nm-device-veth.xml \ diff --git a/introspection/nm-device-statistics.xml b/introspection/nm-device-statistics.xml new file mode 100644 index 0000000000..5d23bf3062 --- /dev/null +++ b/introspection/nm-device-statistics.xml @@ -0,0 +1,37 @@ + + + + + + + + + + + + + + + + + + + diff --git a/libnm-core/nm-dbus-interface.h b/libnm-core/nm-dbus-interface.h index c7ce110029..e5b1b3d6b8 100644 --- a/libnm-core/nm-dbus-interface.h +++ b/libnm-core/nm-dbus-interface.h @@ -68,6 +68,7 @@ #define NM_DBUS_INTERFACE_DEVICE_VXLAN NM_DBUS_INTERFACE_DEVICE ".Vxlan" #define NM_DBUS_INTERFACE_DEVICE_GRE NM_DBUS_INTERFACE_DEVICE ".Gre" #define NM_DBUS_INTERFACE_DEVICE_IP_TUNNEL NM_DBUS_INTERFACE_DEVICE ".IPTunnel" +#define NM_DBUS_INTERFACE_DEVICE_STATISTICS NM_DBUS_INTERFACE_DEVICE ".Statistics" #define NM_DBUS_INTERFACE_SETTINGS "org.freedesktop.NetworkManager.Settings" #define NM_DBUS_PATH_SETTINGS "/org/freedesktop/NetworkManager/Settings" diff --git a/src/Makefile.am b/src/Makefile.am index c460caff67..35bcf93ffa 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -331,6 +331,8 @@ libNetworkManager_la_SOURCES = \ devices/nm-device-generic.h \ devices/nm-device-logging.h \ devices/nm-device-private.h \ + devices/nm-device-statistics.c \ + devices/nm-device-statistics.h \ \ dhcp-manager/nm-dhcp-client.c \ dhcp-manager/nm-dhcp-client.h \ diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index 7381fd930f..58216e175a 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -114,6 +114,9 @@ void nm_device_ip_method_failed (NMDevice *self, int family, NMDeviceStateReason gboolean nm_device_ipv6_sysctl_set (NMDevice *self, const char *property, const char *value); +void nm_device_set_tx_bytes (NMDevice *self, guint64 tx_bytes); +void nm_device_set_rx_bytes (NMDevice *self, guint64 rx_bytes); + #define NM_DEVICE_CLASS_DECLARE_TYPES(klass, conn_type, ...) \ NM_DEVICE_CLASS (klass)->connection_type = conn_type; \ { \ diff --git a/src/devices/nm-device-statistics.c b/src/devices/nm-device-statistics.c new file mode 100644 index 0000000000..b5d4197cfc --- /dev/null +++ b/src/devices/nm-device-statistics.c @@ -0,0 +1,99 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright (C) 2016 Canonical Ltd + * + */ + +#include "nm-default.h" + +#include + +#include "nm-device-statistics.h" +#include "nm-device-private.h" +#include "nm-utils.h" +#include "nm-platform.h" + +#define _NMLOG_DOMAIN LOGD_DEVICE +#define _NMLOG(level, ...) \ + nm_log_obj ((level), _NMLOG_DOMAIN, (self->device), "device-stats", \ + "(%s): " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + nm_device_get_iface (self->device) ?: "(none)" \ + _NM_UTILS_MACRO_REST(__VA_ARGS__)) + +struct _NMDeviceStatistics { + NMDevice *device; + guint stats_update_id; +}; + +static gboolean +update_stats (gpointer user_data) +{ + NMDeviceStatistics *self = user_data; + guint64 rx_packets; + guint64 rx_bytes; + guint64 tx_packets; + guint64 tx_bytes; + int ifindex; + + ifindex = nm_device_get_ip_ifindex (self->device); + + if (nm_platform_link_get_stats (NM_PLATFORM_GET, ifindex, + &rx_packets, &rx_bytes, + &tx_packets, &tx_bytes)) { + _LOGT ("{RX} %"PRIu64" packets %"PRIu64" bytes {TX} %"PRIu64" packets %"PRIu64" bytes", + rx_packets, rx_bytes, tx_packets, tx_bytes); + + nm_device_set_tx_bytes (self->device, tx_bytes); + nm_device_set_rx_bytes (self->device, rx_bytes); + } else { + _LOGE ("error no stats available"); + } + + /* Keep polling */ + nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); + + return TRUE; +} + +/********************************************/ + +NMDeviceStatistics * +nm_device_statistics_new (NMDevice *device, unsigned rate_ms) +{ + NMDeviceStatistics *self; + + self = g_malloc0 (sizeof (*self)); + self->device = device; + + self->stats_update_id = g_timeout_add (rate_ms, update_stats, self); + + return self; +} + +void +nm_device_statistics_unref (NMDeviceStatistics *self) +{ + g_source_remove (self->stats_update_id); + g_free (self); +} + +void +nm_device_statistics_change_rate (NMDeviceStatistics *self, unsigned rate_ms) +{ + g_source_remove (self->stats_update_id); + + self->stats_update_id = g_timeout_add (rate_ms, update_stats, self); +} diff --git a/src/devices/nm-device-statistics.h b/src/devices/nm-device-statistics.h new file mode 100644 index 0000000000..17ff19f484 --- /dev/null +++ b/src/devices/nm-device-statistics.h @@ -0,0 +1,31 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright (C) 2016 Canonical Ltd + */ + +#ifndef __NETWORKMANAGER_DEVICE_STATISTICS_H__ +#define __NETWORKMANAGER_DEVICE_STATISTICS_H__ + +typedef struct _NMDeviceStatistics NMDeviceStatistics; + +NMDeviceStatistics * +nm_device_statistics_new (NMDevice *device, unsigned rate_ms); + +void nm_device_statistics_unref (NMDeviceStatistics *self); + +void nm_device_statistics_change_rate (NMDeviceStatistics *self, unsigned rate_ms); + +#endif /* __NETWORKMANAGER_DEVICE_STATISTICS_H__ */ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9c7319f03d..72ed5e911c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -66,11 +66,13 @@ #include "sd-ipv4ll.h" #include "nm-audit-manager.h" #include "nm-arping-manager.h" +#include "nm-device-statistics.h" #include "nm-device-logging.h" _LOG_DECLARE_SELF (NMDevice); #include "nmdbus-device.h" +#include "nmdbus-device-statistics.h" G_DEFINE_ABSTRACT_TYPE (NMDevice, nm_device, NM_TYPE_EXPORTED_OBJECT) @@ -138,6 +140,9 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDevice, PROP_LLDP_NEIGHBORS, PROP_REAL, PROP_SLAVES, + PROP_REFRESH_RATE_MS, + PROP_TX_BYTES, + PROP_RX_BYTES, ); #define DEFAULT_AUTOCONNECT TRUE @@ -407,6 +412,13 @@ typedef struct _NMDevicePrivate { NMLldpListener *lldp_listener; guint check_delete_unrealized_id; + + guint refresh_rate_ms; + guint64 tx_bytes; + guint64 rx_bytes; + + NMDeviceStatistics *statistics; + } NMDevicePrivate; static gboolean nm_device_set_ip4_config (NMDevice *self, @@ -769,6 +781,36 @@ nm_device_set_ip_iface (NMDevice *self, const char *iface) g_free (old_ip_iface); } +void +nm_device_set_tx_bytes (NMDevice *self, guint64 tx_bytes) +{ + NMDevicePrivate *priv; + + g_return_if_fail (NM_IS_DEVICE (self)); + + priv = NM_DEVICE_GET_PRIVATE (self); + if (tx_bytes == priv->tx_bytes) + return; + + priv->tx_bytes = tx_bytes; + _notify (self, PROP_TX_BYTES); +} + +void +nm_device_set_rx_bytes (NMDevice *self, guint64 rx_bytes) +{ + NMDevicePrivate *priv; + + g_return_if_fail (NM_IS_DEVICE (self)); + + priv = NM_DEVICE_GET_PRIVATE (self); + if (rx_bytes == priv->rx_bytes) + return; + + priv->rx_bytes = rx_bytes; + _notify (self, PROP_RX_BYTES); +} + static gboolean get_ip_iface_identifier (NMDevice *self, NMUtilsIPv6IfaceId *out_iid) { @@ -2199,6 +2241,11 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) priv->carrier = TRUE; } + if (priv->refresh_rate_ms && !priv->statistics) { + priv->statistics = nm_device_statistics_new (self, + priv->refresh_rate_ms); + } + klass->realize_start_notify (self, plink); /* Do not manage externally created software devices until they are IFF_UP @@ -2370,6 +2417,14 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) g_clear_pointer (&priv->physical_port_id, g_free); _notify (self, PROP_PHYSICAL_PORT_ID); } + if (priv->statistics) { + nm_device_statistics_unref (priv->statistics); + priv->statistics = NULL; + priv->tx_bytes = 0; + priv->tx_bytes = 0; + _notify (self, PROP_TX_BYTES); + _notify (self, PROP_RX_BYTES); + } priv->hw_addr_type = HW_ADDR_TYPE_UNSET; g_clear_pointer (&priv->hw_addr_perm, g_free); @@ -11963,6 +12018,11 @@ nm_device_init (NMDevice *self) priv->v4_commit_first_time = TRUE; priv->v6_commit_first_time = TRUE; + + priv->refresh_rate_ms = 0; + priv->tx_bytes = 0; + priv->rx_bytes = 0; + priv->statistics = NULL; } static GObject* @@ -12111,6 +12171,11 @@ dispose (GObject *object) g_clear_object (&priv->lldp_listener); } + if (priv->statistics) { + nm_device_statistics_unref (priv->statistics); + priv->statistics = NULL; + } + G_OBJECT_CLASS (nm_device_parent_class)->dispose (object); if (nm_clear_g_source (&priv->queued_state.id)) { @@ -12243,6 +12308,28 @@ set_property (GObject *object, guint prop_id, /* construct only */ priv->hw_addr_perm = g_value_dup_string (value); break; + case PROP_REFRESH_RATE_MS: { + guint refresh_rate_ms; + + refresh_rate_ms = g_value_get_uint (value); + if (priv->refresh_rate_ms == refresh_rate_ms) + break; + + priv->refresh_rate_ms = refresh_rate_ms; + _LOGI (LOGD_DEVICE, "statistics refresh rate set to %u ms", priv->refresh_rate_ms); + + if (priv->refresh_rate_ms) { + if (priv->statistics) + nm_device_statistics_change_rate (priv->statistics, priv->refresh_rate_ms); + else + priv->statistics = + nm_device_statistics_new (self, priv->refresh_rate_ms); + } else { + nm_device_statistics_unref (priv->statistics); + priv->statistics = NULL; + } + break; + } default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -12405,6 +12492,15 @@ get_property (GObject *object, guint prop_id, g_value_take_boxed (value, slave_list); break; } + case PROP_REFRESH_RATE_MS: + g_value_set_uint (value, priv->refresh_rate_ms); + break; + case PROP_TX_BYTES: + g_value_set_uint64 (value, priv->tx_bytes); + break; + case PROP_RX_BYTES: + g_value_set_uint64 (value, priv->rx_bytes); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; @@ -12655,6 +12751,23 @@ nm_device_class_init (NMDeviceClass *klass) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + /* Statistics */ + obj_properties[PROP_REFRESH_RATE_MS] = + g_param_spec_uint (NM_DEVICE_STATISTICS_REFRESH_RATE_MS, "", "", + 0, UINT32_MAX, 0, + G_PARAM_READWRITE | + G_PARAM_STATIC_STRINGS); + obj_properties[PROP_TX_BYTES] = + g_param_spec_uint64 (NM_DEVICE_STATISTICS_TX_BYTES, "", "", + 0, UINT64_MAX, 0, + G_PARAM_READABLE | + G_PARAM_STATIC_STRINGS); + obj_properties[PROP_RX_BYTES] = + g_param_spec_uint64 (NM_DEVICE_STATISTICS_RX_BYTES, "", "", + 0, UINT64_MAX, 0, + G_PARAM_READABLE | + G_PARAM_STATIC_STRINGS); + g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); /* Signals */ @@ -12725,4 +12838,8 @@ nm_device_class_init (NMDeviceClass *klass) "Disconnect", impl_device_disconnect, "Delete", impl_device_delete, NULL); + + nm_exported_object_class_add_interface (NM_EXPORTED_OBJECT_CLASS (klass), + NMDBUS_TYPE_DEVICE_STATISTICS_SKELETON, + NULL); } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 6a4f22a5f4..433448565d 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -88,6 +88,10 @@ #define NM_DEVICE_STATE_CHANGED "state-changed" #define NM_DEVICE_LINK_INITIALIZED "link-initialized" +#define NM_DEVICE_STATISTICS_REFRESH_RATE_MS "refresh-rate-ms" +#define NM_DEVICE_STATISTICS_TX_BYTES "tx-bytes" +#define NM_DEVICE_STATISTICS_RX_BYTES "rx-bytes" + G_BEGIN_DECLS #define NM_TYPE_DEVICE (nm_device_get_type ()) From 6fb0de0a8bbcd61627bc1c764ec8e3b8d156f29e Mon Sep 17 00:00:00 2001 From: Alfonso Sanchez-Beato Date: Wed, 10 Aug 2016 11:54:32 +0200 Subject: [PATCH 06/18] auth: check when setting statistics refresh rate --- clients/cli/general.c | 2 ++ libnm-glib/nm-client.c | 2 ++ libnm-glib/nm-client.h | 5 ++++- libnm/nm-client.h | 5 ++++- libnm/nm-manager.c | 2 ++ .../org.freedesktop.NetworkManager.policy.in.in | 9 +++++++++ shared/nm-common-macros.h | 1 + src/nm-audit-manager.h | 1 + src/nm-manager.c | 15 +++++++++++++++ 9 files changed, 40 insertions(+), 2 deletions(-) diff --git a/clients/cli/general.c b/clients/cli/general.c index 50e5eeb98c..ccf2292af8 100644 --- a/clients/cli/general.c +++ b/clients/cli/general.c @@ -439,6 +439,8 @@ permission_to_string (NMClientPermission perm) return NM_AUTH_PERMISSION_RELOAD; case NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK: return NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK; + case NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS: + return NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS; default: return _("unknown"); } diff --git a/libnm-glib/nm-client.c b/libnm-glib/nm-client.c index a78c601a19..f0ce64bc31 100644 --- a/libnm-glib/nm-client.c +++ b/libnm-glib/nm-client.c @@ -240,6 +240,8 @@ nm_permission_to_client (const char *nm) return NM_CLIENT_PERMISSION_RELOAD; else if (!strcmp (nm, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK)) return NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK; + else if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS)) + return NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS; return NM_CLIENT_PERMISSION_NONE; } diff --git a/libnm-glib/nm-client.h b/libnm-glib/nm-client.h index a5cfcca0f8..f10b6e54b4 100644 --- a/libnm-glib/nm-client.h +++ b/libnm-glib/nm-client.h @@ -89,6 +89,8 @@ G_BEGIN_DECLS * @NM_CLIENT_PERMISSION_RELOAD: controls access to Reload. * persistent hostname can be changed * @NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK: permission to create checkpoints. + * @NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS: controls whether device + * statistics can be globally enabled or disabled * @NM_CLIENT_PERMISSION_LAST: a reserved boundary value * * #NMClientPermission values indicate various permissions that NetworkManager @@ -110,8 +112,9 @@ typedef enum { NM_CLIENT_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS = 12, NM_CLIENT_PERMISSION_RELOAD = 13, NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK = 14, + NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS = 15, - NM_CLIENT_PERMISSION_LAST = 14, + NM_CLIENT_PERMISSION_LAST = 15, } NMClientPermission; /** diff --git a/libnm/nm-client.h b/libnm/nm-client.h index e2a18b6172..5358ded4a0 100644 --- a/libnm/nm-client.h +++ b/libnm/nm-client.h @@ -98,6 +98,8 @@ G_BEGIN_DECLS * DNS configuration * @NM_CLIENT_PERMISSION_RELOAD: controls access to Reload. * @NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK: permission to create checkpoints. + * @NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS: controls whether device + * statistics can be globally enabled or disabled * @NM_CLIENT_PERMISSION_LAST: a reserved boundary value * * #NMClientPermission values indicate various permissions that NetworkManager @@ -119,8 +121,9 @@ typedef enum { NM_CLIENT_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS = 12, NM_CLIENT_PERMISSION_RELOAD = 13, NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK = 14, + NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS = 15, - NM_CLIENT_PERMISSION_LAST = 14, + NM_CLIENT_PERMISSION_LAST = 15, } NMClientPermission; /** diff --git a/libnm/nm-manager.c b/libnm/nm-manager.c index 4156f651bf..c3d0c51db0 100644 --- a/libnm/nm-manager.c +++ b/libnm/nm-manager.c @@ -234,6 +234,8 @@ nm_permission_to_client (const char *nm) return NM_CLIENT_PERMISSION_RELOAD; else if (!strcmp (nm, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK)) return NM_CLIENT_PERMISSION_CHECKPOINT_ROLLBACK; + else if (!strcmp (nm, NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS)) + return NM_CLIENT_PERMISSION_ENABLE_DISABLE_STATISTICS; return NM_CLIENT_PERMISSION_NONE; } diff --git a/policy/org.freedesktop.NetworkManager.policy.in.in b/policy/org.freedesktop.NetworkManager.policy.in.in index e0a147a107..7de32f6624 100644 --- a/policy/org.freedesktop.NetworkManager.policy.in.in +++ b/policy/org.freedesktop.NetworkManager.policy.in.in @@ -142,5 +142,14 @@ + + <_description>Enable or disable device statistics + <_message>System policy prevents enabling or disabling device statistics + + no + yes + + + diff --git a/shared/nm-common-macros.h b/shared/nm-common-macros.h index 627d72abf6..6e0769cca4 100644 --- a/shared/nm-common-macros.h +++ b/shared/nm-common-macros.h @@ -38,6 +38,7 @@ #define NM_AUTH_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS "org.freedesktop.NetworkManager.settings.modify.global-dns" #define NM_AUTH_PERMISSION_RELOAD "org.freedesktop.NetworkManager.reload" #define NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK "org.freedesktop.NetworkManager.checkpoint-rollback" +#define NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS "org.freedesktop.NetworkManager.enable-disable-statistics" #define NM_CLONED_MAC_PRESERVE "preserve" #define NM_CLONED_MAC_PERMANENT "permanent" diff --git a/src/nm-audit-manager.h b/src/nm-audit-manager.h index a14da5bbb9..2d44c2ca52 100644 --- a/src/nm-audit-manager.h +++ b/src/nm-audit-manager.h @@ -57,6 +57,7 @@ typedef struct { #define NM_AUDIT_OP_SLEEP_CONTROL "sleep-control" #define NM_AUDIT_OP_NET_CONTROL "networking-control" #define NM_AUDIT_OP_RADIO_CONTROL "radio-control" +#define NM_AUDIT_OP_STATISTICS "statistics" #define NM_AUDIT_OP_DEVICE_AUTOCONNECT "device-autoconnect" #define NM_AUDIT_OP_DEVICE_DISCONNECT "device-disconnect" diff --git a/src/nm-manager.c b/src/nm-manager.c index 88723e0298..b8790d46aa 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4415,6 +4415,7 @@ get_permissions_done_cb (NMAuthChain *chain, get_perm_add_result (self, chain, &results, NM_AUTH_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS); get_perm_add_result (self, chain, &results, NM_AUTH_PERMISSION_RELOAD); get_perm_add_result (self, chain, &results, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK); + get_perm_add_result (self, chain, &results, NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS); g_dbus_method_invocation_return_value (context, g_variant_new ("(a{ss})", &results)); @@ -4455,6 +4456,7 @@ impl_manager_get_permissions (NMManager *self, nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_SETTINGS_MODIFY_GLOBAL_DNS, FALSE); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_RELOAD, FALSE); nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_CHECKPOINT_ROLLBACK, FALSE); + nm_auth_chain_add_call (chain, NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS, FALSE); } static void @@ -4915,6 +4917,10 @@ prop_set_auth_done_cb (NMAuthChain *chain, /* ... but set the property on the @object itself. It would be correct to set the property * on the skeleton interface, but as it is now, the result is the same. */ g_object_set (object, pfd->glib_propname, value, NULL); + } else if (!strcmp (pfd->glib_propname, NM_DEVICE_STATISTICS_REFRESH_RATE_MS)) { + g_assert (g_variant_is_of_type (value, G_VARIANT_TYPE_UINT32)); + /* the same here */ + g_object_set (object, pfd->glib_propname, g_variant_get_uint32 (value), NULL); } else { g_assert (g_variant_is_of_type (value, G_VARIANT_TYPE_BOOLEAN)); /* the same here */ @@ -5049,6 +5055,15 @@ prop_filter (GDBusConnection *connection, } else return message; interface_type = NMDBUS_TYPE_DEVICE_SKELETON; + } else if (!strcmp (propiface, NM_DBUS_INTERFACE_DEVICE_STATISTICS)) { + if (!strcmp (propname, "RefreshRateMs")) { + glib_propname = NM_DEVICE_STATISTICS_REFRESH_RATE_MS; + permission = NM_AUTH_PERMISSION_ENABLE_DISABLE_STATISTICS; + audit_op = NM_AUDIT_OP_STATISTICS; + expected_type = G_VARIANT_TYPE ("u"); + } else + return message; + interface_type = NMDBUS_TYPE_DEVICE_SKELETON; } else return message; From ce93bd2da72e8736c6903012b1baa3679092f4b0 Mon Sep 17 00:00:00 2001 From: Alfonso Sanchez-Beato Date: Wed, 10 Aug 2016 11:54:33 +0200 Subject: [PATCH 07/18] docs: add device statistics interface --- docs/api/Makefile.am | 1 + docs/api/network-manager-docs.xml | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/api/Makefile.am b/docs/api/Makefile.am index 5aafc26248..afe5d3847d 100644 --- a/docs/api/Makefile.am +++ b/docs/api/Makefile.am @@ -73,6 +73,7 @@ content_files = \ $(top_builddir)/introspection/nmdbus-settings-org.freedesktop.NetworkManager.Settings.xml \ $(top_builddir)/introspection/nmdbus-device-ethernet-org.freedesktop.NetworkManager.Device.Wired.xml \ $(top_builddir)/introspection/nmdbus-ip4-config-org.freedesktop.NetworkManager.IP4Config.xml \ + $(top_builddir)/introspection/nmdbus-device-statistics-org.freedesktop.NetworkManager.Device.Statistics.xml \ $(top_builddir)/libnm-core/nm-dbus-types.xml \ $(top_builddir)/libnm-core/nm-vpn-dbus-types.xml \ $(top_builddir)/man/nmcli.xml \ diff --git a/docs/api/network-manager-docs.xml b/docs/api/network-manager-docs.xml index 722e33fd19..bb5ab00496 100644 --- a/docs/api/network-manager-docs.xml +++ b/docs/api/network-manager-docs.xml @@ -98,6 +98,7 @@ +
Types From b3c376cd29605646e6b150856080b3acb932c7eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Aug 2016 23:15:05 +0200 Subject: [PATCH 08/18] platform: fix sorting order for nm_platform_link_cmp() nm_platform_link_cmp() shall first compare the ifindex, otherwise the sort-order first considers rather unimportant fields instead of the primary key: the ifindex. Fixes: a3185f22e55484b819859cb4cef8f54385dac1a9 --- src/platform/nm-platform.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 99dc00c812..5d8080fc11 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -3792,10 +3792,6 @@ int nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) { _CMP_SELF (a, b); - _CMP_FIELD (a, b, rx_packets); - _CMP_FIELD (a, b, rx_bytes); - _CMP_FIELD (a, b, tx_packets); - _CMP_FIELD (a, b, tx_bytes); _CMP_FIELD (a, b, ifindex); _CMP_FIELD (a, b, type); _CMP_FIELD_STR (a, b, name); @@ -3813,6 +3809,10 @@ nm_platform_link_cmp (const NMPlatformLink *a, const NMPlatformLink *b) if (a->addr.len) _CMP_FIELD_MEMCMP_LEN (a, b, addr.data, a->addr.len); _CMP_FIELD_MEMCMP (a, b, inet6_token); + _CMP_FIELD (a, b, rx_packets); + _CMP_FIELD (a, b, rx_bytes); + _CMP_FIELD (a, b, tx_packets); + _CMP_FIELD (a, b, tx_bytes); return 0; } From 178bb25a03451a2142ad65338fc4702d43aedcc5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Aug 2016 23:23:25 +0200 Subject: [PATCH 09/18] platform: let _new_from_nl_link() lookup missing tb[IFLA_STATS64] data from cache --- src/platform/nm-linux-platform.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index d185f96f12..13aeb89d30 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1577,7 +1577,8 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr && ( lnk_data_complete_from_cache || address_complete_from_cache || !af_inet6_token_valid - || !af_inet6_addr_gen_mode_valid)) { + || !af_inet6_addr_gen_mode_valid + || !tb[IFLA_STATS64])) { _lookup_cached_link (cache, obj->link.ifindex, completed_from_cache, &link_cached); if (link_cached) { if ( lnk_data_complete_from_cache @@ -1600,6 +1601,12 @@ _new_from_nl_link (NMPlatform *platform, const NMPCache *cache, struct nlmsghdr obj->link.inet6_token = link_cached->link.inet6_token; if (!af_inet6_addr_gen_mode_valid) obj->link.inet6_addr_gen_mode_inv = link_cached->link.inet6_addr_gen_mode_inv; + if (!tb[IFLA_STATS64]) { + obj->link.rx_packets = link_cached->link.rx_packets; + obj->link.rx_bytes = link_cached->link.rx_bytes; + obj->link.tx_packets = link_cached->link.tx_packets; + obj->link.tx_bytes = link_cached->link.tx_bytes; + } } } From 9c5405eba4a2f34383af293f9b0b0a83c6b6405f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Aug 2016 23:28:17 +0200 Subject: [PATCH 10/18] platform: drop nm_platform_link_get_stats() No need to add accessors for fields of NMPlatformLink. Just access them directly. --- src/devices/nm-device-statistics.c | 19 +++++++------------ src/platform/nm-linux-platform.c | 22 ---------------------- src/platform/nm-platform.c | 15 --------------- src/platform/nm-platform.h | 6 ------ 4 files changed, 7 insertions(+), 55 deletions(-) diff --git a/src/devices/nm-device-statistics.c b/src/devices/nm-device-statistics.c index b5d4197cfc..4c7c295c5f 100644 --- a/src/devices/nm-device-statistics.c +++ b/src/devices/nm-device-statistics.c @@ -42,25 +42,20 @@ static gboolean update_stats (gpointer user_data) { NMDeviceStatistics *self = user_data; - guint64 rx_packets; - guint64 rx_bytes; - guint64 tx_packets; - guint64 tx_bytes; int ifindex; + const NMPlatformLink *pllink; ifindex = nm_device_get_ip_ifindex (self->device); - if (nm_platform_link_get_stats (NM_PLATFORM_GET, ifindex, - &rx_packets, &rx_bytes, - &tx_packets, &tx_bytes)) { + pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); + if (pllink) { _LOGT ("{RX} %"PRIu64" packets %"PRIu64" bytes {TX} %"PRIu64" packets %"PRIu64" bytes", - rx_packets, rx_bytes, tx_packets, tx_bytes); + pllink->rx_packets, pllink->rx_bytes, pllink->tx_packets, pllink->tx_bytes); - nm_device_set_tx_bytes (self->device, tx_bytes); - nm_device_set_rx_bytes (self->device, rx_bytes); - } else { + nm_device_set_tx_bytes (self->device, pllink->tx_bytes); + nm_device_set_rx_bytes (self->device, pllink->rx_bytes); + } else _LOGE ("error no stats available"); - } /* Keep polling */ nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 13aeb89d30..98c4e461ab 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -4534,27 +4534,6 @@ link_get_dev_id (NMPlatform *platform, int ifindex) return errno ? 0 : (int) int_val; } -static gboolean -link_get_stats (NMPlatform *platform, int ifindex, - guint64 *rx_packets, guint64 *rx_bytes, - guint64 *tx_packets, guint64 *tx_bytes) -{ - nm_auto_pop_netns NMPNetns *netns = NULL; - const NMPObject *obj; - - obj = cache_lookup_link (platform, ifindex); - - if (!obj) - return FALSE; - - *rx_packets = obj->link.rx_packets; - *rx_bytes = obj->link.rx_bytes; - *tx_packets = obj->link.tx_packets; - *tx_bytes = obj->link.tx_bytes; - - return TRUE; -} - static int vlan_add (NMPlatform *platform, const char *name, @@ -6547,7 +6526,6 @@ nm_linux_platform_class_init (NMLinuxPlatformClass *klass) platform_class->link_get_physical_port_id = link_get_physical_port_id; platform_class->link_get_dev_id = link_get_dev_id; - platform_class->link_get_stats = link_get_stats; platform_class->link_get_wake_on_lan = link_get_wake_on_lan; platform_class->link_get_driver_info = link_get_driver_info; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 5d8080fc11..1a7f8a0f6b 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1265,21 +1265,6 @@ nm_platform_link_get_dev_id (NMPlatform *self, int ifindex) return 0; } -gboolean nm_platform_link_get_stats (NMPlatform *self, int ifindex, - guint64 *rx_packets, guint64 *rx_bytes, - guint64 *tx_packets, guint64 *tx_bytes) -{ - _CHECK_SELF (self, klass, 0); - - g_return_val_if_fail (ifindex >= 0, 0); - - if (klass->link_get_stats) - return klass->link_get_stats (self, ifindex, - rx_packets, rx_bytes, - tx_packets, tx_bytes); - return FALSE; -} - /** * nm_platform_link_get_wake_onlan: * @self: platform instance diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 19148deb4f..fc6965e904 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -538,9 +538,6 @@ typedef struct { char * (*link_get_physical_port_id) (NMPlatform *, int ifindex); guint (*link_get_dev_id) (NMPlatform *, int ifindex); - gboolean (*link_get_stats) (NMPlatform *platform, int ifindex, - guint64 *rx_packets, guint64 *rx_bytes, - guint64 *tx_packets, guint64 *tx_bytes); gboolean (*link_get_wake_on_lan) (NMPlatform *, int ifindex); gboolean (*link_get_driver_info) (NMPlatform *, int ifindex, @@ -772,9 +769,6 @@ gboolean nm_platform_link_set_mtu (NMPlatform *self, int ifindex, guint32 mtu); char *nm_platform_link_get_physical_port_id (NMPlatform *self, int ifindex); guint nm_platform_link_get_dev_id (NMPlatform *self, int ifindex); -gboolean nm_platform_link_get_stats (NMPlatform *self, int ifindex, - guint64 *rx_packets, guint64 *rx_bytes, - guint64 *tx_packets, guint64 *tx_bytes); gboolean nm_platform_link_get_wake_on_lan (NMPlatform *self, int ifindex); gboolean nm_platform_link_get_driver_info (NMPlatform *self, int ifindex, From fc2f1d9cb87a0df904e1f7ff0aaac5d8b9d807d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Aug 2016 23:30:45 +0200 Subject: [PATCH 11/18] device: reset device-stats in update_stats() on missing link First of all, we don't expect missing NMPlatformLink instances. If that actually happens, just reset the counters to zero. --- src/devices/nm-device-statistics.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device-statistics.c b/src/devices/nm-device-statistics.c index 4c7c295c5f..12e595c941 100644 --- a/src/devices/nm-device-statistics.c +++ b/src/devices/nm-device-statistics.c @@ -49,13 +49,16 @@ update_stats (gpointer user_data) pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); if (pllink) { - _LOGT ("{RX} %"PRIu64" packets %"PRIu64" bytes {TX} %"PRIu64" packets %"PRIu64" bytes", - pllink->rx_packets, pllink->rx_bytes, pllink->tx_packets, pllink->tx_bytes); + _LOGT ("ifindex %d: {RX} %"PRIu64" packets %"PRIu64" bytes {TX} %"PRIu64" packets %"PRIu64" bytes", + ifindex, pllink->rx_packets, pllink->rx_bytes, pllink->tx_packets, pllink->tx_bytes); nm_device_set_tx_bytes (self->device, pllink->tx_bytes); nm_device_set_rx_bytes (self->device, pllink->rx_bytes); - } else - _LOGE ("error no stats available"); + } else { + _LOGT ("error no stats available for ifindex %d", ifindex); + nm_device_set_tx_bytes (self->device, 0); + nm_device_set_rx_bytes (self->device, 0); + } /* Keep polling */ nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); From 36f8ffad9f8eee4132eed92497f4facca2e03519 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Aug 2016 23:34:10 +0200 Subject: [PATCH 12/18] device: refresh the link before reading the stats --- src/devices/nm-device-statistics.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device-statistics.c b/src/devices/nm-device-statistics.c index 12e595c941..4b2bbdd08b 100644 --- a/src/devices/nm-device-statistics.c +++ b/src/devices/nm-device-statistics.c @@ -47,6 +47,8 @@ update_stats (gpointer user_data) ifindex = nm_device_get_ip_ifindex (self->device); + nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); + pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); if (pllink) { _LOGT ("ifindex %d: {RX} %"PRIu64" packets %"PRIu64" bytes {TX} %"PRIu64" packets %"PRIu64" bytes", @@ -60,9 +62,6 @@ update_stats (gpointer user_data) nm_device_set_rx_bytes (self->device, 0); } - /* Keep polling */ - nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); - return TRUE; } From 14a7b2a4fec4e1eb7151996f3fdb94126e490a3b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 11:07:36 +0200 Subject: [PATCH 13/18] manager: add explicit cast for g_object_set() Technically, this is not needed because glib requires that int is at least 32 bits. Thus, uint32 will be safely promoted to uint. Just do the cast to be explict about the expected type. --- src/nm-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index b8790d46aa..89dc736044 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4920,7 +4920,7 @@ prop_set_auth_done_cb (NMAuthChain *chain, } else if (!strcmp (pfd->glib_propname, NM_DEVICE_STATISTICS_REFRESH_RATE_MS)) { g_assert (g_variant_is_of_type (value, G_VARIANT_TYPE_UINT32)); /* the same here */ - g_object_set (object, pfd->glib_propname, g_variant_get_uint32 (value), NULL); + g_object_set (object, pfd->glib_propname, (guint) g_variant_get_uint32 (value), NULL); } else { g_assert (g_variant_is_of_type (value, G_VARIANT_TYPE_BOOLEAN)); /* the same here */ From d9509a2db155a9b5fc98d2bf6a33d797c25ce0e7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 11:36:25 +0200 Subject: [PATCH 14/18] device: don't initalize fields in nm_device_init() to NULL They are already guaranteed to be 0/NULL. --- src/devices/nm-device.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 72ed5e911c..69f2a63a36 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -12018,11 +12018,6 @@ nm_device_init (NMDevice *self) priv->v4_commit_first_time = TRUE; priv->v6_commit_first_time = TRUE; - - priv->refresh_rate_ms = 0; - priv->tx_bytes = 0; - priv->rx_bytes = 0; - priv->statistics = NULL; } static GObject* From 02a448e49bbc3f54d32b45cff1a22be836357ffa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 11:35:23 +0200 Subject: [PATCH 15/18] device: namespace fields related to statistics in NMDevicePrivate ... by grouping them together in a struct. --- src/devices/nm-device.c | 66 +++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 69f2a63a36..cd6e280cbd 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -413,11 +413,13 @@ typedef struct _NMDevicePrivate { guint check_delete_unrealized_id; - guint refresh_rate_ms; - guint64 tx_bytes; - guint64 rx_bytes; + struct { + guint refresh_rate_ms; + guint64 tx_bytes; + guint64 rx_bytes; - NMDeviceStatistics *statistics; + NMDeviceStatistics *statistics; + } stats; } NMDevicePrivate; @@ -789,10 +791,10 @@ nm_device_set_tx_bytes (NMDevice *self, guint64 tx_bytes) g_return_if_fail (NM_IS_DEVICE (self)); priv = NM_DEVICE_GET_PRIVATE (self); - if (tx_bytes == priv->tx_bytes) + if (tx_bytes == priv->stats.tx_bytes) return; - priv->tx_bytes = tx_bytes; + priv->stats.tx_bytes = tx_bytes; _notify (self, PROP_TX_BYTES); } @@ -804,10 +806,10 @@ nm_device_set_rx_bytes (NMDevice *self, guint64 rx_bytes) g_return_if_fail (NM_IS_DEVICE (self)); priv = NM_DEVICE_GET_PRIVATE (self); - if (rx_bytes == priv->rx_bytes) + if (rx_bytes == priv->stats.rx_bytes) return; - priv->rx_bytes = rx_bytes; + priv->stats.rx_bytes = rx_bytes; _notify (self, PROP_RX_BYTES); } @@ -2241,9 +2243,9 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) priv->carrier = TRUE; } - if (priv->refresh_rate_ms && !priv->statistics) { - priv->statistics = nm_device_statistics_new (self, - priv->refresh_rate_ms); + if (priv->stats.refresh_rate_ms && !priv->stats.statistics) { + priv->stats.statistics = nm_device_statistics_new (self, + priv->stats.refresh_rate_ms); } klass->realize_start_notify (self, plink); @@ -2417,11 +2419,11 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) g_clear_pointer (&priv->physical_port_id, g_free); _notify (self, PROP_PHYSICAL_PORT_ID); } - if (priv->statistics) { - nm_device_statistics_unref (priv->statistics); - priv->statistics = NULL; - priv->tx_bytes = 0; - priv->tx_bytes = 0; + if (priv->stats.statistics) { + nm_device_statistics_unref (priv->stats.statistics); + priv->stats.statistics = NULL; + priv->stats.tx_bytes = 0; + priv->stats.tx_bytes = 0; _notify (self, PROP_TX_BYTES); _notify (self, PROP_RX_BYTES); } @@ -12166,9 +12168,9 @@ dispose (GObject *object) g_clear_object (&priv->lldp_listener); } - if (priv->statistics) { - nm_device_statistics_unref (priv->statistics); - priv->statistics = NULL; + if (priv->stats.statistics) { + nm_device_statistics_unref (priv->stats.statistics); + priv->stats.statistics = NULL; } G_OBJECT_CLASS (nm_device_parent_class)->dispose (object); @@ -12307,21 +12309,21 @@ set_property (GObject *object, guint prop_id, guint refresh_rate_ms; refresh_rate_ms = g_value_get_uint (value); - if (priv->refresh_rate_ms == refresh_rate_ms) + if (priv->stats.refresh_rate_ms == refresh_rate_ms) break; - priv->refresh_rate_ms = refresh_rate_ms; - _LOGI (LOGD_DEVICE, "statistics refresh rate set to %u ms", priv->refresh_rate_ms); + priv->stats.refresh_rate_ms = refresh_rate_ms; + _LOGI (LOGD_DEVICE, "statistics refresh rate set to %u ms", priv->stats.refresh_rate_ms); - if (priv->refresh_rate_ms) { - if (priv->statistics) - nm_device_statistics_change_rate (priv->statistics, priv->refresh_rate_ms); + if (priv->stats.refresh_rate_ms) { + if (priv->stats.statistics) + nm_device_statistics_change_rate (priv->stats.statistics, priv->stats.refresh_rate_ms); else - priv->statistics = - nm_device_statistics_new (self, priv->refresh_rate_ms); + priv->stats.statistics = + nm_device_statistics_new (self, priv->stats.refresh_rate_ms); } else { - nm_device_statistics_unref (priv->statistics); - priv->statistics = NULL; + nm_device_statistics_unref (priv->stats.statistics); + priv->stats.statistics = NULL; } break; } @@ -12488,13 +12490,13 @@ get_property (GObject *object, guint prop_id, break; } case PROP_REFRESH_RATE_MS: - g_value_set_uint (value, priv->refresh_rate_ms); + g_value_set_uint (value, priv->stats.refresh_rate_ms); break; case PROP_TX_BYTES: - g_value_set_uint64 (value, priv->tx_bytes); + g_value_set_uint64 (value, priv->stats.tx_bytes); break; case PROP_RX_BYTES: - g_value_set_uint64 (value, priv->rx_bytes); + g_value_set_uint64 (value, priv->stats.rx_bytes); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); From 3d9d91b2beaff21b14edf444cb0077ea49a89cac Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 13:47:05 +0200 Subject: [PATCH 16/18] platform: print rx/tx counters in nm_platform_link_to_string() --- src/platform/nm-platform.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 1a7f8a0f6b..9dace69984 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -3115,6 +3115,8 @@ nm_platform_link_to_string (const NMPlatformLink *link, char *buf, gsize len) "%s%s" /* addr */ "%s%s" /* inet6_token */ "%s%s" /* driver */ + " rx:%"G_GUINT64_FORMAT",%"G_GUINT64_FORMAT + " tx:%"G_GUINT64_FORMAT",%"G_GUINT64_FORMAT , link->ifindex, link->name, @@ -3133,7 +3135,9 @@ nm_platform_link_to_string (const NMPlatformLink *link, char *buf, gsize len) link->inet6_token.id ? " inet6token " : "", link->inet6_token.id ? nm_utils_inet6_interface_identifier_to_token (link->inet6_token, str_inet6_token) : "", link->driver ? " driver " : "", - link->driver ? link->driver : ""); + link->driver ? link->driver : "", + link->rx_packets, link->rx_bytes, + link->tx_packets, link->tx_bytes); g_string_free (str_flags, TRUE); return buf; } From c16e14c71c8cc34ac9550cb5e45469c42b9c352a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 12:10:23 +0200 Subject: [PATCH 17/18] device: drop nm-device-statistics.c and refactor tracking device statistics Originally, "nm-device-statistics.c" contained code to fetch the device counters via netlink. As now the netlink part is handled by NMPlatform, the code can be simplified by merging it back to NMDevice. --- src/Makefile.am | 2 - src/devices/nm-device-private.h | 3 - src/devices/nm-device-statistics.c | 96 ----------------- src/devices/nm-device-statistics.h | 31 ------ src/devices/nm-device.c | 162 +++++++++++++++++++---------- 5 files changed, 107 insertions(+), 187 deletions(-) delete mode 100644 src/devices/nm-device-statistics.c delete mode 100644 src/devices/nm-device-statistics.h diff --git a/src/Makefile.am b/src/Makefile.am index 35bcf93ffa..c460caff67 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -331,8 +331,6 @@ libNetworkManager_la_SOURCES = \ devices/nm-device-generic.h \ devices/nm-device-logging.h \ devices/nm-device-private.h \ - devices/nm-device-statistics.c \ - devices/nm-device-statistics.h \ \ dhcp-manager/nm-dhcp-client.c \ dhcp-manager/nm-dhcp-client.h \ diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index 58216e175a..7381fd930f 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -114,9 +114,6 @@ void nm_device_ip_method_failed (NMDevice *self, int family, NMDeviceStateReason gboolean nm_device_ipv6_sysctl_set (NMDevice *self, const char *property, const char *value); -void nm_device_set_tx_bytes (NMDevice *self, guint64 tx_bytes); -void nm_device_set_rx_bytes (NMDevice *self, guint64 rx_bytes); - #define NM_DEVICE_CLASS_DECLARE_TYPES(klass, conn_type, ...) \ NM_DEVICE_CLASS (klass)->connection_type = conn_type; \ { \ diff --git a/src/devices/nm-device-statistics.c b/src/devices/nm-device-statistics.c deleted file mode 100644 index 4b2bbdd08b..0000000000 --- a/src/devices/nm-device-statistics.c +++ /dev/null @@ -1,96 +0,0 @@ -/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ -/* This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2, or (at your option) - * any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Copyright (C) 2016 Canonical Ltd - * - */ - -#include "nm-default.h" - -#include - -#include "nm-device-statistics.h" -#include "nm-device-private.h" -#include "nm-utils.h" -#include "nm-platform.h" - -#define _NMLOG_DOMAIN LOGD_DEVICE -#define _NMLOG(level, ...) \ - nm_log_obj ((level), _NMLOG_DOMAIN, (self->device), "device-stats", \ - "(%s): " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ - nm_device_get_iface (self->device) ?: "(none)" \ - _NM_UTILS_MACRO_REST(__VA_ARGS__)) - -struct _NMDeviceStatistics { - NMDevice *device; - guint stats_update_id; -}; - -static gboolean -update_stats (gpointer user_data) -{ - NMDeviceStatistics *self = user_data; - int ifindex; - const NMPlatformLink *pllink; - - ifindex = nm_device_get_ip_ifindex (self->device); - - nm_platform_link_refresh (NM_PLATFORM_GET, ifindex); - - pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex); - if (pllink) { - _LOGT ("ifindex %d: {RX} %"PRIu64" packets %"PRIu64" bytes {TX} %"PRIu64" packets %"PRIu64" bytes", - ifindex, pllink->rx_packets, pllink->rx_bytes, pllink->tx_packets, pllink->tx_bytes); - - nm_device_set_tx_bytes (self->device, pllink->tx_bytes); - nm_device_set_rx_bytes (self->device, pllink->rx_bytes); - } else { - _LOGT ("error no stats available for ifindex %d", ifindex); - nm_device_set_tx_bytes (self->device, 0); - nm_device_set_rx_bytes (self->device, 0); - } - - return TRUE; -} - -/********************************************/ - -NMDeviceStatistics * -nm_device_statistics_new (NMDevice *device, unsigned rate_ms) -{ - NMDeviceStatistics *self; - - self = g_malloc0 (sizeof (*self)); - self->device = device; - - self->stats_update_id = g_timeout_add (rate_ms, update_stats, self); - - return self; -} - -void -nm_device_statistics_unref (NMDeviceStatistics *self) -{ - g_source_remove (self->stats_update_id); - g_free (self); -} - -void -nm_device_statistics_change_rate (NMDeviceStatistics *self, unsigned rate_ms) -{ - g_source_remove (self->stats_update_id); - - self->stats_update_id = g_timeout_add (rate_ms, update_stats, self); -} diff --git a/src/devices/nm-device-statistics.h b/src/devices/nm-device-statistics.h deleted file mode 100644 index 17ff19f484..0000000000 --- a/src/devices/nm-device-statistics.h +++ /dev/null @@ -1,31 +0,0 @@ -/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ -/* This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2, or (at your option) - * any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Copyright (C) 2016 Canonical Ltd - */ - -#ifndef __NETWORKMANAGER_DEVICE_STATISTICS_H__ -#define __NETWORKMANAGER_DEVICE_STATISTICS_H__ - -typedef struct _NMDeviceStatistics NMDeviceStatistics; - -NMDeviceStatistics * -nm_device_statistics_new (NMDevice *device, unsigned rate_ms); - -void nm_device_statistics_unref (NMDeviceStatistics *self); - -void nm_device_statistics_change_rate (NMDeviceStatistics *self, unsigned rate_ms); - -#endif /* __NETWORKMANAGER_DEVICE_STATISTICS_H__ */ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index cd6e280cbd..de7ec70db4 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -66,7 +66,6 @@ #include "sd-ipv4ll.h" #include "nm-audit-manager.h" #include "nm-arping-manager.h" -#include "nm-device-statistics.h" #include "nm-device-logging.h" _LOG_DECLARE_SELF (NMDevice); @@ -414,11 +413,10 @@ typedef struct _NMDevicePrivate { guint check_delete_unrealized_id; struct { + guint timeout_id; guint refresh_rate_ms; guint64 tx_bytes; guint64 rx_bytes; - - NMDeviceStatistics *statistics; } stats; } NMDevicePrivate; @@ -783,36 +781,112 @@ nm_device_set_ip_iface (NMDevice *self, const char *iface) g_free (old_ip_iface); } -void -nm_device_set_tx_bytes (NMDevice *self, guint64 tx_bytes) +/*****************************************************************************/ + +static void +_stats_update_counters (NMDevice *self, + guint64 tx_bytes, + guint64 rx_bytes) { NMDevicePrivate *priv; - g_return_if_fail (NM_IS_DEVICE (self)); - priv = NM_DEVICE_GET_PRIVATE (self); - if (tx_bytes == priv->stats.tx_bytes) - return; - priv->stats.tx_bytes = tx_bytes; - _notify (self, PROP_TX_BYTES); + if (priv->stats.tx_bytes != tx_bytes) { + priv->stats.tx_bytes = tx_bytes; + _notify (self, PROP_TX_BYTES); + } + if (priv->stats.rx_bytes != rx_bytes) { + priv->stats.rx_bytes = rx_bytes; + _notify (self, PROP_RX_BYTES); + } } -void -nm_device_set_rx_bytes (NMDevice *self, guint64 rx_bytes) +static void +_stats_refresh (NMDevice *self) +{ + 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); +} + +static gboolean +_stats_timeout_cb (gpointer user_data) +{ + NMDevice *self = user_data; + + _LOGT (LOGD_DEVICE, "stats: refresh"); + _stats_refresh (self); + return G_SOURCE_CONTINUE; +} + +static guint +_stats_refresh_rate_real (guint refresh_rate_ms) +{ + const guint STATS_REFRESH_RATE_MS_MIN = 200; + + if (refresh_rate_ms == 0) + return 0; + + 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. Have a lowest number. */ + return STATS_REFRESH_RATE_MS_MIN; + } + + return refresh_rate_ms; +} + +static void +_stats_set_refresh_rate (NMDevice *self, guint refresh_rate_ms) { NMDevicePrivate *priv; - - g_return_if_fail (NM_IS_DEVICE (self)); + guint old_rate; priv = NM_DEVICE_GET_PRIVATE (self); - if (rx_bytes == priv->stats.rx_bytes) + + if (priv->stats.refresh_rate_ms == refresh_rate_ms) return; - priv->stats.rx_bytes = rx_bytes; - _notify (self, PROP_RX_BYTES); + old_rate = priv->stats.refresh_rate_ms; + priv->stats.refresh_rate_ms = refresh_rate_ms; + _notify (self, PROP_REFRESH_RATE_MS); + + _LOGD (LOGD_DEVICE, "stats: set refresh to %u ms", priv->stats.refresh_rate_ms); + + if (!nm_device_is_real (self)) + return; + + refresh_rate_ms = _stats_refresh_rate_real (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); + + priv->stats.timeout_id = g_timeout_add (refresh_rate_ms, _stats_timeout_cb, self); } +/*****************************************************************************/ + static gboolean get_ip_iface_identifier (NMDevice *self, NMUtilsIPv6IfaceId *out_iid) { @@ -1894,6 +1968,7 @@ device_ip_link_changed (NMDevice *self) _notify (self, PROP_IP_IFACE); nm_device_update_dynamic_ip_setup (self); } + return G_SOURCE_REMOVE; } @@ -2152,6 +2227,7 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) static guint32 id = 0; NMDeviceCapabilities capabilities = 0; NMConfig *config; + guint real_rate; g_return_if_fail (NM_IS_DEVICE (self)); @@ -2243,9 +2319,12 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) priv->carrier = TRUE; } - if (priv->stats.refresh_rate_ms && !priv->stats.statistics) { - priv->stats.statistics = nm_device_statistics_new (self, - priv->stats.refresh_rate_ms); + 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); } klass->realize_start_notify (self, plink); @@ -2419,14 +2498,9 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) g_clear_pointer (&priv->physical_port_id, g_free); _notify (self, PROP_PHYSICAL_PORT_ID); } - if (priv->stats.statistics) { - nm_device_statistics_unref (priv->stats.statistics); - priv->stats.statistics = NULL; - priv->stats.tx_bytes = 0; - priv->stats.tx_bytes = 0; - _notify (self, PROP_TX_BYTES); - _notify (self, PROP_RX_BYTES); - } + + nm_clear_g_source (&priv->stats.timeout_id); + _stats_update_counters (self, 0, 0); priv->hw_addr_type = HW_ADDR_TYPE_UNSET; g_clear_pointer (&priv->hw_addr_perm, g_free); @@ -12143,6 +12217,8 @@ dispose (GObject *object) nm_clear_g_source (&priv->check_delete_unrealized_id); + nm_clear_g_source (&priv->stats.timeout_id); + link_disconnect_action_cancel (self); if (priv->settings) { @@ -12168,11 +12244,6 @@ dispose (GObject *object) g_clear_object (&priv->lldp_listener); } - if (priv->stats.statistics) { - nm_device_statistics_unref (priv->stats.statistics); - priv->stats.statistics = NULL; - } - G_OBJECT_CLASS (nm_device_parent_class)->dispose (object); if (nm_clear_g_source (&priv->queued_state.id)) { @@ -12305,28 +12376,9 @@ set_property (GObject *object, guint prop_id, /* construct only */ priv->hw_addr_perm = g_value_dup_string (value); break; - case PROP_REFRESH_RATE_MS: { - guint refresh_rate_ms; - - refresh_rate_ms = g_value_get_uint (value); - if (priv->stats.refresh_rate_ms == refresh_rate_ms) - break; - - priv->stats.refresh_rate_ms = refresh_rate_ms; - _LOGI (LOGD_DEVICE, "statistics refresh rate set to %u ms", priv->stats.refresh_rate_ms); - - if (priv->stats.refresh_rate_ms) { - if (priv->stats.statistics) - nm_device_statistics_change_rate (priv->stats.statistics, priv->stats.refresh_rate_ms); - else - priv->stats.statistics = - nm_device_statistics_new (self, priv->stats.refresh_rate_ms); - } else { - nm_device_statistics_unref (priv->stats.statistics); - priv->stats.statistics = NULL; - } + case PROP_REFRESH_RATE_MS: + _stats_set_refresh_rate (self, g_value_get_uint (value)); break; - } default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; From fbbebc21230b707551b0bb7dc920368059644cdc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Aug 2016 15:55:04 +0200 Subject: [PATCH 18/18] 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 @@ 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);