Re: [RFC PATCH bpf-next v2 5/6] libbpf: Add common attr support for map_create

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

On 2025/9/18 05:46, Andrii Nakryiko wrote:
> On Wed, Sep 17, 2025 at 2:45 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>>
>>> With the previous patch 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.
>>>
>>> This patch extends 'bpf_map_create_opts' with two new fields, 'log_buf'
>>> and 'log_size', allowing users to capture and inspect these log messages.
>>>
>>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>>> ---
>>>  tools/lib/bpf/bpf.c | 16 +++++++++++++++-
>>>  tools/lib/bpf/bpf.h |  5 ++++-
>>>  2 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>>> index 27845e287dd5c..5b58e981a7669 100644
>>> --- a/tools/lib/bpf/bpf.c
>>> +++ b/tools/lib/bpf/bpf.c
>>> @@ -218,7 +218,9 @@ int bpf_map_create(enum bpf_map_type map_type,
>>>                    const struct bpf_map_create_opts *opts)
>>>  {
>>>         const size_t attr_sz = offsetofend(union bpf_attr, map_token_fd);
>>> +       struct bpf_common_attr common_attrs;
>>>         union bpf_attr attr;
>>> +       __u64 log_buf;
>>
>>
>> const char *
>>

Ack.

>>>         int fd;
>>>
>>>         bump_rlimit_memlock();
>>> @@ -249,7 +251,19 @@ int bpf_map_create(enum bpf_map_type map_type,
>>>
>>>         attr.map_token_fd = OPTS_GET(opts, token_fd, 0);
>>>
>>> -       fd = sys_bpf_fd(BPF_MAP_CREATE, &attr, attr_sz);
>>> +       log_buf = (__u64) OPTS_GET(opts, log_buf, NULL);
>>
>> no u64 casting just yet
>>

Ack.

>>> +       if (log_buf) {
>>> +               if (!feat_supported(NULL, FEAT_EXTENDED_SYSCALL))
>>> +                       return libbpf_err(-EOPNOTSUPP);
>>
>> um.. I'm thinking that it would be better usability for libbpf to
>> ignore provided log if kernel doesn't support this feature just yet.
>> Then users don't have to care, they will just opportunistically
>> provide buffer and get extra error log, if kernel supports this
>> feature. Otherwise, log won't be touched, instead of failing an API
>> call.
>>

Agreed.

These two 'if's will be merged into one:

if (log_buf && feat_supported(NULL, FEAT_EXTENDED_SYSCALL)) {
    ...
}

>>> +
>>> +               memset(&common_attrs, 0, sizeof(common_attrs));
>>> +               common_attrs.log_buf = log_buf;
>>
>> ptr_to_u64(log_buf) here
>>

Ack.

>>> +               common_attrs.log_size = OPTS_GET(opts, log_size, 0);
>>> +               fd = sys_bpf_extended(BPF_MAP_CREATE, &attr, attr_sz, &common_attrs,
>>> +                                     sizeof(common_attrs));
>>> +       } 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 38819071ecbe7..3b54d6feb5842 100644
>>> --- a/tools/lib/bpf/bpf.h
>>> +++ b/tools/lib/bpf/bpf.h
>>> @@ -55,9 +55,12 @@ struct bpf_map_create_opts {
>>>         __s32 value_type_btf_obj_fd;
>>>
>>>         __u32 token_fd;
>>> +
>>> +       const char *log_buf;
>>> +       __u32 log_size;
>
> also, what about that log_level ?
>

Should we really introduce log_level here?

I don’t think it makes sense, because logging in map_create is too
simple for different log levels on the kernel side to have any
meaningful effect.

Thanks,
Leon