[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALqUS-6XtJ0Bb9jiykdC3jAY_OHjGuirj06Kzssjvo7eW_so2A@mail.gmail.com>
Date: Wed, 30 Apr 2025 20:43:58 +0800
From: Kafai Wan <mannkafai@...il.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Song Liu <song@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Eduard <eddyz87@...il.com>,
Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
Matt Bobrowski <mattbobrowski@...gle.com>, Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
bpf <bpf@...r.kernel.org>,
linux-trace-kernel <linux-trace-kernel@...r.kernel.org>,
Network Development <netdev@...r.kernel.org>,
"open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>, Leon Hwang <leon.hwang@...ux.dev>
Subject: Re: [PATCH bpf-next 1/4] bpf: Allow get_func_[arg|arg_cnt] helpers in
raw tracepoint programs
On Wed, Apr 30, 2025 at 10:46 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Sat, Apr 26, 2025 at 9:00 AM KaFai Wan <mannkafai@...il.com> wrote:
> >
> > Adding support to use get_func_[arg|arg_cnt] helpers in raw_tp/tp_btf
> > programs.
> >
> > We can use get_func_[arg|ret|arg_cnt] helpers in fentry/fexit/fmod_ret
> > programs currently. If we try to use get_func_[arg|arg_cnt] helpers in
> > raw_tp/tp_btf programs, verifier will fail to load the program with:
> >
> > ; __u64 cnt = bpf_get_func_arg_cnt(ctx);
> > 3: (85) call bpf_get_func_arg_cnt#185
> > unknown func bpf_get_func_arg_cnt#185
> >
> > Adding get_func_[arg|arg_cnt] helpers in raw_tp_prog_func_proto and
> > tracing_prog_func_proto for raw tracepoint.
> >
> > Adding 1 arg on ctx of raw tracepoint program and make it stores number of
> > arguments on ctx-8, so it's easy to verify argument index and find
> > argument's position.
> >
> > Signed-off-by: KaFai Wan <mannkafai@...il.com>
> > ---
> > kernel/trace/bpf_trace.c | 17 ++++++++++++++---
> > net/bpf/test_run.c | 13 +++++--------
> > 2 files changed, 19 insertions(+), 11 deletions(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 52c432a44aeb..eb4c56013493 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -1892,6 +1892,10 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > return &bpf_get_stackid_proto_raw_tp;
> > case BPF_FUNC_get_stack:
> > return &bpf_get_stack_proto_raw_tp;
> > + case BPF_FUNC_get_func_arg:
> > + return &bpf_get_func_arg_proto;
> > + case BPF_FUNC_get_func_arg_cnt:
> > + return &bpf_get_func_arg_cnt_proto;
> > case BPF_FUNC_get_attach_cookie:
> > return &bpf_get_attach_cookie_proto_tracing;
> > default:
> > @@ -1950,10 +1954,16 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > case BPF_FUNC_d_path:
> > return &bpf_d_path_proto;
> > case BPF_FUNC_get_func_arg:
> > + if (prog->type == BPF_PROG_TYPE_TRACING &&
> > + prog->expected_attach_type == BPF_TRACE_RAW_TP)
> > + return &bpf_get_func_arg_proto;
> > return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_proto : NULL;
> > case BPF_FUNC_get_func_ret:
> > return bpf_prog_has_trampoline(prog) ? &bpf_get_func_ret_proto : NULL;
> > case BPF_FUNC_get_func_arg_cnt:
> > + if (prog->type == BPF_PROG_TYPE_TRACING &&
> > + prog->expected_attach_type == BPF_TRACE_RAW_TP)
> > + return &bpf_get_func_arg_cnt_proto;
> > return bpf_prog_has_trampoline(prog) ? &bpf_get_func_arg_cnt_proto : NULL;
> > case BPF_FUNC_get_attach_cookie:
> > if (prog->type == BPF_PROG_TYPE_TRACING &&
> > @@ -2312,7 +2322,7 @@ void __bpf_trace_run(struct bpf_raw_tp_link *link, u64 *args)
> > #define REPEAT(X, FN, DL, ...) REPEAT_##X(FN, DL, __VA_ARGS__)
> >
> > #define SARG(X) u64 arg##X
> > -#define COPY(X) args[X] = arg##X
> > +#define COPY(X) args[X + 1] = arg##X
> >
> > #define __DL_COM (,)
> > #define __DL_SEM (;)
> > @@ -2323,9 +2333,10 @@ void __bpf_trace_run(struct bpf_raw_tp_link *link, u64 *args)
> > void bpf_trace_run##x(struct bpf_raw_tp_link *link, \
> > REPEAT(x, SARG, __DL_COM, __SEQ_0_11)) \
> > { \
> > - u64 args[x]; \
> > + u64 args[x + 1]; \
> > + args[0] = x; \
> > REPEAT(x, COPY, __DL_SEM, __SEQ_0_11); \
> > - __bpf_trace_run(link, args); \
> > + __bpf_trace_run(link, args + 1); \
>
> This is neat, but what is this for?
> The program that attaches to a particular raw_tp knows what it is
> attaching to and how many arguments are there,
> so bpf_get_func_arg_cnt() is a 5th wheel.
>
> If the reason is "for completeness" then it's not a good reason
> to penalize performance. Though it's just an extra 8 byte of stack
> and a single store of a constant.
>
If we try to capture all arguments of a specific raw_tp in tracing programs,
We first obtain the arguments count from the format file in debugfs or BTF
and pass this count to the BPF program via .bss section or cookie (if
available).
If we store the count in ctx and get it via get_func_arg_cnt helper in
the BPF program,
a) It's easier and more efficient to get the arguments count in the BPF program.
b) It could use a single BPF program to capture arguments for multiple raw_tps,
reduce the number of BPF programs when massive tracing.
Thanks,
KaFai
> pw-bot: cr
Powered by blists - more mailing lists