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

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Lorenz Bauer <lmb@cloudflare.com>, Andrey Ignatov <rdna@fb.com>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 1/4] xdp: Support specifying expected existing program when attaching XDP
Date: Fri, 27 Mar 2020 19:26:09 -0700	[thread overview]
Message-ID: <20200328022609.zfupojim7see5cqx@ast-mbp> (raw)
In-Reply-To: <87lfnll0eh.fsf@toke.dk>

On Sat, Mar 28, 2020 at 02:43:18AM +0100, Toke Høiland-Jørgensen wrote:
> 
> No, I was certainly not planning to use that to teach libxdp to just
> nuke any bpf_link it finds attached to an interface. Quite the contrary,
> the point of this series is to allow libxdp to *avoid* replacing
> something on the interface that it didn't put there itself.

Exactly! "that it didn't put there itself".
How are you going to do that?
I really hope you thought it through and came up with magic.
Because I tried and couldn't figure out how to do that with IFLA_XDP*
Please walk me step by step how do you think it's possible.

I'm saying that without bpf_link for xdp libxdp has no ability to identify
an attachment that is theirs.

I suspect what is happening that you found first missing kernel feature
while implementing libxdp and trying to fix it by extending kernel api.
Well the reason libxdp is not part of libbpf is for it to be flexible
in design and have unstable api.
But you're using this unstable project as the reason to add stable apis
both to kernel and libbpf. I don't think that's workable because...

> I could understand why you wouldn't want to do
> that if it was a huge and invasive change; but it really isn't...

Yes. It's a small api extension to both kernel and libbpf.
But it means that by accepting this small change I sign up on maintaining it
forever. And I see how second and third such small experimental change will be
coming in the future. All such design revisions of libxdp will end up on my
plate to support forever in the kernel and in libbpf. I'm not excited to
support all of these experimental code.

I see two ways out of this stalemate:
1. assume that replace_fd extension landed and develop libxdp further
   into fully fledged library. May be not a complete library, but at least
   for few more weeks. If then you still think replace_fd is enough
   I'll land it.
2. I can land replace_fd now, but please don't be surprised that
   I will revert it several weeks from now when it's clear that
   it's not enough.

Which one do you prefer?