BPF List
 help / color / mirror / Atom feed
From: Menglong Dong <menglong.dong@linux.dev>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Menglong Dong <menglong8.dong@gmail.com>,
	ast@kernel.org, andrii@kernel.org, davem@davemloft.net,
	dsahern@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
	eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me,
	haoluo@google.com, jolsa@kernel.org, tglx@linutronix.de,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	x86@kernel.org, hpa@zytor.com, netdev@vger.kernel.org,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next v4 0/9] bpf: tracing session supporting
Date: Sat, 20 Dec 2025 17:01:06 +0800	[thread overview]
Message-ID: <6114986.MhkbZ0Pkbq@7950hx> (raw)
In-Reply-To: <2393471.ElGaqSPkdT@7950hx>

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@linux.dev> wrote:
> > >
> > > On 2025/12/19 08:55 Andrii Nakryiko <andrii.nakryiko@gmail.com> write:
> > > > On Wed, Dec 17, 2025 at 1:54 AM Menglong Dong <menglong8.dong@gmail.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
> > >
> > > >
> [...]
> > >
> > >
> > >
> > >
> 
> 
> 
> 
> 





  reply	other threads:[~2025-12-20  9:01 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-17  9:54 [PATCH bpf-next v4 0/9] bpf: tracing session supporting Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 1/9] bpf: add tracing session support Menglong Dong
2025-12-19  0:55   ` Andrii Nakryiko
2025-12-19  1:24     ` Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 2/9] bpf: use last 8-bits for the nr_args in trampoline Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 3/9] bpf: add the kfunc bpf_fsession_is_return Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 4/9] bpf: add the kfunc bpf_fsession_cookie Menglong Dong
2025-12-19  0:55   ` Andrii Nakryiko
2025-12-19  1:31     ` Menglong Dong
2025-12-19 12:01       ` Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 5/9] bpf,x86: introduce emit_st_r0_imm64() for trampoline Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 6/9] bpf,x86: add tracing session supporting for x86_64 Menglong Dong
2025-12-19  0:55   ` Andrii Nakryiko
2025-12-19  1:41     ` Menglong Dong
2025-12-19 16:56       ` Andrii Nakryiko
2025-12-17  9:54 ` [PATCH bpf-next v4 7/9] libbpf: add support for tracing session Menglong Dong
2025-12-19  0:55   ` Andrii Nakryiko
2025-12-19  1:42     ` Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 8/9] selftests/bpf: add testcases " Menglong Dong
2025-12-17 10:24   ` bot+bpf-ci
2025-12-17 11:42     ` Menglong Dong
2025-12-17  9:54 ` [PATCH bpf-next v4 9/9] selftests/bpf: test fsession mixed with fentry and fexit Menglong Dong
2025-12-17 10:24   ` bot+bpf-ci
2025-12-17 10:37     ` Menglong Dong
2025-12-19  0:55 ` [PATCH bpf-next v4 0/9] bpf: tracing session supporting Andrii Nakryiko
2025-12-19  1:18   ` Menglong Dong
2025-12-19 16:55     ` Andrii Nakryiko
2025-12-20  1:12       ` Menglong Dong
2025-12-20  9:01         ` Menglong Dong [this message]
2025-12-20 12:22           ` Menglong Dong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6114986.MhkbZ0Pkbq@7950hx \
    --to=menglong.dong@linux.dev \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bp@alien8.de \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=hpa@zytor.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=menglong8.dong@gmail.com \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox