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

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: "Toke Høiland-Jørgensen" <toke@redhat.com>
Cc: 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 15:54:34 -0700	[thread overview]
Message-ID: <CAEf4Bza8P3yT08NAaqN2EKaaBFumzydbtYQmSvLxZ99=B6_iHw@mail.gmail.com> (raw)
In-Reply-To: <87wo75l9yj.fsf@toke.dk>

On Fri, Mar 27, 2020 at 3:17 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
>
> > Please stop dodging. Just like with "rest of the kernel", but really
> > "just networking" from before.
>
> Look, if we can't have this conversation without throwing around
> accusations of bad faith, I think it is best we just take Ed's advice
> and leave it until after the merge window.
>

Toke, if me pointing out that you are dodging original discussion and
pivoting offends you, by all means, you don't have to continue. But if
you are still with me, let's look at this particular part of
discussion:

>> >> For XDP there is already a unique handle, it's just implicit: Each
>> >> netdev can have exactly one XDP program loaded. So I don't really see
>> >> how bpf_link adds anything, other than another API for the same thing?
>> >
>> > I certainly failed to explain things clearly if you are still asking
>> > this. See point #2, once you attach bpf_link you can't just replace
>> > it. This is what XDP doesn't have right now.
>>
>> Those are two different things, though. I get that #2 is a new
>> capability provided by bpf_link, I was just saying #1 isn't (for XDP).
>
> bpf_link is combination of those different things... Independently
> they are either impossible or insufficient. I'm not sure how that
> doesn't answer your question:
>
>> So I don't really see
>> how bpf_link adds anything, other than another API for the same thing?
>
> Please stop dodging. Just like with "rest of the kernel", but really
> "just networking" from before.

You said "So I don't really see how bpf_link adds anything, other than
another API for the same thing?". I explained that bpf_link is not the
same thing that exists already, thus it's not another API for the same
thing. You picked one property of bpf_link and claimed it's the same
as what XDP has right now. "I get that #2 is a new capability provided
by bpf_link, I was just saying #1 isn't (for XDP)". So should I read
that as if you are agreeing and your original objection is rescinded?
If yes, then good, this part is concluded and I'm sorry if I
misinterpreted your answer.

But if not, then you again are picking one properly and just saying
"but XDP has it" without considering all of bpf_link properties as a
whole. In that case I do think you are arguing not in good faith.
Simple as that. I also hope I don't have to go all the way back to
"rest of the kernel", pivoted to "just networking" w.r.t.
subsystem-specific configuration/attachment APIs to explain another
reference.

P.S. I don't know how merge window has anything to do with this whole
discussion, honestly...

>> >

> -Toke
>