Message-ID: <CAEf4BzbCyMWr5tq5i45SB3jPvUFd4zOAYwJG3KBBeaoWmEq8kw@mail.gmail.com>
Date: Mon, 5 Jan 2026 13:20:36 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Menglong Dong <menglong8.dong@...il.com>
Cc: ast@...nel.org, andrii@...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, davem@...emloft.net, 
	dsahern@...nel.org, tglx@...utronix.de, mingo@...hat.com, 
	jiang.biao@...ux.dev, bp@...en8.de, dave.hansen@...ux.intel.com, 
	x86@...nel.org, hpa@...or.com, bpf@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v6 00/10] bpf: fsession support

On Sun, Jan 4, 2026 at 4:28 AM Menglong Dong <menglong8.dong@...il.com> wrote:
>
> Hi, all.
>
> No changes in this version, just a rebase to deal with conflicts.
>
> overall
> -------
> 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.
>
> 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.
>
> 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.
>
> kfunc design
> ------------
> The kfunc bpf_fsession_is_return() and bpf_fsession_cookie() are
> introduced, and they are both inlined in the verifier.
>
> In current solution, we can't reuse the existing bpf_session_cookie() and
> bpf_session_is_return(), as their prototype is different from
> bpf_fsession_is_return() and bpf_fsession_cookie(). In
> bpf_fsession_cookie(), we need the function argument "void *ctx" to get
> the cookie. However, the prototype of bpf_session_cookie() is "void".

How critical is it to inline bpf_session_is_return() and
bpf_session_cookie()? they are not inlined for ksessions, and it's
fine (at least for now). Are we micro-optimizing too early here?

>
> Maybe it's possible to reuse the existing bpf_session_cookie() and
> bpf_session_is_return(). First, we move the nr_regs from stack to struct
> bpf_tramp_run_ctx, as Andrii suggested before. Then, we define the session
> cookies as flexible array in bpf_tramp_run_ctx like this:
>     struct bpf_tramp_run_ctx {
>         struct bpf_run_ctx run_ctx;
>         u64 bpf_cookie;
>         struct bpf_run_ctx *saved_run_ctx;
>         u64 func_meta; /* nr_args, cookie_index, etc */
>         u64 fsession_cookies[];
>     };
>
> The problem of this approach is that we can't inlined the bpf helper
> anymore, such as get_func_arg, get_func_ret, get_func_arg_cnt, etc, as
> we can't use the "current" in BPF assembly.
>

We can, as Alexei suggested on your other patch set. Is this still a
valid concern?

I think having separate duplicated ksession and fsession specific
bpf_[f]session_{is_return,session_cookie}() APIs is really bad and
confusing long-term.

> So maybe it's better to use the new kfunc for now? And I'm analyzing that

there is no "for now", this decision will be with us for a really long time...

> if it is possible to inline "current" in verifier. Maybe we can convert to
> the solution above if it success.
>
> architecture
> ------------
> 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 v5:
> * No changes in this version, just a rebase to deal with conflicts.
>
> Changes since v4:
> * use fsession terminology consistently in all patches
> * 1st patch:
>   - use more explicit way in __bpf_trampoline_link_prog()
> * 4th patch:
>   - remove "cookie_cnt" in struct bpf_trampoline
> * 6th patch:
>   - rename nr_regs to func_md
>   - define cookie_off in a new line
> * 7th patch:
>   - remove the handling of BPF_TRACE_SESSION in legacy fallback path for
>     BPF_RAW_TRACEPOINT_OPEN
>
> 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 (10):
>   bpf: add fsession 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 fsession support for x86_64
>   libbpf: add fsession support
>   selftests/bpf: add testcases for fsession
>   selftests/bpf: add testcases for fsession cookie
>   selftests/bpf: test fsession mixed with fentry and fexit
>
>  arch/x86/net/bpf_jit_comp.c                   |  48 ++++-
>  include/linux/bpf.h                           |  37 ++++
>  include/uapi/linux/bpf.h                      |   1 +
>  kernel/bpf/btf.c                              |   2 +
>  kernel/bpf/syscall.c                          |  18 +-
>  kernel/bpf/trampoline.c                       |  53 ++++-
>  kernel/bpf/verifier.c                         |  76 +++++--
>  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                           |   1 +
>  tools/lib/bpf/libbpf.c                        |   3 +
>  .../selftests/bpf/prog_tests/fsession_test.c  | 115 ++++++++++
>  .../bpf/prog_tests/tracing_failure.c          |   2 +-
>  .../selftests/bpf/progs/fsession_test.c       | 198 ++++++++++++++++++
>  17 files changed, 572 insertions(+), 42 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