[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzY3=qjfX385teDBs7G4Ae8LqFKwX0qMmDnSkkLi5qiWBg@mail.gmail.com>
Date: Thu, 18 Dec 2025 16:55:30 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Menglong Dong <menglong8.dong@...il.com>
Cc: 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 1/9] bpf: add tracing session support
On Wed, Dec 17, 2025 at 1:55 AM Menglong Dong <menglong8.dong@...il.com> wrote:
>
> The tracing session is something that similar to kprobe session. It allow
> to attach a single BPF program to both the entry and the exit of the
> target functions.
>
> Introduce the struct bpf_fsession_link, which allows to add the link to
> both the fentry and fexit progs_hlist of the trampoline.
>
> Signed-off-by: Menglong Dong <dongml2@...natelecom.cn>
> Co-developed-by: Leon Hwang <leon.hwang@...ux.dev>
> Signed-off-by: Leon Hwang <leon.hwang@...ux.dev>
> ---
> 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.
> ---
> include/linux/bpf.h | 20 +++++++++++
> include/uapi/linux/bpf.h | 1 +
> kernel/bpf/btf.c | 2 ++
> kernel/bpf/syscall.c | 18 +++++++++-
> kernel/bpf/trampoline.c | 36 +++++++++++++++----
> kernel/bpf/verifier.c | 12 +++++--
> net/bpf/test_run.c | 1 +
> net/core/bpf_sk_storage.c | 1 +
> tools/include/uapi/linux/bpf.h | 1 +
> .../bpf/prog_tests/tracing_failure.c | 2 +-
> 10 files changed, 83 insertions(+), 11 deletions(-)
>
[...]
> int bpf_prog_ctx_arg_info_init(struct bpf_prog *prog,
> const struct bpf_ctx_arg_aux *info, u32 cnt);
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 84ced3ed2d21..696a7d37db0e 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1145,6 +1145,7 @@ enum bpf_attach_type {
> BPF_NETKIT_PEER,
> BPF_TRACE_KPROBE_SESSION,
> BPF_TRACE_UPROBE_SESSION,
> + BPF_TRACE_SESSION,
FSESSION for consistency with FENTRY and FEXIT
> __MAX_BPF_ATTACH_TYPE
> };
>
[...]
> {
> - enum bpf_tramp_prog_type kind;
> - struct bpf_tramp_link *link_exiting;
> + enum bpf_tramp_prog_type kind, okind;
> + struct bpf_tramp_link *link_existing;
> + struct bpf_fsession_link *fslink;
> int err = 0;
> int cnt = 0, i;
>
> - kind = bpf_attach_type_to_tramp(link->link.prog);
> + okind = kind = bpf_attach_type_to_tramp(link->link.prog);
> if (tr->extension_prog)
> /* cannot attach fentry/fexit if extension prog is attached.
> * cannot overwrite extension prog either.
> @@ -621,13 +624,18 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> BPF_MOD_JUMP, NULL,
> link->link.prog->bpf_func);
> }
> + if (kind == BPF_TRAMP_SESSION) {
> + /* deal with fsession as fentry by default */
> + kind = BPF_TRAMP_FENTRY;
> + cnt++;
> + }
this "pretend we are BPF_TRAMP_FENTRY" looks a bit hacky and is very
hard to follow. I think it would be cleaner to have explicit small
special cases for BPF_TRAMP_SESSION, and then generalize
hlist_for_each_entry case by using a local variable for storing
&tr->progs_hlist[kind] (which for TRAMP_SESSION you'll set to
&tr->progs_hlist[BPF_TRAMP_FENTRY]). You'll then just do extra
hlist_add_head/hlist_del_init and count manipulation. IMO, it's better
than keeping in head what kind and okind is...
> if (cnt >= BPF_MAX_TRAMP_LINKS)
> return -E2BIG;
> if (!hlist_unhashed(&link->tramp_hlist))
> /* prog already linked */
> return -EBUSY;
> - hlist_for_each_entry(link_exiting, &tr->progs_hlist[kind], tramp_hlist) {
> - if (link_exiting->link.prog != link->link.prog)
> + hlist_for_each_entry(link_existing, &tr->progs_hlist[kind], tramp_hlist) {
> + if (link_existing->link.prog != link->link.prog)
> continue;
> /* prog already linked */
> return -EBUSY;
[...]
> @@ -23298,6 +23299,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> if (prog_type == BPF_PROG_TYPE_TRACING &&
> insn->imm == BPF_FUNC_get_func_ret) {
> if (eatype == BPF_TRACE_FEXIT ||
> + eatype == BPF_TRACE_SESSION ||
> eatype == BPF_MODIFY_RETURN) {
> /* Load nr_args from ctx - 8 */
> insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> @@ -24242,7 +24244,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> if (tgt_prog->type == BPF_PROG_TYPE_TRACING &&
> prog_extension &&
> (tgt_prog->expected_attach_type == BPF_TRACE_FENTRY ||
> - tgt_prog->expected_attach_type == BPF_TRACE_FEXIT)) {
> + tgt_prog->expected_attach_type == BPF_TRACE_FEXIT ||
> + tgt_prog->expected_attach_type == BPF_TRACE_SESSION)) {
> /* Program extensions can extend all program types
> * except fentry/fexit. The reason is the following.
> * The fentry/fexit programs are used for performance
> @@ -24257,7 +24260,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> * beyond reasonable stack size. Hence extending fentry
> * is not allowed.
> */
> - bpf_log(log, "Cannot extend fentry/fexit\n");
> + bpf_log(log, "Cannot extend fentry/fexit/session\n");
fsession?
> return -EINVAL;
> }
> } else {
> @@ -24341,6 +24344,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> case BPF_LSM_CGROUP:
> case BPF_TRACE_FENTRY:
> case BPF_TRACE_FEXIT:
> + case BPF_TRACE_SESSION:
> if (!btf_type_is_func(t)) {
> bpf_log(log, "attach_btf_id %u is not a function\n",
> btf_id);
[...]
Powered by blists - more mailing lists