Re: [RFC PATCH bpf-next v3 09/10] libbpf: Add common attr support for map_create

[Leon Hwang <leon.hwang@linux.dev>]

On 7/10/25 07:08, Andrii Nakryiko wrote:
> On Thu, Oct 2, 2025 at 8:49 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> With the previous commit adding common attribute support for
>> BPF_MAP_CREATE, it is now possible to retrieve detailed error messages
>> when map creation fails by using the 'log_buf' field from the common
>> attributes.
>>
>> Extend 'bpf_map_create_opts' with these new fields, 'log_buf', 'log_size'
>> , 'log_level' and 'log_true_size', allowing users to capture and inspect
>
> comma
>

Ack.

>> those log messages.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  tools/lib/bpf/bpf.c | 17 +++++++++++++++--
>>  tools/lib/bpf/bpf.h |  9 +++++++--
>>  2 files changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index 9cd79beb13a2d..ca66fcdb3f49f 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -203,10 +203,13 @@ int bpf_map_create(enum bpf_map_type map_type,
>>                    __u32 key_size,
>>                    __u32 value_size,
>>                    __u32 max_entries,
>> -                  const struct bpf_map_create_opts *opts)
>> +                  struct bpf_map_create_opts *opts)
>
> this is a breaking change, see below
>
>>  {
>>         const size_t attr_sz = offsetofend(union bpf_attr, excl_prog_hash_size);
>> +       const size_t common_attrs_sz = sizeof(struct bpf_common_attr);
>> +       struct bpf_common_attr common_attrs;
>>         union bpf_attr attr;
>> +       const char *log_buf;
>>         int fd;
>>
>>         bump_rlimit_memlock();
>> @@ -239,7 +242,17 @@ int bpf_map_create(enum bpf_map_type map_type,
>>         attr.excl_prog_hash = ptr_to_u64(OPTS_GET(opts, excl_prog_hash, NULL));
>>         attr.excl_prog_hash_size = OPTS_GET(opts, excl_prog_hash_size, 0);
>>
>> -       fd = sys_bpf_fd(BPF_MAP_CREATE, &attr, attr_sz);
>> +       log_buf = OPTS_GET(opts, log_buf, NULL);
>> +       if (log_buf && feat_supported(NULL, FEAT_EXTENDED_SYSCALL)) {
>> +               memset(&common_attrs, 0, common_attrs_sz);
>> +               common_attrs.log_buf = ptr_to_u64(log_buf);
>> +               common_attrs.log_size = OPTS_GET(opts, log_size, 0);
>> +               common_attrs.log_level = OPTS_GET(opts, log_level, 0);
>> +               fd = sys_bpf_ext_fd(BPF_MAP_CREATE, &attr, attr_sz, &common_attrs, common_attrs_sz);
>> +               OPTS_SET(opts, log_true_size, common_attrs.log_true_size);
>> +       } else {
>> +               fd = sys_bpf_fd(BPF_MAP_CREATE, &attr, attr_sz);
>> +       }
>>         return libbpf_err_errno(fd);
>>  }
>>
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index e983a3e40d612..77d475e7274a0 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -57,16 +57,21 @@ struct bpf_map_create_opts {
>>
>>         const void *excl_prog_hash;
>>         __u32 excl_prog_hash_size;
>> +
>> +       const char *log_buf;
>> +       __u32 log_size;
>> +       __u32 log_level;
>> +       __u32 log_true_size;
>
> I'm thinking that maybe we should have a separate struct that will
> have these 4 fields, and pass a pointer to it. That way
> bpf_map_create_opts can still be passed as const pointer, but libbpf
> will still be able to write out log_true_size without violating
> constness
>
> and then we can reuse the same type (struct bpf_log? struct
> bpf_log_buf? struct bpf_log_info? not sure, let's bikeshed) across
> different commands that support passing log info through
> bpf_common_attrs
>

Sure, that makes sense.

I add a new 'struct bpf_log_attr' to encapsulate these four fields, and
it works well in my local testing.

This also keeps bpf_map_create_opts const-safe while making it easier to
reuse the same structure for other commands using bpf_common_attrs.

Thanks,
Leon

[...]