* [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support
@ 2025-09-11 16:33 Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 1/6] " Leon Hwang
` (5 more replies)
0 siblings, 6 replies; 25+ messages in thread
From: Leon Hwang @ 2025-09-11 16:33 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, daniel, menglong8.dong, Leon Hwang
This proposal builds upon the discussion in
"[PATCH bpf-next v4 0/4] bpf: Improve error reporting for freplace attachment failure"[1],
and is also relevant to ongoing efforts such as tracing multi-link attach
failures[2].
This patch set introduces support for *common attributes* in the 'bpf()'
syscall, providing a unified mechanism for passing shared metadata across
all BPF commands.
The initial set of common attributes includes:
1. 'log_buf': User-provided buffer for storing log output.
2. 'log_size': Size of the provided log buffer.
3. 'log_level': Verbosity level for logging.
With this extension, the 'bpf()' syscall will be able to return meaningful
error messages (e.g., failures of creating map), improving debuggability
and user experience.
Changes:
RFC v1 -> RFC v2:
* Fix build error reported by test bot.
* Address comments from Alexei:
* Drop new uapi for freplace.
* Add common attributes support for prog_load and btf_load.
* Add common attributes support for map_create.
Links:
[1] https://lore.kernel.org/bpf/20250224153352.64689-1-leon.hwang@linux.dev/
[2] https://lore.kernel.org/bpf/20250703121521.1874196-1-dongml2@chinatelecom.cn/
Leon Hwang (6):
bpf: Extend bpf syscall with common attributes support
libbpf: Add support for extended bpf syscall
bpf: Add common attr support for prog_load and btf_load
bpf: Add common attr support for map_create
libbpf: Add common attr support for map_create
selftests/bpf: Add cases to test map create failure log
include/linux/bpf.h | 3 +-
include/linux/bpf_verifier.h | 2 +-
include/linux/btf.h | 3 +-
include/linux/syscalls.h | 3 +-
include/uapi/linux/bpf.h | 7 +
kernel/bpf/btf.c | 12 +-
kernel/bpf/log.c | 23 +++-
kernel/bpf/syscall.c | 115 ++++++++++++----
kernel/bpf/verifier.c | 8 +-
tools/include/uapi/linux/bpf.h | 7 +
tools/lib/bpf/bpf.c | 61 ++++++++-
tools/lib/bpf/bpf.h | 6 +-
tools/lib/bpf/features.c | 8 ++
tools/lib/bpf/libbpf.map | 2 +
tools/lib/bpf/libbpf_internal.h | 2 +
.../selftests/bpf/prog_tests/map_init.c | 124 ++++++++++++++++++
16 files changed, 341 insertions(+), 45 deletions(-)
--
2.50.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC PATCH bpf-next v2 1/6] bpf: Extend bpf syscall with common attributes support
2025-09-11 16:33 [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support Leon Hwang
@ 2025-09-11 16:33 ` Leon Hwang
2025-09-17 0:06 ` Andrii Nakryiko
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall Leon Hwang
` (4 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: Leon Hwang @ 2025-09-11 16:33 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, daniel, menglong8.dong, Leon Hwang
This patch extends the 'bpf()' syscall to support a set of common
attributes shared across all BPF commands:
1. 'log_buf': User-provided buffer for storing logs
2. 'log_size': Size of the log buffer
3. 'log_level': Log verbosity level
These common attributes are passed as the 4th argument to the 'bpf()'
syscall, with the 5th argument specifying the size of this structure.
To indicate the use of these common attributes from userspace, a new flag
'BPF_COMMON_ATTRS' ('1 << 16') is introduced. This flag is OR-ed into the
'cmd' field of the syscall.
When 'cmd & BPF_COMMON_ATTRS' is set, the kernel will copy the common
attributes from userspace into kernel space for use.
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
include/linux/syscalls.h | 3 ++-
include/uapi/linux/bpf.h | 7 +++++++
kernel/bpf/syscall.c | 19 +++++++++++++++----
tools/include/uapi/linux/bpf.h | 7 +++++++
4 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 77f45e5d44139..94408575dc49b 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -933,7 +933,8 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
asmlinkage long sys_getrandom(char __user *buf, size_t count,
unsigned int flags);
asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
-asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size);
+asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size,
+ struct bpf_common_attr __user *attr_common, unsigned int size_common);
asmlinkage long sys_execveat(int dfd, const char __user *filename,
const char __user *const __user *argv,
const char __user *const __user *envp, int flags);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 233de8677382e..5014baccf065f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1474,6 +1474,13 @@ struct bpf_stack_build_id {
};
};
+struct bpf_common_attr {
+ __u64 log_buf;
+ __u32 log_size;
+ __u32 log_level;
+};
+
+#define BPF_COMMON_ATTRS (1 << 16)
#define BPF_OBJ_NAME_LEN 16U
enum {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 3f178a0f8eb12..d49f822ceea12 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -5987,8 +5987,10 @@ static int prog_stream_read(union bpf_attr *attr)
return ret;
}
-static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
+static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
+ bpfptr_t uattr_common, unsigned int size_common)
{
+ struct bpf_common_attr common_attrs;
union bpf_attr attr;
int err;
@@ -6002,6 +6004,14 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
if (copy_from_bpfptr(&attr, uattr, size) != 0)
return -EFAULT;
+ memset(&common_attrs, 0, sizeof(common_attrs));
+ if (cmd & BPF_COMMON_ATTRS) {
+ cmd &= ~BPF_COMMON_ATTRS;
+ size_common = min_t(u32, size_common, sizeof(common_attrs));
+ if (uattr_common.user && copy_from_bpfptr(&common_attrs, uattr_common, size_common))
+ return -EFAULT;
+ }
+
err = security_bpf(cmd, &attr, size, uattr.is_kernel);
if (err < 0)
return err;
@@ -6134,9 +6144,10 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
return err;
}
-SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
+SYSCALL_DEFINE5(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size,
+ struct bpf_common_attr __user *, uattr_common, unsigned int, size_common)
{
- return __sys_bpf(cmd, USER_BPFPTR(uattr), size);
+ return __sys_bpf(cmd, USER_BPFPTR(uattr), size, USER_BPFPTR(uattr_common), size_common);
}
static bool syscall_prog_is_valid_access(int off, int size,
@@ -6167,7 +6178,7 @@ BPF_CALL_3(bpf_sys_bpf, int, cmd, union bpf_attr *, attr, u32, attr_size)
default:
return -EINVAL;
}
- return __sys_bpf(cmd, KERNEL_BPFPTR(attr), attr_size);
+ return __sys_bpf(cmd, KERNEL_BPFPTR(attr), attr_size, KERNEL_BPFPTR(NULL), 0);
}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 233de8677382e..5014baccf065f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1474,6 +1474,13 @@ struct bpf_stack_build_id {
};
};
+struct bpf_common_attr {
+ __u64 log_buf;
+ __u32 log_size;
+ __u32 log_level;
+};
+
+#define BPF_COMMON_ATTRS (1 << 16)
#define BPF_OBJ_NAME_LEN 16U
enum {
--
2.50.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall
2025-09-11 16:33 [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 1/6] " Leon Hwang
@ 2025-09-11 16:33 ` Leon Hwang
2025-09-17 0:06 ` Andrii Nakryiko
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load Leon Hwang
` (3 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: Leon Hwang @ 2025-09-11 16:33 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, daniel, menglong8.dong, Leon Hwang
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(+)
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,
+ 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));
+}
+
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);
LIBBPF_API int libbpf_set_memlock_rlim(size_t memlock_bytes);
struct bpf_map_create_opts {
diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
index 760657f5224c2..a63172c6343d0 100644
--- a/tools/lib/bpf/features.c
+++ b/tools/lib/bpf/features.c
@@ -507,6 +507,11 @@ static int probe_kern_arg_ctx_tag(int token_fd)
return probe_fd(prog_fd);
}
+static int probe_kern_extended_syscall(int token_fd)
+{
+ return probe_fd(probe_sys_bpf_extended(token_fd));
+}
+
typedef int (*feature_probe_fn)(int /* token_fd */);
static struct kern_feature_cache feature_cache;
@@ -582,6 +587,9 @@ static struct kern_feature_desc {
[FEAT_BTF_QMARK_DATASEC] = {
"BTF DATASEC names starting from '?'", probe_kern_btf_qmark_datasec,
},
+ [FEAT_EXTENDED_SYSCALL] = {
+ "Kernel supports extended syscall", probe_kern_extended_syscall,
+ },
};
bool feat_supported(struct kern_feature_cache *cache, enum kern_feature_id feat_id)
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index d7bd463e7017e..83d3d1af5ed1e 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -448,4 +448,6 @@ LIBBPF_1.6.0 {
} LIBBPF_1.5.0;
LIBBPF_1.7.0 {
+ global:
+ probe_sys_bpf_extended;
} LIBBPF_1.6.0;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 477a3b3389a09..497d845339a6b 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -380,6 +380,8 @@ enum kern_feature_id {
FEAT_ARG_CTX_TAG,
/* Kernel supports '?' at the front of datasec names */
FEAT_BTF_QMARK_DATASEC,
+ /* Kernel supports extended syscall */
+ FEAT_EXTENDED_SYSCALL,
__FEAT_CNT,
};
--
2.50.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load
2025-09-11 16:33 [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 1/6] " Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall Leon Hwang
@ 2025-09-11 16:33 ` Leon Hwang
2025-09-17 21:12 ` Andrii Nakryiko
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create Leon Hwang
` (2 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: Leon Hwang @ 2025-09-11 16:33 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, daniel, menglong8.dong, Leon Hwang
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.
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'.
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
include/linux/bpf.h | 3 ++-
include/linux/bpf_verifier.h | 2 +-
include/linux/btf.h | 3 ++-
kernel/bpf/btf.c | 12 +++++++-----
kernel/bpf/log.c | 23 ++++++++++++++++++++++-
kernel/bpf/syscall.c | 14 ++++++++------
kernel/bpf/verifier.c | 8 ++++----
7 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 8f6e87f0f3a89..adc0e68cb4e50 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2717,7 +2717,8 @@ int bpf_check_uarg_tail_zero(bpfptr_t uaddr, size_t expected_size,
size_t actual_size);
/* verify correctness of eBPF program */
-int bpf_check(struct bpf_prog **fp, union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size);
+int bpf_check(struct bpf_prog **fp, union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size,
+ struct bpf_common_attr *common_attrs);
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth);
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 020de62bd09cd..2d61afec91c92 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -864,7 +864,7 @@ __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
__printf(2, 3) void bpf_log(struct bpf_verifier_log *log,
const char *fmt, ...);
int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
- char __user *log_buf, u32 log_size);
+ char __user *log_buf, u32 log_size, const struct bpf_common_attr *common_attrs);
void bpf_vlog_reset(struct bpf_verifier_log *log, u64 new_pos);
int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual);
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 9eda6b113f9b4..c0acb46930bde 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -145,7 +145,8 @@ const char *btf_get_name(const struct btf *btf);
void btf_get(struct btf *btf);
void btf_put(struct btf *btf);
const struct btf_header *btf_header(const struct btf *btf);
-int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_sz);
+int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_sz,
+ const struct bpf_common_attr *common_attrs);
struct btf *btf_get_by_fd(int fd);
int btf_get_info_by_fd(const struct btf *btf,
const union bpf_attr *attr,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 64739308902f7..4a17ae4842210 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5771,7 +5771,8 @@ static int finalize_log(struct bpf_verifier_log *log, bpfptr_t uattr, u32 uattr_
return err;
}
-static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
+static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size,
+ const struct bpf_common_attr *common_attrs)
{
bpfptr_t btf_data = make_bpfptr(attr->btf, uattr.is_kernel);
char __user *log_ubuf = u64_to_user_ptr(attr->btf_log_buf);
@@ -5791,8 +5792,8 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat
/* user could have requested verbose verifier output
* and supplied buffer to store the verification trace
*/
- err = bpf_vlog_init(&env->log, attr->btf_log_level,
- log_ubuf, attr->btf_log_size);
+ err = bpf_vlog_init(&env->log, attr->btf_log_level, log_ubuf, attr->btf_log_size,
+ common_attrs);
if (err)
goto errout_free;
@@ -8028,12 +8029,13 @@ static int __btf_new_fd(struct btf *btf)
return anon_inode_getfd("btf", &btf_fops, btf, O_RDONLY | O_CLOEXEC);
}
-int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
+int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size,
+ const struct bpf_common_attr *common_attrs)
{
struct btf *btf;
int ret;
- btf = btf_parse(attr, uattr, uattr_size);
+ btf = btf_parse(attr, uattr, uattr_size, common_attrs);
if (IS_ERR(btf))
return PTR_ERR(btf);
diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
index e4983c1303e76..a9a0834884eb9 100644
--- a/kernel/bpf/log.c
+++ b/kernel/bpf/log.c
@@ -29,12 +29,33 @@ static bool bpf_verifier_log_attr_valid(const struct bpf_verifier_log *log)
}
int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level,
- char __user *log_buf, u32 log_size)
+ char __user *log_buf, u32 log_size, const struct bpf_common_attr *common_attrs)
{
+ u32 log_true_size;
+ int err;
+
log->level = log_level;
log->ubuf = log_buf;
log->len_total = log_size;
+ if (log_buf && common_attrs && common_attrs->log_buf &&
+ ((u64) log_buf != common_attrs->log_buf || log_level != common_attrs->log_level ||
+ log_size != common_attrs->log_size)) {
+ if (!bpf_verifier_log_attr_valid(log))
+ return -EINVAL;
+ bpf_log(log, "Conflict log configs between bpf_attr and common_attr.\n");
+ err = bpf_vlog_finalize(log, &log_true_size);
+ if (err)
+ return err;
+ return -EINVAL;
+ }
+
+ if (!log_buf && common_attrs && common_attrs->log_buf) {
+ log->level = common_attrs->log_level;
+ log->ubuf = u64_to_user_ptr(common_attrs->log_buf);
+ log->len_total = common_attrs->log_size;
+ }
+
/* log attributes have to be sane */
if (!bpf_verifier_log_attr_valid(log))
return -EINVAL;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index d49f822ceea12..5e5cf0262a14e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2764,7 +2764,8 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type)
/* last field in 'union bpf_attr' used by this command */
#define BPF_PROG_LOAD_LAST_FIELD fd_array_cnt
-static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
+static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size,
+ struct bpf_common_attr *common_attrs)
{
enum bpf_prog_type type = attr->prog_type;
struct bpf_prog *prog, *dst_prog = NULL;
@@ -2976,7 +2977,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
goto free_prog_sec;
/* run eBPF verifier */
- err = bpf_check(&prog, attr, uattr, uattr_size);
+ err = bpf_check(&prog, attr, uattr, uattr_size, common_attrs);
if (err < 0)
goto free_used_maps;
@@ -5292,7 +5293,8 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
#define BPF_BTF_LOAD_LAST_FIELD btf_token_fd
-static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size)
+static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size,
+ struct bpf_common_attr *common_attrs)
{
struct bpf_token *token = NULL;
@@ -5319,7 +5321,7 @@ static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_
bpf_token_put(token);
- return btf_new_fd(attr, uattr, uattr_size);
+ return btf_new_fd(attr, uattr, uattr_size, common_attrs);
}
#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD fd_by_id_token_fd
@@ -6036,7 +6038,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
err = map_freeze(&attr);
break;
case BPF_PROG_LOAD:
- err = bpf_prog_load(&attr, uattr, size);
+ err = bpf_prog_load(&attr, uattr, size, &common_attrs);
break;
case BPF_OBJ_PIN:
err = bpf_obj_pin(&attr);
@@ -6081,7 +6083,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
err = bpf_raw_tracepoint_open(&attr);
break;
case BPF_BTF_LOAD:
- err = bpf_btf_load(&attr, uattr, size);
+ err = bpf_btf_load(&attr, uattr, size, &common_attrs);
break;
case BPF_BTF_GET_FD_BY_ID:
err = bpf_btf_get_fd_by_id(&attr);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b9394f8fac0ed..77b57289ec097 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -24584,7 +24584,8 @@ static int compute_scc(struct bpf_verifier_env *env)
return err;
}
-int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size)
+int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size,
+ struct bpf_common_attr *common_attrs)
{
u64 start_time = ktime_get_ns();
struct bpf_verifier_env *env;
@@ -24633,9 +24634,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
/* user could have requested verbose verifier output
* and supplied buffer to store the verification trace
*/
- ret = bpf_vlog_init(&env->log, attr->log_level,
- (char __user *) (unsigned long) attr->log_buf,
- attr->log_size);
+ ret = bpf_vlog_init(&env->log, attr->log_level, u64_to_user_ptr(attr->log_buf),
+ attr->log_size, common_attrs);
if (ret)
goto err_unlock;
--
2.50.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create
2025-09-11 16:33 [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support Leon Hwang
` (2 preceding siblings ...)
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load Leon Hwang
@ 2025-09-11 16:33 ` Leon Hwang
2025-09-17 21:39 ` Andrii Nakryiko
2025-09-18 23:29 ` Eduard Zingerman
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 5/6] libbpf: " Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 6/6] selftests/bpf: Add cases to test map create failure log Leon Hwang
5 siblings, 2 replies; 25+ messages in thread
From: Leon Hwang @ 2025-09-11 16:33 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, daniel, menglong8.dong, Leon Hwang
Currently, many 'BPF_MAP_CREATE' failures return '-EINVAL' without
providing any explanation to user space.
With the extended BPF syscall support introduced in the previous patch,
detailed error messages can now be reported. This allows users to
understand the specific reason for a failed map creation, rather than
just receiving a generic '-EINVAL'.
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
kernel/bpf/syscall.c | 82 ++++++++++++++++++++++++++++++++++----------
1 file changed, 63 insertions(+), 19 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 5e5cf0262a14e..2f5e6005671b5 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1340,12 +1340,13 @@ static bool bpf_net_capable(void)
#define BPF_MAP_CREATE_LAST_FIELD map_token_fd
/* called via syscall */
-static int map_create(union bpf_attr *attr, bool kernel)
+static int map_create(union bpf_attr *attr, bool kernel, struct bpf_common_attr *common_attrs)
{
+ u32 map_type = attr->map_type, log_true_size;
+ struct bpf_verifier_log *log = NULL;
const struct bpf_map_ops *ops;
struct bpf_token *token = NULL;
int numa_node = bpf_map_attr_numa_node(attr);
- u32 map_type = attr->map_type;
struct bpf_map *map;
bool token_flag;
int f_flags;
@@ -1355,6 +1356,18 @@ static int map_create(union bpf_attr *attr, bool kernel)
if (err)
return -EINVAL;
+ if (common_attrs->log_buf) {
+ log = kvzalloc(sizeof(*log), GFP_KERNEL);
+ if (!log)
+ return -ENOMEM;
+ err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf),
+ common_attrs->log_size, NULL);
+ if (err) {
+ kvfree(log);
+ return err;
+ }
+ }
+
/* check BPF_F_TOKEN_FD flag, remember if it's set, and then clear it
* to avoid per-map type checks tripping on unknown flag
*/
@@ -1363,16 +1376,24 @@ static int map_create(union bpf_attr *attr, bool kernel)
if (attr->btf_vmlinux_value_type_id) {
if (attr->map_type != BPF_MAP_TYPE_STRUCT_OPS ||
- attr->btf_key_type_id || attr->btf_value_type_id)
- return -EINVAL;
+ attr->btf_key_type_id || attr->btf_value_type_id) {
+ bpf_log(log, "Invalid use of btf_vmlinux_value_type_id.\n");
+ err = -EINVAL;
+ goto put_token;
+ }
} else if (attr->btf_key_type_id && !attr->btf_value_type_id) {
- return -EINVAL;
+ bpf_log(log, "Invalid btf_value_type_id.\n");
+ err = -EINVAL;
+ goto put_token;
}
if (attr->map_type != BPF_MAP_TYPE_BLOOM_FILTER &&
attr->map_type != BPF_MAP_TYPE_ARENA &&
- attr->map_extra != 0)
- return -EINVAL;
+ attr->map_extra != 0) {
+ bpf_log(log, "Invalid map_extra.\n");
+ err = -EINVAL;
+ goto put_token;
+ }
f_flags = bpf_get_file_flag(attr->map_flags);
if (f_flags < 0)
@@ -1380,17 +1401,26 @@ static int map_create(union bpf_attr *attr, bool kernel)
if (numa_node != NUMA_NO_NODE &&
((unsigned int)numa_node >= nr_node_ids ||
- !node_online(numa_node)))
- return -EINVAL;
+ !node_online(numa_node))) {
+ bpf_log(log, "Invalid or offline numa_node.\n");
+ err = -EINVAL;
+ goto put_token;
+ }
/* find map type and init map: hashtable vs rbtree vs bloom vs ... */
map_type = attr->map_type;
- if (map_type >= ARRAY_SIZE(bpf_map_types))
- return -EINVAL;
+ if (map_type >= ARRAY_SIZE(bpf_map_types)) {
+ bpf_log(log, "Invalid map_type.\n");
+ err = -EINVAL;
+ goto put_token;
+ }
map_type = array_index_nospec(map_type, ARRAY_SIZE(bpf_map_types));
ops = bpf_map_types[map_type];
- if (!ops)
- return -EINVAL;
+ if (!ops) {
+ bpf_log(log, "Invalid map_type.\n");
+ err = -EINVAL;
+ goto put_token;
+ }
if (ops->map_alloc_check) {
err = ops->map_alloc_check(attr);
@@ -1399,13 +1429,20 @@ static int map_create(union bpf_attr *attr, bool kernel)
}
if (attr->map_ifindex)
ops = &bpf_map_offload_ops;
- if (!ops->map_mem_usage)
- return -EINVAL;
+ if (!ops->map_mem_usage) {
+ bpf_log(log, "map_mem_usage is required.\n");
+ err = -EINVAL;
+ goto put_token;
+ }
if (token_flag) {
token = bpf_token_get_from_fd(attr->map_token_fd);
- if (IS_ERR(token))
- return PTR_ERR(token);
+ if (IS_ERR(token)) {
+ bpf_log(log, "Invalid map_token_fd.\n");
+ err = PTR_ERR(token);
+ token = NULL;
+ goto put_token;
+ }
/* if current token doesn't grant map creation permissions,
* then we can't use this token, so ignore it and rely on
@@ -1487,8 +1524,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
err = bpf_obj_name_cpy(map->name, attr->map_name,
sizeof(attr->map_name));
- if (err < 0)
+ if (err < 0) {
+ bpf_log(log, "Invalid map_name.\n");
goto free_map;
+ }
preempt_disable();
map->cookie = gen_cookie_next(&bpf_map_cookie);
@@ -1511,6 +1550,7 @@ static int map_create(union bpf_attr *attr, bool kernel)
btf = btf_get_by_fd(attr->btf_fd);
if (IS_ERR(btf)) {
+ bpf_log(log, "Invalid btf_fd.\n");
err = PTR_ERR(btf);
goto free_map;
}
@@ -1565,6 +1605,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
bpf_map_free(map);
put_token:
bpf_token_put(token);
+ if (err && log)
+ (void) bpf_vlog_finalize(log, &log_true_size);
+ if (log)
+ kvfree(log);
return err;
}
@@ -6020,7 +6064,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
switch (cmd) {
case BPF_MAP_CREATE:
- err = map_create(&attr, uattr.is_kernel);
+ err = map_create(&attr, uattr.is_kernel, &common_attrs);
break;
case BPF_MAP_LOOKUP_ELEM:
err = map_lookup_elem(&attr);
--
2.50.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC PATCH bpf-next v2 5/6] libbpf: Add common attr support for map_create
2025-09-11 16:33 [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support Leon Hwang
` (3 preceding siblings ...)
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create Leon Hwang
@ 2025-09-11 16:33 ` Leon Hwang
2025-09-17 21:45 ` Andrii Nakryiko
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 6/6] selftests/bpf: Add cases to test map create failure log Leon Hwang
5 siblings, 1 reply; 25+ messages in thread
From: Leon Hwang @ 2025-09-11 16:33 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, daniel, menglong8.dong, Leon Hwang
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;
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);
+ if (log_buf) {
+ if (!feat_supported(NULL, FEAT_EXTENDED_SYSCALL))
+ return libbpf_err(-EOPNOTSUPP);
+
+ memset(&common_attrs, 0, sizeof(common_attrs));
+ common_attrs.log_buf = log_buf;
+ 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;
size_t :0;
};
-#define bpf_map_create_opts__last_field token_fd
+#define bpf_map_create_opts__last_field log_size
LIBBPF_API int bpf_map_create(enum bpf_map_type map_type,
const char *map_name,
--
2.50.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC PATCH bpf-next v2 6/6] selftests/bpf: Add cases to test map create failure log
2025-09-11 16:33 [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support Leon Hwang
` (4 preceding siblings ...)
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 5/6] libbpf: " Leon Hwang
@ 2025-09-11 16:33 ` Leon Hwang
5 siblings, 0 replies; 25+ messages in thread
From: Leon Hwang @ 2025-09-11 16:33 UTC (permalink / raw)
To: bpf; +Cc: ast, andrii, daniel, menglong8.dong, Leon Hwang
As kernel is able to report log when fail to create map, add test cases
to check those logs.
cd tools/testing/selftests/bpf
./test_progs -t map_create_failure
191/1 map_create_failure/invalid_vmlinux_value_type_id:OK
191/2 map_create_failure/invalid_value_type_id:OK
191/3 map_create_failure/invalid_map_extra:OK
191/4 map_create_failure/invalid_numa_node:OK
191/5 map_create_failure/invalid_map_type:OK
191/6 map_create_failure/invalid_token_fd:OK
191/7 map_create_failure/invalid_map_name:OK
191/8 map_create_failure/invalid_btf_fd:OK
191 map_create_failure:OK
Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
.../selftests/bpf/prog_tests/map_init.c | 124 ++++++++++++++++++
1 file changed, 124 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
index 14a31109dd0e0..ee7c73f5700c9 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_init.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_init.c
@@ -212,3 +212,127 @@ void test_map_init(void)
if (test__start_subtest("pcpu_lru_map_init"))
test_pcpu_lru_map_init();
}
+
+static void test_map_create(enum bpf_map_type map_type, const char *map_name,
+ struct bpf_map_create_opts *opts, const char *exp_msg)
+{
+ char log_buf[128];
+ int fd;
+
+ log_buf[0] = '\0';
+ opts->log_buf = log_buf;
+ opts->log_size = sizeof(log_buf);
+ fd = bpf_map_create(map_type, map_name, 4, 4, 1, opts);
+ if (!ASSERT_LT(fd, 0, "bpf_map_create"))
+ goto out;
+
+ ASSERT_STREQ(log_buf, exp_msg, "unexpected log_buf");
+out:
+ if (fd >= 0)
+ close(fd);
+}
+
+static void test_map_create_simple(struct bpf_map_create_opts *opts, const char *exp_msg)
+{
+ test_map_create(BPF_MAP_TYPE_ARRAY, "test_map_create", opts, exp_msg);
+}
+
+static void test_invalid_vmlinux_value_type_id(void)
+{
+ const char *msg = "Invalid use of btf_vmlinux_value_type_id.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts,
+ .btf_vmlinux_value_type_id = 1,
+ );
+
+ test_map_create_simple(&opts, msg);
+}
+
+static void test_invalid_value_type_id(void)
+{
+ const char *msg = "Invalid btf_value_type_id.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts,
+ .btf_key_type_id = 1,
+ );
+
+ test_map_create_simple(&opts, msg);
+}
+
+static void test_invalid_map_extra(void)
+{
+ const char *msg = "Invalid map_extra.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts,
+ .map_extra = 1,
+ );
+
+ test_map_create_simple(&opts, msg);
+}
+
+static void test_invalid_numa_node(void)
+{
+ const char *msg = "Invalid or offline numa_node.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts,
+ .map_flags = BPF_F_NUMA_NODE,
+ .numa_node = 0xFF,
+ );
+
+ test_map_create_simple(&opts, msg);
+}
+
+static void test_invalid_map_type(void)
+{
+ const char *msg = "Invalid map_type.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts);
+
+ test_map_create(__MAX_BPF_MAP_TYPE, "test_map_create", &opts, msg);
+}
+
+static void test_invalid_token_fd(void)
+{
+ const char *msg = "Invalid map_token_fd.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts,
+ .map_flags = BPF_F_TOKEN_FD,
+ .token_fd = 0xFF,
+ );
+
+ test_map_create_simple(&opts, msg);
+}
+
+static void test_invalid_map_name(void)
+{
+ const char *msg = "Invalid map_name.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts);
+
+ test_map_create(BPF_MAP_TYPE_ARRAY, "test-!@#", &opts, msg);
+}
+
+static void test_invalid_btf_fd(void)
+{
+ const char *msg = "Invalid btf_fd.\n";
+ LIBBPF_OPTS(bpf_map_create_opts, opts,
+ .btf_fd = -1,
+ .btf_key_type_id = 1,
+ .btf_value_type_id = 1,
+ );
+
+ test_map_create_simple(&opts, msg);
+}
+
+void test_map_create_failure(void)
+{
+ if (test__start_subtest("invalid_vmlinux_value_type_id"))
+ test_invalid_vmlinux_value_type_id();
+ if (test__start_subtest("invalid_value_type_id"))
+ test_invalid_value_type_id();
+ if (test__start_subtest("invalid_map_extra"))
+ test_invalid_map_extra();
+ if (test__start_subtest("invalid_numa_node"))
+ test_invalid_numa_node();
+ if (test__start_subtest("invalid_map_type"))
+ test_invalid_map_type();
+ if (test__start_subtest("invalid_token_fd"))
+ test_invalid_token_fd();
+ if (test__start_subtest("invalid_map_name"))
+ test_invalid_map_name();
+ if (test__start_subtest("invalid_btf_fd"))
+ test_invalid_btf_fd();
+}
--
2.50.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 1/6] bpf: Extend bpf syscall with common attributes support
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 1/6] " Leon Hwang
@ 2025-09-17 0:06 ` Andrii Nakryiko
2025-09-23 15:23 ` Leon Hwang
0 siblings, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-17 0:06 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> This patch extends the 'bpf()' syscall to support a set of common
> attributes shared across all BPF commands:
>
> 1. 'log_buf': User-provided buffer for storing logs
> 2. 'log_size': Size of the log buffer
> 3. 'log_level': Log verbosity level
>
> These common attributes are passed as the 4th argument to the 'bpf()'
> syscall, with the 5th argument specifying the size of this structure.
>
> To indicate the use of these common attributes from userspace, a new flag
> 'BPF_COMMON_ATTRS' ('1 << 16') is introduced. This flag is OR-ed into the
> 'cmd' field of the syscall.
>
> When 'cmd & BPF_COMMON_ATTRS' is set, the kernel will copy the common
> attributes from userspace into kernel space for use.
>
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
> include/linux/syscalls.h | 3 ++-
> include/uapi/linux/bpf.h | 7 +++++++
> kernel/bpf/syscall.c | 19 +++++++++++++++----
> tools/include/uapi/linux/bpf.h | 7 +++++++
> 4 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 77f45e5d44139..94408575dc49b 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -933,7 +933,8 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
> asmlinkage long sys_getrandom(char __user *buf, size_t count,
> unsigned int flags);
> asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
> -asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size);
> +asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size,
> + struct bpf_common_attr __user *attr_common, unsigned int size_common);
> asmlinkage long sys_execveat(int dfd, const char __user *filename,
> const char __user *const __user *argv,
> const char __user *const __user *envp, int flags);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 233de8677382e..5014baccf065f 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1474,6 +1474,13 @@ struct bpf_stack_build_id {
> };
> };
>
> +struct bpf_common_attr {
> + __u64 log_buf;
> + __u32 log_size;
> + __u32 log_level;
> +};
> +
> +#define BPF_COMMON_ATTRS (1 << 16)
add this into enum bpf_cmd after __MAX_BPF_CMD (with a small comment
about the purpose of this)? That will keep everything cmd-related in
one place
> #define BPF_OBJ_NAME_LEN 16U
>
> enum {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 3f178a0f8eb12..d49f822ceea12 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -5987,8 +5987,10 @@ static int prog_stream_read(union bpf_attr *attr)
> return ret;
> }
>
> -static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
> +static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
> + bpfptr_t uattr_common, unsigned int size_common)
> {
> + struct bpf_common_attr common_attrs;
> union bpf_attr attr;
> int err;
>
> @@ -6002,6 +6004,14 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
> if (copy_from_bpfptr(&attr, uattr, size) != 0)
> return -EFAULT;
>
> + memset(&common_attrs, 0, sizeof(common_attrs));
> + if (cmd & BPF_COMMON_ATTRS) {
> + cmd &= ~BPF_COMMON_ATTRS;
> + size_common = min_t(u32, size_common, sizeof(common_attrs));
> + if (uattr_common.user && copy_from_bpfptr(&common_attrs, uattr_common, size_common))
> + return -EFAULT;
use bpf_check_uarg_tail_zero() for extra checks, just like we do for uattr
> + }
> +
> err = security_bpf(cmd, &attr, size, uattr.is_kernel);
> if (err < 0)
> return err;
[...]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall Leon Hwang
@ 2025-09-17 0:06 ` Andrii Nakryiko
2025-09-23 15:36 ` Leon Hwang
0 siblings, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-17 0:06 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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)
> + 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.
> +}
> +
> 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?
> LIBBPF_API int libbpf_set_memlock_rlim(size_t memlock_bytes);
>
> struct bpf_map_create_opts {
> diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> index 760657f5224c2..a63172c6343d0 100644
> --- a/tools/lib/bpf/features.c
> +++ b/tools/lib/bpf/features.c
[...]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load Leon Hwang
@ 2025-09-17 21:12 ` Andrii Nakryiko
2025-09-23 15:50 ` Leon Hwang
0 siblings, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-17 21:12 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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).
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.
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
> include/linux/bpf.h | 3 ++-
> include/linux/bpf_verifier.h | 2 +-
> include/linux/btf.h | 3 ++-
> kernel/bpf/btf.c | 12 +++++++-----
> kernel/bpf/log.c | 23 ++++++++++++++++++++++-
> kernel/bpf/syscall.c | 14 ++++++++------
> kernel/bpf/verifier.c | 8 ++++----
> 7 files changed, 46 insertions(+), 19 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create Leon Hwang
@ 2025-09-17 21:39 ` Andrii Nakryiko
2025-09-17 21:49 ` Alexei Starovoitov
2025-09-23 16:27 ` Leon Hwang
2025-09-18 23:29 ` Eduard Zingerman
1 sibling, 2 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-17 21:39 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> Currently, many 'BPF_MAP_CREATE' failures return '-EINVAL' without
> providing any explanation to user space.
>
> With the extended BPF syscall support introduced in the previous patch,
> detailed error messages can now be reported. This allows users to
> understand the specific reason for a failed map creation, rather than
> just receiving a generic '-EINVAL'.
>
> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> ---
> kernel/bpf/syscall.c | 82 ++++++++++++++++++++++++++++++++++----------
> 1 file changed, 63 insertions(+), 19 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 5e5cf0262a14e..2f5e6005671b5 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1340,12 +1340,13 @@ static bool bpf_net_capable(void)
>
> #define BPF_MAP_CREATE_LAST_FIELD map_token_fd
> /* called via syscall */
> -static int map_create(union bpf_attr *attr, bool kernel)
> +static int map_create(union bpf_attr *attr, bool kernel, struct bpf_common_attr *common_attrs)
> {
> + u32 map_type = attr->map_type, log_true_size;
> + struct bpf_verifier_log *log = NULL;
> const struct bpf_map_ops *ops;
> struct bpf_token *token = NULL;
> int numa_node = bpf_map_attr_numa_node(attr);
> - u32 map_type = attr->map_type;
> struct bpf_map *map;
> bool token_flag;
> int f_flags;
> @@ -1355,6 +1356,18 @@ static int map_create(union bpf_attr *attr, bool kernel)
> if (err)
> return -EINVAL;
>
> + if (common_attrs->log_buf) {
> + log = kvzalloc(sizeof(*log), GFP_KERNEL);
> + if (!log)
> + return -ENOMEM;
> + err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf),
> + common_attrs->log_size, NULL);
> + if (err) {
> + kvfree(log);
> + return err;
> + }
> + }
what if we keep bpf_verifier_log on stack? It's 1KB, should be still
fine to be on kernel stack, no?
> +
> /* check BPF_F_TOKEN_FD flag, remember if it's set, and then clear it
> * to avoid per-map type checks tripping on unknown flag
> */
> @@ -1363,16 +1376,24 @@ static int map_create(union bpf_attr *attr, bool kernel)
>
> if (attr->btf_vmlinux_value_type_id) {
> if (attr->map_type != BPF_MAP_TYPE_STRUCT_OPS ||
> - attr->btf_key_type_id || attr->btf_value_type_id)
> - return -EINVAL;
> + attr->btf_key_type_id || attr->btf_value_type_id) {
> + bpf_log(log, "Invalid use of btf_vmlinux_value_type_id.\n");
I don't know how far we want to go here, but I'd split the original
check into map type check and key_type/value_type check, and log a bit
more meaningful error:
a) btf_vmlinux_value_type_id can only be used with struct_ops maps.
and
b) btf_vmlinux_value_type_id is mutually exclusive with
btf_key_type_id and btf_value_type_id
> + err = -EINVAL;
> + goto put_token;
there is no token just yet, add new label for finalizing log?
> + }
> } else if (attr->btf_key_type_id && !attr->btf_value_type_id) {
> - return -EINVAL;
> + bpf_log(log, "Invalid btf_value_type_id.\n");
> + err = -EINVAL;
> + goto put_token;
ditto about token
> }
>
> if (attr->map_type != BPF_MAP_TYPE_BLOOM_FILTER &&
> attr->map_type != BPF_MAP_TYPE_ARENA &&
> - attr->map_extra != 0)
> - return -EINVAL;
> + attr->map_extra != 0) {
> + bpf_log(log, "Invalid map_extra.\n");
> + err = -EINVAL;
> + goto put_token;
> + }
>
> f_flags = bpf_get_file_flag(attr->map_flags);
> if (f_flags < 0)
> @@ -1380,17 +1401,26 @@ static int map_create(union bpf_attr *attr, bool kernel)
>
> if (numa_node != NUMA_NO_NODE &&
> ((unsigned int)numa_node >= nr_node_ids ||
> - !node_online(numa_node)))
> - return -EINVAL;
> + !node_online(numa_node))) {
> + bpf_log(log, "Invalid or offline numa_node.\n");
nit: just "invalid numa_node" ?
> + err = -EINVAL;
> + goto put_token;
> + }
>
> /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
> map_type = attr->map_type;
> - if (map_type >= ARRAY_SIZE(bpf_map_types))
> - return -EINVAL;
> + if (map_type >= ARRAY_SIZE(bpf_map_types)) {
> + bpf_log(log, "Invalid map_type.\n");
> + err = -EINVAL;
> + goto put_token;
> + }
> map_type = array_index_nospec(map_type, ARRAY_SIZE(bpf_map_types));
> ops = bpf_map_types[map_type];
> - if (!ops)
> - return -EINVAL;
> + if (!ops) {
> + bpf_log(log, "Invalid map_type.\n");
> + err = -EINVAL;
> + goto put_token;
> + }
>
> if (ops->map_alloc_check) {
> err = ops->map_alloc_check(attr);
> @@ -1399,13 +1429,20 @@ static int map_create(union bpf_attr *attr, bool kernel)
> }
> if (attr->map_ifindex)
> ops = &bpf_map_offload_ops;
> - if (!ops->map_mem_usage)
> - return -EINVAL;
> + if (!ops->map_mem_usage) {
> + bpf_log(log, "map_mem_usage is required.\n");
this is kernel bug, actually, so let's log "bug: " prefix? same above
with the second "Invalid map_type." message?
and actually, given these are kernel bugs and shouldn't happen, I
wonder if we should log anything here at all?
> + err = -EINVAL;
> + goto put_token;
> + }
>
> if (token_flag) {
> token = bpf_token_get_from_fd(attr->map_token_fd);
> - if (IS_ERR(token))
> - return PTR_ERR(token);
> + if (IS_ERR(token)) {
> + bpf_log(log, "Invalid map_token_fd.\n");
> + err = PTR_ERR(token);
> + token = NULL;
> + goto put_token;
ditto, no token
> + }
>
> /* if current token doesn't grant map creation permissions,
> * then we can't use this token, so ignore it and rely on
> @@ -1487,8 +1524,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
>
> err = bpf_obj_name_cpy(map->name, attr->map_name,
> sizeof(attr->map_name));
> - if (err < 0)
> + if (err < 0) {
> + bpf_log(log, "Invalid map_name.\n");
> goto free_map;
> + }
>
> preempt_disable();
> map->cookie = gen_cookie_next(&bpf_map_cookie);
> @@ -1511,6 +1550,7 @@ static int map_create(union bpf_attr *attr, bool kernel)
>
> btf = btf_get_by_fd(attr->btf_fd);
> if (IS_ERR(btf)) {
> + bpf_log(log, "Invalid btf_fd.\n");
> err = PTR_ERR(btf);
> goto free_map;
> }
> @@ -1565,6 +1605,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
> bpf_map_free(map);
> put_token:
> bpf_token_put(token);
> + if (err && log)
> + (void) bpf_vlog_finalize(log, &log_true_size);
so we'll just drop this size on the floor and never report it to the user?
a) let's either teach bpf_vlog_finalize that log_true_size pointer can
be NULL (and optional),
or b) let's report it back to user through that same commont_attrs
struct, just like we do it for verifier and btf logs?
Also, what if complicating error handling with this goto jumping, can
we make use of __cleanup() attribute to do this automatically? Then
we'd just allocate (or see above, maybe just init on the stack) log
struct, and declare it with cleanup callback that will do this
vlog_finalize and, maybe, report back the log actual size?
> + if (log)
> + kvfree(log);
> return err;
> }
>
> @@ -6020,7 +6064,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
>
> switch (cmd) {
> case BPF_MAP_CREATE:
> - err = map_create(&attr, uattr.is_kernel);
> + err = map_create(&attr, uattr.is_kernel, &common_attrs);
> break;
> case BPF_MAP_LOOKUP_ELEM:
> err = map_lookup_elem(&attr);
> --
> 2.50.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 5/6] libbpf: Add common attr support for map_create
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 5/6] libbpf: " Leon Hwang
@ 2025-09-17 21:45 ` Andrii Nakryiko
2025-09-17 21:46 ` Andrii Nakryiko
0 siblings, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-17 21:45 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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 *
> 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
> + 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.
> +
> + memset(&common_attrs, 0, sizeof(common_attrs));
> + common_attrs.log_buf = log_buf;
ptr_to_u64(log_buf) here
> + 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;
> size_t :0;
> };
> -#define bpf_map_create_opts__last_field token_fd
> +#define bpf_map_create_opts__last_field log_size
>
> LIBBPF_API int bpf_map_create(enum bpf_map_type map_type,
> const char *map_name,
> --
> 2.50.1
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 5/6] libbpf: Add common attr support for map_create
2025-09-17 21:45 ` Andrii Nakryiko
@ 2025-09-17 21:46 ` Andrii Nakryiko
2025-09-23 16:40 ` Leon Hwang
0 siblings, 1 reply; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-17 21:46 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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 *
>
> > 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
>
> > + 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.
>
> > +
> > + memset(&common_attrs, 0, sizeof(common_attrs));
> > + common_attrs.log_buf = log_buf;
>
> ptr_to_u64(log_buf) here
>
> > + 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 ?
> > size_t :0;
> > };
> > -#define bpf_map_create_opts__last_field token_fd
> > +#define bpf_map_create_opts__last_field log_size
> >
> > LIBBPF_API int bpf_map_create(enum bpf_map_type map_type,
> > const char *map_name,
> > --
> > 2.50.1
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create
2025-09-17 21:39 ` Andrii Nakryiko
@ 2025-09-17 21:49 ` Alexei Starovoitov
2025-09-23 15:52 ` Leon Hwang
2025-09-23 16:27 ` Leon Hwang
1 sibling, 1 reply; 25+ messages in thread
From: Alexei Starovoitov @ 2025-09-17 21:49 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Leon Hwang, bpf, Alexei Starovoitov, Andrii Nakryiko,
Daniel Borkmann, Menglong Dong
On Wed, Sep 17, 2025 at 2:39 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
> >
> > Currently, many 'BPF_MAP_CREATE' failures return '-EINVAL' without
> > providing any explanation to user space.
> >
> > With the extended BPF syscall support introduced in the previous patch,
> > detailed error messages can now be reported. This allows users to
> > understand the specific reason for a failed map creation, rather than
> > just receiving a generic '-EINVAL'.
> >
> > Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
> > ---
> > kernel/bpf/syscall.c | 82 ++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 63 insertions(+), 19 deletions(-)
> >
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 5e5cf0262a14e..2f5e6005671b5 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -1340,12 +1340,13 @@ static bool bpf_net_capable(void)
> >
> > #define BPF_MAP_CREATE_LAST_FIELD map_token_fd
> > /* called via syscall */
> > -static int map_create(union bpf_attr *attr, bool kernel)
> > +static int map_create(union bpf_attr *attr, bool kernel, struct bpf_common_attr *common_attrs)
> > {
> > + u32 map_type = attr->map_type, log_true_size;
> > + struct bpf_verifier_log *log = NULL;
> > const struct bpf_map_ops *ops;
> > struct bpf_token *token = NULL;
> > int numa_node = bpf_map_attr_numa_node(attr);
> > - u32 map_type = attr->map_type;
> > struct bpf_map *map;
> > bool token_flag;
> > int f_flags;
> > @@ -1355,6 +1356,18 @@ static int map_create(union bpf_attr *attr, bool kernel)
> > if (err)
> > return -EINVAL;
> >
> > + if (common_attrs->log_buf) {
> > + log = kvzalloc(sizeof(*log), GFP_KERNEL);
> > + if (!log)
> > + return -ENOMEM;
> > + err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf),
> > + common_attrs->log_size, NULL);
> > + if (err) {
> > + kvfree(log);
> > + return err;
> > + }
> > + }
>
> what if we keep bpf_verifier_log on stack? It's 1KB, should be still
> fine to be on kernel stack, no?
1k is a lot. I'm pretty nervous about stack usage. kmalloc is cheap.
Let's use it. I'd drop 'v' part. There is no way it will fallback
to valloc() even if we double BPF_VERIFIER_TMP_LOG_SIZE in the future.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create Leon Hwang
2025-09-17 21:39 ` Andrii Nakryiko
@ 2025-09-18 23:29 ` Eduard Zingerman
2025-09-23 16:31 ` Leon Hwang
1 sibling, 1 reply; 25+ messages in thread
From: Eduard Zingerman @ 2025-09-18 23:29 UTC (permalink / raw)
To: Leon Hwang, bpf; +Cc: ast, andrii, daniel, menglong8.dong
On Fri, 2025-09-12 at 00:33 +0800, Leon Hwang wrote:
[...]
> @@ -1355,6 +1356,18 @@ static int map_create(union bpf_attr *attr, bool kernel)
> if (err)
> return -EINVAL;
>
> + if (common_attrs->log_buf) {
> + log = kvzalloc(sizeof(*log), GFP_KERNEL);
> + if (!log)
> + return -ENOMEM;
> + err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf),
> + common_attrs->log_size, NULL);
Maybe use common_attrs->log_level instead of BPF_LOG_FIXED?
Just for consistent behavior with program and btf load operations.
> + if (err) {
> + kvfree(log);
> + return err;
> + }
> + }
> +
> /* check BPF_F_TOKEN_FD flag, remember if it's set, and then clear it
> * to avoid per-map type checks tripping on unknown flag
> */
[...]
> @@ -1565,6 +1605,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
> bpf_map_free(map);
> put_token:
> bpf_token_put(token);
> + if (err && log)
> + (void) bpf_vlog_finalize(log, &log_true_size);
> + if (log)
> + kvfree(log);
> return err;
> }
+1 to Andrii's suggestion to report log size back,
just for consistency reasons.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 1/6] bpf: Extend bpf syscall with common attributes support
2025-09-17 0:06 ` Andrii Nakryiko
@ 2025-09-23 15:23 ` Leon Hwang
0 siblings, 0 replies; 25+ messages in thread
From: Leon Hwang @ 2025-09-23 15:23 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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:
>>
>> This patch extends the 'bpf()' syscall to support a set of common
>> attributes shared across all BPF commands:
>>
>> 1. 'log_buf': User-provided buffer for storing logs
>> 2. 'log_size': Size of the log buffer
>> 3. 'log_level': Log verbosity level
>>
>> These common attributes are passed as the 4th argument to the 'bpf()'
>> syscall, with the 5th argument specifying the size of this structure.
>>
>> To indicate the use of these common attributes from userspace, a new flag
>> 'BPF_COMMON_ATTRS' ('1 << 16') is introduced. This flag is OR-ed into the
>> 'cmd' field of the syscall.
>>
>> When 'cmd & BPF_COMMON_ATTRS' is set, the kernel will copy the common
>> attributes from userspace into kernel space for use.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>> include/linux/syscalls.h | 3 ++-
>> include/uapi/linux/bpf.h | 7 +++++++
>> kernel/bpf/syscall.c | 19 +++++++++++++++----
>> tools/include/uapi/linux/bpf.h | 7 +++++++
>> 4 files changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 77f45e5d44139..94408575dc49b 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -933,7 +933,8 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
>> asmlinkage long sys_getrandom(char __user *buf, size_t count,
>> unsigned int flags);
>> asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
>> -asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size);
>> +asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size,
>> + struct bpf_common_attr __user *attr_common, unsigned int size_common);
>> asmlinkage long sys_execveat(int dfd, const char __user *filename,
>> const char __user *const __user *argv,
>> const char __user *const __user *envp, int flags);
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 233de8677382e..5014baccf065f 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -1474,6 +1474,13 @@ struct bpf_stack_build_id {
>> };
>> };
>>
>> +struct bpf_common_attr {
>> + __u64 log_buf;
>> + __u32 log_size;
>> + __u32 log_level;
>> +};
>> +
>> +#define BPF_COMMON_ATTRS (1 << 16)
>
> add this into enum bpf_cmd after __MAX_BPF_CMD (with a small comment
> about the purpose of this)? That will keep everything cmd-related in
> one place
>
Ack.
>> #define BPF_OBJ_NAME_LEN 16U
>>
>> enum {
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 3f178a0f8eb12..d49f822ceea12 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -5987,8 +5987,10 @@ static int prog_stream_read(union bpf_attr *attr)
>> return ret;
>> }
>>
>> -static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
>> +static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
>> + bpfptr_t uattr_common, unsigned int size_common)
>> {
>> + struct bpf_common_attr common_attrs;
>> union bpf_attr attr;
>> int err;
>>
>> @@ -6002,6 +6004,14 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size)
>> if (copy_from_bpfptr(&attr, uattr, size) != 0)
>> return -EFAULT;
>>
>> + memset(&common_attrs, 0, sizeof(common_attrs));
>> + if (cmd & BPF_COMMON_ATTRS) {
>> + cmd &= ~BPF_COMMON_ATTRS;
>> + size_common = min_t(u32, size_common, sizeof(common_attrs));
>> + if (uattr_common.user && copy_from_bpfptr(&common_attrs, uattr_common, size_common))
>> + return -EFAULT;
>
> use bpf_check_uarg_tail_zero() for extra checks, just like we do for uattr
>
Ack.
Thanks,
Leon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall
2025-09-17 0:06 ` Andrii Nakryiko
@ 2025-09-23 15:36 ` Leon Hwang
2025-09-24 23:57 ` Andrii Nakryiko
0 siblings, 1 reply; 25+ messages in thread
From: Leon Hwang @ 2025-09-23 15:36 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load
2025-09-17 21:12 ` Andrii Nakryiko
@ 2025-09-23 15:50 ` Leon Hwang
2025-09-25 0:00 ` Andrii Nakryiko
0 siblings, 1 reply; 25+ messages in thread
From: Leon Hwang @ 2025-09-23 15:50 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create
2025-09-17 21:49 ` Alexei Starovoitov
@ 2025-09-23 15:52 ` Leon Hwang
0 siblings, 0 replies; 25+ messages in thread
From: Leon Hwang @ 2025-09-23 15:52 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Menglong Dong
On 2025/9/18 05:49, Alexei Starovoitov wrote:
> On Wed, Sep 17, 2025 at 2:39 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>>
[...]
>>>
>>> + if (common_attrs->log_buf) {
>>> + log = kvzalloc(sizeof(*log), GFP_KERNEL);
>>> + if (!log)
>>> + return -ENOMEM;
>>> + err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf),
>>> + common_attrs->log_size, NULL);
>>> + if (err) {
>>> + kvfree(log);
>>> + return err;
>>> + }
>>> + }
>>
>> what if we keep bpf_verifier_log on stack? It's 1KB, should be still
>> fine to be on kernel stack, no?
>
> 1k is a lot. I'm pretty nervous about stack usage. kmalloc is cheap.
> Let's use it. I'd drop 'v' part. There is no way it will fallback
> to valloc() even if we double BPF_VERIFIER_TMP_LOG_SIZE in the future.
I'll drop 'v' part in the next revision.
Thanks,
Leon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create
2025-09-17 21:39 ` Andrii Nakryiko
2025-09-17 21:49 ` Alexei Starovoitov
@ 2025-09-23 16:27 ` Leon Hwang
1 sibling, 0 replies; 25+ messages in thread
From: Leon Hwang @ 2025-09-23 16:27 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, andrii, daniel, menglong8.dong
On 2025/9/18 05:39, Andrii Nakryiko wrote:
> On Thu, Sep 11, 2025 at 9:33 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>>
>> Currently, many 'BPF_MAP_CREATE' failures return '-EINVAL' without
>> providing any explanation to user space.
>>
>> With the extended BPF syscall support introduced in the previous patch,
>> detailed error messages can now be reported. This allows users to
>> understand the specific reason for a failed map creation, rather than
>> just receiving a generic '-EINVAL'.
>>
>> Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
>> ---
>> kernel/bpf/syscall.c | 82 ++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 63 insertions(+), 19 deletions(-)
>>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 5e5cf0262a14e..2f5e6005671b5 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1340,12 +1340,13 @@ static bool bpf_net_capable(void)
>>
>> #define BPF_MAP_CREATE_LAST_FIELD map_token_fd
>> /* called via syscall */
>> -static int map_create(union bpf_attr *attr, bool kernel)
>> +static int map_create(union bpf_attr *attr, bool kernel, struct bpf_common_attr *common_attrs)
>> {
>> + u32 map_type = attr->map_type, log_true_size;
>> + struct bpf_verifier_log *log = NULL;
>> const struct bpf_map_ops *ops;
>> struct bpf_token *token = NULL;
>> int numa_node = bpf_map_attr_numa_node(attr);
>> - u32 map_type = attr->map_type;
>> struct bpf_map *map;
>> bool token_flag;
>> int f_flags;
>> @@ -1355,6 +1356,18 @@ static int map_create(union bpf_attr *attr, bool kernel)
>> if (err)
>> return -EINVAL;
>>
>> + if (common_attrs->log_buf) {
>> + log = kvzalloc(sizeof(*log), GFP_KERNEL);
>> + if (!log)
>> + return -ENOMEM;
>> + err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf),
>> + common_attrs->log_size, NULL);
>> + if (err) {
>> + kvfree(log);
>> + return err;
>> + }
>> + }
>
> what if we keep bpf_verifier_log on stack? It's 1KB, should be still
> fine to be on kernel stack, no?
>
I'm going to follow Alexei's suggestion.
>
>> +
>> /* check BPF_F_TOKEN_FD flag, remember if it's set, and then clear it
>> * to avoid per-map type checks tripping on unknown flag
>> */
>> @@ -1363,16 +1376,24 @@ static int map_create(union bpf_attr *attr, bool kernel)
>>
>> if (attr->btf_vmlinux_value_type_id) {
>> if (attr->map_type != BPF_MAP_TYPE_STRUCT_OPS ||
>> - attr->btf_key_type_id || attr->btf_value_type_id)
>> - return -EINVAL;
>> + attr->btf_key_type_id || attr->btf_value_type_id) {
>> + bpf_log(log, "Invalid use of btf_vmlinux_value_type_id.\n");
>
> I don't know how far we want to go here, but I'd split the original
> check into map type check and key_type/value_type check, and log a bit
> more meaningful error:
>
> a) btf_vmlinux_value_type_id can only be used with struct_ops maps.
>
> and
>
> b) btf_vmlinux_value_type_id is mutually exclusive with
> btf_key_type_id and btf_value_type_id
>
Sure.
It would be better to separate it like this.
>> + err = -EINVAL;
>> + goto put_token;
>
> there is no token just yet, add new label for finalizing log?
>
Ack.
>> + }
>> } else if (attr->btf_key_type_id && !attr->btf_value_type_id) {
>> - return -EINVAL;
>> + bpf_log(log, "Invalid btf_value_type_id.\n");
>> + err = -EINVAL;
>> + goto put_token;
>
> ditto about token
>
Ack.
>> }
>>
>> if (attr->map_type != BPF_MAP_TYPE_BLOOM_FILTER &&
>> attr->map_type != BPF_MAP_TYPE_ARENA &&
>> - attr->map_extra != 0)
>> - return -EINVAL;
>> + attr->map_extra != 0) {
>> + bpf_log(log, "Invalid map_extra.\n");
>> + err = -EINVAL;
>> + goto put_token;
It'll be changed to the new label.
>> + }
>>
>> f_flags = bpf_get_file_flag(attr->map_flags);
>> if (f_flags < 0)
>> @@ -1380,17 +1401,26 @@ static int map_create(union bpf_attr *attr, bool kernel)
>>
>> if (numa_node != NUMA_NO_NODE &&
>> ((unsigned int)numa_node >= nr_node_ids ||
>> - !node_online(numa_node)))
>> - return -EINVAL;
>> + !node_online(numa_node))) {
>> + bpf_log(log, "Invalid or offline numa_node.\n");
>
>
> nit: just "invalid numa_node" ?
>
Ack.
>> + err = -EINVAL;
>> + goto put_token;
It'll be changed to the new label.
>> + }
>>
>> /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>> map_type = attr->map_type;
>> - if (map_type >= ARRAY_SIZE(bpf_map_types))
>> - return -EINVAL;
>> + if (map_type >= ARRAY_SIZE(bpf_map_types)) {
>> + bpf_log(log, "Invalid map_type.\n");
>> + err = -EINVAL;
>> + goto put_token;
It'll be changed to the new label.
>> + }
>> map_type = array_index_nospec(map_type, ARRAY_SIZE(bpf_map_types));
>> ops = bpf_map_types[map_type];
>> - if (!ops)
>> - return -EINVAL;
>> + if (!ops) {
>> + bpf_log(log, "Invalid map_type.\n");
>> + err = -EINVAL;
>> + goto put_token;
It'll be changed to the new label.
>> + }
>>
>> if (ops->map_alloc_check) {
>> err = ops->map_alloc_check(attr);
>> @@ -1399,13 +1429,20 @@ static int map_create(union bpf_attr *attr, bool kernel)
>> }
>> if (attr->map_ifindex)
>> ops = &bpf_map_offload_ops;
>> - if (!ops->map_mem_usage)
>> - return -EINVAL;
>> + if (!ops->map_mem_usage) {
>> + bpf_log(log, "map_mem_usage is required.\n");
>
>
> this is kernel bug, actually, so let's log "bug: " prefix? same above
> with the second "Invalid map_type." message?
>
> and actually, given these are kernel bugs and shouldn't happen, I
> wonder if we should log anything here at all?
>
As it's a kernel bug if no map_mem_usage, would it be better to
WARN_ONCE here instead of reporting a log?
>> + err = -EINVAL;
>> + goto put_token;
It'll be changed to the new label.
>> + }
>>
>> if (token_flag) {
>> token = bpf_token_get_from_fd(attr->map_token_fd);
>> - if (IS_ERR(token))
>> - return PTR_ERR(token);
>> + if (IS_ERR(token)) {
>> + bpf_log(log, "Invalid map_token_fd.\n");
>> + err = PTR_ERR(token);
>> + token = NULL;
>> + goto put_token;
>
> ditto, no token
>
It'll be changed to the new label.
>> + }
>>
>> /* if current token doesn't grant map creation permissions,
>> * then we can't use this token, so ignore it and rely on
>> @@ -1487,8 +1524,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
>>
>> err = bpf_obj_name_cpy(map->name, attr->map_name,
>> sizeof(attr->map_name));
>> - if (err < 0)
>> + if (err < 0) {
>> + bpf_log(log, "Invalid map_name.\n");
>> goto free_map;
>> + }
>>
>> preempt_disable();
>> map->cookie = gen_cookie_next(&bpf_map_cookie);
>> @@ -1511,6 +1550,7 @@ static int map_create(union bpf_attr *attr, bool kernel)
>>
>> btf = btf_get_by_fd(attr->btf_fd);
>> if (IS_ERR(btf)) {
>> + bpf_log(log, "Invalid btf_fd.\n");
>> err = PTR_ERR(btf);
>> goto free_map;
>> }
>> @@ -1565,6 +1605,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
>> bpf_map_free(map);
>> put_token:
>> bpf_token_put(token);
>> + if (err && log)
>> + (void) bpf_vlog_finalize(log, &log_true_size);
>
> so we'll just drop this size on the floor and never report it to the user?
>
>
> a) let's either teach bpf_vlog_finalize that log_true_size pointer can
> be NULL (and optional),
> or b) let's report it back to user through that same commont_attrs
> struct, just like we do it for verifier and btf logs?
>
Reporting it back to user looks better to me.
>
> Also, what if complicating error handling with this goto jumping, can
> we make use of __cleanup() attribute to do this automatically? Then
> we'd just allocate (or see above, maybe just init on the stack) log
> struct, and declare it with cleanup callback that will do this
> vlog_finalize and, maybe, report back the log actual size?
>
It does seem feasible to simplify error handling with __cleanup().
Instead of initializing log directly on the stack, we could introduce a
small wrapper:
struct bpf_log_wrapper {
struct bpf_verifier_log *log;
};
with a destructor:
static void bpf_log_wrapper_destructor(struct bpf_log_wrapper *w)
{
u32 log_true_size;
if (w->log) {
(void) bpf_vlog_finalize(w->log, &log_true_size);
kfree(w->log);
}
}
In this demo snippet I skipped handling log_true_size, but in practice
it should be dealt with properly.
Then, for example, in map_create():
struct bpf_log_wrapper log_wrapper __cleanup(bpf_log_wrapper_destructor)
= {};
if (common_attrs->log_buf) {
log_wrapper.log = kzalloc(...);
}
I’ll give this approach a try in the next revision.
Thanks,
Leon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create
2025-09-18 23:29 ` Eduard Zingerman
@ 2025-09-23 16:31 ` Leon Hwang
0 siblings, 0 replies; 25+ messages in thread
From: Leon Hwang @ 2025-09-23 16:31 UTC (permalink / raw)
To: Eduard Zingerman, bpf; +Cc: ast, andrii, daniel, menglong8.dong
On 2025/9/19 07:29, Eduard Zingerman wrote:
> On Fri, 2025-09-12 at 00:33 +0800, Leon Hwang wrote:
>
> [...]
>
>> @@ -1355,6 +1356,18 @@ static int map_create(union bpf_attr *attr, bool kernel)
>> if (err)
>> return -EINVAL;
>>
>> + if (common_attrs->log_buf) {
>> + log = kvzalloc(sizeof(*log), GFP_KERNEL);
>> + if (!log)
>> + return -ENOMEM;
>> + err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf),
>> + common_attrs->log_size, NULL);
>
> Maybe use common_attrs->log_level instead of BPF_LOG_FIXED?
> Just for consistent behavior with program and btf load operations.
>
It doesn’t really make sense to let users set log_level in this case,
because logging in map_create is too simple for different log levels to
have any meaningful effect.
>> + if (err) {
>> + kvfree(log);
>> + return err;
>> + }
>> + }
>> +
>> /* check BPF_F_TOKEN_FD flag, remember if it's set, and then clear it
>> * to avoid per-map type checks tripping on unknown flag
>> */
>
> [...]
>
>> @@ -1565,6 +1605,10 @@ static int map_create(union bpf_attr *attr, bool kernel)
>> bpf_map_free(map);
>> put_token:
>> bpf_token_put(token);
>> + if (err && log)
>> + (void) bpf_vlog_finalize(log, &log_true_size);
>> + if (log)
>> + kvfree(log);
>> return err;
>> }
>
> +1 to Andrii's suggestion to report log size back,
> just for consistency reasons.
Right.
The log_true_size will be reported back to users.
Thanks,
Leon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 5/6] libbpf: Add common attr support for map_create
2025-09-17 21:46 ` Andrii Nakryiko
@ 2025-09-23 16:40 ` Leon Hwang
2025-09-25 0:02 ` Andrii Nakryiko
0 siblings, 1 reply; 25+ messages in thread
From: Leon Hwang @ 2025-09-23 16:40 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, andrii, daniel, menglong8.dong
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall
2025-09-23 15:36 ` Leon Hwang
@ 2025-09-24 23:57 ` Andrii Nakryiko
0 siblings, 0 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-24 23:57 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
On Tue, Sep 23, 2025 at 8:36 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
>
>
> 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?
see libbpf_internal.h, it's not the first non-UAPI global function we
have in libbpf.
Just don't add this function to libbpf.map and it should be fine
>
> Thanks,
> Leon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load
2025-09-23 15:50 ` Leon Hwang
@ 2025-09-25 0:00 ` Andrii Nakryiko
0 siblings, 0 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-25 0:00 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
On Tue, Sep 23, 2025 at 8:50 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
>
>
> 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.
We are talking about duplicating 1-2 if conditions. One-time thing,
we won't be having any more commands going forward that will allow
specifying log in two different ways. Let's not add a zoo of
bpf_vlog_xxx constructors
>
> > 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.
>
TIL about -EUSERS, works for me
> Thanks,
> Leon
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC PATCH bpf-next v2 5/6] libbpf: Add common attr support for map_create
2025-09-23 16:40 ` Leon Hwang
@ 2025-09-25 0:02 ` Andrii Nakryiko
0 siblings, 0 replies; 25+ messages in thread
From: Andrii Nakryiko @ 2025-09-25 0:02 UTC (permalink / raw)
To: Leon Hwang; +Cc: bpf, ast, andrii, daniel, menglong8.dong
On Tue, Sep 23, 2025 at 9:40 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
>
>
> 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?
>
yes, at the very least for API consistency. bpf() syscall's log is a)
buffer, b) size of that bufer, c) log_level / flags. Let's keep it
consistent.
> 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
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2025-09-25 0:02 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-11 16:33 [RFC PATCH bpf-next v2 0/6] bpf: Extend bpf syscall with common attributes support Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 1/6] " Leon Hwang
2025-09-17 0:06 ` Andrii Nakryiko
2025-09-23 15:23 ` Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 2/6] libbpf: Add support for extended bpf syscall Leon Hwang
2025-09-17 0:06 ` Andrii Nakryiko
2025-09-23 15:36 ` Leon Hwang
2025-09-24 23:57 ` Andrii Nakryiko
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 3/6] bpf: Add common attr support for prog_load and btf_load Leon Hwang
2025-09-17 21:12 ` Andrii Nakryiko
2025-09-23 15:50 ` Leon Hwang
2025-09-25 0:00 ` Andrii Nakryiko
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 4/6] bpf: Add common attr support for map_create Leon Hwang
2025-09-17 21:39 ` Andrii Nakryiko
2025-09-17 21:49 ` Alexei Starovoitov
2025-09-23 15:52 ` Leon Hwang
2025-09-23 16:27 ` Leon Hwang
2025-09-18 23:29 ` Eduard Zingerman
2025-09-23 16:31 ` Leon Hwang
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 5/6] libbpf: " Leon Hwang
2025-09-17 21:45 ` Andrii Nakryiko
2025-09-17 21:46 ` Andrii Nakryiko
2025-09-23 16:40 ` Leon Hwang
2025-09-25 0:02 ` Andrii Nakryiko
2025-09-11 16:33 ` [RFC PATCH bpf-next v2 6/6] selftests/bpf: Add cases to test map create failure log Leon Hwang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox