On 2025/12/20 09:12, Menglong Dong wrote:

On 2025/12/20 00:55, Andrii Nakryiko wrote:

quoted

On Thu, Dec 18, 2025 at 5:18 PM Menglong Dong [off-list ref] wrote:

quoted

On 2025/12/19 08:55 Andrii Nakryiko [off-list ref] write:

quoted

On Wed, Dec 17, 2025 at 1:54 AM Menglong Dong [off-list ref] wrote:

quoted

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().

quoted

quoted

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?

quoted

quoted

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

quoted

quoted

I'll check more to see if there is a more simple way to reuse them.

Thanks!
Menglong Dong

quoted

[...]

quoted

quoted