Re: [PATCH net-next v3 6/6] net: dsa: motorcomm: Use safe 64-bit counter reader

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] The patch is a functional bug fix (it removes spurious ~4 GiB
  jumps in the 64-bit MIB byte counters that are exported to userspace)…

--

net: dsa: motorcomm: Use safe 64-bit counter reader

This switches the multi-word MIB counters in yt921x_mib_read() over to a
new helper, yt921x_counter_read(), which re-reads the low word after the
high word and retries until the pair is consistent. The single-word path
keeps its existing software wrap extension.

> diff --git a/drivers/net/dsa/motorcomm/mib.c b/drivers/net/dsa/motorcomm/mib.c
> index 8601d25fe9887..cfae2977a77b9 100644
> --- a/drivers/net/dsa/motorcomm/mib.c
> +++ b/drivers/net/dsa/motorcomm/mib.c
> @@ -102,29 +102,26 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
>  	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
>  		const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
>  		u32 reg = YT921X_MIBn_DATA0(port) + desc->offset;
> -		u32 val0;
> -		u64 val;
> -
> -		res = yt921x_reg_read(priv, reg, &val0);
> -		if (res)
> -			break;
>  
>  		if (desc->size <= 1) {
>  			u64 old_val = buf[i];
> +			u32 val0;
> +			u64 val;
> +
> +			res = yt921x_reg_read(priv, reg, &val0);
> +			if (res)
> +				break;
>  
>  			val = (old_val & ~(u64)U32_MAX) | val0;
>  			if (val < old_val)
>  				val += 1ull << 32;
> -		} else {
> -			u32 val1;
>  
> -			res = yt921x_reg_read(priv, reg + 4, &val1);
> +			buf[i] = val;
> +		} else {
> +			res = yt921x_counter_read(priv, reg, &buf[i]);
>  			if (res)
>  				break;
> -			val = ((u64)val1 << 32) | val0;
>  		}
> -
> -		buf[i] = val;
>  	}

[Severity: Low]
Should this hunk carry a Fixes: tag and a stable Cc?

The commit message describes a concrete user-visible misbehaviour
("producing a spurious ~4 GiB jump"), and the racy sequence it replaces is
already in a released kernel. At the baseline the same code lives in
drivers/net/dsa/motorcomm/chip.c:yt921x_mib_read():

	res = yt921x_reg_read(priv, reg, &val0);
	...
		res = yt921x_reg_read(priv, reg + 4, &val1);
		if (res)
			break;
		val = ((u64)val1 << 32) | val0;

git blame attributes that two-transaction read to 186623f4aa724c ("net:
dsa: Add Motorcomm YT921x DSA switch support"), and the earlier patches in
this series only moved it from chip.c into mib.c:yt921x_mib_read().

The affected entries are the desc->size > 1 counters, i.e. rx_good_bytes,
rx_bad_bytes and tx_good_bytes in struct yt921x_mib_stats, which are
reported through the periodic MIB poll and the DSA get_stats64()/ethtool
paths.

Without a Fixes: tag pointing at 186623f4aa724c, would this hunk be
distinguishable from the surrounding refactor for backporting purposes?

[ ... smi.c and smi.h hunks snipped, no comments ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906131414.2683362-1-mmyangfl%40gmail.com