netdev - Re: [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP

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: <87k130iwrb.fsf@toke.dk>
Date:   Tue, 31 Mar 2020 13:34:00 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Edward Cree <ecree@...arflare.com>
Cc:     Andrii Nakryiko <andrii.nakryiko@...il.com>,
        John Fastabend <john.fastabend@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Lorenz Bauer <lmb@...udflare.com>,
        Andrey Ignatov <rdna@...com>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP

Alexei Starovoitov <alexei.starovoitov@...il.com> writes:

> On Mon, Mar 30, 2020 at 04:41:46PM +0100, Edward Cree wrote:
>> On 29/03/2020 21:23, Andrii Nakryiko wrote:
>> > But you can't say the same about other XDP applications that do not
>> > use libxdp. So will your library come with a huge warning
>> What about a system-wide policy switch to decide whether replacing/
>>  removing an XDP program without EXPECTED_FD is allowed?  That way
>>  the sysadmin gets to choose whether it's the firewall or the packet
>>  analyser that breaks, rather than baking a policy into the design.
>> Then libxdp just needs to say in the README "you might want to turn
>>  on this switch".  Or maybe it defaults to on, and the other program
>>  has to talk you into turning it off if it wants to be 'ill-behaved'.
>
> yeah. something like this can work for xdp only, but
> it won't work for tc, since ownership is missing.
> It looks like such policy knob will bere-inventing bpf_link for
> one specific xdp case only because xdp has one program per attachment.

You keep talking about this as though bpf_link was the existing API and
we're discussing adding another, when in reality it's the other way
around.

> Imagine it was easy to come up with sensible policy and allow
> multiple progs in xdp hook.
> How would you implement such policy knob?
> processA attaches prog XDP_A. processB attaches prog XDP-B.
> Unless they start tagging their indivdual programs with BTF tags
> (as Toke is planning to do) there is no way to tell them apart.
> Then processA can iterate all progs in a hook, finds its prog
> based on tag and tell kernel: "find and replace an xdp prog with old_fd
> with new_fd on this ifindex".
> Kinda works, but it doesn't stop processB to accidently detach prog XDP_A
> that was installed by processA.
>
> The kernel job is to share the system resources. Like memory, cpu time.
> The hook is such resource too. The owner concept part of bpf_link
> allows such sharing.

FWIW I actually agree that the bpf_link ownership concept makes sense
for the individual attachments in a multi-prog hook; including for XDP.
And I've started thinking about whether the bpf_link fd can work as the
reference being returned by libxdp after a component program is
attached. I have some reservations, but I'll start a new thread on that
once I'm a bit further along with it...

[...]

> XDP is the hardest, since it does single prog only.
> That's what we're trying to solve with libdispatcher.
> I think if it goes well it can become part of the kernel and kernel
> will do multi prog XDP attach. And all hooks will be symmetrical.

Now *that* I'd like to see! I've said from the beginning that I think
XDP multi-prog should be part of the kernel, so if we can get there via
this detour I'm all for it.

> But looking at the size of this thread and still lots of
> misunderstanding about basic concept like bpf_link I'm not hopeful
> that libdispatcher will ever become part of the kernel.

I don't share your pessimism. If we can stop writing off honest
disagreement about design tradeoffs as just "misunderstanding", I think
we can get there.

-Toke