Date: Tue, 19 Dec 2023 17:36:38 +0100
From: Daniel Borkmann <daniel@...earbox.net>
To: Christian Brauner <brauner@...nel.org>,
 Linus Torvalds <torvalds@...uxfoundation.org>,
 Alexei Starovoitov <alexei.starovoitov@...il.com>, andrii@...nel.org
Cc: kuba@...nel.org, davem@...emloft.net, edumazet@...gle.com,
 pabeni@...hat.com, peterz@...radead.org, netdev@...r.kernel.org,
 bpf@...r.kernel.org, kernel-team@...com, linux-fsdevel@...r.kernel.org
Subject: Re: pull-request: bpf-next 2023-12-18

On 12/19/23 11:23 AM, Christian Brauner wrote:
> On Mon, Dec 18, 2023 at 05:11:23PM -0800, Linus Torvalds wrote:
>> On Mon, 18 Dec 2023 at 16:05, Alexei Starovoitov
>> <alexei.starovoitov@...il.com> wrote:
>>>
>>> 2) Introduce BPF token object, from Andrii Nakryiko.
>>
>> I assume this is why I and some other unusual recipients are cc'd,
>> because the networking people feel like they can't judge this and
>> shouldn't merge non-networking code like this.
>>
>> Honestly, I was told - and expected - that this part would come in a
>> branch of its own, so that it would be sanely reviewable.
>>
>> Now it's mixed in with everything else.
>>
>> This is *literally* why we have branches in git, so that people can
>> make more independent changes and judgements, and so that we don't
>> have to be in a situation where "look, here's ten different things,
>> pull it all or nothing".
>>
>> Many of the changes *look* like they are in branches, but they've been
>> the "fake branches" that are just done as "patch series in a branch,
>> with the cover letter as the merge message".
>>
>> Which is great for maintaining that cover letter information and a
>> certain amount of historical clarity, but not helpful AT ALL for the
>> "independent changes" thing when it is all mixed up in history, where
>> independent things are mostly serialized and not actually independent
>> in history at all.
>>
>> So now it appears to be one big mess, and exactly that "all or
>> nothing" thing that isn't great, since the whole point was that the
>> networking people weren't comfortable with the reviewing filesystem
>> side.
>>
>> And honestly, the bpf side *still* seems to be absolutely conbfused
>> and complkete crap when it comes to file descriptors.
>>
>> I took a quick look, and I *still* see new code being introduced there
>> that thinks that file descriptor zero is special, and we tols you a
>> *year* ago that that wasn't true, and that you need to fix this.
>>
>> I literally see complete garbage like tghis:
>>
>>          ..
>>          __u32 btf_token_fd;
>>          ...
>>          if (attr->btf_token_fd) {
>>                  token = bpf_token_get_from_fd(attr->btf_token_fd);
>>
>> and this is all *new* code that makes that same bogus sh*t-for-brains
>> mistake that was wrong the first time.
>>
>> So now I'm saying NAK. Enough is enough.  No more of this crazy "I
>> don't understand even the _basics_ of file descriptors, and yet I'm
>> introducing new random interfaces".
>>
>> I know you thought fd zero was something invalid. You were told
>> otherwise. Apparently you just ignored being wrong, and have decided
>> to double down on being wrong.
>>
>> We don't take this kind of flat-Earther crap.
>>
>> File descriptors don't start at 1. Deal with reality. Stop making the
>> same mistake over and over. If you ant to have a "no file descriptor"
>> flag, you use a signed type, and a signed value for that, because file
>> descriptor zero is perfectly valid, and I don't want to hear any more
>> uninformed denialism.
>>
>> Stop polluting the kernel with incorrect assumptions.
>>
>> So yes, I will keep NAK'ing this until this kind of fundamental
>> mistake is fixed. This is not rocket science, and this is not
>> something that wasn't discussed before. Your ignorance has now turned
>> from "I didn't know" to "I didn 't care", and at that point I really
>> don't want to see new code any more.
> 
> Alexei, Andrii, this is a massive breach of trust and flatout
> disrespectful. I barely reword mails and believe me I've reworded this
> mail many times. I'm furious.
> 
> Over the last couple of months since LSFMM in May 2023 until almost last
> week I've given you extensive design and review for this whole approach
> to get this into even remotely sane shape from a VFS perspective.
> 
> The VFS maintainers including Linus have explicitly NAKed this "zero is
> not a valid fd nonsense" and told you to stop doing that. We told you
> that such fundamental VFS semantics are not yours to decide.
> 
> And yet you put a patch into a series that did exactly that and then had
> the unbelievable audacity to repeatedly ask me to put my ACK under this
> - both in person and on list.
> 
> I'm glad I only gave my ACK to the two patches that I extensivly
> reviewed and never to the whole series.

Sincere apologies for this whole mess. All token series related patches
have been reverted in bpf-next now, and I'm prepping a PR for net-next
so that this is also fully removed from there.

Thanks,
Daniel

Powered by blists - more mailing lists