Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit
@ 2026-09-04  9:24 Julius Bairaktaris
  0 siblings, 0 replies; 5+ messages in thread
From: Julius Bairaktaris @ 2026-09-04  9:24 UTC (permalink / raw)
  To: Andrew Lunn, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, linux-kernel, Pablo Neira Ayuso

A flowtable bound to a DSA user port is forwarded to the conduit netdev
by dsa_user_setup_ft_block(), for a flow engine that sits on the
conduit as mtk_eth's does. It does not go through
ds->ops->port_setup_tc, so a switch that owns its flow engine is never
offered the flowtable.

Offer TC_SETUP_FT to the switch first and forward it to the conduit
only when the switch answers -EOPNOTSUPP. The side that takes the bind
is recorded on the port so that the unbind goes to the same side. Every
in-tree .port_setup_tc returns -EOPNOTSUPP for TC_SETUP_FT, so a
conduit-side flow engine is reached as before.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
v2:
- decide the target on FLOW_BLOCK_BIND and record it on the port, so
  the unbind goes to the side that holds the block callback
- reword the comment
v1: https://lore.kernel.org/netdev/20260901092546.369232-1-julius@bairaktaris.de/

Tested on IPQ8074 with an out-of-tree DSA switch driver that handles
TC_SETUP_FT (openwrt/openwrt#24806): three flowtable unbind/bind cycles,
hardware offload of routed flows after each.

 include/net/dsa.h |  6 ++++++
 net/dsa/user.c    | 25 ++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7507d632e7c6..dbcb02de06fc 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -297,6 +297,12 @@ struct dsa_port {
 
 	u8			setup:1;
 
+	/* Flowtable blocks on this user port go to the switch, not the
+	 * conduit; decided at bind time. Written without rtnl, so not one of
+	 * the bit fields above.
+	 */
+	bool			ft_on_switch;
+
 	struct device_node	*dn;
 	unsigned int		ageing_time;
 
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef..f4cd48df2519 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1718,15 +1718,30 @@ static int dsa_user_setup_tc_block(struct net_device *dev,
 	}
 }
 
-static int dsa_user_setup_ft_block(struct dsa_switch *ds, int port,
-				   void *type_data)
+static int dsa_user_setup_ft_block(struct dsa_port *dp,
+				   struct flow_block_offload *bo)
 {
-	struct net_device *conduit = dsa_port_to_conduit(dsa_to_port(ds, port));
+	struct net_device *conduit = dsa_port_to_conduit(dp);
+	struct dsa_switch *ds = dp->ds;
+	int err;
+
+	/* The unbind goes to the side that took the bind. */
+	if (bo->command == FLOW_BLOCK_BIND) {
+		err = -EOPNOTSUPP;
+		if (ds->ops->port_setup_tc)
+			err = ds->ops->port_setup_tc(ds, dp->index, TC_SETUP_FT,
+						     bo);
+		dp->ft_on_switch = err != -EOPNOTSUPP;
+		if (dp->ft_on_switch)
+			return err;
+	} else if (dp->ft_on_switch) {
+		return ds->ops->port_setup_tc(ds, dp->index, TC_SETUP_FT, bo);
+	}
 
 	if (!conduit->netdev_ops->ndo_setup_tc)
 		return -EOPNOTSUPP;
 
-	return conduit->netdev_ops->ndo_setup_tc(conduit, TC_SETUP_FT, type_data);
+	return conduit->netdev_ops->ndo_setup_tc(conduit, TC_SETUP_FT, bo);
 }
 
 static int dsa_user_setup_tc(struct net_device *dev, enum tc_setup_type type,
@@ -1739,7 +1754,7 @@ static int dsa_user_setup_tc(struct net_device *dev, enum tc_setup_type type,
 	case TC_SETUP_BLOCK:
 		return dsa_user_setup_tc_block(dev, type_data);
 	case TC_SETUP_FT:
-		return dsa_user_setup_ft_block(ds, dp->index, type_data);
+		return dsa_user_setup_ft_block(dp, type_data);
 	default:
 		break;
 	}
-- 
2.53.0


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

* [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit
@ 2026-09-04  9:27 Julius Bairaktaris
  2026-09-05  1:23 ` Jakub Kicinski
  2026-09-08 21:29 ` netdev-bot+sashiko
  0 siblings, 2 replies; 5+ messages in thread
From: Julius Bairaktaris @ 2026-09-04  9:27 UTC (permalink / raw)
  To: Andrew Lunn, Vladimir Oltean
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, linux-kernel, Pablo Neira Ayuso

A flowtable bound to a DSA user port is forwarded to the conduit netdev
by dsa_user_setup_ft_block(), for a flow engine that sits on the
conduit as mtk_eth's does. It does not go through
ds->ops->port_setup_tc, so a switch that owns its flow engine is never
offered the flowtable.

Offer TC_SETUP_FT to the switch first and forward it to the conduit
only when the switch answers -EOPNOTSUPP. The side that takes the bind
is recorded on the port so that the unbind goes to the same side. Every
in-tree .port_setup_tc returns -EOPNOTSUPP for TC_SETUP_FT, so a
conduit-side flow engine is reached as before.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Julius Bairaktaris <julius@bairaktaris.de>
---
v2:
- decide the target on FLOW_BLOCK_BIND and record it on the port, so
  the unbind goes to the side that holds the block callback
- reword the comment
v1: https://lore.kernel.org/netdev/20260901092546.369232-1-julius@bairaktaris.de/

Tested on IPQ8074 with an out-of-tree DSA switch driver that handles
TC_SETUP_FT (openwrt/openwrt#24806): three flowtable unbind/bind cycles,
hardware offload of routed flows after each.

 include/net/dsa.h |  6 ++++++
 net/dsa/user.c    | 25 ++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7507d632e7c6..dbcb02de06fc 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -297,6 +297,12 @@ struct dsa_port {
 
 	u8			setup:1;
 
+	/* Flowtable blocks on this user port go to the switch, not the
+	 * conduit; decided at bind time. Written without rtnl, so not one of
+	 * the bit fields above.
+	 */
+	bool			ft_on_switch;
+
 	struct device_node	*dn;
 	unsigned int		ageing_time;
 
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 041f9060c8ef..f4cd48df2519 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1718,15 +1718,30 @@ static int dsa_user_setup_tc_block(struct net_device *dev,
 	}
 }
 
-static int dsa_user_setup_ft_block(struct dsa_switch *ds, int port,
-				   void *type_data)
+static int dsa_user_setup_ft_block(struct dsa_port *dp,
+				   struct flow_block_offload *bo)
 {
-	struct net_device *conduit = dsa_port_to_conduit(dsa_to_port(ds, port));
+	struct net_device *conduit = dsa_port_to_conduit(dp);
+	struct dsa_switch *ds = dp->ds;
+	int err;
+
+	/* The unbind goes to the side that took the bind. */
+	if (bo->command == FLOW_BLOCK_BIND) {
+		err = -EOPNOTSUPP;
+		if (ds->ops->port_setup_tc)
+			err = ds->ops->port_setup_tc(ds, dp->index, TC_SETUP_FT,
+						     bo);
+		dp->ft_on_switch = err != -EOPNOTSUPP;
+		if (dp->ft_on_switch)
+			return err;
+	} else if (dp->ft_on_switch) {
+		return ds->ops->port_setup_tc(ds, dp->index, TC_SETUP_FT, bo);
+	}
 
 	if (!conduit->netdev_ops->ndo_setup_tc)
 		return -EOPNOTSUPP;
 
-	return conduit->netdev_ops->ndo_setup_tc(conduit, TC_SETUP_FT, type_data);
+	return conduit->netdev_ops->ndo_setup_tc(conduit, TC_SETUP_FT, bo);
 }
 
 static int dsa_user_setup_tc(struct net_device *dev, enum tc_setup_type type,
@@ -1739,7 +1754,7 @@ static int dsa_user_setup_tc(struct net_device *dev, enum tc_setup_type type,
 	case TC_SETUP_BLOCK:
 		return dsa_user_setup_tc_block(dev, type_data);
 	case TC_SETUP_FT:
-		return dsa_user_setup_ft_block(ds, dp->index, type_data);
+		return dsa_user_setup_ft_block(dp, type_data);
 	default:
 		break;
 	}
-- 
2.53.0


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

* Re: [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit
  2026-09-04  9:27 [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit Julius Bairaktaris
@ 2026-09-05  1:23 ` Jakub Kicinski
  2026-09-08 15:24   ` Julius Bairaktaris
  2026-09-08 21:29 ` netdev-bot+sashiko
  1 sibling, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2026-09-05  1:23 UTC (permalink / raw)
  To: Julius Bairaktaris
  Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Paolo Abeni, Simon Horman, netdev, linux-kernel,
	Pablo Neira Ayuso

On Fri,  4 Sep 2026 11:27:18 +0200 Julius Bairaktaris wrote:
> Tested on IPQ8074 with an out-of-tree DSA switch driver that handles
> TC_SETUP_FT (openwrt/openwrt#24806): three flowtable unbind/bind cycles,
> hardware offload of routed flows after each.

We don't accept patches without an in-tree user.

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

* Re: [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit
  2026-09-05  1:23 ` Jakub Kicinski
@ 2026-09-08 15:24   ` Julius Bairaktaris
  0 siblings, 0 replies; 5+ messages in thread
From: Julius Bairaktaris @ 2026-09-08 15:24 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Paolo Abeni, Simon Horman, netdev, linux-kernel,
	Pablo Neira Ayuso

Understood. Thanks!

Julius

Am Sa., 5. Sept. 2026 um 01:23 Uhr schrieb Jakub Kicinski <kuba@kernel.org>:
>
> On Fri,  4 Sep 2026 11:27:18 +0200 Julius Bairaktaris wrote:
> > Tested on IPQ8074 with an out-of-tree DSA switch driver that handles
> > TC_SETUP_FT (openwrt/openwrt#24806): three flowtable unbind/bind cycles,
> > hardware offload of routed flows after each.
>
> We don't accept patches without an in-tree user.

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

* Re: [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit
  2026-09-04  9:27 [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit Julius Bairaktaris
  2026-09-05  1:23 ` Jakub Kicinski
@ 2026-09-08 21:29 ` netdev-bot+sashiko
  1 sibling, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-08 21:29 UTC (permalink / raw)
  To: julius
  Cc: andrew, olteanv, davem, edumazet, kuba, pabeni, horms, netdev,
	linux-kernel, pablo

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

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

- [High] dsa_user_setup_ft_block() (net/dsa/user.c:1721) records which
  side accepted a flowtable bind in a single per-port boolean,…
- [Medium] In dsa_user_setup_ft_block() (net/dsa/user.c:1734) the
  ownership record is computed as `dp->ft_on_switch = err !=…

--

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

TC_SETUP_FT on a DSA user port is now offered to the switch driver via
ds->ops->port_setup_tc first, and only forwarded to the conduit netdev when
the switch answers -EOPNOTSUPP.  The side that accepted the bind is recorded
in a new per-port boolean, dp->ft_on_switch, so that the matching unbind is
sent to the same side.

> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 7507d632e7c6d..dbcb02de06fce 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -297,6 +297,12 @@ struct dsa_port {
>  
>  	u8			setup:1;
>  
> +	/* Flowtable blocks on this user port go to the switch, not the
> +	 * conduit; decided at bind time. Written without rtnl, so not one of
> +	 * the bit fields above.
> +	 */
> +	bool			ft_on_switch;
> +

[Severity: High]
Can a single boolean per dsa_port track something the flow block API keys
per block?

nf_flow_table_block_offload_init() sets bo->block = &flowtable->flow_block,
so each flowtable is a distinct binding.  And more than one flowtable can
bind the same DSA user netdev, because the duplicate device check in
nft_register_flowtable_net_hooks() only looks at flowtables of the same nft
table:

net/netfilter/nf_tables_api.c:nft_register_flowtable_net_hooks() {
	list_for_each_entry(ft, &table->flowtables, list) {
		if (!nft_is_active_next(net, ft))
			continue;
		if (nft_hook_list_find(&ft->hook_list, hook)) {
			err = -EEXIST;
			...
}

Two flowtables in two different nft tables therefore produce two independent
FLOW_BLOCK_BINDs on the same port, and both write the one flag.  Compare
with dsa_user_setup_tc_block(), which keys the block on f->block via
flow_block_cb_lookup().

>  	struct device_node	*dn;
>  	unsigned int		ageing_time;
>  
> diff --git a/net/dsa/user.c b/net/dsa/user.c
> index 041f9060c8ef0..f4cd48df2519d 100644
> --- a/net/dsa/user.c
> +++ b/net/dsa/user.c
> @@ -1718,15 +1718,30 @@ static int dsa_user_setup_tc_block(struct net_device *dev,
>  	}
>  }
>  
> -static int dsa_user_setup_ft_block(struct dsa_switch *ds, int port,
> -				   void *type_data)
> +static int dsa_user_setup_ft_block(struct dsa_port *dp,
> +				   struct flow_block_offload *bo)
>  {
> -	struct net_device *conduit = dsa_port_to_conduit(dsa_to_port(ds, port));
> +	struct net_device *conduit = dsa_port_to_conduit(dp);
> +	struct dsa_switch *ds = dp->ds;
> +	int err;
> +
> +	/* The unbind goes to the side that took the bind. */
> +	if (bo->command == FLOW_BLOCK_BIND) {
> +		err = -EOPNOTSUPP;
> +		if (ds->ops->port_setup_tc)
> +			err = ds->ops->port_setup_tc(ds, dp->index, TC_SETUP_FT,
> +						     bo);
> +		dp->ft_on_switch = err != -EOPNOTSUPP;
> +		if (dp->ft_on_switch)
> +			return err;
> +	} else if (dp->ft_on_switch) {
> +		return ds->ops->port_setup_tc(ds, dp->index, TC_SETUP_FT, bo);
> +	}

[Severity: High]
Following on from the per-block question above: since each
FLOW_BLOCK_BIND overwrites dp->ft_on_switch, and the FLOW_BLOCK_UNBIND
routing decision reads only that flag and never looks at bo->block, can an
unbind reach the side that never took the bind?

	BIND(FT_A)   switch returns 0            -> ft_on_switch = true
	BIND(FT_B)   switch has no room for a
	             second block, -EOPNOTSUPP   -> ft_on_switch = false,
	                                            conduit takes FT_B
	UNBIND(FT_A) reads false                 -> goes to the conduit

The conduit provider looks its block up by bo->block, so it simply fails:

drivers/net/ethernet/mediatek/mtk_ppe_offload.c:mtk_eth_setup_tc_block() {
	...
	block_cb = flow_block_cb_lookup(f->block, cb, dev);
	if (!block_cb)
		return -ENOENT;
	...
}

nft_unregister_flowtable_ops() discards that error, and
nf_flow_table_offload_setup() returns before reaching the freeing loop in
nf_flow_table_block_setup():

net/netfilter/nf_flow_table_offload.c:nf_flow_table_block_setup() {
	...
	case FLOW_BLOCK_UNBIND:
		list_for_each_entry_safe(block_cb, next, &bo->cb_list, list) {
			list_del(&block_cb->list);
			flow_block_cb_free(block_cb);
	...
}

nf_flow_table_free() has no leftover-cb cleanup either.  Does the switch's
flow_block_cb then stay linked on the driver's block list with ->block
pointing into the freed nf_flowtable, with the hardware offload entries
never torn down, so that a later flow_block_cb_lookup() can match a stale
cb if a new flowtable is allocated at the same address?  The mirrored
ordering (switch answers -EOPNOTSUPP for FT_A, accepts FT_B) sends a
conduit-owned unbind to the switch instead.

Would keying the ownership on bo->block, the way dsa_user_setup_tc_block()
does, avoid this?

[Severity: Medium]
Separately, should ft_on_switch be set for a bind that failed?

	dp->ft_on_switch = err != -EOPNOTSUPP;
	if (dp->ft_on_switch)
		return err;

Any hard error from ds->ops->port_setup_tc() (-ENOMEM, -EBUSY, -ENOSPC)
also satisfies err != -EOPNOTSUPP, so the port is latched as "the switch
owns the flowtable block" although nothing was bound anywhere -- the switch
failed, and the conduit was never offered the bind because of the early
return.  Nothing clears the flag afterwards; it is only recomputed on the
next FLOW_BLOCK_BIND, never on unbind.

An unbind for a different block that the conduit genuinely holds on the
same port then takes the else-if branch and is delivered to the switch:

	BIND(FT_A)   switch -EOPNOTSUPP     -> conduit takes FT_A
	BIND(FT_B)   switch -ENOMEM         -> ft_on_switch = true, err
	                                       returned, nothing bound
	UNBIND(FT_A) reads true             -> goes to the switch

The conduit's flow_block_cb for FT_A is then never removed and keeps a
->block pointer into the freed flowtable.  Should the flag only be set when
the switch actually accepted the bind, that is err == 0?

>  
>  	if (!conduit->netdev_ops->ndo_setup_tc)
>  		return -EOPNOTSUPP;
>  
> -	return conduit->netdev_ops->ndo_setup_tc(conduit, TC_SETUP_FT, type_data);
> +	return conduit->netdev_ops->ndo_setup_tc(conduit, TC_SETUP_FT, bo);
>  }
>  

[ ... ]

For both of the above: no in-tree .port_setup_tc handles TC_SETUP_FT today
(hellcreek, ksz, yt921x, mt753x, felix/vsc9959 and sja1105 all return
-EOPNOTSUPP), so the switch branch is not reachable in mainline yet, but
this patch is what enables such drivers.

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

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

end of thread, other threads:[~2026-09-08 21:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-04  9:27 [PATCH net-next v2] net: dsa: offer a flowtable to the switch before the conduit Julius Bairaktaris
2026-09-05  1:23 ` Jakub Kicinski
2026-09-08 15:24   ` Julius Bairaktaris
2026-09-08 21:29 ` netdev-bot+sashiko
  -- strict thread matches above, loose matches on Subject: below --
2026-09-04  9:24 Julius Bairaktaris

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