[PATCH net-next] net: dsa: offer a flowtable to the switch before the conduit

[PATCH net-next] net: dsa: offer a flowtable to the switch before the conduit

[Julius Bairaktaris]

A netfilter flowtable bound to a DSA user port reaches hardware through
dsa_user_setup_ft_block(), which forwards the block to the conduit
netdev. That route serves a flow engine that sits on the conduit, as
mtk_eth's does. TC_SETUP_FT is the only tc setup type handled this way:
every other type falls through to ds->ops->port_setup_tc, so a switch
that owns its flow engine is never offered the flowtable at all.

Reaching back from the conduit's ndo_setup_tc through conduit->dsa_ptr
is not a substitute: dsa_tree_teardown() tears down the conduit before
the ports, so a block unbound while the tree goes down can no longer be
resolved to the port it was bound on.

Offer TC_SETUP_FT to the switch first and keep the conduit forward as
the fallback. Every driver implementing .port_setup_tc returns
-EOPNOTSUPP for a setup type it does not handle, so a conduit-side flow
engine is reached exactly as before.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
The switch-side consumer is a DSA driver for the IPQ8074 PPE, maintained
in OpenWrt; a flowtable bound to its user ports is exercised there on
IPQ8074 hardware, against a live PPPoE uplink. The conduit path is
unchanged by inspection: every in-tree .port_setup_tc implementation
returns -EOPNOTSUPP for a setup type it does not handle, so mtk_eth
behind mt7530 keeps receiving the forward exactly as before.

 net/dsa/user.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef..c5adb557056f 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1734,11 +1734,22 @@ static int dsa_user_setup_tc(struct net_device *dev, enum tc_setup_type type,
 {
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	struct dsa_switch *ds = dp->ds;
+	int err;
 
 	switch (type) {
 	case TC_SETUP_BLOCK:
 		return dsa_user_setup_tc_block(dev, type_data);
 	case TC_SETUP_FT:
+		/* A switch that owns the flow tables answers for itself; only
+		 * a conduit-side flow engine needs the block forwarded, and
+		 * that forward is gone by the time the port is torn down.
+		 */
+		if (ds->ops->port_setup_tc) {
+			err = ds->ops->port_setup_tc(ds, dp->index, type,
+						     type_data);
+			if (err != -EOPNOTSUPP)
+				return err;
+		}
 		return dsa_user_setup_ft_block(ds, dp->index, type_data);
 	default:
 		break;

base-commit: 25c1f6111034aef7fc06cfbdcf1e4f0d6e5ee74b
-- 
2.53.0

Re: [net-next] net: dsa: offer a flowtable to the switch before the conduit

[netdev-bot+sashiko]

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

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

- [Medium] dsa_user_setup_tc() now selects the TC_SETUP_FT offload target
  (switch vs. conduit) independently on every call, by probing…
- [Low] The new comment at net/dsa/user.c:1743-1746 says '... only a
  conduit-side flow engine needs the block forwarded, and that forward…

--

net: dsa: offer a flowtable to the switch before the conduit

TC_SETUP_FT is now offered to the switch driver via
ds->ops->port_setup_tc() first, and only forwarded to the conduit netdev
via dsa_user_setup_ft_block() when the switch answers -EOPNOTSUPP. This
lets a switch that owns its own flow engine see a flowtable bound to one
of its user ports.

> diff --git a/net/dsa/user.c b/net/dsa/user.c
> index 041f9060c8ef0..c5adb557056f7 100644
> --- a/net/dsa/user.c
> +++ b/net/dsa/user.c
> @@ -1734,11 +1734,22 @@ static int dsa_user_setup_tc(struct net_device *dev, enum tc_setup_type type,
>  {
>  	struct dsa_port *dp = dsa_user_to_port(dev);
>  	struct dsa_switch *ds = dp->ds;
> +	int err;
>  
>  	switch (type) {
>  	case TC_SETUP_BLOCK:
>  		return dsa_user_setup_tc_block(dev, type_data);
>  	case TC_SETUP_FT:
> +		/* A switch that owns the flow tables answers for itself; only
> +		 * a conduit-side flow engine needs the block forwarded, and
> +		 * that forward is gone by the time the port is torn down.
> +		 */

[Severity: Low]
The comment says "that forward is gone by the time the port is torn
down", but the statement right below it runs that conduit forward on
every TC_SETUP_FT command, teardown unbind included. After -EOPNOTSUPP
from the switch, dsa_user_setup_ft_block() still resolves
dsa_port_to_conduit() and calls conduit->netdev_ops->ndo_setup_tc().
Nothing suppresses the forward during teardown.

The changelog's argument is about the other direction — a conduit-side
handler reaching back through conduit->dsa_ptr, which does not work
because dsa_tree_teardown() in net/dsa/dsa.c orders the teardown as:

	dsa_tree_teardown_conduit(dst);

	dsa_tree_teardown_ports(dst);

That ordering claim checks out, so this looks like just the comment
wording talking about a route the patch does not take. Could the comment
be reworded so it describes the fallback that is actually emitted here?

> +		if (ds->ops->port_setup_tc) {
> +			err = ds->ops->port_setup_tc(ds, dp->index, type,
> +						     type_data);
> +			if (err != -EOPNOTSUPP)
> +				return err;
> +		}
>  		return dsa_user_setup_ft_block(ds, dp->index, type_data);

[Severity: Medium]
TC_SETUP_FT carries a stateful FLOW_BLOCK_BIND / FLOW_BLOCK_UNBIND pair,
but this picks the offload target fresh on every call from the driver's
return code. Nothing records which side accepted the bind.

If a switch driver's TC_SETUP_FT verdict differs between the bind and the
matching unbind — for example its flow engine is unavailable at bind time
and available later, or offload is toggled at runtime — does the unbind
then land on the other target?

The target that did not see the bind has no matching block_cb, so its
flow_block_cb_lookup() fails and it returns -ENOENT. Since -ENOENT is not
-EOPNOTSUPP, dsa_user_setup_tc() propagates it (and the conduit's return
value is propagated unconditionally by dsa_user_setup_ft_block()).

On the netfilter side that error short-circuits the cleanup in
net/netfilter/nf_flow_table_offload.c:

nf_flow_table_offload_cmd() {
	...
	down_write(&flowtable->flow_block_lock);
	err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_FT, bo);
	up_write(&flowtable->flow_block_lock);
	if (err < 0)
		return err;
	...
}

nf_flow_table_offload_setup() {
	...
	if (err < 0)
		return err;

	return nf_flow_table_block_setup(flowtable, &bo, cmd);
}

nf_flow_table_block_setup() is the only place an unbind does list_del()
plus flow_block_cb_free(), so does a misrouted unbind leak the
flow_block_cb allocated at bind time, leaving it linked on the owner
driver's private block list with cb_priv pointing at flowtable state that
is going away?

Worth noting no in-tree .port_setup_tc handles TC_SETUP_FT today — they
all return -EOPNOTSUPP unconditionally — so this cannot trigger in tree
right now, and the behaviour of the out-of-tree consumer is unknown.
Would it make sense to decide the target only for FLOW_BLOCK_BIND and
remember it on the port, so the unbind always goes back to whoever
accepted the bind?

>  	default:
>  		break;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260901092546.369232-1-julius%40bairaktaris.de