On Wed, Feb 2, 2022 at 9:24 AM Jiri Olsa <[email protected]> wrote:
>
> On Wed, Feb 02, 2022 at 09:09:53AM -0800, Alexei Starovoitov wrote:
> > On Wed, Feb 2, 2022 at 5:53 AM Jiri Olsa <[email protected]> wrote:
> > >
> > > hi,
> > > this patchset adds new link type BPF_LINK_TYPE_FPROBE that attaches kprobe
> > > program through fprobe API [1] instroduced by Masami.
> >
> > No new prog type please.
> > I thought I made my reasons clear earlier.
> > It's a multi kprobe. Not a fprobe or any other name.
> > The kernel internal names should not leak into uapi.
> >
>
> well it's not new prog type, it's new link type that allows
> to attach kprobe program to multiple functions
>
> the original change used BPF_LINK_TYPE_RAW_KPROBE, which did not
> seem to fit anymore, so I moved to FPROBE, because that's what
> it is ;-)
Now I don't like the fprobe name even more.
Why invent new names? It's an ftrace interface.
> but if you don't want new name in uapi we could make this more
> obvious with link name:
> BPF_LINK_TYPE_MULTI_KPROBE
>
> and bpf_attach_type:
> BPF_TRACE_MULTI_KPROBE
I'd rather get rid of fprobe name first.
On Wed, Feb 02, 2022 at 09:30:21AM -0800, Alexei Starovoitov wrote:
> On Wed, Feb 2, 2022 at 9:24 AM Jiri Olsa <[email protected]> wrote:
> >
> > On Wed, Feb 02, 2022 at 09:09:53AM -0800, Alexei Starovoitov wrote:
> > > On Wed, Feb 2, 2022 at 5:53 AM Jiri Olsa <[email protected]> wrote:
> > > >
> > > > hi,
> > > > this patchset adds new link type BPF_LINK_TYPE_FPROBE that attaches kprobe
> > > > program through fprobe API [1] instroduced by Masami.
> > >
> > > No new prog type please.
> > > I thought I made my reasons clear earlier.
> > > It's a multi kprobe. Not a fprobe or any other name.
> > > The kernel internal names should not leak into uapi.
> > >
> >
> > well it's not new prog type, it's new link type that allows
> > to attach kprobe program to multiple functions
> >
> > the original change used BPF_LINK_TYPE_RAW_KPROBE, which did not
> > seem to fit anymore, so I moved to FPROBE, because that's what
> > it is ;-)
>
> Now I don't like the fprobe name even more.
> Why invent new names? It's an ftrace interface.
how about ftrace_probe ?
>
> > but if you don't want new name in uapi we could make this more
> > obvious with link name:
> > BPF_LINK_TYPE_MULTI_KPROBE
> >
> > and bpf_attach_type:
> > BPF_TRACE_MULTI_KPROBE
>
> I'd rather get rid of fprobe name first.
>
Masami, any idea?
thanks,
jirka
On Thu, 3 Feb 2022 16:06:36 +0100
Jiri Olsa <[email protected]> wrote:
> On Wed, Feb 02, 2022 at 09:30:21AM -0800, Alexei Starovoitov wrote:
> > On Wed, Feb 2, 2022 at 9:24 AM Jiri Olsa <[email protected]> wrote:
> > >
> > > On Wed, Feb 02, 2022 at 09:09:53AM -0800, Alexei Starovoitov wrote:
> > > > On Wed, Feb 2, 2022 at 5:53 AM Jiri Olsa <[email protected]> wrote:
> > > > >
> > > > > hi,
> > > > > this patchset adds new link type BPF_LINK_TYPE_FPROBE that attaches kprobe
> > > > > program through fprobe API [1] instroduced by Masami.
> > > >
> > > > No new prog type please.
> > > > I thought I made my reasons clear earlier.
> > > > It's a multi kprobe. Not a fprobe or any other name.
> > > > The kernel internal names should not leak into uapi.
> > > >
> > >
> > > well it's not new prog type, it's new link type that allows
> > > to attach kprobe program to multiple functions
> > >
> > > the original change used BPF_LINK_TYPE_RAW_KPROBE, which did not
> > > seem to fit anymore, so I moved to FPROBE, because that's what
> > > it is ;-)
> >
> > Now I don't like the fprobe name even more.
> > Why invent new names? It's an ftrace interface.
>
> how about ftrace_probe ?
I thought What Alexei pointed was that don't expose the FPROBE name
to user space. If so, I agree with that. We can continue to use
KPROBE for user space. Using fprobe is just for kernel implementation.
It means that we may better to keep simple mind model (there are only
static event or dynamic kprobe event).
> > > but if you don't want new name in uapi we could make this more
> > > obvious with link name:
> > > BPF_LINK_TYPE_MULTI_KPROBE
> > >
> > > and bpf_attach_type:
> > > BPF_TRACE_MULTI_KPROBE
> >
> > I'd rather get rid of fprobe name first.
> >
>
> Masami, any idea?
Can't we continue to use kprobe prog type for user interface
and internally, if there are multiple kprobes or kretprobes
required, switch to use fprobe?
Thank you,
>
> thanks,
> jirka
>
--
Masami Hiramatsu <[email protected]>
On Thu, Feb 3, 2022 at 4:46 PM Masami Hiramatsu <[email protected]> wrote:
>
> I thought What Alexei pointed was that don't expose the FPROBE name
> to user space. If so, I agree with that. We can continue to use
> KPROBE for user space. Using fprobe is just for kernel implementation.
Clearly that intent is not working.
The "fprobe" name is already leaking outside of the kernel internals.
The module interface is being proposed.
You'd need to document it, etc.
I think it's only causing confusion to users.
The new name serves no additional purpose other than
being new and unheard of.
fprobe is kprobe on ftrace. That's it.
Just call it kprobe on ftrace in api and everywhere.
Please?