Re: [PATCH v2 net-next 3/6] net: dsa: mv88e6xxx: Fix mv88e6352_serdes_get_stats error path

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Dec 05, 2023 at 05:04:15PM +0100, Tobias Waldekranz wrote:
> mv88e6xxx_get_stats, which collects stats from various sources,
> expects all callees to return the number of stats read. If an error
> occurs, 0 should be returned.
> 
> Prevent future mishaps of this kind by updating the return type to
> reflect this contract.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
> index 3b4b42651fa3..01ea53940786 100644
> --- a/drivers/net/dsa/mv88e6xxx/serdes.c
> +++ b/drivers/net/dsa/mv88e6xxx/serdes.c
> @@ -187,7 +187,7 @@ int mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
>  
>  	err = mv88e6352_g2_scratch_port_has_serdes(chip, port);
>  	if (err <= 0)
> -		return err;
> +		return 0;

Ok, you're saying we don't care enough about handling the catastrophic
event where an MDIO access error takes place in mv88e6xxx_g2_scratch_read()
to submit this to "stable".

I guess the impact in such a case is that the error (interpreted as negative
count) makes us go back by -EIO (5) entries or whatever into the "data"
array provided to user space, overwriting some previous stats and making
everything after the failed counter minus the error code be reported in
the wrong place relative to its string. I don't think that the error
codes are high enough to overcome the ~60 port stats and cause memory
accesses behind the "data" array.

Anyway, for the patch content:

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>