Re: [PATCH v6 net-next 08/15] net: allow ndo_get_stats64 to return an int error code
From: Saeed Mahameed <saeed@kernel.org>
Date: 2021-01-11 22:26:05
On Sat, 2021-01-09 at 19:26 +0200, Vladimir Oltean wrote:
From: Vladimir Oltean <vladimir.oltean@nxp.com> Some drivers need to do special tricks to comply with the new policy of ndo_get_stats64 being sleepable. For example, the bonding driver, which derives its stats from its lower interfaces, must recurse with dev_get_stats into its lowers with no locks held. But for that to work, it needs to dynamically allocate some memory for a refcounted copy of its array of slave interfaces (because recursing unlocked means that the original one is subject to disappearing). And since memory allocation can fail under pressure, we should not let it go unnoticed, but instead propagate the error code. This patch converts all implementations of .ndo_get_stats64 to return int, and propagates that to the dev_get_stats calling site. Error checking will be done in further patches. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- Changes in v6: Remove the unused "int err" from __bond_release_one and add it in the patch it belongs to. Changes in v5: None. Changes in v4: Patch is new (Eric's suggestion).
Just Give Eric the proper credit and add: Suggested-by: Eric .. [...]
quoted hunk ↗ jump to hunk
@@ -354,4 +356,4 @@ int rmnet_vnd_update_dev_mtu(struct rmnet_port*port, } return 0; -} \ No newline at end of file +}
Not related? [...]
quoted hunk ↗ jump to hunk
-void dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 *storage) +int dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 *storage) { const struct net_device_ops *ops = dev->netdev_ops; + int err = 0; if (ops->ndo_get_stats64) { memset(storage, 0, sizeof(*storage)); - ops->ndo_get_stats64(dev, storage); + err = ops->ndo_get_stats64(dev, storage); } else if (ops->ndo_get_stats) { netdev_stats_to_stats64(storage, ops-quoted
ndo_get_stats(dev));} else {@@ -10418,6 +10419,8 @@ void dev_get_stats(struct net_device *dev,struct rtnl_link_stats64 *storage) storage->rx_dropped += (unsigned long)atomic_long_read(&dev-quoted
rx_dropped);storage->tx_dropped += (unsigned long)atomic_long_read(&dev-quoted
tx_dropped);storage->rx_nohandler += (unsigned long)atomic_long_read(&dev-quoted
rx_nohandler);
Must be consistent here, if there was an error you should abort without touching the caller provided storage, even if you can for some stats.
+ + return err; } EXPORT_SYMBOL(dev_get_stats);