netdev - Re: [RFC] netfilter: nf_tables: ignore errors on flowtable device hw offload setup

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: <40ce1ec0-0527-7659-2da0-a9d643580f9e@nbd.name>
Date:   Mon, 30 May 2022 20:52:36 +0200
From:   Felix Fietkau <nbd@....name>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
        Jo-Philipp Wich <jo@...n.io>
Subject: Re: [RFC] netfilter: nf_tables: ignore errors on flowtable device hw
 offload setup


On 30.05.22 18:55, Pablo Neira Ayuso wrote:
> On Fri, May 20, 2022 at 11:48:01PM +0200, Pablo Neira Ayuso wrote:
>> On Fri, May 20, 2022 at 08:07:44PM +0200, Felix Fietkau wrote:
>> > 
>> > On 20.05.22 09:50, Pablo Neira Ayuso wrote:
>> > > I'm sssuming we relax the requirement as I proposed, ie. allow for not
>> > > allow devices to support for hardware offload, but at least one.
>> > > 
>> > > Then, it should be possible to extend the netlink interface to promote
>> > > a flowtable to support hardware offload, e.g.
>> > > 
>> > >   add flowtable inet x y { hook ingress devices = { eth0, eth1 } priority 0; flags offload; }
>> > > 
>> > > For an existing flowtable, that will add eth0 and eth1, and it will
>> > > request to turn hardware offload.
>> > > 
>> > > This is not supported, these bits are missing in the netlink interface.
>> > > 
>> > > > I still think the best course of action is to silently accept the offload
>> > > > flag even if none of the devices support hw offload.
>> > > 
>> > > Silent means user is asking for something that is actually not
>> > > supported, there will be no effective way from the control plane to
>> > > check if what they request is actually being applied.
>> > > 
>> > > I'd propose two changes:
>> > > 
>> > > - relax the existing requirement, so if one device support hw offload,
>> > >    then accept the configuration.
>> > > 
>> > > - allow to update a flowtable to on/off hardware offload from netlink
>> > >    interface without needing to reload your whole ruleset.
>> >
>> > I still don't see the value in forcing user space to do the
>> > failure-and-retry dance if none of the devices support hw offload.
>> > If this is about notifying user space about the hw offload status, I think
>> > it's much better to simply accept such configurations as-is and extend the
>> > netlink api to report which of the member devices hw offload was actually
>> > enabled for.
>> > This would be much more valuable to users that actually care about the hw
>> > offload status than knowing if one of the devices in the list has hw offload
>> > support, and it would simplify the code as well, for kernel and user space
>> > alike.
>> 
>> I would suggest to extend the API to expose if the device actually
>> support for the flowtable hardware offload, then after the listing,
>> the user knows if the feature is available, so they can turn it on.
> 
> Thinking it well, something in between your proposal and mine.
> 
> Allow to set on 'offload', then the kernel will disable this flag if
> no devices support for hardware offload. The update path would also
> need to allow for this new behaviour.
> 
> The user can check via 'nft list ruleset' if the flag is on / off.

I think that's reasonable. Let's implement it this way for now.

Thanks,

- Felix