On Mon, Jan 19, 2026 at 11:31 PM Menglong Dong <[email protected]> wrote: > > For now, bpf_get_func_arg() and bpf_get_func_arg_cnt() is not supported by > the BPF_TRACE_RAW_TP, which is not convenient to get the argument of the > tracepoint, especially for the case that the position of the arguments in > a tracepoint can change. > > The target tracepoint BTF type id is specified during loading time, > therefore we can get the function argument count from the function > prototype instead of the stack. > > Signed-off-by: Menglong Dong <[email protected]> > --- > v4: > - fix the error of using bpf_get_func_arg() for BPF_TRACE_ITER > > v3: > - remove unnecessary NULL checking for prog->aux->attach_func_proto > > v2: > - for nr_args, skip first 'void *__data' argument in btf_trace_##name > typedef > --- > kernel/bpf/verifier.c | 32 ++++++++++++++++++++++++++++---- > kernel/trace/bpf_trace.c | 4 ++++ > 2 files changed, 32 insertions(+), 4 deletions(-) >
other than stylistical choices, looks good to me Acked-by: Andrii Nakryiko <[email protected]> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 9de0ec0c3ed9..0b281b7c41eb 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -23323,8 +23323,20 @@ static int do_misc_fixups(struct bpf_verifier_env > *env) > /* Implement bpf_get_func_arg inline. */ > if (prog_type == BPF_PROG_TYPE_TRACING && > insn->imm == BPF_FUNC_get_func_arg) { > - /* Load nr_args from ctx - 8 */ > - insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, > BPF_REG_1, -8); > + if (eatype == BPF_TRACE_RAW_TP) { > + int nr_args = > btf_type_vlen(prog->aux->attach_func_proto); > + > + /* > + * skip first 'void *__data' argument in > btf_trace_##name > + * typedef > + */ > + nr_args--; > + /* Save nr_args to reg0 */ > + insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, > nr_args); > + } else { > + /* Load nr_args from ctx - 8 */ > + insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, > BPF_REG_1, -8); > + } > insn_buf[1] = BPF_JMP32_REG(BPF_JGE, BPF_REG_2, > BPF_REG_0, 6); > insn_buf[2] = BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 3); > insn_buf[3] = BPF_ALU64_REG(BPF_ADD, BPF_REG_2, > BPF_REG_1); > @@ -23376,8 +23388,20 @@ static int do_misc_fixups(struct bpf_verifier_env > *env) > /* Implement get_func_arg_cnt inline. */ > if (prog_type == BPF_PROG_TYPE_TRACING && > insn->imm == BPF_FUNC_get_func_arg_cnt) { > - /* Load nr_args from ctx - 8 */ > - insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, > BPF_REG_1, -8); > + if (eatype == BPF_TRACE_RAW_TP) { > + int nr_args = > btf_type_vlen(prog->aux->attach_func_proto); > + > + /* > + * skip first 'void *__data' argument in > btf_trace_##name > + * typedef > + */ > + nr_args--; > + /* Save nr_args to reg0 */ > + insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, > nr_args); nit: isn't this just a very verbose way of writing: int nr_args = btf_type_vlen(prog->aux->attach_func_proto); /* skip 'void *__data' in btf_trace_##name() and save to reg0 */ insn_buf[0] = BPF_MOV64_IMM(BPF_REG_0, nr_args - 1); even if you want to preserve nr_args-- for clarity, at least make that 4-line comment into a single-line one, please > + } else { > + /* Load nr_args from ctx - 8 */ > + insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, > BPF_REG_1, -8); > + } > > new_prog = bpf_patch_insn_data(env, i + delta, > insn_buf, 1); > if (!new_prog) > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index f73e08c223b5..0efdad3adcce 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1734,10 +1734,14 @@ 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->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->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; hm, wouldn't "has trampoline or is raw_tp" a more logical grouping? if (bpf_prog_has_trampoline(prog) || prog->expected_attach_type == BPF_TRACE_RAW_TP) return &bpf_get_func_arg_cnt_proto; return NULL; maybe you'll need to wrap that condition, but still, at least no one has to double check that we return exactly the same prototype in both cases, no? > case BPF_FUNC_get_attach_cookie: > if (prog->type == BPF_PROG_TYPE_TRACING && > -- > 2.52.0 >
