Message-ID: <6114986.MhkbZ0Pkbq@7950hx>
Date: Sat, 20 Dec 2025 17:01:06 +0800
From: Menglong Dong <menglong.dong@...ux.dev>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
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 2025/12/20 09:12, Menglong Dong wrote:
> On 2025/12/20 00:55, Andrii Nakryiko wrote:
> > 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
> 
> Ah, I remember it now. We do can use different kfunc version
> for different case in specialize_kfunc().
> 
> > 
> > >
> > > 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.
> 
> I see. The problem now is that the prototype of bpf_session_cookie()
> or bpf_session_is_return() don't satisfy our need. For bpf_session_cookie(),
> we at least need the context to be the argument. However, both
> of them don't have any function argument. After all, the prototype of
> different version of logically the same kfunc should be the same.

Hi, Andrii. I see that you want to make the API consistent between
ksession and fsession, which is more friendly for the user.

After my analysis, I think we have following approach:
1. change the function prototype of bpf_session_cookie and bpf_session_is_return
to:
    bool bpf_session_is_return(void *ctx);
    bool bpf_session_cookie(void *ctx);
And we do the fix up in specialize_kfunc(), which I think is the easiest
way. The defect is that it will break existing users.

2. We define a fixup_kfunc_call_early() and call it in add_subprog_and_kfunc.
In the fixup_kfunc_call_early(), we will change the target kfunc(which is insn->imm)
from bpf_session_cookie() to bpf_fsession_cookie(). For the bpf_session_cookie(),
we make its prototype to:
    __bpf_kfunc __u64 *bpf_session_cookie(void *ctx__ign)
Therefore, it won't break the existing users. For the ksession that uses the
old prototype, it can pass the verifier too. Following is a demo patch of this
approach. In this way, we can allow a extension in the prototype for a kfunc
in the feature too.

What do you think?

Thanks!
Menglong Dong

>patch<

+static int fixup_kfunc_call_early(struct bpf_verifier_env *env, struct bpf_insn *insn)
+{
+       struct bpf_prog *prog = env->prog;
+
+       if (prog->expected_attach_type == BPF_TRACE_FSESSION) {
+               if (insn->imm == special_kfunc_list[KF_bpf_session_cookie])
+                       insn->imm = special_kfunc_list[KF_bpf_fsession_cookie];
+               else if (insn->imm == special_kfunc_list[KF_bpf_session_is_return])
+                       insn->imm = special_kfunc_list[KF_bpf_fsession_is_return];
+       }
+
+       return 0;
+}

@@ -3489,10 +3490,12 @@ static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
                        return -EPERM;
                }
 
-               if (bpf_pseudo_func(insn) || bpf_pseudo_call(insn))
+               if (bpf_pseudo_func(insn) || bpf_pseudo_call(insn)) {
                        ret = add_subprog(env, i + insn->imm + 1);
-               else
-                       ret = add_kfunc_call(env, insn->imm, insn->off);
+               } else {
+                       ret = fixup_kfunc_call_early(env, insn);
+                       ret = ret ?: add_kfunc_call(env, insn->imm, insn->off);
+               }

@@ -3316,7 +3321,7 @@ static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
 
 __bpf_kfunc_start_defs();
 
-__bpf_kfunc bool bpf_session_is_return(void)
+__bpf_kfunc bool bpf_session_is_return(void *ctx__ign)
 {
        struct bpf_session_run_ctx *session_ctx;
 
@@ -3324,7 +3329,7 @@ __bpf_kfunc bool bpf_session_is_return(void)
        return session_ctx->is_return;
 }
 
-__bpf_kfunc __u64 *bpf_session_cookie(void)
+__bpf_kfunc __u64 *bpf_session_cookie(void *ctx__ign)
 {
        struct bpf_session_run_ctx *session_ctx;

> 
> I think it's not a good idea to modify the prototype of existing kfunc,
> can we?
> 
> > 
> > >
> > > 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.
> 
> Yeah, we can inline it depend on the program type if we can solve
> the prototype problem.
> 
> Thanks!
> Menglong Dong
> 
> 
> > 
> > > I'll check more to see if there is a more simple way to reuse them.
> > >
> > > Thanks!
> > > Menglong Dong
> > >
> > > >
> [...]
> > >
> > >
> > >
> > >
> 
> 
> 
> 
> 

Powered by blists - more mailing lists