Re: [PATCH bpf-next 0/7] bpf: Introduce kprobe_multi session attach

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Apr 23, 2024 at 10:12:07PM -0700, John Fastabend wrote:
> Andrii Nakryiko wrote:
> > On Mon, Apr 22, 2024 at 5:12 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > hi,
> > > adding support to attach kprobe program through kprobe_multi link
> > > in a session mode, which means:
> > >   - program is attached to both function entry and return
> > >   - entry program can decided if the return program gets executed
> > >   - entry program can share u64 cookie value with return program
> > >
> > > The initial RFC for this was posted in [0] and later discussed more
> > > and which ended up with the session idea [1]
> > >
> > > Having entry together with return probe for given function is common
> > > use case for tetragon, bpftrace and most likely for others.
> > >
> > > At the moment if we want both entry and return probe to execute bpf
> > > program we need to create two (entry and return probe) links. The link
> > > for return probe creates extra entry probe to setup the return probe.
> > > The extra entry probe execution could be omitted if we had a way to
> > > use just single link for both entry and exit probe.
> > >
> > > In addition the possibility to control the return program execution
> > > and sharing data within entry and return probe allows for other use
> > > cases.
> > >
> > > Changes from last RFC version [1]:
> > >   - changed wrapper name to session
> > >   - changed flag to adding new attach type for session:
> > >       BPF_TRACE_KPROBE_MULTI_SESSION
> > >     it's more convenient wrt filtering on kfuncs setup and seems
> > >     to make more sense alltogether
> > >   - renamed bpf_kprobe_multi_is_return to bpf_session_is_return
> > >   - added bpf_session_cookie kfunc, which actually already works
> > >     on current fprobe implementation (not just fprobe-on-fgraph)
> > >     and it provides the shared data between entry/return probes [Andrii]
> > >
> > >     we could actually make the cookie size configurable.. thoughts?
> > >     (it's 8 bytes atm)
> > >
> > 
> > Attach cookie is fixed at 8 bytes and that works pretty well. I think
> > beyond 8 bytes there is no clearly "right" size. A common case would
> > be to capture arguments in kprobe to handle in kretprobe, and there
> > you might need at least 40+ bytes, which seems wasteful. So I want to
> > say that it's probably good to hard-code it to just 8 bytes (enough to
> > store timestamp and you can even fit in some flags if you reduce
> > timestamp precision from nanoseconds to microseconds), or use it as an
> > index into array or some other data structure.
> > 
> > let's keep it simple?
> 
> +1 for keeping it simple. Use it as a key if you need more.

ok, it's true then it'd be tricky wrt verifier being able to check
the proper size and it does work with attach cookie so far

jirka