From: Masami Hiramatsu <mhiramat@kernel•org>
To: Jiri Olsa <jolsa@redhat•com>
Cc: Alexei Starovoitov <ast@kernel•org>,
Daniel Borkmann <daniel@iogearbox•net>,
Andrii Nakryiko <andrii@kernel•org>,
netdev@vger•kernel.org, 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 00/13] kprobe/bpf: Add support to attach multiple kprobes
Date: Fri, 7 Jan 2022 14:42:47 +0900 [thread overview]
Message-ID: <20220107144247.caf007b0c080de6a26acbf96@kernel.org> (raw)
In-Reply-To: <YdcDRqmgOeOMmyoM@krava>
On Thu, 6 Jan 2022 15:57:10 +0100
Jiri Olsa <jolsa@redhat•com> wrote:
> On Thu, Jan 06, 2022 at 10:59:43PM +0900, Masami Hiramatsu wrote:
>
> SNIP
>
> > > >
> > > > Hmm, I think there may be a time to split the "kprobe as an
> > > > interface for the software breakpoint" and "kprobe as a wrapper
> > > > interface for the callbacks of various instrumentations", like
> > > > 'raw_kprobe'(or kswbp) and 'kprobes'.
> > > > And this may be called as 'fprobe' as ftrace_ops wrapper.
> > > > (But if the bpf is enough flexible, this kind of intermediate layer
> > > > may not be needed, it can use ftrace_ops directly, eventually)
> > > >
> > > > Jiri, have you already considered to use ftrace_ops from the
> > > > bpf directly? Are there any issues?
> > > > (bpf depends on 'kprobe' widely?)
> > >
> > > at the moment there's not ftrace public interface for the return
> > > probe merged in, so to get the kretprobe working I had to use
> > > kprobe interface
> >
> > Yeah, I found that too. We have to ask Steve to salvage it ;)
>
> I got those patches rebased like half a year ago upstream code,
> so should be easy to revive them
Nice! :)
>
> >
> > > but.. there are patches Steven shared some time ago, that do that
> > > and make graph_ops available as kernel interface
> > >
> > > I recall we considered graph_ops interface before as common attach
> > > layer for trampolines, which was bad, but it might actually make
> > > sense for kprobes
> >
> > I started working on making 'fprobe' which will provide multiple
> > function probe with similar interface of kprobes. See attached
> > patch. Then you can use it in bpf, maybe with an union like
> >
> > union {
> > struct kprobe kp; // for function body
> > struct fprobe fp; // for function entry and return
> > };
> >
> > At this moment, fprobe only support entry_handler, but when we
> > re-start the generic graph_ops interface, it is easy to expand
> > to support exit_handler.
> > If this works, I think kretprobe can be phased out, since at that
> > moment, kprobe_event can replace it with the fprobe exit_handler.
> > (This is a benefit of decoupling the instrumentation layer from
> > the event layer. It can choose the best way without changing
> > user interface.)
> >
>
> I can resend out graph_ops patches if you want to base
> it directly on that
Yes, that's very helpful. Now I'm considering to use it (or via fprobe)
from kretprobes like ftrace-based kprobe.
> > > I'll need to check it in more details but I think both graph_ops and
> > > kprobe do about similar thing wrt hooking return probe, so it should
> > > be comparable.. and they are already doing the same for the entry hook,
> > > because kprobe is mostly using ftrace for that
> > >
> > > we would not need to introduce new program type - kprobe programs
> > > should be able to run from ftrace callbacks just fine
> >
> > That seems to bind your mind. The program type is just a programing
> > 'model' of the bpf. You can choose the best implementation to provide
> > equal functionality. 'kprobe' in bpf is just a name that you call some
> > instrumentations which can probe kernel code.
>
> I don't want to introduce new type, there's some dependencies
> in bpf verifier and helpers code we'd need to handle for that
>
> I'm looking for solution for current kprobe bpf program type
> to be registered for multiple addresses quickly
Yes, as I replied to Alex, the bpf program type itself keeps 'kprobe'.
For example, you've introduced bpf_kprobe_link at [8/13],
struct bpf_kprobe_link {
struct bpf_link link;
union {
struct kretprobe rp;
struct fprobe fp;
};
bool is_return;
bool is_fentry;
kprobe_opcode_t **addrs;
u32 cnt;
u64 bpf_cookie;
};
If all "addrs" are function entry, ::fp will be used.
If cnt == 1 then use ::rp.
> > > so we would have:
> > > - kprobe type programs attaching to:
> > > - new BPF_LINK_TYPE_FPROBE link using the graph_ops as attachment layer
> > >
> > > jirka
> > >
> >
> >
> > --
> > Masami Hiramatsu <mhiramat@kernel•org>
>
> > From 269b86597c166d6d4c5dd564168237603533165a Mon Sep 17 00:00:00 2001
> > From: Masami Hiramatsu <mhiramat@kernel•org>
> > Date: Thu, 6 Jan 2022 15:40:36 +0900
> > Subject: [PATCH] fprobe: Add ftrace based probe APIs
> >
> > The fprobe is a wrapper API for ftrace function tracer.
> > Unlike kprobes, this probes only supports the function entry, but
> > it can probe multiple functions by one fprobe. The usage is almost
> > same as the kprobe, user will specify the function names by
> > fprobe::syms, the number of syms by fprobe::nsyms, and the user
> > handler by fprobe::handler.
> >
> > struct fprobe = { 0 };
> > const char *targets[] = {"func1", "func2", "func3"};
> >
> > fprobe.handler = user_handler;
> > fprobe.nsyms = ARRAY_SIZE(targets);
> > fprobe.syms = targets;
> >
> > ret = register_fprobe(&fprobe);
> > ...
> >
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel•org>
> > ---
> > include/linux/fprobes.h | 52 ++++++++++++++++
> > kernel/trace/Kconfig | 10 ++++
> > kernel/trace/Makefile | 1 +
> > kernel/trace/fprobes.c | 128 ++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 191 insertions(+)
> > create mode 100644 include/linux/fprobes.h
> > create mode 100644 kernel/trace/fprobes.c
> >
> > diff --git a/include/linux/fprobes.h b/include/linux/fprobes.h
> > new file mode 100644
> > index 000000000000..22db748bf491
> > --- /dev/null
> > +++ b/include/linux/fprobes.h
> > @@ -0,0 +1,52 @@
> > +#ifndef _LINUX_FPROBES_H
> > +#define _LINUX_FPROBES_H
> > +/* Simple ftrace probe wrapper */
> > +
> > +#include <linux/compiler.h>
> > +#include <linux/ftrace.h>
> > +
> > +struct fprobe {
> > + const char **syms;
> > + unsigned long *addrs;
>
> could you add array of user data for each addr/sym?
OK, something like this?
void **user_data;
But note that you need O(N) to search the entry corresponding to
a specific address. To reduce the overhead, we may need to sort
the array in advance (e.g. when registering it).
>
> SNIP
>
> > +static int populate_func_addresses(struct fprobe *fp)
> > +{
> > + unsigned int i;
> > +
> > + fp->addrs = kmalloc(sizeof(void *) * fp->nsyms, GFP_KERNEL);
> > + if (!fp->addrs)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < fp->nsyms; i++) {
> > + fp->addrs[i] = kallsyms_lookup_name(fp->syms[i]);
> > + if (!fp->addrs[i]) {
> > + kfree(fp->addrs);
> > + fp->addrs = NULL;
> > + return -ENOENT;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * register_fprobe - Register fprobe to ftrace
> > + * @fp: A fprobe data structure to be registered.
> > + *
> > + * This expects the user set @fp::syms or @fp::addrs (not both),
> > + * @fp::nsyms (number of entries of @fp::syms or @fp::addrs) and
> > + * @fp::handler. Other fields are initialized by this function.
> > + */
> > +int register_fprobe(struct fprobe *fp)
> > +{
> > + unsigned int i;
> > + int ret;
> > +
> > + if (!fp)
> > + return -EINVAL;
> > +
> > + if (!fp->nsyms || (!fp->syms && !fp->addrs) || (fp->syms && fp->addrs))
> > + return -EINVAL;
> > +
> > + if (fp->syms) {
> > + ret = populate_func_addresses(fp);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + fp->ftrace.func = fprobe_handler;
> > + fp->ftrace.flags = FTRACE_OPS_FL_SAVE_REGS;
> > +
> > + for (i = 0; i < fp->nsyms; i++) {
> > + ret = ftrace_set_filter_ip(&fp->ftrace, fp->addrs[i], 0, 0);
> > + if (ret < 0)
> > + goto error;
> > + }
>
> I introduced ftrace_set_filter_ips, because loop like above was slow:
> https://lore.kernel.org/bpf/20211118112455.475349-4-jolsa@kernel.org/
Ah, thanks for noticing!
Thank you,
>
> thanks,
> jirka
>
> > +
> > + fp->nmissed = 0;
> > + ret = register_ftrace_function(&fp->ftrace);
> > + if (!ret)
> > + return ret;
> > +
> > +error:
> > + if (fp->syms) {
> > + kfree(fp->addrs);
> > + fp->addrs = NULL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * unregister_fprobe - Unregister fprobe from ftrace
> > + * @fp: A fprobe data structure to be unregistered.
> > + */
> > +int unregister_fprobe(struct fprobe *fp)
> > +{
> > + int ret;
> > +
> > + if (!fp)
> > + return -EINVAL;
> > +
> > + if (!fp->nsyms || !fp->addrs)
> > + return -EINVAL;
> > +
> > + ret = unregister_ftrace_function(&fp->ftrace);
> > +
> > + if (fp->syms) {
> > + /* fp->addrs is allocated by register_fprobe() */
> > + kfree(fp->addrs);
> > + fp->addrs = NULL;
> > + }
> > +
> > + return ret;
> > +}
> > --
> > 2.25.1
> >
>
--
Masami Hiramatsu <mhiramat@kernel•org>
next prev parent reply other threads:[~2022-01-07 5:43 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-04 8:09 [RFC 00/13] kprobe/bpf: Add support to attach multiple kprobes Jiri Olsa
2022-01-04 8:09 ` [PATCH 01/13] ftrace: Add ftrace_set_filter_ips function Jiri Olsa
2022-01-04 8:09 ` [PATCH 02/13] kprobe: Keep traced function address Jiri Olsa
2022-01-05 14:32 ` Masami Hiramatsu
2022-01-06 8:30 ` Jiri Olsa
2022-01-06 4:30 ` Andrii Nakryiko
2022-01-06 8:31 ` Jiri Olsa
2022-01-04 8:09 ` [PATCH 03/13] kprobe: Add support to register multiple ftrace kprobes Jiri Olsa
2022-01-05 15:00 ` Masami Hiramatsu
2022-01-04 8:09 ` [PATCH 04/13] kprobe: Add support to register multiple ftrace kretprobes Jiri Olsa
2022-01-04 8:09 ` [PATCH 05/13] kprobe: Allow to get traced function address for multi ftrace kprobes Jiri Olsa
2022-01-06 4:30 ` Andrii Nakryiko
2022-01-04 8:09 ` [PATCH 06/13] samples/kprobes: Add support for multi kprobe interface Jiri Olsa
2022-01-04 8:09 ` [PATCH 07/13] samples/kprobes: Add support for multi kretprobe interface Jiri Olsa
2022-01-04 8:09 ` [PATCH 08/13] bpf: Add kprobe link for attaching raw kprobes Jiri Olsa
2022-01-06 4:30 ` Andrii Nakryiko
2022-01-06 8:41 ` Jiri Olsa
2022-01-06 16:32 ` Alexei Starovoitov
2022-01-06 21:53 ` Andrii Nakryiko
2022-01-04 8:09 ` [PATCH 09/13] libbpf: Add libbpf__kallsyms_parse function Jiri Olsa
2022-01-04 8:09 ` [PATCH 10/13] libbpf: Add bpf_link_create support for multi kprobes Jiri Olsa
2022-01-04 8:09 ` [PATCH 11/13] libbpf: Add bpf_program__attach_kprobe_opts " Jiri Olsa
2022-01-04 8:09 ` [PATCH 12/13] selftest/bpf: Add raw kprobe attach test Jiri Olsa
2022-01-04 8:09 ` [PATCH 13/13] selftest/bpf: Add bpf_cookie test for raw_k[ret]probe Jiri Olsa
2022-01-04 18:53 ` [RFC 00/13] kprobe/bpf: Add support to attach multiple kprobes Alexei Starovoitov
2022-01-05 9:15 ` Jiri Olsa
2022-01-05 15:24 ` Masami Hiramatsu
2022-01-06 8:29 ` Jiri Olsa
2022-01-06 13:59 ` Masami Hiramatsu
2022-01-06 14:57 ` Jiri Olsa
2022-01-07 5:42 ` Masami Hiramatsu [this message]
2022-01-06 15:02 ` Steven Rostedt
2022-01-06 17:40 ` Alexei Starovoitov
2022-01-06 23:52 ` Masami Hiramatsu
2022-01-07 0:20 ` Alexei Starovoitov
2022-01-07 12:55 ` Masami Hiramatsu
2022-01-11 15:00 ` [RFC PATCH 0/6] fprobe: Introduce fprobe function entry/exit probe Masami Hiramatsu
2022-01-11 15:00 ` [RFC PATCH 1/6] fprobe: Add ftrace based probe APIs Masami Hiramatsu
2022-01-11 15:00 ` [RFC PATCH 2/6] rethook: Add a generic return hook Masami Hiramatsu
2022-01-11 15:00 ` [RFC PATCH 3/6] rethook: x86: Add rethook x86 implementation Masami Hiramatsu
2022-01-11 15:01 ` [RFC PATCH 4/6] fprobe: Add exit_handler support Masami Hiramatsu
2022-01-11 15:01 ` [RFC PATCH 5/6] fprobe: Add sample program for fprobe Masami Hiramatsu
2022-01-11 15:01 ` [RFC PATCH 6/6] bpf: Add kprobe link for attaching raw kprobes Masami Hiramatsu
2022-01-11 22:39 ` [RFC PATCH 0/6] fprobe: Introduce fprobe function entry/exit probe Alexei Starovoitov
2022-01-12 7:33 ` Masami Hiramatsu
2022-01-12 11:08 ` Masami Hiramatsu
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=20220107144247.caf007b0c080de6a26acbf96@kernel.org \
--to=mhiramat@kernel$(echo .)org \
--cc=andrii@kernel$(echo .)org \
--cc=anil.s.keshavamurthy@intel$(echo .)com \
--cc=ast@kernel$(echo .)org \
--cc=bpf@vger$(echo .)kernel.org \
--cc=daniel@iogearbox$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=john.fastabend@gmail$(echo .)com \
--cc=jolsa@redhat$(echo .)com \
--cc=kafai@fb$(echo .)com \
--cc=kpsingh@chromium$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=naveen.n.rao@linux$(echo .)ibm.com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=rostedt@goodmis$(echo .)org \
--cc=songliubraving@fb$(echo .)com \
--cc=yhs@fb$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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