netdev - Re: [PATCH bpf-next 2/7] bpf, xdp: add 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87k0z3enpr.fsf@toke.dk>
Date:   Thu, 16 Jul 2020 12:52:32 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>,
        David Ahern <dsahern@...il.com>,
        Jakub Kicinski <kicinski@...com>, Andrey Ignatov <rdna@...com>,
        Takshak Chahande <ctakshak@...com>
Subject: Re: [PATCH bpf-next 2/7] bpf, xdp: add bpf_link-based XDP attachment API

Andrii Nakryiko <andrii.nakryiko@...il.com> writes:

> On Wed, Jul 15, 2020 at 8:48 AM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>>
>> Andrii Nakryiko <andrii.nakryiko@...il.com> writes:
>>
>> >> Yup, that was helpful, thanks! I think our difference of opinion on this
>> >> stems from the same place as our disagreement about point 2.: You are
>> >> assuming that an application that uses XDP sticks around and holds on to
>> >> its bpf_link, while I'm assuming it doesn't (and so has to rely on
>> >> pinning for any persistence). In the latter case you don't really gain
>> >> much from the bpf_link auto-cleanup, and whether it's a prog fd or a
>> >> link fd you go find in your bpffs doesn't make that much difference...
>> >
>> > Right. But if I had to pick just one implementation (prog fd-based vs
>> > bpf_link), I'd stick with bpf_link because it is flexible enough to
>> > "emulate" prog fd attachment (through BPF FS), but the same isn't true
>> > about prog fd attachment emulating bpf_link. That's it. I really don't
>> > enjoy harping on that point, but it feels to be silently dismissed all
>> > the time based on one particular arrangement for particular existing
>> > XDP flow.
>>
>> It can; kinda. But you introduce a dependency on bpffs that wasn't there
>> before, and you end up with resources that are kept around in the kernel
>> if the interface disappears (because they are still pinned). So I
>> consider it a poor emulation.
>
> Yes, it's not exactly 100% the same semantics.
> It is possible with slight additions to API to support essentially
> exactly the same semantics you want with prog attachment. E.g., we can
> either have a flag at LINK_CREATE time, or a separate command (e.g.,
> LINK_PIN or something), that would mark bpf_link as "sticky", bump
> it's refcnt. What happens then is that even if last FD is closed,
> there is still refcnt 1 there, and then there are two ways to detach
> that link:
>
> 1) interface/cgroup/whatever is destroyed and bpf_link is
> auto-detached. At that point auto-detach handler will see that it's a
> "sticky" bpf_link, will decrement refcnt and subsequently free
> bpf_link kernel object (unless some application still has FD open, of
> course).
>
> 2) a new LINK_DESTROY BPF command will be introduced, which will only
> work with "sticky" bpf_links. It will decrement refcnt and do the same
> stuff as the auto-detach handler does today (so bpf_link->dev = NULL,
> for XDP link).
>
> I don't mind this, as long as this is not a default semantics and
> require conscious opt-in from whoever creates the link.

Now *this* I would like to see! I have the issue with component progs of
the multiprog dispatcher being pinned and making everything stick
around.

[...]

> Sure, thanks, enjoy your vacation! I'll post v3 then with build fixes
> I have so far.

Thanks! :)

-Toke