Re: [RFC PATCH v3 0/9] fprobe: Introduce fprobe function entry/exit probe

[Andrii Nakryiko]

On Mon, Jan 24, 2022 at 2:23 PM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> On Mon, Jan 24, 2022 at 12:22:10PM -0800, Andrii Nakryiko wrote:
>
> SNIP
>
> > > > > > > > > (This testing patch is just for confirming the rethook is correctly
> > > > > > > > >  implemented.)
> > > > > > > > >
> > > > > > > > > BTW, on the x86, ftrace (with fentry) location address is same as
> > > > > > > > > symbol address. But on other archs, it will be different (e.g. arm64
> > > > > > > > > will need 2 instructions to save link-register and call ftrace, the
> > > > > > > > > 2nd instruction will be the ftrace location.)
> > > > > > > > > Does libbpf correctly handle it?
> > > > >
> > > > > hm, I'm probably missing something, but should this be handled by arm
> > > > > specific kernel code? user passes whatever is found in kallsyms, right?
> > > >
> > > > In x86, fentry nop is always placed at the first instruction of the function,
> > > > but the other arches couldn't do that if they use LR (link register) for
> > > > storing return address instead of stack. E.g. arm64 saves lr and call the
> > > > ftrace. Then ftrace location address of a function is not the symbol address.
> > > >
> > > > Anyway, I updated fprobe to handle those cases. I also found some issues
> > > > on rethook, so let me update the series again.
> > >
> > > great, I reworked the bpf fprobe link change and need to add the
> > > symbols attachment support, so you don't need to include it in
> > > new version.. I'll rebase it and send on top of your patchset
> >
> > Using just addresses (IPs) for retsnoop and bpftrace is fine because
> > such generic tools are already parsing kallsyms and probably building
> > some lookup table. But in general, having IP-based attachment is a
> > regression from current perf_event_open-based kprobe, where user is
> > expected to pass symbolic function name. Using IPs has an advantage of
> > being unambiguous (e.g., when same static function name in kernel
> > belongs to multiple actual functions), so there is that. But I was
> > also wondering wouldn't kernel need to do symbol to IP resolution
> > anyways just to check that we are attaching to function entry?
>
> ftrace does its own check for address to attach, it keeps record
> for every attachable address.. so less work for us ;-)

Oh, makes sense, thanks!

>
> >
> > I'll wait for your patch set to see how did you go about it in a new revision.
>
> I agree we should have the support to use symbols as well, I'll add it

sounds good, thanks

>
> jirka
>