mbox series

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

Message ID 164260419349.657731.13913104835063027148.stgit@devnote2 (mailing list archive)
Headers show
Series fprobe: Introduce fprobe function entry/exit probe | expand

Message

Masami Hiramatsu (Google) Jan. 19, 2022, 2:56 p.m. UTC
Hello Jiri,

Here is the 3rd version of fprobe. I added some comments and
fixed some issues. But I still saw some problems when I add
your selftest patches.

This series introduces the fprobe, the function entry/exit probe
with multiple probe point support. This also introduces the rethook
for hooking function return as same as kretprobe does. This
abstraction will help us to generalize the fgraph tracer,
because we can just switch it from rethook in fprobe, depending
on the kernel configuration.

The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
patches will not be affected by this change.

[1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u

However, when I applied all other patches on top of this series,
I saw the "#8 bpf_cookie" test case has been stacked (maybe related
to the bpf_cookie issue which Andrii and Jiri talked?) And when I
remove the last selftest patch[2], the selftest stopped at "#112
raw_tp_test_run".

[2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483 

Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.

This added 2 more out-of-tree patches. [8/9] is for adding wildcard
support to the sample program, [9/9] is a testing patch for replacing
kretprobe trampoline with rethook.
According to this work, I noticed that using rethook in kretprobe
needs 2 steps.
 1. port the rethook on all architectures which supports kretprobes.
    (some arch requires CONFIG_KPROBES for rethook)
 2. replace kretprobe trampoline with rethook for all archs, at once.
    This must be done by one treewide patch.

Anyway, I'll do the kretprobe update in the next step as another series.
(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?

Thank you,

---

Jiri Olsa (2):
      ftrace: Add ftrace_set_filter_ips function
      bpf: Add kprobe link for attaching raw kprobes

Masami Hiramatsu (7):
      fprobe: Add ftrace based probe APIs
      rethook: Add a generic return hook
      rethook: x86: Add rethook x86 implementation
      fprobe: Add exit_handler support
      fprobe: Add sample program for fprobe
      [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
      [DO NOT MERGE] out-of-tree: kprobes: Use rethook for kretprobe


 arch/x86/Kconfig                |    1 
 arch/x86/include/asm/unwind.h   |    8 +
 arch/x86/kernel/Makefile        |    1 
 arch/x86/kernel/kprobes/core.c  |  106 --------------
 arch/x86/kernel/rethook.c       |  115 +++++++++++++++
 include/linux/bpf_types.h       |    1 
 include/linux/fprobe.h          |   84 +++++++++++
 include/linux/ftrace.h          |    3 
 include/linux/kprobes.h         |   85 +----------
 include/linux/rethook.h         |   99 +++++++++++++
 include/linux/sched.h           |    4 -
 include/uapi/linux/bpf.h        |   12 ++
 kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++-
 kernel/exit.c                   |    3 
 kernel/fork.c                   |    4 -
 kernel/kallsyms.c               |    1 
 kernel/kprobes.c                |  265 +++++------------------------------
 kernel/trace/Kconfig            |   22 +++
 kernel/trace/Makefile           |    2 
 kernel/trace/fprobe.c           |  179 ++++++++++++++++++++++++
 kernel/trace/ftrace.c           |   54 ++++++-
 kernel/trace/rethook.c          |  295 +++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_kprobe.c     |    4 -
 kernel/trace/trace_output.c     |    2 
 samples/Kconfig                 |    7 +
 samples/Makefile                |    1 
 samples/fprobe/Makefile         |    3 
 samples/fprobe/fprobe_example.c |  154 ++++++++++++++++++++
 tools/include/uapi/linux/bpf.h  |   12 ++
 29 files changed, 1283 insertions(+), 439 deletions(-)
 create mode 100644 arch/x86/kernel/rethook.c
 create mode 100644 include/linux/fprobe.h
 create mode 100644 include/linux/rethook.h
 create mode 100644 kernel/trace/fprobe.c
 create mode 100644 kernel/trace/rethook.c
 create mode 100644 samples/fprobe/Makefile
 create mode 100644 samples/fprobe/fprobe_example.c

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

Comments

Andrii Nakryiko Jan. 20, 2022, 10:24 p.m. UTC | #1
On Wed, Jan 19, 2022 at 6:56 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Hello Jiri,
>
> Here is the 3rd version of fprobe. I added some comments and
> fixed some issues. But I still saw some problems when I add
> your selftest patches.
>
> This series introduces the fprobe, the function entry/exit probe
> with multiple probe point support. This also introduces the rethook
> for hooking function return as same as kretprobe does. This
> abstraction will help us to generalize the fgraph tracer,
> because we can just switch it from rethook in fprobe, depending
> on the kernel configuration.
>
> The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
> patches will not be affected by this change.
>
> [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
>
> However, when I applied all other patches on top of this series,
> I saw the "#8 bpf_cookie" test case has been stacked (maybe related
> to the bpf_cookie issue which Andrii and Jiri talked?) And when I
> remove the last selftest patch[2], the selftest stopped at "#112
> raw_tp_test_run".
>
> [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483
>
> Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.
>
> This added 2 more out-of-tree patches. [8/9] is for adding wildcard
> support to the sample program, [9/9] is a testing patch for replacing
> kretprobe trampoline with rethook.
> According to this work, I noticed that using rethook in kretprobe
> needs 2 steps.
>  1. port the rethook on all architectures which supports kretprobes.
>     (some arch requires CONFIG_KPROBES for rethook)
>  2. replace kretprobe trampoline with rethook for all archs, at once.
>     This must be done by one treewide patch.
>
> Anyway, I'll do the kretprobe update in the next step as another series.
> (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?

libbpf doesn't do anything there. The interface for kprobe is based on
function name and kernel performs name lookups internally to resolve
IP. For fentry it's similar (kernel handles IP resolution), but
instead of function name we specify BTF ID of a function type.

>
> Thank you,
>
> ---
>
> Jiri Olsa (2):
>       ftrace: Add ftrace_set_filter_ips function
>       bpf: Add kprobe link for attaching raw kprobes
>
> Masami Hiramatsu (7):
>       fprobe: Add ftrace based probe APIs
>       rethook: Add a generic return hook
>       rethook: x86: Add rethook x86 implementation
>       fprobe: Add exit_handler support
>       fprobe: Add sample program for fprobe
>       [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
>       [DO NOT MERGE] out-of-tree: kprobes: Use rethook for kretprobe
>
>
>  arch/x86/Kconfig                |    1
>  arch/x86/include/asm/unwind.h   |    8 +
>  arch/x86/kernel/Makefile        |    1
>  arch/x86/kernel/kprobes/core.c  |  106 --------------
>  arch/x86/kernel/rethook.c       |  115 +++++++++++++++
>  include/linux/bpf_types.h       |    1
>  include/linux/fprobe.h          |   84 +++++++++++
>  include/linux/ftrace.h          |    3
>  include/linux/kprobes.h         |   85 +----------
>  include/linux/rethook.h         |   99 +++++++++++++
>  include/linux/sched.h           |    4 -
>  include/uapi/linux/bpf.h        |   12 ++
>  kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++-
>  kernel/exit.c                   |    3
>  kernel/fork.c                   |    4 -
>  kernel/kallsyms.c               |    1
>  kernel/kprobes.c                |  265 +++++------------------------------
>  kernel/trace/Kconfig            |   22 +++
>  kernel/trace/Makefile           |    2
>  kernel/trace/fprobe.c           |  179 ++++++++++++++++++++++++
>  kernel/trace/ftrace.c           |   54 ++++++-
>  kernel/trace/rethook.c          |  295 +++++++++++++++++++++++++++++++++++++++
>  kernel/trace/trace_kprobe.c     |    4 -
>  kernel/trace/trace_output.c     |    2
>  samples/Kconfig                 |    7 +
>  samples/Makefile                |    1
>  samples/fprobe/Makefile         |    3
>  samples/fprobe/fprobe_example.c |  154 ++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h  |   12 ++
>  29 files changed, 1283 insertions(+), 439 deletions(-)
>  create mode 100644 arch/x86/kernel/rethook.c
>  create mode 100644 include/linux/fprobe.h
>  create mode 100644 include/linux/rethook.h
>  create mode 100644 kernel/trace/fprobe.c
>  create mode 100644 kernel/trace/rethook.c
>  create mode 100644 samples/fprobe/Makefile
>  create mode 100644 samples/fprobe/fprobe_example.c
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
Masami Hiramatsu (Google) Jan. 21, 2022, 4:55 a.m. UTC | #2
On Thu, 20 Jan 2022 14:24:15 -0800
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Wed, Jan 19, 2022 at 6:56 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Hello Jiri,
> >
> > Here is the 3rd version of fprobe. I added some comments and
> > fixed some issues. But I still saw some problems when I add
> > your selftest patches.
> >
> > This series introduces the fprobe, the function entry/exit probe
> > with multiple probe point support. This also introduces the rethook
> > for hooking function return as same as kretprobe does. This
> > abstraction will help us to generalize the fgraph tracer,
> > because we can just switch it from rethook in fprobe, depending
> > on the kernel configuration.
> >
> > The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
> > patches will not be affected by this change.
> >
> > [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> >
> > However, when I applied all other patches on top of this series,
> > I saw the "#8 bpf_cookie" test case has been stacked (maybe related
> > to the bpf_cookie issue which Andrii and Jiri talked?) And when I
> > remove the last selftest patch[2], the selftest stopped at "#112
> > raw_tp_test_run".
> >
> > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483
> >
> > Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.
> >
> > This added 2 more out-of-tree patches. [8/9] is for adding wildcard
> > support to the sample program, [9/9] is a testing patch for replacing
> > kretprobe trampoline with rethook.
> > According to this work, I noticed that using rethook in kretprobe
> > needs 2 steps.
> >  1. port the rethook on all architectures which supports kretprobes.
> >     (some arch requires CONFIG_KPROBES for rethook)
> >  2. replace kretprobe trampoline with rethook for all archs, at once.
> >     This must be done by one treewide patch.
> >
> > Anyway, I'll do the kretprobe update in the next step as another series.
> > (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?
> 
> libbpf doesn't do anything there. The interface for kprobe is based on
> function name and kernel performs name lookups internally to resolve
> IP. For fentry it's similar (kernel handles IP resolution), but
> instead of function name we specify BTF ID of a function type.

Hmm, according to Jiri's original patch, it seems to pass an array of
addresses. So I thought that has been resolved by libbpf.

+			struct {
+				__aligned_u64	addrs;
+				__u32		cnt;
+				__u64		bpf_cookie;
+			} kprobe;

Anyway, fprobe itself also has same issue. I'll try to fix it.

Thank you!

> 
> >
> > Thank you,
> >
> > ---
> >
> > Jiri Olsa (2):
> >       ftrace: Add ftrace_set_filter_ips function
> >       bpf: Add kprobe link for attaching raw kprobes
> >
> > Masami Hiramatsu (7):
> >       fprobe: Add ftrace based probe APIs
> >       rethook: Add a generic return hook
> >       rethook: x86: Add rethook x86 implementation
> >       fprobe: Add exit_handler support
> >       fprobe: Add sample program for fprobe
> >       [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
> >       [DO NOT MERGE] out-of-tree: kprobes: Use rethook for kretprobe
> >
> >
> >  arch/x86/Kconfig                |    1
> >  arch/x86/include/asm/unwind.h   |    8 +
> >  arch/x86/kernel/Makefile        |    1
> >  arch/x86/kernel/kprobes/core.c  |  106 --------------
> >  arch/x86/kernel/rethook.c       |  115 +++++++++++++++
> >  include/linux/bpf_types.h       |    1
> >  include/linux/fprobe.h          |   84 +++++++++++
> >  include/linux/ftrace.h          |    3
> >  include/linux/kprobes.h         |   85 +----------
> >  include/linux/rethook.h         |   99 +++++++++++++
> >  include/linux/sched.h           |    4 -
> >  include/uapi/linux/bpf.h        |   12 ++
> >  kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++-
> >  kernel/exit.c                   |    3
> >  kernel/fork.c                   |    4 -
> >  kernel/kallsyms.c               |    1
> >  kernel/kprobes.c                |  265 +++++------------------------------
> >  kernel/trace/Kconfig            |   22 +++
> >  kernel/trace/Makefile           |    2
> >  kernel/trace/fprobe.c           |  179 ++++++++++++++++++++++++
> >  kernel/trace/ftrace.c           |   54 ++++++-
> >  kernel/trace/rethook.c          |  295 +++++++++++++++++++++++++++++++++++++++
> >  kernel/trace/trace_kprobe.c     |    4 -
> >  kernel/trace/trace_output.c     |    2
> >  samples/Kconfig                 |    7 +
> >  samples/Makefile                |    1
> >  samples/fprobe/Makefile         |    3
> >  samples/fprobe/fprobe_example.c |  154 ++++++++++++++++++++
> >  tools/include/uapi/linux/bpf.h  |   12 ++
> >  29 files changed, 1283 insertions(+), 439 deletions(-)
> >  create mode 100644 arch/x86/kernel/rethook.c
> >  create mode 100644 include/linux/fprobe.h
> >  create mode 100644 include/linux/rethook.h
> >  create mode 100644 kernel/trace/fprobe.c
> >  create mode 100644 kernel/trace/rethook.c
> >  create mode 100644 samples/fprobe/Makefile
> >  create mode 100644 samples/fprobe/fprobe_example.c
> >
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
Andrii Nakryiko Jan. 21, 2022, 5:29 p.m. UTC | #3
On Thu, Jan 20, 2022 at 8:55 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Thu, 20 Jan 2022 14:24:15 -0800
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> > On Wed, Jan 19, 2022 at 6:56 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > Hello Jiri,
> > >
> > > Here is the 3rd version of fprobe. I added some comments and
> > > fixed some issues. But I still saw some problems when I add
> > > your selftest patches.
> > >
> > > This series introduces the fprobe, the function entry/exit probe
> > > with multiple probe point support. This also introduces the rethook
> > > for hooking function return as same as kretprobe does. This
> > > abstraction will help us to generalize the fgraph tracer,
> > > because we can just switch it from rethook in fprobe, depending
> > > on the kernel configuration.
> > >
> > > The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
> > > patches will not be affected by this change.
> > >
> > > [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> > >
> > > However, when I applied all other patches on top of this series,
> > > I saw the "#8 bpf_cookie" test case has been stacked (maybe related
> > > to the bpf_cookie issue which Andrii and Jiri talked?) And when I
> > > remove the last selftest patch[2], the selftest stopped at "#112
> > > raw_tp_test_run".
> > >
> > > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483
> > >
> > > Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.
> > >
> > > This added 2 more out-of-tree patches. [8/9] is for adding wildcard
> > > support to the sample program, [9/9] is a testing patch for replacing
> > > kretprobe trampoline with rethook.
> > > According to this work, I noticed that using rethook in kretprobe
> > > needs 2 steps.
> > >  1. port the rethook on all architectures which supports kretprobes.
> > >     (some arch requires CONFIG_KPROBES for rethook)
> > >  2. replace kretprobe trampoline with rethook for all archs, at once.
> > >     This must be done by one treewide patch.
> > >
> > > Anyway, I'll do the kretprobe update in the next step as another series.
> > > (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?
> >
> > libbpf doesn't do anything there. The interface for kprobe is based on
> > function name and kernel performs name lookups internally to resolve
> > IP. For fentry it's similar (kernel handles IP resolution), but
> > instead of function name we specify BTF ID of a function type.
>
> Hmm, according to Jiri's original patch, it seems to pass an array of
> addresses. So I thought that has been resolved by libbpf.
>
> +                       struct {
> +                               __aligned_u64   addrs;

I think this is a pointer to an array of pointers to zero-terminated C strings

> +                               __u32           cnt;
> +                               __u64           bpf_cookie;
> +                       } kprobe;
>
> Anyway, fprobe itself also has same issue. I'll try to fix it.
>
> Thank you!
>
> >
> > >
> > > Thank you,
> > >
> > > ---
> > >
> > > Jiri Olsa (2):
> > >       ftrace: Add ftrace_set_filter_ips function
> > >       bpf: Add kprobe link for attaching raw kprobes
> > >
> > > Masami Hiramatsu (7):
> > >       fprobe: Add ftrace based probe APIs
> > >       rethook: Add a generic return hook
> > >       rethook: x86: Add rethook x86 implementation
> > >       fprobe: Add exit_handler support
> > >       fprobe: Add sample program for fprobe
> > >       [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
> > >       [DO NOT MERGE] out-of-tree: kprobes: Use rethook for kretprobe
> > >
> > >
> > >  arch/x86/Kconfig                |    1
> > >  arch/x86/include/asm/unwind.h   |    8 +
> > >  arch/x86/kernel/Makefile        |    1
> > >  arch/x86/kernel/kprobes/core.c  |  106 --------------
> > >  arch/x86/kernel/rethook.c       |  115 +++++++++++++++
> > >  include/linux/bpf_types.h       |    1
> > >  include/linux/fprobe.h          |   84 +++++++++++
> > >  include/linux/ftrace.h          |    3
> > >  include/linux/kprobes.h         |   85 +----------
> > >  include/linux/rethook.h         |   99 +++++++++++++
> > >  include/linux/sched.h           |    4 -
> > >  include/uapi/linux/bpf.h        |   12 ++
> > >  kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++-
> > >  kernel/exit.c                   |    3
> > >  kernel/fork.c                   |    4 -
> > >  kernel/kallsyms.c               |    1
> > >  kernel/kprobes.c                |  265 +++++------------------------------
> > >  kernel/trace/Kconfig            |   22 +++
> > >  kernel/trace/Makefile           |    2
> > >  kernel/trace/fprobe.c           |  179 ++++++++++++++++++++++++
> > >  kernel/trace/ftrace.c           |   54 ++++++-
> > >  kernel/trace/rethook.c          |  295 +++++++++++++++++++++++++++++++++++++++
> > >  kernel/trace/trace_kprobe.c     |    4 -
> > >  kernel/trace/trace_output.c     |    2
> > >  samples/Kconfig                 |    7 +
> > >  samples/Makefile                |    1
> > >  samples/fprobe/Makefile         |    3
> > >  samples/fprobe/fprobe_example.c |  154 ++++++++++++++++++++
> > >  tools/include/uapi/linux/bpf.h  |   12 ++
> > >  29 files changed, 1283 insertions(+), 439 deletions(-)
> > >  create mode 100644 arch/x86/kernel/rethook.c
> > >  create mode 100644 include/linux/fprobe.h
> > >  create mode 100644 include/linux/rethook.h
> > >  create mode 100644 kernel/trace/fprobe.c
> > >  create mode 100644 kernel/trace/rethook.c
> > >  create mode 100644 samples/fprobe/Makefile
> > >  create mode 100644 samples/fprobe/fprobe_example.c
> > >
> > > --
> > > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
Jiri Olsa Jan. 23, 2022, 11:50 p.m. UTC | #4
On Fri, Jan 21, 2022 at 09:29:00AM -0800, Andrii Nakryiko wrote:
> On Thu, Jan 20, 2022 at 8:55 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > On Thu, 20 Jan 2022 14:24:15 -0800
> > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > > On Wed, Jan 19, 2022 at 6:56 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > >
> > > > Hello Jiri,
> > > >
> > > > Here is the 3rd version of fprobe. I added some comments and
> > > > fixed some issues. But I still saw some problems when I add
> > > > your selftest patches.
> > > >
> > > > This series introduces the fprobe, the function entry/exit probe
> > > > with multiple probe point support. This also introduces the rethook
> > > > for hooking function return as same as kretprobe does. This
> > > > abstraction will help us to generalize the fgraph tracer,
> > > > because we can just switch it from rethook in fprobe, depending
> > > > on the kernel configuration.
> > > >
> > > > The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
> > > > patches will not be affected by this change.
> > > >
> > > > [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> > > >
> > > > However, when I applied all other patches on top of this series,
> > > > I saw the "#8 bpf_cookie" test case has been stacked (maybe related
> > > > to the bpf_cookie issue which Andrii and Jiri talked?) And when I
> > > > remove the last selftest patch[2], the selftest stopped at "#112
> > > > raw_tp_test_run".
> > > >
> > > > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483
> > > >
> > > > Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.
> > > >
> > > > This added 2 more out-of-tree patches. [8/9] is for adding wildcard
> > > > support to the sample program, [9/9] is a testing patch for replacing
> > > > kretprobe trampoline with rethook.
> > > > According to this work, I noticed that using rethook in kretprobe
> > > > needs 2 steps.
> > > >  1. port the rethook on all architectures which supports kretprobes.
> > > >     (some arch requires CONFIG_KPROBES for rethook)
> > > >  2. replace kretprobe trampoline with rethook for all archs, at once.
> > > >     This must be done by one treewide patch.
> > > >
> > > > Anyway, I'll do the kretprobe update in the next step as another series.
> > > > (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?


> > >
> > > libbpf doesn't do anything there. The interface for kprobe is based on
> > > function name and kernel performs name lookups internally to resolve
> > > IP. For fentry it's similar (kernel handles IP resolution), but
> > > instead of function name we specify BTF ID of a function type.
> >
> > Hmm, according to Jiri's original patch, it seems to pass an array of
> > addresses. So I thought that has been resolved by libbpf.
> >
> > +                       struct {
> > +                               __aligned_u64   addrs;
> 
> I think this is a pointer to an array of pointers to zero-terminated C strings

I used direct addresses, because bpftrace already has them, so there was
no point passing strings, I cann add support for that

jirka

> 
> > +                               __u32           cnt;
> > +                               __u64           bpf_cookie;
> > +                       } kprobe;
> >
> > Anyway, fprobe itself also has same issue. I'll try to fix it.
> >
> > Thank you!
> >
> > >
> > > >
> > > > Thank you,
> > > >
> > > > ---
> > > >
> > > > Jiri Olsa (2):
> > > >       ftrace: Add ftrace_set_filter_ips function
> > > >       bpf: Add kprobe link for attaching raw kprobes
> > > >
> > > > Masami Hiramatsu (7):
> > > >       fprobe: Add ftrace based probe APIs
> > > >       rethook: Add a generic return hook
> > > >       rethook: x86: Add rethook x86 implementation
> > > >       fprobe: Add exit_handler support
> > > >       fprobe: Add sample program for fprobe
> > > >       [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample
> > > >       [DO NOT MERGE] out-of-tree: kprobes: Use rethook for kretprobe
> > > >
> > > >
> > > >  arch/x86/Kconfig                |    1
> > > >  arch/x86/include/asm/unwind.h   |    8 +
> > > >  arch/x86/kernel/Makefile        |    1
> > > >  arch/x86/kernel/kprobes/core.c  |  106 --------------
> > > >  arch/x86/kernel/rethook.c       |  115 +++++++++++++++
> > > >  include/linux/bpf_types.h       |    1
> > > >  include/linux/fprobe.h          |   84 +++++++++++
> > > >  include/linux/ftrace.h          |    3
> > > >  include/linux/kprobes.h         |   85 +----------
> > > >  include/linux/rethook.h         |   99 +++++++++++++
> > > >  include/linux/sched.h           |    4 -
> > > >  include/uapi/linux/bpf.h        |   12 ++
> > > >  kernel/bpf/syscall.c            |  195 +++++++++++++++++++++++++-
> > > >  kernel/exit.c                   |    3
> > > >  kernel/fork.c                   |    4 -
> > > >  kernel/kallsyms.c               |    1
> > > >  kernel/kprobes.c                |  265 +++++------------------------------
> > > >  kernel/trace/Kconfig            |   22 +++
> > > >  kernel/trace/Makefile           |    2
> > > >  kernel/trace/fprobe.c           |  179 ++++++++++++++++++++++++
> > > >  kernel/trace/ftrace.c           |   54 ++++++-
> > > >  kernel/trace/rethook.c          |  295 +++++++++++++++++++++++++++++++++++++++
> > > >  kernel/trace/trace_kprobe.c     |    4 -
> > > >  kernel/trace/trace_output.c     |    2
> > > >  samples/Kconfig                 |    7 +
> > > >  samples/Makefile                |    1
> > > >  samples/fprobe/Makefile         |    3
> > > >  samples/fprobe/fprobe_example.c |  154 ++++++++++++++++++++
> > > >  tools/include/uapi/linux/bpf.h  |   12 ++
> > > >  29 files changed, 1283 insertions(+), 439 deletions(-)
> > > >  create mode 100644 arch/x86/kernel/rethook.c
> > > >  create mode 100644 include/linux/fprobe.h
> > > >  create mode 100644 include/linux/rethook.h
> > > >  create mode 100644 kernel/trace/fprobe.c
> > > >  create mode 100644 kernel/trace/rethook.c
> > > >  create mode 100644 samples/fprobe/Makefile
> > > >  create mode 100644 samples/fprobe/fprobe_example.c
> > > >
> > > > --
> > > > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> >
> >
> > --
> > Masami Hiramatsu <mhiramat@kernel.org>
>
Masami Hiramatsu (Google) Jan. 24, 2022, 12:24 a.m. UTC | #5
On Mon, 24 Jan 2022 00:50:13 +0100
Jiri Olsa <jolsa@redhat.com> wrote:

> On Fri, Jan 21, 2022 at 09:29:00AM -0800, Andrii Nakryiko wrote:
> > On Thu, Jan 20, 2022 at 8:55 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > On Thu, 20 Jan 2022 14:24:15 -0800
> > > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > >
> > > > On Wed, Jan 19, 2022 at 6:56 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > >
> > > > > Hello Jiri,
> > > > >
> > > > > Here is the 3rd version of fprobe. I added some comments and
> > > > > fixed some issues. But I still saw some problems when I add
> > > > > your selftest patches.
> > > > >
> > > > > This series introduces the fprobe, the function entry/exit probe
> > > > > with multiple probe point support. This also introduces the rethook
> > > > > for hooking function return as same as kretprobe does. This
> > > > > abstraction will help us to generalize the fgraph tracer,
> > > > > because we can just switch it from rethook in fprobe, depending
> > > > > on the kernel configuration.
> > > > >
> > > > > The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
> > > > > patches will not be affected by this change.
> > > > >
> > > > > [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> > > > >
> > > > > However, when I applied all other patches on top of this series,
> > > > > I saw the "#8 bpf_cookie" test case has been stacked (maybe related
> > > > > to the bpf_cookie issue which Andrii and Jiri talked?) And when I
> > > > > remove the last selftest patch[2], the selftest stopped at "#112
> > > > > raw_tp_test_run".
> > > > >
> > > > > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483
> > > > >
> > > > > Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.
> > > > >
> > > > > This added 2 more out-of-tree patches. [8/9] is for adding wildcard
> > > > > support to the sample program, [9/9] is a testing patch for replacing
> > > > > kretprobe trampoline with rethook.
> > > > > According to this work, I noticed that using rethook in kretprobe
> > > > > needs 2 steps.
> > > > >  1. port the rethook on all architectures which supports kretprobes.
> > > > >     (some arch requires CONFIG_KPROBES for rethook)
> > > > >  2. replace kretprobe trampoline with rethook for all archs, at once.
> > > > >     This must be done by one treewide patch.
> > > > >
> > > > > Anyway, I'll do the kretprobe update in the next step as another series.
> > > > > (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.

> > > >
> > > > libbpf doesn't do anything there. The interface for kprobe is based on
> > > > function name and kernel performs name lookups internally to resolve
> > > > IP. For fentry it's similar (kernel handles IP resolution), but
> > > > instead of function name we specify BTF ID of a function type.
> > >
> > > Hmm, according to Jiri's original patch, it seems to pass an array of
> > > addresses. So I thought that has been resolved by libbpf.
> > >
> > > +                       struct {
> > > +                               __aligned_u64   addrs;
> > 
> > I think this is a pointer to an array of pointers to zero-terminated C strings
> 
> I used direct addresses, because bpftrace already has them, so there was
> no point passing strings, I cann add support for that

So now both direct address array or symbol array are OK.

Thank you,
Jiri Olsa Jan. 24, 2022, 12:21 p.m. UTC | #6
On Mon, Jan 24, 2022 at 09:24:05AM +0900, Masami Hiramatsu wrote:
> On Mon, 24 Jan 2022 00:50:13 +0100
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > On Fri, Jan 21, 2022 at 09:29:00AM -0800, Andrii Nakryiko wrote:
> > > On Thu, Jan 20, 2022 at 8:55 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > >
> > > > On Thu, 20 Jan 2022 14:24:15 -0800
> > > > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > >
> > > > > On Wed, Jan 19, 2022 at 6:56 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > > >
> > > > > > Hello Jiri,
> > > > > >
> > > > > > Here is the 3rd version of fprobe. I added some comments and
> > > > > > fixed some issues. But I still saw some problems when I add
> > > > > > your selftest patches.
> > > > > >
> > > > > > This series introduces the fprobe, the function entry/exit probe
> > > > > > with multiple probe point support. This also introduces the rethook
> > > > > > for hooking function return as same as kretprobe does. This
> > > > > > abstraction will help us to generalize the fgraph tracer,
> > > > > > because we can just switch it from rethook in fprobe, depending
> > > > > > on the kernel configuration.
> > > > > >
> > > > > > The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
> > > > > > patches will not be affected by this change.
> > > > > >
> > > > > > [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> > > > > >
> > > > > > However, when I applied all other patches on top of this series,
> > > > > > I saw the "#8 bpf_cookie" test case has been stacked (maybe related
> > > > > > to the bpf_cookie issue which Andrii and Jiri talked?) And when I
> > > > > > remove the last selftest patch[2], the selftest stopped at "#112
> > > > > > raw_tp_test_run".
> > > > > >
> > > > > > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483
> > > > > >
> > > > > > Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.
> > > > > >
> > > > > > This added 2 more out-of-tree patches. [8/9] is for adding wildcard
> > > > > > support to the sample program, [9/9] is a testing patch for replacing
> > > > > > kretprobe trampoline with rethook.
> > > > > > According to this work, I noticed that using rethook in kretprobe
> > > > > > needs 2 steps.
> > > > > >  1. port the rethook on all architectures which supports kretprobes.
> > > > > >     (some arch requires CONFIG_KPROBES for rethook)
> > > > > >  2. replace kretprobe trampoline with rethook for all archs, at once.
> > > > > >     This must be done by one treewide patch.
> > > > > >
> > > > > > Anyway, I'll do the kretprobe update in the next step as another series.
> > > > > > (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

thanks,
jirka

> 
> > > > >
> > > > > libbpf doesn't do anything there. The interface for kprobe is based on
> > > > > function name and kernel performs name lookups internally to resolve
> > > > > IP. For fentry it's similar (kernel handles IP resolution), but
> > > > > instead of function name we specify BTF ID of a function type.
> > > >
> > > > Hmm, according to Jiri's original patch, it seems to pass an array of
> > > > addresses. So I thought that has been resolved by libbpf.
> > > >
> > > > +                       struct {
> > > > +                               __aligned_u64   addrs;
> > > 
> > > I think this is a pointer to an array of pointers to zero-terminated C strings
> > 
> > I used direct addresses, because bpftrace already has them, so there was
> > no point passing strings, I cann add support for that
> 
> So now both direct address array or symbol array are OK.
> 
> Thank you,
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>
>
Andrii Nakryiko Jan. 24, 2022, 8:22 p.m. UTC | #7
On Mon, Jan 24, 2022 at 4:21 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Jan 24, 2022 at 09:24:05AM +0900, Masami Hiramatsu wrote:
> > On Mon, 24 Jan 2022 00:50:13 +0100
> > Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > > On Fri, Jan 21, 2022 at 09:29:00AM -0800, Andrii Nakryiko wrote:
> > > > On Thu, Jan 20, 2022 at 8:55 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > >
> > > > > On Thu, 20 Jan 2022 14:24:15 -0800
> > > > > Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> > > > >
> > > > > > On Wed, Jan 19, 2022 at 6:56 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > > > > > >
> > > > > > > Hello Jiri,
> > > > > > >
> > > > > > > Here is the 3rd version of fprobe. I added some comments and
> > > > > > > fixed some issues. But I still saw some problems when I add
> > > > > > > your selftest patches.
> > > > > > >
> > > > > > > This series introduces the fprobe, the function entry/exit probe
> > > > > > > with multiple probe point support. This also introduces the rethook
> > > > > > > for hooking function return as same as kretprobe does. This
> > > > > > > abstraction will help us to generalize the fgraph tracer,
> > > > > > > because we can just switch it from rethook in fprobe, depending
> > > > > > > on the kernel configuration.
> > > > > > >
> > > > > > > The patch [1/9] and [7/9] are from Jiri's series[1]. Other libbpf
> > > > > > > patches will not be affected by this change.
> > > > > > >
> > > > > > > [1] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#u
> > > > > > >
> > > > > > > However, when I applied all other patches on top of this series,
> > > > > > > I saw the "#8 bpf_cookie" test case has been stacked (maybe related
> > > > > > > to the bpf_cookie issue which Andrii and Jiri talked?) And when I
> > > > > > > remove the last selftest patch[2], the selftest stopped at "#112
> > > > > > > raw_tp_test_run".
> > > > > > >
> > > > > > > [2] https://lore.kernel.org/all/20220104080943.113249-1-jolsa@kernel.org/T/#m242d2b3a3775eeb5baba322424b15901e5e78483
> > > > > > >
> > > > > > > Note that I used tools/testing/selftests/bpf/vmtest.sh to check it.
> > > > > > >
> > > > > > > This added 2 more out-of-tree patches. [8/9] is for adding wildcard
> > > > > > > support to the sample program, [9/9] is a testing patch for replacing
> > > > > > > kretprobe trampoline with rethook.
> > > > > > > According to this work, I noticed that using rethook in kretprobe
> > > > > > > needs 2 steps.
> > > > > > >  1. port the rethook on all architectures which supports kretprobes.
> > > > > > >     (some arch requires CONFIG_KPROBES for rethook)
> > > > > > >  2. replace kretprobe trampoline with rethook for all archs, at once.
> > > > > > >     This must be done by one treewide patch.
> > > > > > >
> > > > > > > Anyway, I'll do the kretprobe update in the next step as another series.
> > > > > > > (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?

I'll wait for your patch set to see how did you go about it in a new revision.


>
> thanks,
> jirka
>
> >
> > > > > >
> > > > > > libbpf doesn't do anything there. The interface for kprobe is based on
> > > > > > function name and kernel performs name lookups internally to resolve
> > > > > > IP. For fentry it's similar (kernel handles IP resolution), but
> > > > > > instead of function name we specify BTF ID of a function type.
> > > > >
> > > > > Hmm, according to Jiri's original patch, it seems to pass an array of
> > > > > addresses. So I thought that has been resolved by libbpf.
> > > > >
> > > > > +                       struct {
> > > > > +                               __aligned_u64   addrs;
> > > >
> > > > I think this is a pointer to an array of pointers to zero-terminated C strings
> > >
> > > I used direct addresses, because bpftrace already has them, so there was
> > > no point passing strings, I cann add support for that
> >
> > So now both direct address array or symbol array are OK.
> >
> > Thank you,
> >
> > --
> > Masami Hiramatsu <mhiramat@kernel.org>
> >
>
Jiri Olsa Jan. 24, 2022, 10:23 p.m. UTC | #8
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 ;-)

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

jirka
Andrii Nakryiko Jan. 24, 2022, 10:58 p.m. UTC | #9
On Mon, Jan 24, 2022 at 2:23 PM Jiri Olsa <jolsa@redhat.com> 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
>