Re: [PATCH bpf-next v4 2/6] bpf: introduce BPF dispatcher

[Alexei Starovoitov]

On Mon, Aug 15, 2022 at 7:13 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Wed, 11 Dec 2019 13:30:13 +0100
> Björn Töpel <bjorn.topel@xxxxxxxxx> wrote:
>
> > From: Björn Töpel <bjorn.topel@xxxxxxxxx>
> >
> > The BPF dispatcher is a multi-way branch code generator, mainly
> > targeted for XDP programs. When an XDP program is executed via the
> > bpf_prog_run_xdp(), it is invoked via an indirect call. The indirect
> > call has a substantial performance impact, when retpolines are
> > enabled. The dispatcher transform indirect calls to direct calls, and
> > therefore avoids the retpoline. The dispatcher is generated using the
> > BPF JIT, and relies on text poking provided by bpf_arch_text_poke().
> >
> > The dispatcher hijacks a trampoline function it via the __fentry__ nop
>
> Why was the ftrace maintainers not Cc'd on this patch?  I would have NACKED
> it. Hell, it wasn't even sent to LKML! This was BPF being sneaky in
> updating major infrastructure of the Linux kernel without letting the
> stakeholders of this change know about it.
>
> For some reason, the BPF folks think they own the entire kernel!
>
> When I heard that ftrace was broken by BPF I thought it was something
> unique they were doing, but unfortunately, I didn't investigate what they
> were doing at the time.

ftrace is still broken and refusing to accept the fact doesn't make it
non-broken.

> Then they started sending me patches to hide fentry locations from ftrace.
> And even telling me that fentry != ftrace

It sounds that you've invented nop5 and kernel's ability
to replace nop5 with a jump or call.
ftrace should really stop trying to own all of the kernel text rewrites.
It's in the way. Like this case.

>    https://lore.kernel.org/all/CAADnVQJTT7h3MniVqdBEU=eLwvJhEKNLSjbUAK4sOrhN=zggCQ@xxxxxxxxxxxxxx/
>
> Even though fentry was created for ftrace
>
>    https://lore.kernel.org/lkml/1258720459.22249.1018.camel@xxxxxxxxxxxxxxxxxxx/
>
> and all the work with fentry was for the ftrace infrastructure. Ftrace
> takes a lot of care for security and use cases for other users (like
> live kernel patching). But BPF has the NIH syndrome, and likes to own
> everything and recreate the wheel so that they have full control.
>
> > of the trampoline. One dispatcher instance currently supports up to 64
> > dispatch points. A user creates a dispatcher with its corresponding
> > trampoline with the DEFINE_BPF_DISPATCHER macro.
>
> Anyway, this patch just looks like a re-implementation of static_calls:

It was implemented long before static_calls made it to the kernel
and it's different. Please do your home work.