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@xxxxxxxxx> 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
