netdev - Re: [PATCH bpf-next 1/3] libbpf: xsk: use bpf

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:   Tue, 16 Feb 2021 21:15:02 +0100
From:   Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Björn Töpel <bjorn.topel@...el.com>,
        daniel@...earbox.net, ast@...nel.org, bpf@...r.kernel.org,
        netdev@...r.kernel.org, andrii@...nel.org,
        magnus.karlsson@...el.com, ciara.loftus@...el.com
Subject: Re: [PATCH bpf-next 1/3] libbpf: xsk: use bpf_link

On Tue, Feb 16, 2021 at 11:27:55AM +0100, Toke Høiland-Jørgensen wrote:
> Maciej Fijalkowski <maciej.fijalkowski@...el.com> writes:
> >
> > Am I reading this right or you're trying to reject the fix of the long
> > standing issue due to a PR that is not ready yet on a standalone
> > project/library? :P
> 
> Haha, no, that is not what I'm saying. As I said up-thread I agree that
> this is something we should fix, obviously. I'm just suggesting we fix
> it in a way that will also be compatible with libxdp and multiprog so we
> won't have to introduce yet-another-flag that users will have to decide
> on.
> 
> However, now that I'm looking at your patch again I think my fears may
> have been overblown. I got hung up on the bit in the commit message
> where you said "refcounting of BPF resources will be done automatically
> by bpf_link itself", and wrongly assumed that you were exposing the
> bpf_link fd to the application. But I see now that it's kept in the
> private xsk_ctx structure, and applications still just call
> xsk_socket__delete(). So in libxdp we can just implement the same API
> with a different synchronisation mechanism; that's fine. Apologies for
> jumping to conclusions :/

No worries, this shows how important a thorough commit message is, seems
that I failed on that part.

> 
> > Once libxdp is the standard way of playing with AF-XDP and there are
> > actual users of that, then I'm happy to work/help on that issue.
> 
> That is good to know, thanks! I opened an issue in the xdp-tools
> repository to track this for the libxdp side (Magnus and I agreed that
> we'll merge the existing code first, then fix this on top):
> https://github.com/xdp-project/xdp-tools/issues/93

Thanks! And good to hear that there's some sort of agreement.

> 
> As noted above, the mechanism may have to change, but I think it's
> possible to achieve the same thing in a libxdp context :)
> 
> -Toke
>