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

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 15 Aug 2022 07:31:23 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >
> > 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.

I extended Jiri's patch to make it work again.

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

Actually I did invent it.

   https://lore.kernel.org/lkml/20080210072109.GR4100@elte.hu/


I'm the one that introduced the code to convert mcount into the 5 byte nop,
and did the research and development to make it work at run time. I had one
hiccup along the way that caused the e1000e network card breakage.

The "daemon" approach was horrible, and then I created the recordmcount.pl
perl script to accomplish the same thing at compile time.

> ftrace should really stop trying to own all of the kernel text rewrites.
> It's in the way. Like this case.

It's not trying to own all modifications (static_calls is not ftrace). But
the code at the start of functions with fentry does belong to it.

> 
> >    https://lore.kernel.org/all/CAADnVQJTT7h3MniVqdBEU=eLwvJhEKNLSjbUAK4sOrhN=zggCQ@mail.gmail.com/
> >
> > Even though fentry was created for ftrace
> >
> >    https://lore.kernel.org/lkml/1258720459.22249.1018.camel@gandalf.stny.rr.com/
> >
> > 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.

Long before? This code made it into the kernel in Dec 2019. Yes static calls
finally made it into the kernel in 2020, but it was first introduced in
October 2018:

  https://lore.kernel.org/all/20181006015110.653946300@goodmis.org/

If you had Cc'd us on this patch, we could have collaborated and come up
with something that would have worked for you.

It took time to get in because we don't just push our features in, we make
sure that they are generic and work for others, and is secure and robust.

I sent a proof of concept, Josh took over, Linus had issues, and finally
Peter pushed it through the gate. It's a long process, but we don't break
others code while doing it!

-- Steve