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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190710090628.GF2282@nanopsycho>
Date:   Wed, 10 Jul 2019 11:06:28 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        thomas.lendacky@....com, f.fainelli@...il.com,
        ariel.elior@...ium.com, michael.chan@...adcom.com,
        madalin.bucur@....com, yisen.zhuang@...wei.com,
        salil.mehta@...wei.com, jeffrey.t.kirsher@...el.com,
        tariqt@...lanox.com, saeedm@...lanox.com, jiri@...lanox.com,
        idosch@...lanox.com, jakub.kicinski@...ronome.com,
        peppe.cavallaro@...com, grygorii.strashko@...com, andrew@...n.ch,
        vivien.didelot@...il.com, alexandre.torgue@...com,
        joabreu@...opsys.com, linux-net-drivers@...arflare.com,
        ogerlitz@...lanox.com, Manish.Chopra@...ium.com,
        marcelo.leitner@...il.com, mkubecek@...e.cz,
        venkatkumar.duvvuru@...adcom.com, maxime.chevallier@...tlin.com,
        cphealy@...il.com, phil@....cc, netfilter-devel@...r.kernel.org
Subject: Re: [PATCH net-next,v4 05/12] net: flow_offload: add list handling
 functions

Wed, Jul 10, 2019 at 09:36:52AM CEST, jiri@...nulli.us wrote:
>Tue, Jul 09, 2019 at 10:55:43PM CEST, pablo@...filter.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);
>>+
>
>[...]