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

[John Fastabend <john.fastabend@gmail.com>]

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.