public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v6 0/3] net: dsa: yt921x: Fix MIB overflow wraparound routine
@ 2026-01-18  1:30 David Yang
  2026-01-18  1:30 ` [PATCH net-next v6 1/3] " David Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: David Yang @ 2026-01-18  1:30 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

Fix warning reported by static checker.

v5: https://lore.kernel.org/r/20260114114745.213252-1-mmyangfl@gmail.com
  - avoid type casting
  - split patches
v4: https://lore.kernel.org/r/20260108004309.4087448-1-mmyangfl@gmail.com
  - add missing u64_stats_init
v3: https://lore.kernel.org/r/20260105020905.3522484-1-mmyangfl@gmail.com
  - use u64_stats_t
  - fix calculations of rx_frames/tx_frames
v2: https://lore.kernel.org/r/20251025171314.1939608-1-mmyangfl@gmail.com
  - run tests and fix MIB parsing in 510026a39849
  - no major changes between versions
v1: https://lore.kernel.org/r/20251024084918.1353031-1-mmyangfl@gmail.com
  - take suggestion from David Laight
  - protect MIB stats with a lock

David Yang (3):
  net: dsa: yt921x: Fix MIB overflow wraparound routine
  net: dsa: yt921x: Return early for failed MIB read
  net: dsa: yt921x: Use u64_stats_t for MIB stats

 drivers/net/dsa/yt921x.c | 258 +++++++++++++++++++++++----------------
 drivers/net/dsa/yt921x.h | 114 +++++++++--------
 2 files changed, 218 insertions(+), 154 deletions(-)

-- 
2.51.0


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

* [PATCH net-next v6 1/3] net: dsa: yt921x: Fix MIB overflow wraparound routine
  2026-01-18  1:30 [PATCH net-next v6 0/3] net: dsa: yt921x: Fix MIB overflow wraparound routine David Yang
@ 2026-01-18  1:30 ` David Yang
  2026-01-22  9:41   ` Paolo Abeni
  2026-01-18  1:30 ` [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read David Yang
  2026-01-18  1:30 ` [PATCH net-next v6 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats David Yang
  2 siblings, 1 reply; 10+ messages in thread
From: David Yang @ 2026-01-18  1:30 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel,
	Dan Carpenter, David Laight

Reported by the following Smatch static checker warning:

  drivers/net/dsa/yt921x.c:702 yt921x_read_mib()
  warn: was expecting a 64 bit value instead of '(~0)'

Fixes: 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm YT921x")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/netdev/aPsjYKQMzpY0nSXm@stanley.mountain/
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index 0b3df732c0d1..5e4e8093ba16 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -682,21 +682,22 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 		const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
 		u32 reg = YT921X_MIBn_DATA0(port) + desc->offset;
 		u64 *valp = &((u64 *)mib)[i];
-		u64 val = *valp;
 		u32 val0;
-		u32 val1;
+		u64 val;
 
 		res = yt921x_reg_read(priv, reg, &val0);
 		if (res)
 			break;
 
 		if (desc->size <= 1) {
-			if (val < (u32)val)
-				/* overflow */
-				val += (u64)U32_MAX + 1;
-			val &= ~U32_MAX;
-			val |= val0;
+			u64 old_val = *valp;
+
+			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);
 			if (res)
 				break;
-- 
2.51.0


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

* [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read
  2026-01-18  1:30 [PATCH net-next v6 0/3] net: dsa: yt921x: Fix MIB overflow wraparound routine David Yang
  2026-01-18  1:30 ` [PATCH net-next v6 1/3] " David Yang
@ 2026-01-18  1:30 ` David Yang
  2026-01-18 16:06   ` Andrew Lunn
  2026-01-18  1:30 ` [PATCH net-next v6 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats David Yang
  2 siblings, 1 reply; 10+ messages in thread
From: David Yang @ 2026-01-18  1:30 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

This patch does not change anything effectively, but serves as a
prerequisite for another patch.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index 5e4e8093ba16..fe08385445d2 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -707,6 +707,12 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 		WRITE_ONCE(*valp, val);
 	}
 
+	if (res) {
+		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
+			port, res);
+		return res;
+	}
+
 	pp->rx_frames = mib->rx_64byte + mib->rx_65_127byte +
 			mib->rx_128_255byte + mib->rx_256_511byte +
 			mib->rx_512_1023byte + mib->rx_1024_1518byte +
@@ -716,10 +722,7 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 			mib->tx_512_1023byte + mib->tx_1024_1518byte +
 			mib->tx_jumbo;
 
-	if (res)
-		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
-			port, res);
-	return res;
+	return 0;
 }
 
 static void yt921x_poll_mib(struct work_struct *work)
-- 
2.51.0


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

* [PATCH net-next v6 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats
  2026-01-18  1:30 [PATCH net-next v6 0/3] net: dsa: yt921x: Fix MIB overflow wraparound routine David Yang
  2026-01-18  1:30 ` [PATCH net-next v6 1/3] " David Yang
  2026-01-18  1:30 ` [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read David Yang
@ 2026-01-18  1:30 ` David Yang
  2 siblings, 0 replies; 10+ messages in thread
From: David Yang @ 2026-01-18  1:30 UTC (permalink / raw)
  To: netdev
  Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel

64-bit variables might not be atomic on 32-bit architectures, thus
cannot be unconditionally made lock-free. Use u64_stats_t so it would
still be lock-free on 64-bit architectures.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/dsa/yt921x.c | 236 +++++++++++++++++++++++----------------
 drivers/net/dsa/yt921x.h | 114 ++++++++++---------
 2 files changed, 205 insertions(+), 145 deletions(-)

diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index fe08385445d2..f850117e7a68 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -18,6 +18,7 @@
 #include <linux/of.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
+#include <linux/u64_stats_sync.h>
 
 #include <net/dsa.h>
 
@@ -666,22 +667,20 @@ yt921x_mbus_ext_init(struct yt921x_priv *priv, struct device_node *mnp)
 static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 {
 	struct yt921x_port *pp = &priv->ports[port];
+	struct yt921x_mib *mib_new = &pp->mib_new;
 	struct device *dev = to_device(priv);
 	struct yt921x_mib *mib = &pp->mib;
+	u64 rx_frames;
+	u64 tx_frames;
 	int res = 0;
 
-	/* Reading of yt921x_port::mib is not protected by a lock and it's vain
-	 * to keep its consistency, since we have to read registers one by one
-	 * and there is no way to make a snapshot of MIB stats.
-	 *
-	 * Writing (by this function only) is and should be protected by
-	 * reg_lock.
+	/* u64_stats_read/set is redundant for mib_new, but I don't want to
+	 * declare a plain u64 yt921x_mib variant.
 	 */
 
 	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;
-		u64 *valp = &((u64 *)mib)[i];
 		u32 val0;
 		u64 val;
 
@@ -690,7 +689,7 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 			break;
 
 		if (desc->size <= 1) {
-			u64 old_val = *valp;
+			u64 old_val = u64_stats_read(&mib->stats[i]);
 
 			val = (old_val & ~(u64)U32_MAX) | val0;
 			if (val < old_val)
@@ -704,7 +703,7 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 			val = ((u64)val1 << 32) | val0;
 		}
 
-		WRITE_ONCE(*valp, val);
+		u64_stats_set(&mib_new->stats[i], val);
 	}
 
 	if (res) {
@@ -713,14 +712,29 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
 		return res;
 	}
 
-	pp->rx_frames = mib->rx_64byte + mib->rx_65_127byte +
-			mib->rx_128_255byte + mib->rx_256_511byte +
-			mib->rx_512_1023byte + mib->rx_1024_1518byte +
-			mib->rx_jumbo;
-	pp->tx_frames = mib->tx_64byte + mib->tx_65_127byte +
-			mib->tx_128_255byte + mib->tx_256_511byte +
-			mib->tx_512_1023byte + mib->tx_1024_1518byte +
-			mib->tx_jumbo;
+	rx_frames = u64_stats_read(&mib_new->rx_64byte) +
+		    u64_stats_read(&mib_new->rx_65_127byte) +
+		    u64_stats_read(&mib_new->rx_128_255byte) +
+		    u64_stats_read(&mib_new->rx_256_511byte) +
+		    u64_stats_read(&mib_new->rx_512_1023byte) +
+		    u64_stats_read(&mib_new->rx_1024_1518byte) +
+		    u64_stats_read(&mib_new->rx_jumbo);
+	tx_frames = u64_stats_read(&mib_new->tx_64byte) +
+		    u64_stats_read(&mib_new->tx_65_127byte) +
+		    u64_stats_read(&mib_new->tx_128_255byte) +
+		    u64_stats_read(&mib_new->tx_256_511byte) +
+		    u64_stats_read(&mib_new->tx_512_1023byte) +
+		    u64_stats_read(&mib_new->tx_1024_1518byte) +
+		    u64_stats_read(&mib_new->tx_jumbo);
+
+	u64_stats_update_begin(&pp->syncp);
+	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
+		u64_stats_set(&mib->stats[i],
+			      u64_stats_read(&mib_new->stats[i]));
+	}
+	u64_stats_set(&pp->rx_frames, rx_frames);
+	u64_stats_set(&pp->tx_frames, tx_frames);
+	u64_stats_update_end(&pp->syncp);
 
 	return 0;
 }
@@ -765,22 +779,27 @@ yt921x_dsa_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 	size_t j;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	j = 0;
-	for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
-		const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
 
-		if (!desc->name)
-			continue;
+		j = 0;
+		for (size_t i = 0; i < ARRAY_SIZE(yt921x_mib_descs); i++) {
+			const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
 
-		data[j] = ((u64 *)mib)[i];
-		j++;
-	}
+			if (!desc->name)
+				continue;
+
+			data[j] = u64_stats_read(&((u64_stats_t *)mib)[i]);
+			j++;
+		}
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static int yt921x_dsa_get_sset_count(struct dsa_switch *ds, int port, int sset)
@@ -807,33 +826,38 @@ yt921x_dsa_get_eth_mac_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	mac_stats->FramesTransmittedOK = pp->tx_frames;
-	mac_stats->SingleCollisionFrames = mib->tx_single_collisions;
-	mac_stats->MultipleCollisionFrames = mib->tx_multiple_collisions;
-	mac_stats->FramesReceivedOK = pp->rx_frames;
-	mac_stats->FrameCheckSequenceErrors = mib->rx_crc_errors;
-	mac_stats->AlignmentErrors = mib->rx_alignment_errors;
-	mac_stats->OctetsTransmittedOK = mib->tx_good_bytes;
-	mac_stats->FramesWithDeferredXmissions = mib->tx_deferred;
-	mac_stats->LateCollisions = mib->tx_late_collisions;
-	mac_stats->FramesAbortedDueToXSColls = mib->tx_aborted_errors;
-	/* mac_stats->FramesLostDueToIntMACXmitError */
-	/* mac_stats->CarrierSenseErrors */
-	mac_stats->OctetsReceivedOK = mib->rx_good_bytes;
-	/* mac_stats->FramesLostDueToIntMACRcvError */
-	mac_stats->MulticastFramesXmittedOK = mib->tx_multicast;
-	mac_stats->BroadcastFramesXmittedOK = mib->tx_broadcast;
-	/* mac_stats->FramesWithExcessiveDeferral */
-	mac_stats->MulticastFramesReceivedOK = mib->rx_multicast;
-	mac_stats->BroadcastFramesReceivedOK = mib->rx_broadcast;
-	/* mac_stats->InRangeLengthErrors */
-	/* mac_stats->OutOfRangeLengthField */
-	mac_stats->FrameTooLongErrors = mib->rx_oversize_errors;
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		mac_stats->FramesTransmittedOK = u64_stats_read(&pp->tx_frames);
+		mac_stats->SingleCollisionFrames = u64_stats_read(&mib->tx_single_collisions);
+		mac_stats->MultipleCollisionFrames = u64_stats_read(&mib->tx_multiple_collisions);
+		mac_stats->FramesReceivedOK = u64_stats_read(&pp->rx_frames);
+		mac_stats->FrameCheckSequenceErrors = u64_stats_read(&mib->rx_crc_errors);
+		mac_stats->AlignmentErrors = u64_stats_read(&mib->rx_alignment_errors);
+		mac_stats->OctetsTransmittedOK = u64_stats_read(&mib->tx_good_bytes);
+		mac_stats->FramesWithDeferredXmissions = u64_stats_read(&mib->tx_deferred);
+		mac_stats->LateCollisions = u64_stats_read(&mib->tx_late_collisions);
+		mac_stats->FramesAbortedDueToXSColls = u64_stats_read(&mib->tx_aborted_errors);
+		/* mac_stats->FramesLostDueToIntMACXmitError */
+		/* mac_stats->CarrierSenseErrors */
+		mac_stats->OctetsReceivedOK = u64_stats_read(&mib->rx_good_bytes);
+		/* mac_stats->FramesLostDueToIntMACRcvError */
+		mac_stats->MulticastFramesXmittedOK = u64_stats_read(&mib->tx_multicast);
+		mac_stats->BroadcastFramesXmittedOK = u64_stats_read(&mib->tx_broadcast);
+		/* mac_stats->FramesWithExcessiveDeferral */
+		mac_stats->MulticastFramesReceivedOK = u64_stats_read(&mib->rx_multicast);
+		mac_stats->BroadcastFramesReceivedOK = u64_stats_read(&mib->rx_broadcast);
+		/* mac_stats->InRangeLengthErrors */
+		/* mac_stats->OutOfRangeLengthField */
+		mac_stats->FrameTooLongErrors = u64_stats_read(&mib->rx_oversize_errors);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static void
@@ -843,14 +867,19 @@ yt921x_dsa_get_eth_ctrl_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	ctrl_stats->MACControlFramesTransmitted = mib->tx_pause;
-	ctrl_stats->MACControlFramesReceived = mib->rx_pause;
-	/* ctrl_stats->UnsupportedOpcodesReceived */
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		ctrl_stats->MACControlFramesTransmitted = u64_stats_read(&mib->tx_pause);
+		ctrl_stats->MACControlFramesReceived = u64_stats_read(&mib->rx_pause);
+		/* ctrl_stats->UnsupportedOpcodesReceived */
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static const struct ethtool_rmon_hist_range yt921x_rmon_ranges[] = {
@@ -872,6 +901,7 @@ yt921x_dsa_get_rmon_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
@@ -879,26 +909,30 @@ yt921x_dsa_get_rmon_stats(struct dsa_switch *ds, int port,
 
 	*ranges = yt921x_rmon_ranges;
 
-	rmon_stats->undersize_pkts = mib->rx_undersize_errors;
-	rmon_stats->oversize_pkts = mib->rx_oversize_errors;
-	rmon_stats->fragments = mib->rx_alignment_errors;
-	/* rmon_stats->jabbers */
-
-	rmon_stats->hist[0] = mib->rx_64byte;
-	rmon_stats->hist[1] = mib->rx_65_127byte;
-	rmon_stats->hist[2] = mib->rx_128_255byte;
-	rmon_stats->hist[3] = mib->rx_256_511byte;
-	rmon_stats->hist[4] = mib->rx_512_1023byte;
-	rmon_stats->hist[5] = mib->rx_1024_1518byte;
-	rmon_stats->hist[6] = mib->rx_jumbo;
-
-	rmon_stats->hist_tx[0] = mib->tx_64byte;
-	rmon_stats->hist_tx[1] = mib->tx_65_127byte;
-	rmon_stats->hist_tx[2] = mib->tx_128_255byte;
-	rmon_stats->hist_tx[3] = mib->tx_256_511byte;
-	rmon_stats->hist_tx[4] = mib->tx_512_1023byte;
-	rmon_stats->hist_tx[5] = mib->tx_1024_1518byte;
-	rmon_stats->hist_tx[6] = mib->tx_jumbo;
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		rmon_stats->undersize_pkts = u64_stats_read(&mib->rx_undersize_errors);
+		rmon_stats->oversize_pkts = u64_stats_read(&mib->rx_oversize_errors);
+		rmon_stats->fragments = u64_stats_read(&mib->rx_alignment_errors);
+		/* rmon_stats->jabbers */
+
+		rmon_stats->hist[0] = u64_stats_read(&mib->rx_64byte);
+		rmon_stats->hist[1] = u64_stats_read(&mib->rx_65_127byte);
+		rmon_stats->hist[2] = u64_stats_read(&mib->rx_128_255byte);
+		rmon_stats->hist[3] = u64_stats_read(&mib->rx_256_511byte);
+		rmon_stats->hist[4] = u64_stats_read(&mib->rx_512_1023byte);
+		rmon_stats->hist[5] = u64_stats_read(&mib->rx_1024_1518byte);
+		rmon_stats->hist[6] = u64_stats_read(&mib->rx_jumbo);
+
+		rmon_stats->hist_tx[0] = u64_stats_read(&mib->tx_64byte);
+		rmon_stats->hist_tx[1] = u64_stats_read(&mib->tx_65_127byte);
+		rmon_stats->hist_tx[2] = u64_stats_read(&mib->tx_128_255byte);
+		rmon_stats->hist_tx[3] = u64_stats_read(&mib->tx_256_511byte);
+		rmon_stats->hist_tx[4] = u64_stats_read(&mib->tx_512_1023byte);
+		rmon_stats->hist_tx[5] = u64_stats_read(&mib->tx_1024_1518byte);
+		rmon_stats->hist_tx[6] = u64_stats_read(&mib->tx_jumbo);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static void
@@ -908,33 +942,41 @@ yt921x_dsa_get_stats64(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
+
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		stats->rx_length_errors = u64_stats_read(&mib->rx_undersize_errors) +
+					  u64_stats_read(&mib->rx_fragment_errors);
+		stats->rx_over_errors = u64_stats_read(&mib->rx_oversize_errors);
+		stats->rx_crc_errors = u64_stats_read(&mib->rx_crc_errors);
+		stats->rx_frame_errors = u64_stats_read(&mib->rx_alignment_errors);
+		/* stats->rx_fifo_errors */
+		/* stats->rx_missed_errors */
+
+		stats->tx_aborted_errors = u64_stats_read(&mib->tx_aborted_errors);
+		/* stats->tx_carrier_errors */
+		stats->tx_fifo_errors = u64_stats_read(&mib->tx_undersize_errors);
+		/* stats->tx_heartbeat_errors */
+		stats->tx_window_errors = u64_stats_read(&mib->tx_late_collisions);
+
+		stats->rx_packets = u64_stats_read(&pp->rx_frames);
+		stats->tx_packets = u64_stats_read(&pp->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_dropped = u64_stats_read(&mib->rx_dropped);
+		/* stats->tx_dropped */
+		stats->multicast = u64_stats_read(&mib->rx_multicast);
+		stats->collisions = u64_stats_read(&mib->tx_collisions);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 
-	stats->rx_length_errors = mib->rx_undersize_errors +
-				  mib->rx_fragment_errors;
-	stats->rx_over_errors = mib->rx_oversize_errors;
-	stats->rx_crc_errors = mib->rx_crc_errors;
-	stats->rx_frame_errors = mib->rx_alignment_errors;
-	/* stats->rx_fifo_errors */
-	/* stats->rx_missed_errors */
-
-	stats->tx_aborted_errors = mib->tx_aborted_errors;
-	/* stats->tx_carrier_errors */
-	stats->tx_fifo_errors = mib->tx_undersize_errors;
-	/* stats->tx_heartbeat_errors */
-	stats->tx_window_errors = mib->tx_late_collisions;
-
-	stats->rx_packets = pp->rx_frames;
-	stats->tx_packets = pp->tx_frames;
-	stats->rx_bytes = mib->rx_good_bytes - ETH_FCS_LEN * stats->rx_packets;
-	stats->tx_bytes = mib->tx_good_bytes - ETH_FCS_LEN * stats->tx_packets;
 	stats->rx_errors = stats->rx_length_errors + stats->rx_over_errors +
 			   stats->rx_crc_errors + stats->rx_frame_errors;
 	stats->tx_errors = stats->tx_aborted_errors + stats->tx_fifo_errors +
 			   stats->tx_window_errors;
-	stats->rx_dropped = mib->rx_dropped;
-	/* stats->tx_dropped */
-	stats->multicast = mib->rx_multicast;
-	stats->collisions = mib->tx_collisions;
 }
 
 static void
@@ -944,13 +986,18 @@ yt921x_dsa_get_pause_stats(struct dsa_switch *ds, int port,
 	struct yt921x_priv *priv = to_yt921x_priv(ds);
 	struct yt921x_port *pp = &priv->ports[port];
 	struct yt921x_mib *mib = &pp->mib;
+	unsigned int start;
 
 	mutex_lock(&priv->reg_lock);
 	yt921x_read_mib(priv, port);
 	mutex_unlock(&priv->reg_lock);
 
-	pause_stats->tx_pause_frames = mib->tx_pause;
-	pause_stats->rx_pause_frames = mib->rx_pause;
+	do {
+		start = u64_stats_fetch_begin(&pp->syncp);
+
+		pause_stats->tx_pause_frames = u64_stats_read(&mib->tx_pause);
+		pause_stats->rx_pause_frames = u64_stats_read(&mib->rx_pause);
+	} while (u64_stats_fetch_retry(&pp->syncp, start));
 }
 
 static int
@@ -2971,6 +3018,7 @@ static int yt921x_mdio_probe(struct mdio_device *mdiodev)
 
 		pp->index = i;
 		INIT_DELAYED_WORK(&pp->mib_read, yt921x_poll_mib);
+		u64_stats_init(&pp->syncp);
 	}
 
 	ds = &priv->ds;
diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
index 61bb0ab3b09a..29d82f2b7733 100644
--- a/drivers/net/dsa/yt921x.h
+++ b/drivers/net/dsa/yt921x.h
@@ -6,6 +6,8 @@
 #ifndef __YT921X_H
 #define __YT921X_H
 
+#include <linux/u64_stats_sync.h>
+
 #include <net/dsa.h>
 
 #define YT921X_SMI_SWITCHID_M		GENMASK(3, 2)
@@ -475,55 +477,61 @@ enum yt921x_fdb_entry_status {
 #define yt921x_port_is_external(port) (8 <= (port) && (port) < 9)
 
 struct yt921x_mib {
-	u64 rx_broadcast;
-	u64 rx_pause;
-	u64 rx_multicast;
-	u64 rx_crc_errors;
-
-	u64 rx_alignment_errors;
-	u64 rx_undersize_errors;
-	u64 rx_fragment_errors;
-	u64 rx_64byte;
-
-	u64 rx_65_127byte;
-	u64 rx_128_255byte;
-	u64 rx_256_511byte;
-	u64 rx_512_1023byte;
-
-	u64 rx_1024_1518byte;
-	u64 rx_jumbo;
-	u64 rx_good_bytes;
-
-	u64 rx_bad_bytes;
-	u64 rx_oversize_errors;
-
-	u64 rx_dropped;
-	u64 tx_broadcast;
-	u64 tx_pause;
-	u64 tx_multicast;
-
-	u64 tx_undersize_errors;
-	u64 tx_64byte;
-	u64 tx_65_127byte;
-	u64 tx_128_255byte;
-
-	u64 tx_256_511byte;
-	u64 tx_512_1023byte;
-	u64 tx_1024_1518byte;
-	u64 tx_jumbo;
-
-	u64 tx_good_bytes;
-	u64 tx_collisions;
-
-	u64 tx_aborted_errors;
-	u64 tx_multiple_collisions;
-	u64 tx_single_collisions;
-	u64 tx_good;
-
-	u64 tx_deferred;
-	u64 tx_late_collisions;
-	u64 rx_oam;
-	u64 tx_oam;
+	union {
+		struct {
+			u64_stats_t rx_broadcast;
+			u64_stats_t rx_pause;
+			u64_stats_t rx_multicast;
+			u64_stats_t rx_crc_errors;
+
+			u64_stats_t rx_alignment_errors;
+			u64_stats_t rx_undersize_errors;
+			u64_stats_t rx_fragment_errors;
+			u64_stats_t rx_64byte;
+
+			u64_stats_t rx_65_127byte;
+			u64_stats_t rx_128_255byte;
+			u64_stats_t rx_256_511byte;
+			u64_stats_t rx_512_1023byte;
+
+			u64_stats_t rx_1024_1518byte;
+			u64_stats_t rx_jumbo;
+			u64_stats_t rx_good_bytes;
+
+			u64_stats_t rx_bad_bytes;
+			u64_stats_t rx_oversize_errors;
+
+			u64_stats_t rx_dropped;
+			u64_stats_t tx_broadcast;
+			u64_stats_t tx_pause;
+			u64_stats_t tx_multicast;
+
+			u64_stats_t tx_undersize_errors;
+			u64_stats_t tx_64byte;
+			u64_stats_t tx_65_127byte;
+			u64_stats_t tx_128_255byte;
+
+			u64_stats_t tx_256_511byte;
+			u64_stats_t tx_512_1023byte;
+			u64_stats_t tx_1024_1518byte;
+			u64_stats_t tx_jumbo;
+
+			u64_stats_t tx_good_bytes;
+			u64_stats_t tx_collisions;
+
+			u64_stats_t tx_aborted_errors;
+			u64_stats_t tx_multiple_collisions;
+			u64_stats_t tx_single_collisions;
+			u64_stats_t tx_good;
+
+			u64_stats_t tx_deferred;
+			u64_stats_t tx_late_collisions;
+			u64_stats_t rx_oam;
+			u64_stats_t tx_oam;
+		};
+
+		u64_stats_t stats[39];
+	};
 };
 
 struct yt921x_port {
@@ -533,9 +541,13 @@ struct yt921x_port {
 	bool isolated;
 
 	struct delayed_work mib_read;
+	struct u64_stats_sync syncp;
 	struct yt921x_mib mib;
-	u64 rx_frames;
-	u64 tx_frames;
+	u64_stats_t rx_frames;
+	u64_stats_t tx_frames;
+
+	/* only used by read routine to avoid huge allocations on the stack */
+	struct yt921x_mib mib_new;
 };
 
 struct yt921x_reg_ops {
-- 
2.51.0


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

* Re: [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read
  2026-01-18  1:30 ` [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read David Yang
@ 2026-01-18 16:06   ` Andrew Lunn
  2026-01-18 19:24     ` Yangfl
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2026-01-18 16:06 UTC (permalink / raw)
  To: David Yang
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

On Sun, Jan 18, 2026 at 09:30:15AM +0800, David Yang wrote:
> This patch does not change anything effectively, but serves as a
> prerequisite for another patch.
> 
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
>  drivers/net/dsa/yt921x.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
> index 5e4e8093ba16..fe08385445d2 100644
> --- a/drivers/net/dsa/yt921x.c
> +++ b/drivers/net/dsa/yt921x.c
> @@ -707,6 +707,12 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
>  		WRITE_ONCE(*valp, val);
>  	}
>  
> +	if (res) {
> +		dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
> +			port, res);
> +		return res;
> +	}

I know you are just moving code around, so i can understand a straight
cut/paste.

However, when i look at the code, what is the point of %s and the
constant "read stats for"?

	Andrew

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

* Re: [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read
  2026-01-18 16:06   ` Andrew Lunn
@ 2026-01-18 19:24     ` Yangfl
  2026-01-22  9:58       ` Paolo Abeni
  0 siblings, 1 reply; 10+ messages in thread
From: Yangfl @ 2026-01-18 19:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-kernel

On Mon, Jan 19, 2026 at 12:06 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sun, Jan 18, 2026 at 09:30:15AM +0800, David Yang wrote:
> > This patch does not change anything effectively, but serves as a
> > prerequisite for another patch.
> >
> > Signed-off-by: David Yang <mmyangfl@gmail.com>
> > ---
> >  drivers/net/dsa/yt921x.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
> > index 5e4e8093ba16..fe08385445d2 100644
> > --- a/drivers/net/dsa/yt921x.c
> > +++ b/drivers/net/dsa/yt921x.c
> > @@ -707,6 +707,12 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
> >               WRITE_ONCE(*valp, val);
> >       }
> >
> > +     if (res) {
> > +             dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
> > +                     port, res);
> > +             return res;
> > +     }
>
> I know you are just moving code around, so i can understand a straight
> cut/paste.
>
> However, when i look at the code, what is the point of %s and the
> constant "read stats for"?
>
>         Andrew

The error format is used many times across the file, so using the same
string helps reduce the data size a bit.

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

* Re: [PATCH net-next v6 1/3] net: dsa: yt921x: Fix MIB overflow wraparound routine
  2026-01-18  1:30 ` [PATCH net-next v6 1/3] " David Yang
@ 2026-01-22  9:41   ` Paolo Abeni
  2026-01-22 13:19     ` Yangfl
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Abeni @ 2026-01-22  9:41 UTC (permalink / raw)
  To: David Yang, netdev
  Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, linux-kernel, Dan Carpenter, David Laight

On 1/18/26 2:30 AM, David Yang wrote:
> Reported by the following Smatch static checker warning:
> 
>   drivers/net/dsa/yt921x.c:702 yt921x_read_mib()
>   warn: was expecting a 64 bit value instead of '(~0)'
> 
> Fixes: 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm YT921x")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/netdev/aPsjYKQMzpY0nSXm@stanley.mountain/
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
>  drivers/net/dsa/yt921x.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
> index 0b3df732c0d1..5e4e8093ba16 100644
> --- a/drivers/net/dsa/yt921x.c
> +++ b/drivers/net/dsa/yt921x.c
> @@ -682,21 +682,22 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
>  		const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
>  		u32 reg = YT921X_MIBn_DATA0(port) + desc->offset;
>  		u64 *valp = &((u64 *)mib)[i];
> -		u64 val = *valp;
>  		u32 val0;
> -		u32 val1;
> +		u64 val;
>  
>  		res = yt921x_reg_read(priv, reg, &val0);
>  		if (res)
>  			break;
>  
>  		if (desc->size <= 1) {
> -			if (val < (u32)val)
> -				/* overflow */
> -				val += (u64)U32_MAX + 1;
> -			val &= ~U32_MAX;
> -			val |= val0;
> +			u64 old_val = *valp;

Why targeting net-next here? the blamed commit is already in Linus'tree
and it looks like the above could causes functional issues, ad the wrong
value is written into the mib.

I think this should go via net.

/P


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

* Re: [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read
  2026-01-18 19:24     ` Yangfl
@ 2026-01-22  9:58       ` Paolo Abeni
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Abeni @ 2026-01-22  9:58 UTC (permalink / raw)
  To: Yangfl, Andrew Lunn
  Cc: netdev, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, linux-kernel

 1/18/26 8:24 PM, Yangfl wrote:
> On Mon, Jan 19, 2026 at 12:06 AM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>> On Sun, Jan 18, 2026 at 09:30:15AM +0800, David Yang wrote:
>>> This patch does not change anything effectively, but serves as a
>>> prerequisite for another patch.
>>>
>>> Signed-off-by: David Yang <mmyangfl@gmail.com>
>>> ---
>>>  drivers/net/dsa/yt921x.c | 11 +++++++----
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
>>> index 5e4e8093ba16..fe08385445d2 100644
>>> --- a/drivers/net/dsa/yt921x.c
>>> +++ b/drivers/net/dsa/yt921x.c
>>> @@ -707,6 +707,12 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
>>>               WRITE_ONCE(*valp, val);
>>>       }
>>>
>>> +     if (res) {
>>> +             dev_err(dev, "Failed to %s port %d: %i\n", "read stats for",
>>> +                     port, res);
>>> +             return res;
>>> +     }
>>
>> I know you are just moving code around, so i can understand a straight
>> cut/paste.
>>
>> However, when i look at the code, what is the point of %s and the
>> constant "read stats for"?
>>
>>         Andrew
> 
> The error format is used many times across the file, so using the same
> string helps reduce the data size a bit.

I think you could/should clean the implementation encapsulating the
dev_err in an helper tacking the variable string as an argument.

Since you have to repost, you could introduce a pre-req patch doing that.

Thanks,

Paolo


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

* Re: [PATCH net-next v6 1/3] net: dsa: yt921x: Fix MIB overflow wraparound routine
  2026-01-22  9:41   ` Paolo Abeni
@ 2026-01-22 13:19     ` Yangfl
  2026-01-22 15:21       ` Jakub Kicinski
  0 siblings, 1 reply; 10+ messages in thread
From: Yangfl @ 2026-01-22 13:19 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev, Andrew Lunn, Vladimir Oltean, David S. Miller,
	Eric Dumazet, Jakub Kicinski, linux-kernel, Dan Carpenter,
	David Laight

On Thu, Jan 22, 2026 at 5:41 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 1/18/26 2:30 AM, David Yang wrote:
> > Reported by the following Smatch static checker warning:
> >
> >   drivers/net/dsa/yt921x.c:702 yt921x_read_mib()
> >   warn: was expecting a 64 bit value instead of '(~0)'
> >
> > Fixes: 186623f4aa72 ("net: dsa: yt921x: Add support for Motorcomm YT921x")
> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Closes: https://lore.kernel.org/netdev/aPsjYKQMzpY0nSXm@stanley.mountain/
> > Suggested-by: David Laight <david.laight.linux@gmail.com>
> > Signed-off-by: David Yang <mmyangfl@gmail.com>
> > ---
> >  drivers/net/dsa/yt921x.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
> > index 0b3df732c0d1..5e4e8093ba16 100644
> > --- a/drivers/net/dsa/yt921x.c
> > +++ b/drivers/net/dsa/yt921x.c
> > @@ -682,21 +682,22 @@ static int yt921x_read_mib(struct yt921x_priv *priv, int port)
> >               const struct yt921x_mib_desc *desc = &yt921x_mib_descs[i];
> >               u32 reg = YT921X_MIBn_DATA0(port) + desc->offset;
> >               u64 *valp = &((u64 *)mib)[i];
> > -             u64 val = *valp;
> >               u32 val0;
> > -             u32 val1;
> > +             u64 val;
> >
> >               res = yt921x_reg_read(priv, reg, &val0);
> >               if (res)
> >                       break;
> >
> >               if (desc->size <= 1) {
> > -                     if (val < (u32)val)
> > -                             /* overflow */
> > -                             val += (u64)U32_MAX + 1;
> > -                     val &= ~U32_MAX;
> > -                     val |= val0;
> > +                     u64 old_val = *valp;
>
> Why targeting net-next here? the blamed commit is already in Linus'tree
> and it looks like the above could causes functional issues, ad the wrong
> value is written into the mib.
>
> I think this should go via net.
>
> /P
>

There are following patches which are not fully suitable for net tree,
and the issue does not block the main functionality, only gives wrong
statistics.

Should the whole series be sent to net, or split into two parts?

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

* Re: [PATCH net-next v6 1/3] net: dsa: yt921x: Fix MIB overflow wraparound routine
  2026-01-22 13:19     ` Yangfl
@ 2026-01-22 15:21       ` Jakub Kicinski
  0 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2026-01-22 15:21 UTC (permalink / raw)
  To: Yangfl
  Cc: Paolo Abeni, netdev, Andrew Lunn, Vladimir Oltean,
	David S. Miller, Eric Dumazet, linux-kernel, Dan Carpenter,
	David Laight

On Thu, 22 Jan 2026 21:19:29 +0800 Yangfl wrote:
> > Why targeting net-next here? the blamed commit is already in Linus'tree
> > and it looks like the above could causes functional issues, ad the wrong
> > value is written into the mib.
> >
> > I think this should go via net.
>
> There are following patches which are not fully suitable for net tree,
> and the issue does not block the main functionality, only gives wrong
> statistics.
> 
> Should the whole series be sent to net, or split into two parts?

Split into two parts please (and the second one posted after net is
merged back into net-next).

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

end of thread, other threads:[~2026-01-22 15:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-18  1:30 [PATCH net-next v6 0/3] net: dsa: yt921x: Fix MIB overflow wraparound routine David Yang
2026-01-18  1:30 ` [PATCH net-next v6 1/3] " David Yang
2026-01-22  9:41   ` Paolo Abeni
2026-01-22 13:19     ` Yangfl
2026-01-22 15:21       ` Jakub Kicinski
2026-01-18  1:30 ` [PATCH net-next v6 2/3] net: dsa: yt921x: Return early for failed MIB read David Yang
2026-01-18 16:06   ` Andrew Lunn
2026-01-18 19:24     ` Yangfl
2026-01-22  9:58       ` Paolo Abeni
2026-01-18  1:30 ` [PATCH net-next v6 3/3] net: dsa: yt921x: Use u64_stats_t for MIB stats David Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox