Date:   Thu, 6 Jan 2022 09:29:02 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Masami Hiramatsu <mhiramat@kernel.org>
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
Message-ID: <YdaoTuWjEeT33Zzm@krava>
References: <20220104080943.113249-1-jolsa@kernel.org>
 <20220106002435.d73e4010c93462fbee9ef074@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220106002435.d73e4010c93462fbee9ef074@kernel.org>
Precedence: bulk

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@redhat.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

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

so we would have:
  - kprobe type programs attaching to:
  - new BPF_LINK_TYPE_FPROBE link using the graph_ops as attachment layer

jirka