Message-Id: <20220106225943.87701fcc674202dc3e172289@kernel.org>
Date:   Thu, 6 Jan 2022 22:59:43 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [RFC 00/13] kprobe/bpf: Add support to attach multiple kprobes

On Thu, 6 Jan 2022 09:29:02 +0100
Jiri Olsa <jolsa@...hat.com> wrote:

> On Thu, Jan 06, 2022 at 12:24:35AM +0900, Masami Hiramatsu wrote:
> > On Tue,  4 Jan 2022 09:09:30 +0100
> > Jiri Olsa <jolsa@...hat.com> wrote:
> > 
> > > hi,
> > > adding support to attach multiple kprobes within single syscall
> > > and speed up attachment of many kprobes.
> > > 
> > > The previous attempt [1] wasn't fast enough, so coming with new
> > > approach that adds new kprobe interface.
> > 
> > Yes, since register_kprobes() just registers multiple kprobes on
> > array. This is designed for dozens of kprobes.
> > 
> > > The attachment speed of of this approach (tested in bpftrace)
> > > is now comparable to ftrace tracer attachment speed.. fast ;-)
> > 
> > Yes, because that if ftrace, not kprobes.
> > 
> > > The limit of this approach is forced by using ftrace as attach
> > > layer, so it allows only kprobes on function's entry (plus
> > > return probes).
> > 
> > Note that you also need to multiply the number of instances.
> > 
> > > 
> > > This patchset contains:
> > >   - kprobes support to register multiple kprobes with current
> > >     kprobe API (patches 1 - 8)
> > >   - bpf support ot create new kprobe link allowing to attach
> > >     multiple addresses (patches 9 - 14)
> > > 
> > > We don't need to care about multiple probes on same functions
> > > because it's taken care on the ftrace_ops layer.
> > 
> > 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 ;)

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

Thank you,

> 
> 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@...nel.org>

View attachment "0001-fprobe-Add-ftrace-based-probe-APIs.patch" of type "text/x-diff" (6394 bytes)

Powered by blists - more mailing lists