Re: [PATCH net] net: flow_offload: protect driver_block_list in flow_block_cb_setup_simple()

[Florian Westphal]

Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> On Fri, 13 Feb 2026 12:30:58 +0100 Florian Westphal wrote:
> > > > Looking at the *upper layer*, I don't think it expected drivers to use
> > > > a single global list for this bit something that is scoped to the
> > > > net_device.  
> > > 
> > > Maybe subjective but the fix seems a little off to me.
> > > Isn't flow_block_cb_setup_simple() just a "simple" implementation 
> > > for reuse in drivers locking in there doesn't really guarantee much?  
> > 
> > Not sure what you mean.  I see the same pattern as netdevsim in all
> > drivers using this API. 
> 
> Grep for flow_block_cb_add(). Not all drivers use

static int
mtk_eth_setup_tc_block(struct net_device *dev, struct flow_block_offload *f)
{
        struct mtk_mac *mac = netdev_priv(dev);
        struct mtk_eth *eth = mac->hw;
        static LIST_HEAD(block_cb_list);
	~~~~~~
I have a question.

[..]
        f->driver_block_list = &block_cb_list;

Now I have many questions!

How is this supposed to work?
How is ownership handled, what locks protect, or what locks are supposed
to protect this?

> the flow_block_cb_setup_simple() helper, it's just a convenience helper,
> not a mandatory part of the flow. We should probably add a helper for
> add like the one added for  flow_block_cb_remove_driver() instead of
> taking the lock directly in flow_block_cb_setup_simple()?

No idea, I don't understand how any of this is supposed to work.

I will try to play with this next week but I'm not sure I will send
patches.  AFAICS there are not even netdevsim based tests for this
feature anywhere.