Re: [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall

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

On 2025/9/17 08:06, Andrii Nakryiko wrote:
> On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> To support the extended 'bpf()' syscall introduced in the previous commit,
>> this patch adds the following APIs:
>>
>> 1. *Internal:*
>>
>>    * 'sys_bpf_extended()'
>>    * 'sys_bpf_fd_extended()'
>>      These wrap the raw 'syscall()' interface to support passing extended
>>      attributes.
>>
>> 2. *Exported:*
>>
>>    * 'probe_sys_bpf_extended()'
>>      This function checks whether the running kernel supports the extended
>>      'bpf()' syscall with common attributes.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>>  tools/lib/bpf/bpf.c             | 45 +++++++++++++++++++++++++++++++++
>>  tools/lib/bpf/bpf.h             |  1 +
>>  tools/lib/bpf/features.c        |  8 ++++++
>>  tools/lib/bpf/libbpf.map        |  2 ++
>>  tools/lib/bpf/libbpf_internal.h |  2 ++
>>  5 files changed, 58 insertions(+)
>>
>
> (ran out of time, will continue reviewing the rest of patches
> tomorrow, so please don't yet send new revision)
>
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index ab40dbf9f020f..27845e287dd5c 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -69,6 +69,51 @@ static inline __u64 ptr_to_u64(const void *ptr)
>>         return (__u64) (unsigned long) ptr;
>>  }
>>
>> +static inline int sys_bpf_extended(enum bpf_cmd cmd, union bpf_attr *attr,
>> +                                  unsigned int size,
>> +                                  struct bpf_common_attr *common_attrs,
>> +                                  unsigned int size_common)
>> +{
>> +       cmd = common_attrs ? cmd | BPF_COMMON_ATTRS : cmd & ~BPF_COMMON_ATTRS;
>> +       return syscall(__NR_bpf, cmd, attr, size, common_attrs, size_common);
>> +}
>> +
>> +static inline int sys_bpf_fd_extended(enum bpf_cmd cmd, union bpf_attr *attr,
>
> please shorten to sys_bpf_ext() and sys_bpf_ext_fd() (also note ext before fd)
>

The short ones look good to me.

>
>> +                                     unsigned int size,
>> +                                     struct bpf_common_attr *common_attrs,
>> +                                     unsigned int size_common)
>> +{
>> +       int fd;
>> +
>> +       fd = sys_bpf_extended(cmd, attr, size, common_attrs, size_common);
>> +       return ensure_good_fd(fd);
>> +}
>> +
>> +int probe_sys_bpf_extended(int token_fd)
>> +{
>> +       const size_t attr_sz = offsetofend(union bpf_attr, prog_token_fd);
>> +       struct bpf_common_attr common_attrs;
>> +       struct bpf_insn insns[] = {
>> +               BPF_MOV64_IMM(BPF_REG_0, 0),
>> +               BPF_EXIT_INSN(),
>> +       };
>> +       union bpf_attr attr;
>> +
>> +       memset(&attr, 0, attr_sz);
>> +       attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
>> +       attr.license = ptr_to_u64("GPL");
>> +       attr.insns = ptr_to_u64(insns);
>> +       attr.insn_cnt = (__u32)ARRAY_SIZE(insns);
>> +       attr.prog_token_fd = token_fd;
>> +       if (token_fd)
>> +               attr.prog_flags |= BPF_F_TOKEN_FD;
>> +       libbpf_strlcpy(attr.prog_name, "libbpf_sysbpftest", sizeof(attr.prog_name));
>> +       memset(&common_attrs, 0, sizeof(common_attrs));
>> +
>> +       return sys_bpf_fd_extended(BPF_PROG_LOAD, &attr, attr_sz, &common_attrs,
>> +                                  sizeof(common_attrs));
>
> I think we can set up this feature detector such that we get -EINVAL
> due to BPF_COMMON_ATTRS not supported on old kernels, while -EFAULT on
> newer kernels due to NULL passed in common_attrs. This would be cheap
> and simple. Try it.
>

Let me give that a try.

>> +}
>> +
>>  static inline int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr,
>>                           unsigned int size)
>>  {
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index 7252150e7ad35..38819071ecbe7 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -35,6 +35,7 @@
>>  extern "C" {
>>  #endif
>>
>> +LIBBPF_API int probe_sys_bpf_extended(int token_fd);
>
> why adding this as a public UAPI?
>

If we don’t mark it with LIBBPF_API, the build fails when compiling libbpf.

My intention here wasn’t to introduce a new public UAPI, but simply to
provide a way for 'features.c' to probe whether the kernel supports the
extended BPF syscall, without directly exposing 'sys_bpf_fd_extended()'.

Do you have a suggestion on how we can perform this probe without
introducing a new LIBBPF_API symbol?

Thanks,
Leon