Message-ID: <CAEf4BzYm3=zzmCRg3zr1F99sBkxEZ_pDgjtKMBurb9LGu6JJKQ@mail.gmail.com>
Date: Fri, 19 Dec 2025 08:55:28 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Menglong Dong <menglong.dong@...ux.dev>
Cc: Menglong Dong <menglong8.dong@...il.com>, ast@...nel.org, andrii@...nel.org, 
	davem@...emloft.net, dsahern@...nel.org, daniel@...earbox.net, 
	martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org, 
	sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, tglx@...utronix.de, 
	mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, 
	hpa@...or.com, netdev@...r.kernel.org, bpf@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 0/9] bpf: tracing session supporting

On Thu, Dec 18, 2025 at 5:18 PM Menglong Dong <menglong.dong@...ux.dev> wrote:
>
> On 2025/12/19 08:55 Andrii Nakryiko <andrii.nakryiko@...il.com> write:
> > On Wed, Dec 17, 2025 at 1:54 AM Menglong Dong <menglong8.dong@...il.com> wrote:
> > >
> > > Hi, all.
> > >
> > > In this version, I combined Alexei and Andrii's advice, which makes the
> > > architecture specific code much simpler.
> > >
> > > Sometimes, we need to hook both the entry and exit of a function with
> > > TRACING. Therefore, we need define a FENTRY and a FEXIT for the target
> > > function, which is not convenient.
> > >
> > > Therefore, we add a tracing session support for TRACING. Generally
> > > speaking, it's similar to kprobe session, which can hook both the entry
> > > and exit of a function with a single BPF program. Session cookie is also
> > > supported with the kfunc bpf_fsession_cookie(). In order to limit the
> > > stack usage, we limit the maximum number of cookies to 4.
> > >
> > > The kfunc bpf_fsession_is_return() and bpf_fsession_cookie() are both
> > > inlined in the verifier.
> >
> > We have generic bpf_session_is_return() and bpf_session_cookie() (that
> > currently works for ksession), can't you just implement them for the
> > newly added program type instead of adding type-specific kfuncs?
>
> Hi, Andrii. I tried and found that it's a little hard to reuse them. The
> bpf_session_is_return() and bpf_session_cookie() are defined as kfunc, which
> makes we can't implement different functions for different attach type, like
> what bpf helper does.

Are you sure? We certainly support kfunc implementation specialization
for sleepable vs non-sleepable BPF programs. Check specialize_kfunc()
in verifier.c

>
> The way we store "is_return" and "cookie" in fsession is different with
> ksession. For ksession, it store the "is_return" in struct bpf_session_run_ctx.
> Even if we move the "nr_regs" from stack to struct bpf_tramp_run_ctx,
> it's still hard to reuse the bpf_session_is_return() or bpf_session_cookie(),
> as the way of storing the "is_return" and "cookie" in fsession and ksession
> is different, and it's a little difficult and complex to unify them.

I'm not saying we should unify the implementation, you have to
implement different version of logically the same kfunc, of course.

>
> What's more, we will lose the advantage of inline bpf_fsession_is_return
> and bpf_fsession_cookie in verifier.
>

I'd double check that either. BPF verifier and JIT do know program
type, so you can pick how to inline
bpf_session_is_return()/bpf_session_cookie() based on that.

> I'll check more to see if there is a more simple way to reuse them.
>
> Thanks!
> Menglong Dong
>
> >
> > >
> > > We allow the usage of bpf_get_func_ret() to get the return value in the
> > > fentry of the tracing session, as it will always get "0", which is safe
> > > enough and is OK. Maybe we can prohibit the usage of bpf_get_func_ret()
> > > in the fentry in verifier, which can make the architecture specific code
> > > simpler.
> > >
> > > The fsession stuff is arch related, so the -EOPNOTSUPP will be returned if
> > > it is not supported yet by the arch. In this series, we only support
> > > x86_64. And later, other arch will be implemented.
> > >
> > > Changes since v3:
> > > * instead of adding a new hlist to progs_hlist in trampoline, add the bpf
> > >   program to both the fentry hlist and the fexit hlist.
> > > * introduce the 2nd patch to reuse the nr_args field in the stack to
> > >   store all the information we need(except the session cookies).
> > > * limit the maximum number of cookies to 4.
> > > * remove the logic to skip fexit if the fentry return non-zero.
> > >
> > > Changes since v2:
> > > * squeeze some patches:
> > >   - the 2 patches for the kfunc bpf_tracing_is_exit() and
> > >     bpf_fsession_cookie() are merged into the second patch.
> > >   - the testcases for fsession are also squeezed.
> > >
> > > * fix the CI error by move the testcase for bpf_get_func_ip to
> > >   fsession_test.c
> > >
> > > Changes since v1:
> > > * session cookie support.
> > >   In this version, session cookie is implemented, and the kfunc
> > >   bpf_fsession_cookie() is added.
> > >
> > > * restructure the layout of the stack.
> > >   In this version, the session stuff that stored in the stack is changed,
> > >   and we locate them after the return value to not break
> > >   bpf_get_func_ip().
> > >
> > > * testcase enhancement.
> > >   Some nits in the testcase that suggested by Jiri is fixed. Meanwhile,
> > >   the testcase for get_func_ip and session cookie is added too.
> > >
> > > Menglong Dong (9):
> > >   bpf: add tracing session support
> > >   bpf: use last 8-bits for the nr_args in trampoline
> > >   bpf: add the kfunc bpf_fsession_is_return
> > >   bpf: add the kfunc bpf_fsession_cookie
> > >   bpf,x86: introduce emit_st_r0_imm64() for trampoline
> > >   bpf,x86: add tracing session supporting for x86_64
> > >   libbpf: add support for tracing session
> > >   selftests/bpf: add testcases for tracing session
> > >   selftests/bpf: test fsession mixed with fentry and fexit
> > >
> > >  arch/x86/net/bpf_jit_comp.c                   |  47 +++-
> > >  include/linux/bpf.h                           |  39 +++
> > >  include/uapi/linux/bpf.h                      |   1 +
> > >  kernel/bpf/btf.c                              |   2 +
> > >  kernel/bpf/syscall.c                          |  18 +-
> > >  kernel/bpf/trampoline.c                       |  50 +++-
> > >  kernel/bpf/verifier.c                         |  75 ++++--
> > >  kernel/trace/bpf_trace.c                      |  56 ++++-
> > >  net/bpf/test_run.c                            |   1 +
> > >  net/core/bpf_sk_storage.c                     |   1 +
> > >  tools/bpf/bpftool/common.c                    |   1 +
> > >  tools/include/uapi/linux/bpf.h                |   1 +
> > >  tools/lib/bpf/bpf.c                           |   2 +
> > >  tools/lib/bpf/libbpf.c                        |   3 +
> > >  .../selftests/bpf/prog_tests/fsession_test.c  |  90 +++++++
> > >  .../bpf/prog_tests/tracing_failure.c          |   2 +-
> > >  .../selftests/bpf/progs/fsession_test.c       | 226 ++++++++++++++++++
> > >  17 files changed, 571 insertions(+), 44 deletions(-)
> > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/fsession_test.c
> > >  create mode 100644 tools/testing/selftests/bpf/progs/fsession_test.c
> > >
> > > --
> > > 2.52.0
> > >
> >
>
>
>
>

Powered by blists - more mailing lists