netdev - Re: [PATCH net-next 1/2] net: flow_offload: mangle 128-bit packet field with one action

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] [day] [month] [year] [list]

Message-ID: <5f7f561d-36b5-2611-e051-4a1549e35f09@solarflare.com>
Date:   Wed, 21 Aug 2019 16:05:49 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>
CC:     <netfilter-devel@...r.kernel.org>, <davem@...emloft.net>,
        <netdev@...r.kernel.org>, <jakub.kicinski@...ronome.com>,
        <jiri@...nulli.us>, <vladbu@...lanox.com>
Subject: Re: [PATCH net-next 1/2] net: flow_offload: mangle 128-bit packet
 field with one action

On 20/08/2019 19:35, Pablo Neira Ayuso wrote:
> With one action that says "mangle an IPv6 at offset ip6 daddr field"
> the driver has more global view on what is going on, rather than
> having four actions to mangle four 32-bit words at some offset.
But the action doesn't say that, it still says "mangle four 32-bit
 words", it's just that they're now contiguous.  The driver doesn't
 know whether that's an IPv6 address or just a bunch of fields that
 happened to be next to one another.
(Besides, the driver can't rely on that 'global view', because if
 the actions did come from the TC uAPI, they're still going to be
 single u32 mangles.)

> If this patch adds some loops here is because I did not want to make
> too smart changes on the drivers.
The thing is, the drivers are already looping over TC actions, so they
 already naturally support multiple pedits.  You don't gain any
 expressiveness by combining them into batches of four, meanwhile you
 make the API less orthogonal and more laborious to implement.

> Please, allow for incremental updates on the flow_offload API to get
> it better now. Later we'll have way more drivers it will become harder
> to update this.
I'm not opposed to making the API better.  I just don't believe that
 this patch series achieves that.