Message-ID: <CAEf4BzZX+hDww=MvMrqx3h9uKdB+Sef8rVMxrvxcm+9jj7MUjg@mail.gmail.com>
Date: Wed, 21 Jan 2026 16:09:56 -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 v10 00/12] bpf: fsession support

On Thu, Jan 15, 2026 at 3:23 AM Menglong Dong <menglong8.dong@...il.com> wrote:
>
> Hi, all.
>
> In this version, I followed Andrii's suggestions in v9, and did many
> adjustment.
>
> 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_session_cookie().
> In order to limit the stack usage, we limit the maximum number of cookies
> to 4.
>
> kfunc design
> ------------
> In order to keep consistency with existing kfunc, we don't introduce new
> kfunc for fsession. Instead, we reuse the existing kfunc
> bpf_session_cookie() and bpf_session_is_return().
>
> The prototype of bpf_session_cookie() and bpf_session_is_return() don't
> satisfy our needs, so we change their prototype by adding the argument
> "void *ctx" to them.
>
> We inline bpf_session_cookie() and bpf_session_is_return() for fsession
> in the verifier directly. Therefore, we don't need to introduce new
> functions for them.
>
> 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 v9 -> v10:
> * 1st patch: some small adjustment, such as use switch in
>   bpf_prog_has_trampoline()
> * 2nd patch: some adjustment to the commit log and comment
> * 3rd patch:
>   - drop the declaration of bpf_session_is_return() and
>     bpf_session_cookie()
>   - use vmlinux.h instead of bpf_kfuncs.h in uprobe_multi_session.c,
>     kprobe_multi_session_cookie.c and uprobe_multi_session_cookie.c
> * 4th patch:
>   - some adjustment to the comment and commit log
>   - rename the prefix from BPF_TRAMP_M_ tp BPF_TRAMP_SHIFT_

This is minor nit, but I think BPF_TRAMP_IS_RETURN_SHIFT and
BPF_TRAMP_COOKIE_INDEX_SHIFT (note added INDEX, because that's what we
store, not the cookie itself) makes more sense naming-wise and is more
natural to read. Consider updating this.

>   - remove the definition of BPF_TRAMP_M_NR_ARGS
>   - check the program type in bpf_session_filter()
> * 5th patch: some adjustment to the commit log
> * 6th patch:
>   - add the "reg" to the function arguments of emit_store_stack_imm64()
>   - use the positive offset in emit_store_stack_imm64()
> * 7th patch:
>   - use "|" for func_meta instead of "+"
>   - pass the "func_meta_off" to invoke_bpf() explicitly, instead of
>     computing it with "stack_size + 8"
>   - pass the "cookie_off" to invoke_bpf() instead of computing the current
>     cookie index with "func_meta"
> * 8th patch:
>   - split the modification to bpftool to a separate patch
> * v9: https://lore.kernel.org/bpf/20260110141115.537055-1-dongml2@chinatelecom.cn/
>

Overall this looks great, I was actually trying to apply and push it
despite those minor nits I left. But unfortunately there is some
conflict, so you'll have to rebase and resend. Hopefully just one last
time :)

> Changes v8 -> v9:
> * remove the definition of bpf_fsession_cookie and bpf_fsession_is_return
>   in the 4th and 5th patch
> * rename emit_st_r0_imm64() to emit_store_stack_imm64() in the 6th patch
> * v8: https://lore.kernel.org/bpf/20260108022450.88086-1-dongml2@chinatelecom.cn/
>
> Changes v7 -> v8:
> * use the last byte of nr_args for bpf_get_func_arg_cnt() in the 2nd patch
> * v7: https://lore.kernel.org/bpf/20260107064352.291069-1-dongml2@chinatelecom.cn/
>
> Changes v6 -> v7:
> * change the prototype of bpf_session_cookie() and bpf_session_is_return(),
>   and reuse them instead of introduce new kfunc for fsession.
> * v6: https://lore.kernel.org/bpf/20260104122814.183732-1-dongml2@chinatelecom.cn/
>
> Changes v5 -> v6:
> * No changes in this version, just a rebase to deal with conflicts.
> * v5: https://lore.kernel.org/bpf/20251224130735.201422-1-dongml2@chinatelecom.cn/
>
> Changes v4 -> v5:
> * 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
> * v4: https://lore.kernel.org/bpf/20251217095445.218428-1-dongml2@chinatelecom.cn/
>
> Changes v3 -> v4:
> * 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.
> * v3: https://lore.kernel.org/bpf/20251026030143.23807-1-dongml2@chinatelecom.cn/
>
> Changes v2 -> v3:
> * 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
> * v2: https://lore.kernel.org/bpf/20251022080159.553805-1-dongml2@chinatelecom.cn/
>
> Changes v1 -> v2:
> * 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.
> * v1: https://lore.kernel.org/bpf/20251018142124.783206-1-dongml2@chinatelecom.cn/
>
> Menglong Dong (12):
>   bpf: add fsession support
>   bpf: use the least significant byte for the nr_args in trampoline
>   bpf: change prototype of bpf_session_{cookie,is_return}
>   bpf: support fsession for bpf_session_is_return
>   bpf: support fsession for bpf_session_cookie
>   bpf,x86: introduce emit_store_stack_imm64() for trampoline
>   bpf,x86: add fsession support for x86_64
>   libbpf: add fsession support
>   bpftool: 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                   |  71 +++++--
>  include/linux/bpf.h                           |  36 ++++
>  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                         |  86 +++++++--
>  kernel/trace/bpf_trace.c                      |  49 +++--
>  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 +
>  tools/testing/selftests/bpf/bpf_kfuncs.h      |   3 -
>  .../selftests/bpf/prog_tests/fsession_test.c  |  90 +++++++++
>  .../bpf/prog_tests/tracing_failure.c          |   2 +-
>  .../selftests/bpf/progs/fsession_test.c       | 179 ++++++++++++++++++
>  .../bpf/progs/kprobe_multi_session_cookie.c   |  15 +-
>  .../bpf/progs/uprobe_multi_session.c          |   7 +-
>  .../bpf/progs/uprobe_multi_session_cookie.c   |  15 +-
>  .../progs/uprobe_multi_session_recursive.c    |  11 +-
>  22 files changed, 550 insertions(+), 96 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