netdev - Re: [PATCH RFC bpf-next v2 10/17] bnxt: Propagate trait presence to skb

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: <87cyd07jhf.fsf@toke.dk>
Date: Fri, 25 Apr 2025 10:06:52 +0200
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Jakub Sitnicki <jakub@...udflare.com>, Stanislav Fomichev
 <stfomichev@...il.com>
Cc: Arthur Fabre <arthur@...hurfabre.com>, netdev@...r.kernel.org,
 bpf@...r.kernel.org, hawk@...nel.org, yan@...udflare.com,
 jbrandeburg@...udflare.com, lbiancon@...hat.com, ast@...nel.org,
 kuba@...nel.org, edumazet@...gle.com, kernel-team@...udflare.com
Subject: Re: [PATCH RFC bpf-next v2 10/17] bnxt: Propagate trait presence to
 skb

Jakub Sitnicki <jakub@...udflare.com> writes:

> On Thu, Apr 24, 2025 at 08:39 AM -07, Stanislav Fomichev wrote:
>> On 04/24, Toke Høiland-Jørgensen wrote:
>
> [...]
>
>>> Being able to change the placement (and format) of the data store is the
>>> reason why we should absolutely *not* expose the internal trait storage
>>> to AF_XDP. Once we do that, we effectively make the whole thing UAPI.
>>> The trait get/set API very deliberately does not expose any details
>>> about the underlying storage for exactly this reason :)
>>
>> I was under the impression that we want to eventually expose trait
>> blobs to the userspace via getsockopt (or some other similar means),
>> is it not the case? How is userspace supposed to consume it?
>
> Yes, we definitely want to consume and produce traits from userspace.
>
> Before last Netdev [1], our plan was for the socket glue layer to
> convert between the in-kernel format and a TLV-based stable format for
> uAPI.
>
> But then Alexei suggested something that did not occur to us. The traits
> format can be something that BPF and userspace agree on. The kernel just
> passes it back and forth without needing to understand the content. This
> naturally simplifies changes in the socket glue layer.
>
> As Eric pointed out, this is similar to exposing raw TCP SYN headers via
> getsockopt(TCP_SAVED_SYN). BPF can set a custom TCP option, and
> userspace can consume it.
>
> The trade-off is that then the traits can only be used in parts of the
> network stack where a BPF hook exist.

It also means that we can't change the format later because it becomes
an API consumed by userspace. Regardless of whether we deem it "UAPI"
and not, it is bound to ossify. TCP headers are actually an excellent
example of this; they are ostensibly modifiable, but it's all but
impossible to do so in practice, because implementations make
assumptions on the expected format and break if it changes.

Also, making the format "agreed upon between BPF and userspace", means
that the kernel won't be able to use the data stored in traits itself
(since it does not know the format). We do want fields stored in traits
to be consumable by the kernel as well, so for this reason it is not a
good idea to leave it up to BPF to define the format either.

> Is that an acceptable limitation? That's what we're hoping to hear your
> thoughts on.

I absolutely don't think this is acceptable, see above.

> One concern that comes to mind, since the network stack is unaware of
> traits contents, we will be limited to simple merge strategies (like
> "drop all" or "keep first") in the GRO layer.

Yeah, this is another limitation of the kernel not understanding the
format, but not the only one.

-Toke