Re: pull-request: bpf-next 2023-12-18

[Christian Brauner <brauner@kernel.org>]

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@gmail.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.

@Linus, I'd like to ask you to please not pull any BPF code that touches
fs/ in any way without an explicit ACK/RVB from Al, Jan, or myself. For
now, everything BPF related to fs/ is proactively NAKed by me.

This is disrespectful to the whole fs community and to me personally and
precisely why we will keep resisting meaningful BPF integration in fs/
until we can be sure that we can trust this subsystem.

Pleasant in-person interactions are one thing. But they're meaningless
if they're inconsistent with on-list behavior and matters of trust.
Disgraceful and tasteless is what I keep coming back to.