All of lore.kernel.org
 help / color / mirror / Atom feed
From: Menglong Dong <menglong.dong@linux.dev>
To: Menglong Dong <menglong8.dong@gmail.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: 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: Fri, 19 Dec 2025 09:18:04 +0800	[thread overview]
Message-ID: <1943128.tdWV9SEqCh@7940hx> (raw)
In-Reply-To: <CAEf4Bzb8oooWbxctuVhNPsziRBd1Fv9Y11-5TiEML8ckjrCt+g@mail.gmail.com>

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.

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.

What's more, we will lose the advantage of inline bpf_fsession_is_return
and bpf_fsession_cookie in verifier.

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

Thanks!
Menglong Dong

> 
> >
> > 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. Maybe we can prohibit the usage of bpf_get_func_ret()
> > in the fentry in verifier, which can make the architecture specific code
> > simpler.
> >
> > 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 since v3:
> > * 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.
> >
> > Changes since v2:
> > * 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
> >
> > Changes since v1:
> > * 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.
> >
> > Menglong Dong (9):
> >   bpf: add tracing session support
> >   bpf: use last 8-bits for the nr_args in trampoline
> >   bpf: add the kfunc bpf_fsession_is_return
> >   bpf: add the kfunc bpf_fsession_cookie
> >   bpf,x86: introduce emit_st_r0_imm64() for trampoline
> >   bpf,x86: add tracing session supporting for x86_64
> >   libbpf: add support for tracing session
> >   selftests/bpf: add testcases for tracing session
> >   selftests/bpf: test fsession mixed with fentry and fexit
> >
> >  arch/x86/net/bpf_jit_comp.c                   |  47 +++-
> >  include/linux/bpf.h                           |  39 +++
> >  include/uapi/linux/bpf.h                      |   1 +
> >  kernel/bpf/btf.c                              |   2 +
> >  kernel/bpf/syscall.c                          |  18 +-
> >  kernel/bpf/trampoline.c                       |  50 +++-
> >  kernel/bpf/verifier.c                         |  75 ++++--
> >  kernel/trace/bpf_trace.c                      |  56 ++++-
> >  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                           |   2 +
> >  tools/lib/bpf/libbpf.c                        |   3 +
> >  .../selftests/bpf/prog_tests/fsession_test.c  |  90 +++++++
> >  .../bpf/prog_tests/tracing_failure.c          |   2 +-
> >  .../selftests/bpf/progs/fsession_test.c       | 226 ++++++++++++++++++
> >  17 files changed, 571 insertions(+), 44 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
> >
> 





  reply	other threads:[~2025-12-19  1:18 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 [this message]
2025-12-19 16:55     ` Andrii Nakryiko
2025-12-20  1:12       ` Menglong Dong
2025-12-20  9:01         ` Menglong Dong
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=1943128.tdWV9SEqCh@7940hx \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.