net/dsa/user.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
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
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
© 2016 - 2026 Red Hat, Inc.