diff mbox series

[net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count

Message ID 20210509115513.4121549-1-olteanv@gmail.com
State New
Headers show
Series [net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count | expand

Commit Message

Vladimir Oltean May 9, 2021, 11:55 a.m. UTC
From: Vladimir Oltean <vladimir.oltean@nxp.com>

DSA implements a bunch of 'standardized' ethtool statistics counters,
namely tx_packets, tx_bytes, rx_packets, rx_bytes. So whatever the
hardware driver returns in .get_sset_count(), we need to add 4 to that.

That is ok, except that .get_sset_count() can return a negative error
code, for example:

b53_get_sset_count
-> phy_ethtool_get_sset_count
   -> return -EIO

-EIO is -5, and with 4 added to it, it becomes -1, aka -EPERM. One can
imagine that certain error codes may even become positive, although
based on code inspection I did not see instances of that.

Check the error code first, if it is negative return it as-is.

Based on a similar patch for dsa_master_get_strings from Dan Carpenter:
https://patchwork.kernel.org/project/netdevbpf/patch/YJaSe3RPgn7gKxZv@mwanda/

Fixes: 91da11f870f0 ("net: Distributed Switch Architecture protocol support")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/dsa/slave.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Florian Fainelli May 10, 2021, 1:18 a.m. UTC | #1
On 5/9/2021 2:54 PM, Vladimir Oltean wrote:
> On Sun, May 09, 2021 at 10:35:27AM -0700, Florian Fainelli wrote:
>> On 5/9/2021 4:55 AM, Vladimir Oltean wrote:
>> I would have a preference for keeping the count variable and treating it
>> specifically in case it is negative.
> 
> Thanks. I hope I've understood you correctly that renaming "err" back to
> "count" was all that you were asking for. Anyway, patch is superseded by
> https://patchwork.kernel.org/project/netdevbpf/patch/20210509193338.451174-1-olteanv@gmail.com/

You did, thanks for the quick spin.
diff mbox series

Patch

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3689ffa2dbb8..dda232a8d6f5 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -798,13 +798,15 @@  static int dsa_slave_get_sset_count(struct net_device *dev, int sset)
 	struct dsa_switch *ds = dp->ds;
 
 	if (sset == ETH_SS_STATS) {
-		int count;
+		int err = 0;
 
-		count = 4;
-		if (ds->ops->get_sset_count)
-			count += ds->ops->get_sset_count(ds, dp->index, sset);
+		if (ds->ops->get_sset_count) {
+			err = ds->ops->get_sset_count(ds, dp->index, sset);
+			if (err < 0)
+				return err;
+		}
 
-		return count;
+		return err + 4;
 	} else if (sset ==  ETH_SS_TEST) {
 		return net_selftest_get_count();
 	}