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

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Alexei Starovoitov <alexei.starovoitov@gmail.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