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

[Jiri Pirko <jiri@resnulli.us>]

Wed, Jul 10, 2019 at 09:36:52AM CEST, jiri@resnulli.us wrote:
>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 &&

Looking at this a bit more, this is wrong. This breaks block sharing
concept. The original lookup look up the block_cb in certain block - the
block to be shared. With this, you broke the block sharing feature for
mlxsw.

We need to maintain the existing block concept (changed to flow_block).


>
>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);
>>+
>
>[...]