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: <20210929104615.6179efee@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 29 Sep 2021 10:46:15 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Lorenz Bauer <lmb@...udflare.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        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

On Wed, 29 Sep 2021 11:36:33 +0100 Lorenz Bauer wrote:
> 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?

Sure. We can leave the checking to the program then, but that ties
our hands for the implementation changes later on.

Not sure which pointer type will be chosen for the ret value but it 
may allow error checking at verification.

> How about this: instead of taking stack_ptr, take the return value
> from header_pointer as an argument. Then use the already existing
> (right ;P) inlining to do the following:
> 
>    if (md->ptr + args->off != ret_ptr)
>      __pointer_flush(...)

That only checks for the case where pointer is in the "head" frag,
and is not generally correct. You need to check the length of the 
first frag is smaller than off. Otherwise BPF stack may "happen"
to follow the head page and math will work out.

It would also be slower than Lorenzo's current code, which allows
access to tail pages without copying.