netdev - Re: [PATCH nf-next 7/8] netfilter: nf_tables: add flowtable map for xdp offload

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: <87il5re7su.fsf@toke.dk>
Date: Fri, 24 Nov 2023 11:59:13 +0100
From: Toke Høiland-Jørgensen <toke@...e.dk>
To: Florian Westphal <fw@...len.de>, netfilter-devel@...r.kernel.org
Cc: lorenzo@...nel.org, netdev@...r.kernel.org, Florian Westphal <fw@...len.de>
Subject: Re: [PATCH nf-next 7/8] netfilter: nf_tables: add flowtable map for
 xdp offload

Florian Westphal <fw@...len.de> writes:

> A device cannot be added to multiple flowtables, the mapping needs
> to be unique.  This is enforced when a flowtables with the
> NF_FLOWTABLE_XDP_OFFLOAD was added.
>
> Exposure of this NF_FLOWTABLE_XDP_OFFLOAD in UAPI could be avoided,
> iff the 'net_device maps to 0 or 1 flowtable' paradigm is enforced
> regardless of offload-or-not flag.
>
> HOWEVER, that does break existing behaviour.

I am not a huge fan of this flag, especially not as UAPI. Using the XDP
offload functionality is already an explicit opt-in by userspace (you
need to load the XDP program). So adding a second UAPI flag that you
have to set for the flowtable to be compatible with XDP seems to just
constrain things needlessly (and is bound to lead to bugs)?

If we can't change the behaviour, we could change the lookup mechanism?
BPF is pretty flexible, nothing says it has to use an ifindex as the
lookup key? The neatest thing would be to have some way for userspace to
directly populate a reference to the flowtable struct in a map, but a
simpler solution would be to just introduce an opaque ID for each
flowtable instance and use that as the lookup key (userspace could
trivially put that into a map for the BPF program to find)?

-Toke