Re: [PATCH RFC bpf-next 0/4] bpf: Add support to attach return prog in kprobe multi

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Feb 19, 2024 at 12:20:08PM +0100, Viktor Malik wrote:
> On 2/13/24 22:09, Jiri Olsa wrote:
> > On Tue, Feb 13, 2024 at 10:20:46AM -0800, Andrii Nakryiko wrote:
> >> On Tue, Feb 13, 2024 at 4:09 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >>>
> >>> On Mon, Feb 12, 2024 at 08:06:06PM -0800, Andrii Nakryiko wrote:
> >>>
> >>> SNIP
> >>>
> >>>>>> But the way you implement it with extra flag and extra fd parameter
> >>>>>> makes it harder to have a nice high-level support in libbpf (and
> >>>>>> presumably other BPF loader libraries) for this.
> >>>>>>
> >>>>>> When I was thinking about doing something like this, I was considering
> >>>>>> adding a new program type, actually. That way it's possible to define
> >>>>>> this "let's skip return probe" protocol without backwards
> >>>>>> compatibility concerns. It's easier to use it declaratively in libbpf.
> >>>>>
> >>>>> ok, that seems cleaner.. but we need to use current kprobe programs,
> >>>>> so not sure at the moment how would that fit in.. did you mean new
> >>>>> link type?
> >>>>
> >>>> It's kind of a less important detail, actually. New program type would
> >>>> allow us to have an entirely different context type, but I think we
> >>>> can make do with the existing kprobe program type. We can have a
> >>>> separate attach_type and link type, just like multi-kprobe and
> >>>> multi-uprobe are still kprobe programs.
> >>>
> >>> ok, having new attach type on top of kprobe_multi link makes sense
> >>>
> >>>>
> >>>>>
> >>>>>> You just declare SEC("kprobe.wrap/...") (or whatever the name,
> >>>>>> something to designate that it's both entry and exit probe) as one
> >>>>>> program and in the code there would be some way to determine whether
> >>>>>> we are in entry mode or exit mode (helper or field in the custom
> >>>>>> context type, the latter being faster and more usable, but it's
> >>>>>> probably not critical).
> >>>>>
> >>>>> hum, so the single program would be for both entry and exit probe,
> >>>>> I'll need to check how bad it'd be for us, but it'd probably mean
> >>>>> just one extra tail call, so it's likely ok
> >>>>
> >>>> I guess, I don't know what you are doing there :) I'd recommend
> >>>> looking at utilizing BPF global subprogs instead of tail calls, if
> >>>> your kernel allows for that, as that's a saner way to scale BPF
> >>>> verification.
> >>>
> >>> ok, we should probably do that.. given this enhancement will be
> >>> available on latest kernel anyway, we could use global subprogs
> >>> as well
> >>>
> >>> the related bpftrace might be bit more challenging.. will have to
> >>> generate program calling entry or return program now, but seems
> >>> doable of course
> >>
> >> So you want users to still have separate kprobe and kretprobe in
> >> bpftrace, but combine them into this kwrapper transparently? It does
> > 
> > no I meant I'd need to generate the wrapper program for the new
> > interface.. which is extra compared to current bpftrace changes
> 
> If you end up introducing this new kwrapper program type in libbpf, I
> think that it'll make sense to have something similar in bpftrace, too.
> Allowing users to write separate kprobe and kretprobe programs and
> transparently combining them into kwrapper doesn't seem to bring much
> value to me.

I kind of liked the idea of not introducing new probe type and silently
making things faster ;-) but having wrapper probe type makes also sense

> 
> Jirka, if you need help with implementing bpftrace support for this, let
> me know. I'm very interested in having this capability there.

that'd be great, I'll send new version of kernel changes soon

thanks,
jirka