[PATCH net-next 0/2] net: dsa: user: minor cleanups

[PATCH net-next 0/2] net: dsa: user: minor cleanups

[Kai Kuang]

Small, self-contained cleanups in the DSA user port code, found with
checkpatch.pl --strict and targeted at net-next:

Patch 1: use the kernel types (u32/u8/u64) in the ethtool_ops method
implementations, matching the prototypes in include/linux/ethtool.h
instead of the uint32_t/uint8_t/uint64_t typedefs.

Patch 2: document the plain wmb() in the LAG DSA conduit migration
path, which orders the dp->cpu_dp stores before lag_dev->dsa_ptr is
published to the concurrent RX path readers.

Both patches have no functional change.

Kai Kuang (2):
  net: dsa: user: use kernel types in ethtool callbacks
  net: dsa: user: document the write memory barrier on LAG conduit
    migration

 net/dsa/user.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

[PATCH net-next 1/2] net: dsa: user: use kernel types in ethtool callbacks

[Kai Kuang]

Use u32/u8/u64 in the ->get_strings() and ->get_ethtool_stats()
callbacks to match the ethtool_ops method prototypes.

No functional change.

Signed-off-by: Kai Kuang <kuangkai@kylinos.cn>
---
 net/dsa/user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef..cfad269be75c 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1023,7 +1023,7 @@ static int dsa_user_set_eeprom(struct net_device *dev,
 }
 
 static void dsa_user_get_strings(struct net_device *dev,
-				 uint32_t stringset, uint8_t *data)
+				 u32 stringset, u8 *data)
 {
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct dsa_switch *ds = dp->ds;
@@ -1043,7 +1043,7 @@ static void dsa_user_get_strings(struct net_device *dev,
 
 static void dsa_user_get_ethtool_stats(struct net_device *dev,
 				       struct ethtool_stats *stats,
-				       uint64_t *data)
+				       u64 *data)
 {
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct dsa_switch *ds = dp->ds;
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH net-next 1/2] net: dsa: user: use kernel types in ethtool callbacks

[Andrew Lunn]

On Wed, Oct 28, 2026 at 08:56:15PM +0800, Kai Kuang wrote:
> Use u32/u8/u64 in the ->get_strings() and ->get_ethtool_stats()
> callbacks to match the ethtool_ops method prototypes.
> 
> No functional change.

net-next is closed at the moment:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

The patch itself looks O.K, please repost when net-next re-opens.

    Andrew

[PATCH net-next 0/2] net: dsa: user: minor cleanups

[Kai Kuang]

Two small, self-contained cleanups in the DSA user port code, targeted
at net-next:

Patch 1: use u32/u8/u64 in the ->get_strings() and
->get_ethtool_stats() implementations to match the ethtool_ops method
prototypes in include/linux/ethtool.h.

Patch 2: document the plain wmb() in the LAG DSA conduit migration
path. The barrier orders the dp->cpu_dp stores before the new
lag_dev->dsa_ptr is published to the concurrent RX path readers.

Neither patch changes behavior. Reposting now that net-next has
re-opened, as previously requested during the merge window; the patches
are unchanged.

Kai Kuang (2):
  net: dsa: user: use kernel types in ethtool callbacks
  net: dsa: user: document the write memory barrier on LAG conduit
    migration

 net/dsa/user.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

[PATCH net-next 1/2] net: dsa: user: use kernel types in ethtool callbacks

[Kai Kuang]

Use u32/u8/u64 in the ->get_strings() and ->get_ethtool_stats()
callbacks to match the ethtool_ops method prototypes.

No functional change.

Signed-off-by: Kai Kuang <kuangkai@kylinos.cn>
---
 net/dsa/user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef..cfad269be75c 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1023,7 +1023,7 @@ static int dsa_user_set_eeprom(struct net_device *dev,
 }
 
 static void dsa_user_get_strings(struct net_device *dev,
-				 uint32_t stringset, uint8_t *data)
+				 u32 stringset, u8 *data)
 {
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct dsa_switch *ds = dp->ds;
@@ -1043,7 +1043,7 @@ static void dsa_user_get_strings(struct net_device *dev,
 
 static void dsa_user_get_ethtool_stats(struct net_device *dev,
 				       struct ethtool_stats *stats,
-				       uint64_t *data)
+				       u64 *data)
 {
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct dsa_switch *ds = dp->ds;
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

[PATCH net-next 2/2] net: dsa: user: document the write memory barrier on LAG conduit migration

[Kai Kuang]

The wmb() orders the dp->cpu_dp stores before the new lag_dev->dsa_ptr
is published to the concurrent RX path readers. Document this, since a
plain barrier without a comment is hard to interpret.

Signed-off-by: Kai Kuang <kuangkai@kylinos.cn>
---
 net/dsa/user.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index cfad269be75c..96393d4b5da1 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -3435,6 +3435,10 @@ static void dsa_conduit_lag_leave(struct net_device *conduit,
 		 * physical CPU port
 		 */
 		lag_dev->dsa_ptr = new_cpu_dp;
+		/* Ensure the new dp->cpu_dp values are visible to concurrent
+		 * RX path readers of lag_dev->dsa_ptr before the pointer
+		 * itself becomes visible
+		 */
 		wmb();
 	} else {
 		/* If the LAG DSA conduit has no ports left, migrate back all
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH net-next 2/2] net: dsa: user: document the write memory barrier on LAG conduit migration

[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 comment added in dsa_conduit_lag_leave()
  (net/dsa/user.c:3438-3441) and the commit message both say the wmb()…

--

net: dsa: user: document the write memory barrier on LAG conduit migration

This patch adds a comment above the wmb() in dsa_conduit_lag_leave(). The
comment says the barrier makes the new dp->cpu_dp values visible to RX
path readers of lag_dev->dsa_ptr before the new pointer itself is visible.

> diff --git a/net/dsa/user.c b/net/dsa/user.c
> index cfad269be75c8..96393d4b5da1b 100644
> --- a/net/dsa/user.c
> +++ b/net/dsa/user.c
> @@ -3435,6 +3435,10 @@ static void dsa_conduit_lag_leave(struct net_device *conduit,
>  		 * physical CPU port
>  		 */
>  		lag_dev->dsa_ptr = new_cpu_dp;
> +		/* Ensure the new dp->cpu_dp values are visible to concurrent
> +		 * RX path readers of lag_dev->dsa_ptr before the pointer
> +		 * itself becomes visible
> +		 */
>  		wmb();

[Severity: Medium]
Can the wmb() in this position give the ordering the new comment
describes?

In dsa_conduit_lag_leave(), the dp->cpu_dp stores and the store to
lag_dev->dsa_ptr all happen before the barrier:

net/dsa/user.c:dsa_conduit_lag_leave() {
    ...
		dsa_tree_for_each_user_port(dp, dst)
			if (dsa_port_to_conduit(dp) == lag_dev)
				dp->cpu_dp = new_cpu_dp;
    ...
		lag_dev->dsa_ptr = new_cpu_dp;
		wmb();
    ...
}

A write barrier orders the stores before it against the stores after it.
Here all of these stores come before it, so nothing orders the dp->cpu_dp
stores against the dsa_ptr store. On a weakly ordered CPU, the new
dsa_ptr can become visible before some of the dp->cpu_dp updates.

The publish pattern in dsa_conduit_setup() puts the barrier before the
pointer store:

net/dsa/conduit.c:dsa_conduit_setup() {
    ...
	wmb();

	dev->dsa_ptr = cpu_dp;
    ...
}

The placement in dsa_conduit_lag_leave() instead matches
dsa_conduit_teardown(). That function stores dev->dsa_ptr = NULL, then
calls wmb(), and its comment says only that packets "from this point"
take the new path.

The readers also use plain loads, with no matching read barrier or
acquire. For example, dsa_find_designated_bridge_port_by_vid() in
net/dsa/tag.h and dsa_tag_8021q_find_port_by_vbid() in
net/dsa/tag_8021q.c both do this:

	struct dsa_port *cpu_dp = conduit->dsa_ptr;
	...
		if (dp->cpu_dp != cpu_dp)
			continue;

Both run on the RX path, dsa_switch_rcv() -> tagger rcv with
dev == lag_dev. That path runs in softirq without rtnl, while
dsa_conduit_lag_leave() runs from the netdev notifier under rtnl.

Suppose a reader sees the new dsa_ptr but a stale dp->cpu_dp. Would it
skip the designated bridge port, return NULL, and drop the packet?

The opposite case, an old dsa_ptr with a new dp->cpu_dp, also produces a
mismatch. So no single barrier on the writer side can fully give what the
comment promises.

The barrier placement and the reader-side race date back to commit
acc43b7bf52a ("net: dsa: allow masters to join a LAG"). What this patch
adds is the comment, plus this statement in the commit message, which
claim an ordering the code does not give:

  "The wmb() orders the dp->cpu_dp stores before the new lag_dev->dsa_ptr
   is published to the concurrent RX path readers."

Should the barrier move between the dp->cpu_dp loop and the dsa_ptr
store, using smp_wmb() or smp_store_release() paired with READ_ONCE() or
an acquire on the readers? Or should the comment describe what the
barrier does order, which is all the earlier stores against the later
ones in dsa_conduit_lag_teardown()?

Also, this is ordering between CPUs, so smp_wmb() would be enough here
rather than the mandatory wmb().

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109120607.344544-2-kuangkai%40kylinos.cn

Re: [PATCH net-next 2/2] net: dsa: user: document the write memory barrier on LAG conduit migration

[Andrew Lunn]

> [Severity: Medium]
> Can the wmb() in this position give the ordering the new comment
> describes?

Please consider which is better: No comment, or a wrong comment.

If you are adding comments, you need to be 100% sure they are correct.
wmb() and the like are not simple. Do you really understand what is
going on here? If you do, please respond to the LLM indicating how it
is wrong.

   Andrew

[PATCH net-next 2/2] net: dsa: user: document the write memory barrier on LAG conduit migration

[Kai Kuang]

The wmb() orders the dp->cpu_dp stores before the new lag_dev->dsa_ptr
is published to the concurrent RX path readers. Document this, since a
plain barrier without a comment is hard to interpret.

Signed-off-by: Kai Kuang <kuangkai@kylinos.cn>
---
 net/dsa/user.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index cfad269be75c..96393d4b5da1 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -3435,6 +3435,10 @@ static void dsa_conduit_lag_leave(struct net_device *conduit,
 		 * physical CPU port
 		 */
 		lag_dev->dsa_ptr = new_cpu_dp;
+		/* Ensure the new dp->cpu_dp values are visible to concurrent
+		 * RX path readers of lag_dev->dsa_ptr before the pointer
+		 * itself becomes visible
+		 */
 		wmb();
 	} else {
 		/* If the LAG DSA conduit has no ports left, migrate back all
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus