Re: [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load

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

On 2025/9/18 05:12, Andrii Nakryiko wrote:
> On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> The log buffer of common attributes would be confusing with the one in
>> 'union bpf_attr' for BPF_PROG_LOAD and BPF_BTF_LOAD.
>>
>> In order to clarify the usage of these two 'log_buf's, they both can be
>> used for logging if:
>>
>> * They are same, including 'log_buf', 'log_level' and 'log_size'.
>> * One of them is missing, then another one will be used for logging.
>>
>
> I agree with the logic above, but I'm not sure whether we need to
> plumb common_attrs all the way into bpf_vlog_init, tbh. There are only
> two commands that can have log specified through both bpf_attr and
> bpf_common_attrs. I'd have those two commands check and resolve the
> log buffer pointer, size and flags on their own (sure, a bit of
> duplicated logic, but we won't have any new command having to do that,
> so that's fine in my book).
>
> And then I'd keep bpf_vlog_init completely unaware of common_attrs
> (which eventually have more stuff in it that's irrelevant to logging).
>

To avoid modifying bpf_vlog_init directly, one option would be to
introduce a new helper, e.g. bpf_vlog_init2 or
bpf_vlog_init_with_cattrs, to handle the case with common_attrs.

This way, bpf_vlog_init_with_cattrs could be used for BPF_PROG_LOAD and
BPF_BTF_LOAD, while the existing bpf_vlog_init remains unchanged and
could be used for BPF_MAP_CREATE.

That would avoid duplicating the log handling logic, while also keeping
the separation between the two cases clear.

> This seems cleaner than plumbing this through so deeply.
>
>> If they both have 'log_buf' but they are not same, a log message will be
>> written to the log buffer of 'union bpf_attr'.
>>
>
> Meh, whatever, this is unlikely user error, just error out with
> -EINVAL or something. Let's not invent "log here, but not here" logic.
>

In that case, we can return -EUSERS, as Alexei suggested earlier.

Thanks,
Leon