On 08/09/2026 17:01, [email protected] wrote:
>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst 
>> b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
>> index d0a36f442db7..1cdecf3e4fa5 100644
>> --- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
>> +++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> 
> [ ... ]
> 
>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
>> index a50540ef6521..0fcfe20ff515 100644
>> --- a/tools/bpf/bpftool/gen.c
>> +++ b/tools/bpf/bpftool/gen.c
>> @@ -583,6 +583,9 @@ static void codegen_attach_detach(struct bpf_object 
>> *obj, const char *obj_name)
>>      bpf_object__for_each_program(prog, obj) {
>>              const char *tp_name;
>>
>> +            if (!bpf_program__autoload(prog))
>> +                    continue;
>> +
>>              codegen("\
>>                      \n\
>>                      \n\
> 
> [ ... ]
> 
>> @@ -1339,6 +1348,8 @@ static int do_skeleton(int argc, char **argv)
>>              map_cnt++;
>>      }
>>      bpf_object__for_each_program(prog, obj) {
>> +            if (use_loader && !bpf_program__autoload(prog))
>> +                    continue;
>>              prog_cnt++;
>>      }
> 
> Can the autoload flag change between when this check runs and when
> codegen_attach_detach() checks it?
> 
> Looking at the code flow, do_skeleton() samples bpf_program__autoload() here
> before calling bpf_object__load(), but codegen_attach_detach() and
> codegen_destroy() (which also check autoload at gen.c:586, 635, 654, and 686)
> run inside gen_trace() after bpf_object__load() completes.
> 
> In tools/lib/bpf/libbpf.c, bpf_object__load() ->
> bpf_object_prepare() modifies prog->autoload for struct_ops programs:
> 
> 1. bpf_object__init_kern_struct_ops_maps() (libbpf.c:1251, 1290) sets
>    st_ops->progs[i]->autoload = false when the member is missing from kernel
>    BTF or the slot was replaced/NULLed.
> 
> 2. bpf_object_adjust_struct_ops_autoload() (libbpf.c:1166) sets
>    prog->autoload = should_load for programs referenced by struct_ops maps,
>    which can flip SEC("?struct_ops/...") programs from false back to true when
>    the referencing map has autocreate set.
> 
> For the false->true case (SEC("?struct_ops/foo") with an autocreated map):
> do_skeleton() omits both the progs and links struct fields, but
> codegen_attach_detach()/codegen_destroy() then emit attach functions
> referencing skel->progs.foo.prog_fd and skel->links.foo_fd, which weren't
> declared.
> 
> For the true->false case (plain SEC("struct_ops/foo") whose member is missing
> from kernel BTF): the fields are emitted but attach/detach/destroy skip the
> program, and the loader doesn't reserve a prog_fd slot for it, so subsequent
> program FDs get written into preceding fields.
> 
> Would it work to sample autoload once and reuse that decision, or to move the
> struct field emission after bpf_object__load()?

It looks like both Sashiko and the bpf-ci bot raised valid concerns.
Here, it's probably cleaner to move the struct field emission after
bpf_object__load()? Seems a bit more involved though, we'd need to move
these struct field emission to gen_trace() for the use_loader case, or
move bpf_object__load() out of gen_trace(), not sure which is best. This
way we'd have the fields printed after bpf_object__load() has updated
(if necessary) the autoload status for "obj", and
codegen_attach_detach() would then use the correct fields.

Quentin

Reply via email to