diff mbox series

[v5,net-next,12/16] net: terminate errors from dev_get_stats

Message ID 20210108163159.358043-13-olteanv@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Make .ndo_get_stats64 sleepable | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 25 maintainers not CCed: jesse.brandeburg@intel.com gerald.schaefer@linux.ibm.com daniel@iogearbox.net borntraeger@de.ibm.com pavel@ucw.cz gor@linux.ibm.com hca@linux.ibm.com dmurphy@ti.com wangyunjian@huawei.com iyappan@os.amperecomputing.com anthony.l.nguyen@intel.com keyur@os.amperecomputing.com yisen.zhuang@huawei.com bjorn.topel@intel.com intel-wired-lan@lists.osuosl.org quan@os.amperecomputing.com jiri@mellanox.com gustavoars@kernel.org luwei32@huawei.com salil.mehta@huawei.com ast@kernel.org linux-s390@vger.kernel.org andriin@fb.com vladimir.oltean@nxp.com linux-leds@vger.kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 7181 this patch: 7181
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 158 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 7586 this patch: 7586
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Vladimir Oltean Jan. 8, 2021, 4:31 p.m. UTC 
From: Vladimir Oltean <vladimir.oltean@nxp.com>

dev_get_stats can now return errors. Some call sites are coming from a
context that returns void (ethtool stats, workqueue context). So since
we can't report to the upper layer, do the next best thing: shout.

This patch wraps up the conversion of existing dev_get_stats callers, so
we can add the __must_check attribute now to ensure that future callers
keep doing this too.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v5:
Added the __must_check to dev_get_stats.

Changes in v4:
Patch is new (Eric's suggestion).

 arch/s390/appldata/appldata_net_sum.c               | 10 ++++++++--
 drivers/leds/trigger/ledtrig-netdev.c               |  9 ++++++++-
 drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c |  9 +++++++--
 drivers/net/ethernet/hisilicon/hns/hns_ethtool.c    |  7 ++++++-
 drivers/net/ethernet/intel/e1000e/ethtool.c         |  9 +++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c    |  9 +++++++--
 drivers/net/ethernet/intel/ixgbevf/ethtool.c        |  9 +++++++--
 include/linux/netdevice.h                           |  3 ++-
 net/core/dev.c                                      |  3 ++-
 9 files changed, 54 insertions(+), 14 deletions(-)
diff mbox series

Patch

diff --git a/arch/s390/appldata/appldata_net_sum.c b/arch/s390/appldata/appldata_net_sum.c
index 6146606ac9a3..72cb5344e488 100644
--- a/arch/s390/appldata/appldata_net_sum.c
+++ b/arch/s390/appldata/appldata_net_sum.c
@@ -58,11 +58,11 @@  struct appldata_net_sum_data {
  */
 static void appldata_get_net_sum_data(void *data)
 {
-	int i;
 	struct appldata_net_sum_data *net_data;
 	struct net_device *dev;
 	unsigned long rx_packets, tx_packets, rx_bytes, tx_bytes, rx_errors,
 			tx_errors, rx_dropped, tx_dropped, collisions;
+	int ret, i;
 
 	net_data = data;
 	net_data->sync_count_1++;
@@ -83,7 +83,13 @@  static void appldata_get_net_sum_data(void *data)
 	for_each_netdev(&init_net, dev) {
 		struct rtnl_link_stats64 stats;
 
-		dev_get_stats(dev, &stats);
+		ret = dev_get_stats(dev, &stats);
+		if (ret) {
+			netif_lists_unlock(&init_net);
+			netdev_err(dev, "dev_get_stats returned %d\n", ret);
+			return;
+		}
+
 		rx_packets += stats.rx_packets;
 		tx_packets += stats.tx_packets;
 		rx_bytes   += stats.rx_bytes;
diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 4382ee278309..c717b7e7dd81 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -351,6 +351,7 @@  static void netdev_trig_work(struct work_struct *work)
 	unsigned int new_activity;
 	unsigned long interval;
 	int invert;
+	int err;
 
 	/* If we dont have a device, insure we are off */
 	if (!trigger_data->net_dev) {
@@ -363,7 +364,13 @@  static void netdev_trig_work(struct work_struct *work)
 	    !test_bit(NETDEV_LED_RX, &trigger_data->mode))
 		return;
 
-	dev_get_stats(trigger_data->net_dev, &dev_stats);
+	err = dev_get_stats(trigger_data->net_dev, &dev_stats);
+	if (err) {
+		netdev_err(trigger_data->net_dev,
+			   "dev_get_stats returned %d\n", err);
+		return;
+	}
+
 	new_activity =
 	    (test_bit(NETDEV_LED_TX, &trigger_data->mode) ?
 		dev_stats.tx_packets : 0) +
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
index ada70425b48c..aab6a81f0438 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_ethtool.c
@@ -266,9 +266,14 @@  static void xgene_get_ethtool_stats(struct net_device *ndev,
 {
 	struct xgene_enet_pdata *pdata = netdev_priv(ndev);
 	struct rtnl_link_stats64 stats;
-	int i;
+	int err, i;
+
+	err = dev_get_stats(ndev, &stats);
+	if (err) {
+		netdev_err(ndev, "dev_get_stats returned %d\n", err);
+		return;
+	}
 
-	dev_get_stats(ndev, &stats);
 	for (i = 0; i < XGENE_STATS_LEN; i++)
 		data[i] = *(u64 *)((char *)&stats + gstrings_stats[i].offset);
 
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index ee2172011051..d05fa7b3f6e0 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -840,6 +840,7 @@  static void hns_get_ethtool_stats(struct net_device *netdev,
 	struct hns_nic_priv *priv = netdev_priv(netdev);
 	struct hnae_handle *h = priv->ae_handle;
 	struct rtnl_link_stats64 net_stats;
+	int err;
 
 	if (!h->dev->ops->get_stats || !h->dev->ops->update_stats) {
 		netdev_err(netdev, "get_stats or update_stats is null!\n");
@@ -848,7 +849,11 @@  static void hns_get_ethtool_stats(struct net_device *netdev,
 
 	h->dev->ops->update_stats(h, &netdev->stats);
 
-	dev_get_stats(netdev, &net_stats);
+	err = dev_get_stats(netdev, &net_stats);
+	if (err) {
+		netdev_err(netdev, "dev_get_stats returned %d\n", err);
+		return;
+	}
 
 	/* get netdev statistics */
 	p[0] = net_stats.rx_packets;
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index 06442e6bef73..41bd3e0598ce 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -2060,15 +2060,20 @@  static void e1000_get_ethtool_stats(struct net_device *netdev,
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	struct rtnl_link_stats64 net_stats;
-	int i;
 	char *p = NULL;
+	int err, i;
 
 	pm_runtime_get_sync(netdev->dev.parent);
 
-	dev_get_stats(netdev, &net_stats);
+	err = dev_get_stats(netdev, &net_stats);
 
 	pm_runtime_put_sync(netdev->dev.parent);
 
+	if (err) {
+		netdev_err(netdev, "dev_get_stats returned %d\n", err);
+		return;
+	}
+
 	for (i = 0; i < E1000_GLOBAL_STATS_LEN; i++) {
 		switch (e1000_gstrings_stats[i].type) {
 		case NETDEV_STATS:
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 2b8084664403..a647e2774f76 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -1298,11 +1298,16 @@  static void ixgbe_get_ethtool_stats(struct net_device *netdev,
 	struct rtnl_link_stats64 net_stats;
 	unsigned int start;
 	struct ixgbe_ring *ring;
-	int i, j;
 	char *p = NULL;
+	int err, i, j;
 
 	ixgbe_update_stats(adapter);
-	dev_get_stats(netdev, &net_stats);
+	err = dev_get_stats(netdev, &net_stats);
+	if (err) {
+		netdev_err(netdev, "dev_get_stats returned %d\n", err);
+		return;
+	}
+
 	for (i = 0; i < IXGBE_GLOBAL_STATS_LEN; i++) {
 		switch (ixgbe_gstrings_stats[i].type) {
 		case NETDEV_STATS:
diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index 3b9b7e5c2998..665e39301092 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -423,11 +423,16 @@  static void ixgbevf_get_ethtool_stats(struct net_device *netdev,
 	struct rtnl_link_stats64 net_stats;
 	unsigned int start;
 	struct ixgbevf_ring *ring;
-	int i, j;
+	int err, i, j;
 	char *p;
 
 	ixgbevf_update_stats(adapter);
-	dev_get_stats(netdev, &net_stats);
+	err = dev_get_stats(netdev, &net_stats);
+	if (err) {
+		netdev_err(netdev, "dev_get_stats returned %d\n", err);
+		return;
+	}
+
 	for (i = 0; i < IXGBEVF_GLOBAL_STATS_LEN; i++) {
 		switch (ixgbevf_gstrings_stats[i].type) {
 		case NETDEV_STATS:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index bd471f1e1fa3..b1aebab916a8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4545,7 +4545,8 @@  void netdev_notify_peers(struct net_device *dev);
 void netdev_features_change(struct net_device *dev);
 /* Load a device via the kmod */
 void dev_load(struct net *net, const char *name);
-int dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 *storage);
+int __must_check dev_get_stats(struct net_device *dev,
+			       struct rtnl_link_stats64 *storage);
 void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 			     const struct net_device_stats *netdev_stats);
 void dev_fetch_sw_netstats(struct rtnl_link_stats64 *s,
diff --git a/net/core/dev.c b/net/core/dev.c
index dfbd66ba3cad..30facac95d5e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10403,7 +10403,8 @@  EXPORT_SYMBOL(netdev_stats_to_stats64);
  *	dev->netdev_ops->get_stats64 or dev->netdev_ops->get_stats;
  *	otherwise the internal statistics structure is used.
  */
-int dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 *storage)
+int __must_check dev_get_stats(struct net_device *dev,
+			       struct rtnl_link_stats64 *storage)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
 	int err = 0;