Re: [PATCH net-next,v4 05/12] net: flow_offload: add list handling functions

[Jiri Pirko <jiri@resnulli.us>]

Tue, Jul 09, 2019 at 10:55:43PM CEST, pablo@netfilter.org wrote:

[...]


>@@ -176,6 +176,7 @@ struct flow_block_cb *flow_block_cb_alloc(struct net *net, tc_setup_cb_t *cb,
> 	if (!block_cb)
> 		return ERR_PTR(-ENOMEM);
> 
>+	block_cb->net = net;
> 	block_cb->cb = cb;
> 	block_cb->cb_ident = cb_ident;
> 	block_cb->cb_priv = cb_priv;
>@@ -194,6 +195,22 @@ void flow_block_cb_free(struct flow_block_cb *block_cb)
> }
> EXPORT_SYMBOL(flow_block_cb_free);
> 
>+struct flow_block_cb *flow_block_cb_lookup(struct flow_block_offload *f,
>+					   tc_setup_cb_t *cb, void *cb_ident)
>+{
>+	struct flow_block_cb *block_cb;
>+
>+	list_for_each_entry(block_cb, f->driver_block_list, driver_list) {
>+		if (block_cb->net == f->net &&

I don't understand why you need net for this. You should have a list of
cbs per subsystem (tc/nft) go over it here.

The clash of 2 suybsytems is prevented later on by
flow_block_cb_is_busy().

Am I missing something?
If not, could you please remove use of net from flow_block_cb_alloc()
and from here and replace it by some shared flow structure holding the
cb list that would be used by both tc and nft?



>+		    block_cb->cb == cb &&
>+		    block_cb->cb_ident == cb_ident)
>+			return block_cb;
>+	}
>+
>+	return NULL;
>+}
>+EXPORT_SYMBOL(flow_block_cb_lookup);
>+

[...]