netdev - Re: [PATCH v14 bpf-next 00/18] mvneta: introduce XDP multi-buffer support

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: <87sfxnin6i.fsf@toke.dk>
Date:   Wed, 29 Sep 2021 14:25:09 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Lorenz Bauer <lmb@...udflare.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        bpf <bpf@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, shayagr@...zon.com,
        David Ahern <dsahern@...nel.org>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Eelco Chaudron <echaudro@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Saeed Mahameed <saeed@...nel.org>,
        "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        tirthendu.sarkar@...el.com
Subject: Re: [PATCH v14 bpf-next 00/18] mvneta: introduce XDP multi-buffer
 support

Lorenz Bauer <lmb@...udflare.com> writes:

> On Mon, 20 Sept 2021 at 23:46, Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>> > The draft API was:
>> >
>> > void *xdp_mb_pointer_flush(struct xdp_buff *xdp_md, u32 flags,
>> >                            u32 offset, u32 len, void *stack_buf)
>> >
>> > Which does not take the ptr returned by header_pointer(), but that's
>> > easy to add (well, easy other than the fact it'd be the 6th arg).
>>
>> I guess we could play some trickery with stuffing offset/len/flags into
>> one or two u64s to save an argument or two?
>
> Adding another pointer arg seems really hard to explain as an API.
> What happens if I pass the "wrong" ptr? What happens if I pass NULL?
>
> How about this: instead of taking stack_ptr, take the return value
> from header_pointer as an argument.

Hmm, that's a good point; I do think that passing the return value from
header pointer is more natural as well (you're flushing pointer you just
wrote to, after all).

> Then use the already existing (right ;P) inlining to do the following:
>
>    if (md->ptr + args->off != ret_ptr)
>      __pointer_flush(...)

The inlining is orthogonal, though, right? The helper can do this check
whether or not it's a proper CALL or not (although obviously for
performance reasons we do want it to inline, at least eventually). In
particular, I believe we can make progress on this patch series without
working out the inlining :)

> This means that __pointer_flush has to deal with aliased memory
> though, so it would always have to memmove. Probably OK for the "slow"
> path?

Erm, not sure what you mean here? Yeah, flushing is going to take longer
if you ended up using the stack pointer instead of writing directly to
the packet. That's kinda intrinsic? Or am I misunderstanding you?

-Toke