[PATCH net v2] net: dsa: motorcomm: Publish a consistent MIB sample

[PATCH net v2] net: dsa: motorcomm: Publish a consistent MIB sample

[David Yang]

get_stats64() derived rx_bytes and tx_bytes by subtracting ETH_FCS_LEN
per packet from the published byte counters, but u64_stats_sync gives
neither ordering nor retry across a multi-field read on 64-bit kernels,
and a failed register read published a partially refreshed buffer as
well. A fresh packet count could then pair with a stale byte counter,
and the subtraction underflowed on unsigned arithmetic, reporting
rx_bytes or tx_bytes as about 2^64.

Do the arithmetic in the writer, on the complete sample collected under
priv->reg_lock, and publish the adjusted byte counters next to the
frame totals, so that get_stats64() only copies published values. On a
failed register read, keep the last complete sample.

Found by LLM. The issue is transient and abnormal values will be flushed
by the next function call.

Fixes: 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm YT921x")
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
v2:
 - more issues found by sashiko
v1: https://lore.kernel.org/r/20261102101334.3525485-1-mmyangfl@gmail.com
 drivers/net/dsa/motorcomm/mib.c | 23 +++++++++++++++--------
 drivers/net/dsa/motorcomm/mib.h |  2 ++
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/motorcomm/mib.c b/drivers/net/dsa/motorcomm/mib.c
index 6b170fc6e187..d47aafc468a3 100644
--- a/drivers/net/dsa/motorcomm/mib.c
+++ b/drivers/net/dsa/motorcomm/mib.c
@@ -95,6 +95,8 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
 	unsigned long flags;
 	u64 rx_frames;
 	u64 tx_frames;
+	u64 rx_bytes;
+	u64 tx_bytes;
 	int res = 0;
 
 	mib = &pm->stats;
@@ -127,6 +129,12 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
 		buf[i] = val;
 	}
 
+	if (res) {
+		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
+			port, res);
+		return res;
+	}
+
 	flags = u64_stats_update_begin_irqsave(&pm->syncp);
 	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++)
 		u64_stats_set(&((u64_stats_t *)mib)[i], buf[i]);
@@ -144,14 +152,15 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
 		    u64_stats_read(&mib->tx_512_1023byte) +
 		    u64_stats_read(&mib->tx_1024_1518byte) +
 		    u64_stats_read(&mib->tx_jumbo);
+	rx_bytes = u64_stats_read(&mib->rx_good_bytes) - ETH_FCS_LEN * rx_frames;
+	tx_bytes = u64_stats_read(&mib->tx_good_bytes) - ETH_FCS_LEN * tx_frames;
 	u64_stats_set(&pm->rx_frames, rx_frames);
 	u64_stats_set(&pm->tx_frames, tx_frames);
+	u64_stats_set(&pm->rx_bytes, rx_bytes);
+	u64_stats_set(&pm->tx_bytes, tx_bytes);
 	u64_stats_update_end_irqrestore(&pm->syncp, flags);
 
-	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
-			port, res);
-	return res;
+	return 0;
 }
 
 void yt921x_mib_poll(struct work_struct *work)
@@ -391,10 +400,8 @@ yt921x_dsa_get_stats64(struct dsa_switch *ds, int port,
 
 		stats->rx_packets = u64_stats_read(&pm->rx_frames);
 		stats->tx_packets = u64_stats_read(&pm->tx_frames);
-		stats->rx_bytes = u64_stats_read(&mib->rx_good_bytes) -
-				  ETH_FCS_LEN * stats->rx_packets;
-		stats->tx_bytes = u64_stats_read(&mib->tx_good_bytes) -
-				  ETH_FCS_LEN * stats->tx_packets;
+		stats->rx_bytes = u64_stats_read(&pm->rx_bytes);
+		stats->tx_bytes = u64_stats_read(&pm->tx_bytes);
 		stats->rx_dropped = u64_stats_read(&mib->rx_dropped);
 		/* stats->tx_dropped */
 		stats->multicast = u64_stats_read(&mib->rx_multicast);
diff --git a/drivers/net/dsa/motorcomm/mib.h b/drivers/net/dsa/motorcomm/mib.h
index 9c24a52d46cc..2e2b832cb245 100644
--- a/drivers/net/dsa/motorcomm/mib.h
+++ b/drivers/net/dsa/motorcomm/mib.h
@@ -136,6 +136,8 @@ struct yt921x_mib {
 	struct yt921x_mib_stats stats;
 	u64_stats_t rx_frames;
 	u64_stats_t tx_frames;
+	u64_stats_t rx_bytes;
+	u64_stats_t tx_bytes;
 	/* protected by priv->reg_lock */
 	u64 data[YT921X_MIB_STAT_NUM];
 };
-- 
2.53.0

Re: [PATCH net v2] net: dsa: motorcomm: Publish a consistent MIB sample

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] net: dsa: motorcomm: Publish a consistent MIB sample

[David Yang]

On Tue, Nov 3, 2026 at 9:53 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Sequence related, cannot reliably reproduce.

Re: [PATCH net v2] net: dsa: motorcomm: Publish a consistent MIB sample

[netdev-bot+sashiko]

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

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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The underflow this patch says it fixes can still happen when
  the MIB sample is fully consistent.

--

net: dsa: motorcomm: Publish a consistent MIB sample

This patch moves the "good_bytes - ETH_FCS_LEN * frames" calculation out
of yt921x_dsa_get_stats64() and into the writer, yt921x_mib_read(). The
writer then publishes the result as pm->rx_bytes and pm->tx_bytes. It
also skips publishing the sample when a register read fails.

> diff --git a/drivers/net/dsa/motorcomm/mib.c b/drivers/net/dsa/motorcomm/mib.c
> index 6b170fc6e1875..d47aafc468a3f 100644
> --- a/drivers/net/dsa/motorcomm/mib.c
> +++ b/drivers/net/dsa/motorcomm/mib.c

[ ... ]

> @@ -144,14 +152,15 @@ static int yt921x_mib_read(struct yt921x_priv *priv, int port)
>  		    u64_stats_read(&mib->tx_512_1023byte) +
>  		    u64_stats_read(&mib->tx_1024_1518byte) +
>  		    u64_stats_read(&mib->tx_jumbo);
> +	rx_bytes = u64_stats_read(&mib->rx_good_bytes) - ETH_FCS_LEN * rx_frames;
> +	tx_bytes = u64_stats_read(&mib->tx_good_bytes) - ETH_FCS_LEN * tx_frames;

[Severity: Medium]
Can this subtraction still underflow when the sample is fully consistent?

rx_frames and tx_frames are sums of the packet-length histogram buckets
(rx_64byte through rx_jumbo, and tx_64byte through tx_jumbo). The byte
term comes from a separate counter, RX_GOOD_BYTES or TX_GOOD_BYTES, which
appears to count good frames only. The hardware also has a separate
RxBadBytes counter:

	MIB_DESC(2, YT921X_MIB_DATA_RX_BAD_BYTES, "RxBadBytes"),

yt921x_dsa_get_rmon_stats() exports the same buckets as RMON histogram
counters:

	rmon_stats->hist[0] = u64_stats_read(&mib->rx_64byte);
	...
	rmon_stats->hist_tx[0] = u64_stats_read(&mib->tx_64byte);

Those are the RFC 2819 etherStatsPktsXtoYOctets counters, and RFC 2819
counts bad packets in them.

If the YT921x buckets also count CRC or alignment errored frames, a port
that receives only errored frames would see rx_64byte grow while
rx_good_bytes stays the same. rx_bytes would then wrap to about 2^64.
This is the same symptom the commit message describes.

Because the value is recomputed and published on every poll, would the
wrapped value persist? The commit message says:

    The issue is transient and abnormal values will be flushed
    by the next function call.

That claim would not hold in this case.

There also seems to be a mismatch with struct rtnl_link_stats64. It says
rx_packets counts good packets and rx_bytes corresponds to rx_packets.
For TX, the driver already reads a good-frame counter into mib->tx_good:

	MIB_DESC(1, YT921X_MIB_DATA_TX_PKT, NULL),

Would tx_good be a better match for tx_good_bytes than the histogram sum?
Alternatively, should the subtraction be clamped to avoid wrapping?

>  	u64_stats_set(&pm->rx_frames, rx_frames);
>  	u64_stats_set(&pm->tx_frames, tx_frames);
> +	u64_stats_set(&pm->rx_bytes, rx_bytes);
> +	u64_stats_set(&pm->tx_bytes, tx_bytes);
>  	u64_stats_update_end_irqrestore(&pm->syncp, flags);

[ ... ]

> @@ -391,10 +400,8 @@ yt921x_dsa_get_stats64(struct dsa_switch *ds, int port,
>  
>  		stats->rx_packets = u64_stats_read(&pm->rx_frames);
>  		stats->tx_packets = u64_stats_read(&pm->tx_frames);
> -		stats->rx_bytes = u64_stats_read(&mib->rx_good_bytes) -
> -				  ETH_FCS_LEN * stats->rx_packets;
> -		stats->tx_bytes = u64_stats_read(&mib->tx_good_bytes) -
> -				  ETH_FCS_LEN * stats->tx_packets;
> +		stats->rx_bytes = u64_stats_read(&pm->rx_bytes);
> +		stats->tx_bytes = u64_stats_read(&pm->tx_bytes);

[ ... ]

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