Re: [PATCH bpf-next 01/13] bpf: match program context types by name without vmlinux BTF

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 4138 bytes --]

> bpf: match program context types by name without vmlinux BTF

TL;DR: Commit message only: a "TODO-NEEDS-IHOR: Signed-off-by:" placeholder
isn't a real trailer, so the author's Signed-off-by is missing (checkpatch
would flag it). It should be fixed or removed; other series commits may have
it too.

> bpf: match program context types by name without vmlinux BTF
>
> btf_is_prog_ctx_type() and btf_validate_prog_ctx_type() decide whether
> a BTF function argument is the program's context.  They look up the
> canonical context type of the program type in bpf_ctx_convert, a type
> that only exists in the vmlinux BTF, but then use nothing but its name:
> the argument's type name is compared with it, with the __sk_buff ->
> sk_buff and xdp_md -> xdp_buff projections, and with one typedef
> resolution step for bpf_user_pt_regs_t.
>
> btf_prepare_func_args() runs this check for the main program of every
> load that carries func_info (e.g. the socket filter and cgroup programs
> systemd loads at boot) and for subprograms.  Once the vmlinux BTF is
> parsed on first use (CONFIG_DEBUG_INFO_BTF_LAZY, later in this series),
> fetching the BTF here would make all of them pay for parsing it, and
> skipping the check when the BTF is not there yet would make the
> verification result (e.g. func_info "unreliable") depend on whether
> something else parsed the BTF first.
>
> Take the names from include/linux/bpf_types.h instead: a table indexed
> by program type is built from the prog_ctx_type argument of
> BPF_PROG_TYPE(), and prog_ctx_tname() returns the struct tag, or the
> typedef name.  For bpf_user_pt_regs_t it also returns the name of the
> underlying struct, found with __builtin_types_compatible_p() the same
> way btf_validate_prog_ctx_type() already does for perf_event programs.
> The other forms are equivalent to what the BTF lookup produced:
>
>  - typedefs of integers (__u64, __u32): the old code compared the
>    typedef name and then the integer type name, which never equals a
>    struct tag; only the typedef name is compared now;
>  - "void *" (tracing, struct_ops, ext, lsm, syscall): no named context
>    type, as before;
>  - s390 typedefs bpf_user_pt_regs_t to an anonymous struct: nothing
>    matches, as before; only the __arg_ctx error message changes from
>    "should be `struct  *`" to "should be void pointer";
>  - program types that are not configured in used to index
>    bpf_ctx_convert_map[] with 0 (socket filter); they now get no name.
>    Such programs cannot be loaded anyway.
>
> find_kern_ctx_type_id() and get_kern_ctx_btf_id() still need the kernel
> context type (tracing/EXT access to a BPF program's context argument,
> bpf_cast_to_kern_ctx()).  Kernel types enter the program there, so they
> now fetch the vmlinux BTF with bpf_get_btf_vmlinux() instead of relying
> on bpf_check() having done it.
>
> No change with CONFIG_DEBUG_INFO_BTF=y.  With CONFIG_DEBUG_INFO_BTF=n,
> global subprograms taking e.g. a struct __sk_buff *ctx now verify
> instead of failing with "btf_vmlinux is malformed", as they do with =y.
>
> TODO-NEEDS-IHOR: Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> [split the BPF_PROG_TYPE() definition over two lines]
> Signed-off-by: Jay Wang <wanjay@amazon.com>

Is the "TODO-NEEDS-IHOR: Signed-off-by:" line at the end of the commit
message meant to stay in the submitted patch?

Because of the prefix, git and checkpatch do not parse that line as a
trailer, so the only real Signed-off-by is Jay Wang's, while the patch is
authored by Ihor Solodrai.  checkpatch would report "Missing Signed-off-by:
line by nominal patch author".

Should this become a real Signed-off-by line from the author (or be removed),
as required by the Developer's Certificate of Origin in
Documentation/process/submitting-patches.rst?  The same placeholder appears in
other commits of this series (for example 32ad8adbb1a8).


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/41205212136