From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>, Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
lkml <linux-kernel@vger.kernel.org>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@chromium.org>,
Steven Rostedt <rostedt@goodmis.org>,
"Naveen N . Rao" <naveen.n.rao@linux.ibm.com>,
Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
"David S . Miller" <davem@davemloft.net>
Subject: Re: [RFC PATCH v3 0/9] fprobe: Introduce fprobe function entry/exit probe
Date: Fri, 21 Jan 2022 09:29:00 -0800 [thread overview]
Message-ID: <CAEf4Bza0eTft2kjcm9HhKpAm=AuXnGwZfZ+sYpVVBvj93PBreQ@mail.gmail.com> (raw)
In-Reply-To: <20220121135510.7cfa6540e31824aa39b1c1b8@kernel.org>
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>
next prev parent reply other threads:[~2022-01-21 17:29 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-19 14:56 [RFC PATCH v3 0/9] fprobe: Introduce fprobe function entry/exit probe Masami Hiramatsu
2022-01-19 14:56 ` [RFC PATCH v3 1/9] ftrace: Add ftrace_set_filter_ips function Masami Hiramatsu
2022-01-19 14:56 ` [RFC PATCH v3 2/9] fprobe: Add ftrace based probe APIs Masami Hiramatsu
2022-01-19 14:57 ` [RFC PATCH v3 3/9] rethook: Add a generic return hook Masami Hiramatsu
2022-01-19 14:57 ` [RFC PATCH v3 4/9] rethook: x86: Add rethook x86 implementation Masami Hiramatsu
2022-01-19 14:57 ` [RFC PATCH v3 5/9] fprobe: Add exit_handler support Masami Hiramatsu
2022-01-19 14:57 ` [RFC PATCH v3 6/9] fprobe: Add sample program for fprobe Masami Hiramatsu
2022-01-19 14:57 ` [RFC PATCH v3 7/9] bpf: Add kprobe link for attaching raw kprobes Masami Hiramatsu
2022-01-20 10:44 ` Jiri Olsa
2022-01-20 12:15 ` Masami Hiramatsu
2022-01-19 14:58 ` [RFC PATCH v3 8/9] [DO NOT MERGE] Out-of-tree: Support wildcard symbol option to sample Masami Hiramatsu
2022-01-19 14:58 ` [RFC PATCH v3 9/9] [DO NOT MERGE] out-of-tree: kprobes: Use rethook for kretprobe Masami Hiramatsu
2022-01-20 22:24 ` [RFC PATCH v3 0/9] fprobe: Introduce fprobe function entry/exit probe Andrii Nakryiko
2022-01-21 4:55 ` Masami Hiramatsu
2022-01-21 17:29 ` Andrii Nakryiko [this message]
2022-01-23 23:50 ` Jiri Olsa
2022-01-24 0:24 ` Masami Hiramatsu
2022-01-24 12:21 ` Jiri Olsa
2022-01-24 20:22 ` Andrii Nakryiko
2022-01-24 22:23 ` Jiri Olsa
2022-01-24 22:58 ` Andrii Nakryiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEf4Bza0eTft2kjcm9HhKpAm=AuXnGwZfZ+sYpVVBvj93PBreQ@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=anil.s.keshavamurthy@intel.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=john.fastabend@gmail.com \
--cc=jolsa@redhat.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=naveen.n.rao@linux.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).