> libbpf: Factor out __sys_bpf_fd helper
TL;DR: Nit only, not a bug: the new libbpf helper __sys_bpf_fd() has a
misleading name and a different argument order (cmd not first) than
sys_bpf_fd(); a name like sys_bpf_fd_log() is suggested.
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index b49822d212ae..9a2107d4f6d8 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -203,6 +203,30 @@ int bump_rlimit_memlock(void)
> return 0;
> }
>
> +static int __sys_bpf_fd(union bpf_attr *attr, const size_t attr_sz, enum
> bpf_cmd cmd,
> + struct bpf_log_opts *log_opts)
> +{
> + const size_t attr_common_sz = sizeof(struct bpf_common_attr);
> + struct bpf_common_attr attr_common;
> + int fd;
> +
> + if (!OPTS_VALID(log_opts, bpf_log_opts))
> + return libbpf_err(-EINVAL);
> +
> + if (log_opts && feat_supported(NULL, FEAT_BPF_SYSCALL_COMMON_ATTRS)) {
> + memset(&attr_common, 0, attr_common_sz);
> + attr_common.log_buf = ptr_to_u64(OPTS_GET(log_opts, buf, NULL));
> + attr_common.log_size = OPTS_GET(log_opts, size, 0);
> + attr_common.log_level = OPTS_GET(log_opts, level, 0);
> + fd = sys_bpf_ext_fd(cmd, attr, attr_sz, &attr_common,
> attr_common_sz);
> + OPTS_SET(log_opts, true_size, attr_common.log_true_size);
> + } else {
> + fd = sys_bpf_fd(cmd, attr, attr_sz);
> + OPTS_SET(log_opts, true_size, 0);
> + }
> + return fd;
> +}
> +
[ ... ]
> @@ -248,20 +270,7 @@ int bpf_map_create(enum bpf_map_type map_type,
> attr.excl_prog_hash_size = OPTS_GET(opts, excl_prog_hash_size, 0);
>
> log_opts = OPTS_GET(opts, log_opts, NULL);
> - if (!OPTS_VALID(log_opts, bpf_log_opts))
> - return libbpf_err(-EINVAL);
> -
> - if (log_opts && feat_supported(NULL, FEAT_BPF_SYSCALL_COMMON_ATTRS)) {
> - memset(&attr_common, 0, attr_common_sz);
> - attr_common.log_buf = ptr_to_u64(OPTS_GET(log_opts, buf, NULL));
> - attr_common.log_size = OPTS_GET(log_opts, size, 0);
> - attr_common.log_level = OPTS_GET(log_opts, level, 0);
> - fd = sys_bpf_ext_fd(BPF_MAP_CREATE, &attr, attr_sz,
> &attr_common, attr_common_sz);
> - OPTS_SET(log_opts, true_size, attr_common.log_true_size);
> - } else {
> - fd = sys_bpf_fd(BPF_MAP_CREATE, &attr, attr_sz);
> - OPTS_SET(log_opts, true_size, 0);
> - }
> + fd = __sys_bpf_fd(&attr, attr_sz, BPF_MAP_CREATE, log_opts);
> return libbpf_err_errno(fd);
> }
This isn't a bug, but __sys_bpf_fd() is a higher-level wrapper that
dispatches to sys_bpf_fd() or sys_bpf_ext_fd() after validating log_opts and
probing FEAT_BPF_SYSCALL_COMMON_ATTRS, while a leading double underscore
usually marks the lower-level variant of a function.
The helper also takes (attr, attr_sz, cmd, log_opts), whereas sys_bpf(),
sys_bpf_fd(), sys_bpf_ext() and sys_bpf_ext_fd() all take cmd first, so the
new call reads differently from the nearby calls:
fd = __sys_bpf_fd(&attr, attr_sz, BPF_MAP_CREATE, log_opts);
Would a name that describes what the helper adds (for example something like
sys_bpf_fd_log()), with cmd as the first argument like the other sys_bpf*()
wrappers, read more naturally next to sys_bpf_fd() and sys_bpf_ext_fd()?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40108748084
