netdev - Re: [PATCH bpf-next 06/13] xsk: optimize for aligned case

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]

Date:   Wed, 29 Sep 2021 07:52:58 +0200
From:   Magnus Karlsson <magnus.karlsson@...il.com>
To:     Nathan Chancellor <nathan@...nel.org>
Cc:     "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Network Development <netdev@...r.kernel.org>,
        "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>,
        Ciara Loftus <ciara.loftus@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        bpf <bpf@...r.kernel.org>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        llvm@...ts.linux.dev
Subject: Re: [PATCH bpf-next 06/13] xsk: optimize for aligned case

On Wed, Sep 29, 2021 at 1:15 AM Nathan Chancellor <nathan@...nel.org> wrote:
>
> On Wed, Sep 22, 2021 at 09:56:06AM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@...el.com>
> >
> > Optimize for the aligned case by precomputing the parameter values of
> > the xdp_buff_xsk and xdp_buff structures in the heads array. We can do
> > this as the heads array size is equal to the number of chunks in the
> > umem for the aligned case. Then every entry in this array will reflect
> > a certain chunk/frame and can therefore be prepopulated with the
> > correct values and we can drop the use of the free_heads stack. Note
> > that it is not possible to allocate more buffers than what has been
> > allocated in the aligned case since each chunk can only contain a
> > single buffer.
> >
> > We can unfortunately not do this in the unaligned case as one chunk
> > might contain multiple buffers. In this case, we keep the old scheme
> > of populating a heads entry every time it is used and using
> > the free_heads stack.
> >
> > Also move xp_release() and xp_get_handle() to xsk_buff_pool.h. They
> > were for some reason in xsk.c even though they are buffer pool
> > operations.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
>
> My apologies if this has already been reported (I have not seen a report
> on netdev nor a report from Intel around it) but this patch as
> commit 94033cd8e73b ("xsk: Optimize for aligned case") in -next causes
> the following build failure with clang + x86_64 allmodconfig:
>
> net/xdp/xsk_buff_pool.c:465:15: error: variable 'xskb' is uninitialized when used here [-Werror,-Wuninitialized]
>                         xp_release(xskb);
>                                    ^~~~
> net/xdp/xsk_buff_pool.c:455:27: note: initialize the variable 'xskb' to silence this warning
>         struct xdp_buff_xsk *xskb;
>                                  ^
>                                   = NULL
> 1 error generated.

Thanks for reporting this Nathan. Will fix right away.

/Magnus

> Cheers,
> Nathan