Message-ID: <CAHk-=wg7JuFYwGy=GOMbRCtOL+jwSQsdUaBsRWkDVYbxipbM5A@mail.gmail.com>
Date: Mon, 18 Dec 2023 17:11:23 -0800
From: Linus Torvalds <torvalds@...uxfoundation.org>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: kuba@...nel.org, davem@...emloft.net, edumazet@...gle.com, 
	pabeni@...hat.com, daniel@...earbox.net, andrii@...nel.org, 
	peterz@...radead.org, brauner@...nel.org, netdev@...r.kernel.org, 
	bpf@...r.kernel.org, kernel-team@...com
Subject: Re: pull-request: bpf-next 2023-12-18

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.

               Linus

Powered by blists - more mailing lists