All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count
@ 2021-05-09 19:33 Vladimir Oltean
  2021-05-10  0:35 ` Andrew Lunn
  2021-05-10 21:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Vladimir Oltean @ 2021-05-09 19:33 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, netdev
  Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, Dan Carpenter,
	Vladimir Oltean

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>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
v1 -> v2: preserved variable name of "count" instead of "err".

 net/dsa/slave.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 3689ffa2dbb8..988af45dd38f 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 count = 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) {
+			count = ds->ops->get_sset_count(ds, dp->index, sset);
+			if (count < 0)
+				return count;
+		}
 
-		return count;
+		return count + 4;
 	} else if (sset ==  ETH_SS_TEST) {
 		return net_selftest_get_count();
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count
  2021-05-09 19:33 [PATCH v2 net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count Vladimir Oltean
@ 2021-05-10  0:35 ` Andrew Lunn
  2021-05-10 21:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Lunn @ 2021-05-10  0:35 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: David S . Miller, Jakub Kicinski, netdev, Florian Fainelli,
	Vivien Didelot, Dan Carpenter, Vladimir Oltean

On Sun, May 09, 2021 at 10:33:38PM +0300, Vladimir Oltean wrote:
> 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>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count
  2021-05-09 19:33 [PATCH v2 net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count Vladimir Oltean
  2021-05-10  0:35 ` Andrew Lunn
@ 2021-05-10 21:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-05-10 21:40 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: davem, kuba, netdev, f.fainelli, andrew, vivien.didelot,
	dan.carpenter, vladimir.oltean

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Sun,  9 May 2021 22:33:38 +0300 you wrote:
> 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:
> 
> [...]

Here is the summary with links:
  - [v2,net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count
    https://git.kernel.org/netdev/net/c/b94cbc909f1d

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-10 21:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-09 19:33 [PATCH v2 net] net: dsa: fix error code getting shifted with 4 in dsa_slave_get_sset_count Vladimir Oltean
2021-05-10  0:35 ` Andrew Lunn
2021-05-10 21:40 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.