This is a bpf-treewide change that annotates all kfuncs as such inside
.BTF_ids. This annotation eventually allows us to automatically generate
kfunc prototypes from bpftool.
We store this metadata inside a yet-unused flags field inside struct
btf_id_set8 (thanks Kumar!). pahole will be taught where to look.
More details about the full chain of events are available in commit 2's
description.
Daniel Xu (2):
bpf: btf: Support optional flags for BTF_SET8 sets
bpf: treewide: Annotate BPF kfuncs in BTF
drivers/hid/bpf/hid_bpf_dispatch.c | 4 ++--
fs/verity/measure.c | 2 +-
include/linux/btf_ids.h | 17 ++++++++++++-----
kernel/bpf/btf.c | 3 +++
kernel/bpf/cpumask.c | 2 +-
kernel/bpf/helpers.c | 4 ++--
kernel/bpf/map_iter.c | 2 +-
kernel/cgroup/rstat.c | 2 +-
kernel/trace/bpf_trace.c | 4 ++--
net/bpf/test_run.c | 4 ++--
net/core/filter.c | 8 ++++----
net/core/xdp.c | 2 +-
net/ipv4/bpf_tcp_ca.c | 2 +-
net/ipv4/fou_bpf.c | 2 +-
net/ipv4/tcp_bbr.c | 2 +-
net/ipv4/tcp_cubic.c | 2 +-
net/ipv4/tcp_dctcp.c | 2 +-
net/netfilter/nf_conntrack_bpf.c | 2 +-
net/netfilter/nf_nat_bpf.c | 2 +-
net/xfrm/xfrm_interface_bpf.c | 2 +-
net/xfrm/xfrm_state_bpf.c | 2 +-
.../selftests/bpf/bpf_testmod/bpf_testmod.c | 2 +-
22 files changed, 42 insertions(+), 32 deletions(-)
--
2.42.1
This commit adds support for optional flags on BTF_SET8s.
struct btf_id_set8 already supported 32 bits worth of flags, but was
only used for alignment purposes before.
We now use these bits to encode flags. The next commit will tag all
kfunc sets with a flag so that pahole can recognize which
BTF_ID_FLAGS(func, ..) are actual kfuncs.
Signed-off-by: Daniel Xu <[email protected]>
---
include/linux/btf_ids.h | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index a9cb10b0e2e9..88f914579fa1 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -183,17 +183,21 @@ extern struct btf_id_set name;
* .word (1 << 3) | (1 << 1) | (1 << 2)
*
*/
-#define __BTF_SET8_START(name, scope) \
+#define ___BTF_SET8_START(name, scope, flags) \
asm( \
".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
"." #scope " __BTF_ID__set8__" #name "; \n" \
"__BTF_ID__set8__" #name ":; \n" \
-".zero 8 \n" \
+".zero 4 \n" \
+".long " #flags "\n" \
".popsection; \n");
-#define BTF_SET8_START(name) \
+#define __BTF_SET8_START(name, scope, flags, ...) \
+___BTF_SET8_START(name, scope, flags)
+
+#define BTF_SET8_START(name, ...) \
__BTF_ID_LIST(name, local) \
-__BTF_SET8_START(name, local)
+__BTF_SET8_START(name, local, ##__VA_ARGS__, 0)
#define BTF_SET8_END(name) \
asm( \
@@ -214,7 +218,7 @@ extern struct btf_id_set8 name;
#define BTF_SET_START(name) static struct btf_id_set __maybe_unused name = { 0 };
#define BTF_SET_START_GLOBAL(name) static struct btf_id_set __maybe_unused name = { 0 };
#define BTF_SET_END(name)
-#define BTF_SET8_START(name) static struct btf_id_set8 __maybe_unused name = { 0 };
+#define BTF_SET8_START(name, ...) static struct btf_id_set8 __maybe_unused name = { 0 };
#define BTF_SET8_END(name)
#endif /* CONFIG_DEBUG_INFO_BTF */
--
2.42.1
This commit marks kfuncs as such inside the .BTF_ids section. The upshot
of these annotations is that we'll be able to automatically generate
kfunc prototypes for downstream users. The process is as follows:
1. In source, tag kfunc BTF_SET8 sets with BTF_SET8_KFUNC flag
2. During build, pahole injects into BTF a "bpf_kfunc" BTF_DECL_TAG for
each function inside BTF_SET8_KFUNC sets
3. At runtime, vmlinux or module BTF is made available in sysfs
4. At runtime, bpftool (or similar) can look at provided BTF and
generate appropriate prototypes for functions with "bpf_kfunc" tag
To ensure future kfunc are similarly tagged, we add a WARN_ON() inside
kfunc registration so developers will notice issues duringn development.
Signed-off-by: Daniel Xu <[email protected]>
---
drivers/hid/bpf/hid_bpf_dispatch.c | 4 ++--
fs/verity/measure.c | 2 +-
include/linux/btf_ids.h | 3 +++
kernel/bpf/btf.c | 3 +++
kernel/bpf/cpumask.c | 2 +-
kernel/bpf/helpers.c | 4 ++--
kernel/bpf/map_iter.c | 2 +-
kernel/cgroup/rstat.c | 2 +-
kernel/trace/bpf_trace.c | 4 ++--
net/bpf/test_run.c | 4 ++--
net/core/filter.c | 8 ++++----
net/core/xdp.c | 2 +-
net/ipv4/bpf_tcp_ca.c | 2 +-
net/ipv4/fou_bpf.c | 2 +-
net/ipv4/tcp_bbr.c | 2 +-
net/ipv4/tcp_cubic.c | 2 +-
net/ipv4/tcp_dctcp.c | 2 +-
net/netfilter/nf_conntrack_bpf.c | 2 +-
net/netfilter/nf_nat_bpf.c | 2 +-
net/xfrm/xfrm_interface_bpf.c | 2 +-
net/xfrm/xfrm_state_bpf.c | 2 +-
tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 2 +-
22 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
index d9ef45fcaeab..e277b74e8831 100644
--- a/drivers/hid/bpf/hid_bpf_dispatch.c
+++ b/drivers/hid/bpf/hid_bpf_dispatch.c
@@ -172,7 +172,7 @@ hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, const size_t rdwr
* The following set contains all functions we agree BPF programs
* can use.
*/
-BTF_SET8_START(hid_bpf_kfunc_ids)
+BTF_SET8_START(hid_bpf_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, hid_bpf_get_data, KF_RET_NULL)
BTF_SET8_END(hid_bpf_kfunc_ids)
@@ -440,7 +440,7 @@ static const struct btf_kfunc_id_set hid_bpf_fmodret_set = {
};
/* for syscall HID-BPF */
-BTF_SET8_START(hid_bpf_syscall_kfunc_ids)
+BTF_SET8_START(hid_bpf_syscall_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, hid_bpf_attach_prog)
BTF_ID_FLAGS(func, hid_bpf_allocate_context, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, hid_bpf_release_context, KF_RELEASE)
diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index bf7a5f4cccaf..7e6c4ad9f8ce 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -159,7 +159,7 @@ __bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr_ker
__bpf_kfunc_end_defs();
-BTF_SET8_START(fsverity_set_ids)
+BTF_SET8_START(fsverity_set_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_get_fsverity_digest, KF_TRUSTED_ARGS)
BTF_SET8_END(fsverity_set_ids)
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
index 88f914579fa1..771e29762a2d 100644
--- a/include/linux/btf_ids.h
+++ b/include/linux/btf_ids.h
@@ -8,6 +8,9 @@ struct btf_id_set {
u32 ids[];
};
+/* This flag implies BTF_SET8 holds kfunc(s) */
+#define BTF_SET8_KFUNC (1 << 0)
+
struct btf_id_set8 {
u32 cnt;
u32 flags;
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 51e8b4bee0c8..b8ba00a4179f 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7769,6 +7769,9 @@ static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
struct btf *btf;
int ret, i;
+ /* All kfuncs need to be tagged as such in BTF */
+ WARN_ON(!(kset->set->flags & BTF_SET8_KFUNC));
+
btf = btf_get_module_btf(kset->owner);
if (!btf) {
if (!kset->owner && IS_ENABLED(CONFIG_DEBUG_INFO_BTF)) {
diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index 2e73533a3811..4155c479cc2f 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -424,7 +424,7 @@ __bpf_kfunc u32 bpf_cpumask_weight(const struct cpumask *cpumask)
__bpf_kfunc_end_defs();
-BTF_SET8_START(cpumask_kfunc_btf_ids)
+BTF_SET8_START(cpumask_kfunc_btf_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_cpumask_create, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_cpumask_release, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_cpumask_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index be72824f32b2..c4c2a3bd36f6 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2543,7 +2543,7 @@ __bpf_kfunc void bpf_throw(u64 cookie)
__bpf_kfunc_end_defs();
-BTF_SET8_START(generic_btf_ids)
+BTF_SET8_START(generic_btf_ids, BTF_SET8_KFUNC)
#ifdef CONFIG_KEXEC_CORE
BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
#endif
@@ -2588,7 +2588,7 @@ BTF_ID(struct, cgroup)
BTF_ID(func, bpf_cgroup_release_dtor)
#endif
-BTF_SET8_START(common_btf_ids)
+BTF_SET8_START(common_btf_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_cast_to_kern_ctx)
BTF_ID_FLAGS(func, bpf_rdonly_cast)
BTF_ID_FLAGS(func, bpf_rcu_read_lock)
diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
index 6abd7c5df4b3..225c70e88cb7 100644
--- a/kernel/bpf/map_iter.c
+++ b/kernel/bpf/map_iter.c
@@ -213,7 +213,7 @@ __bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map)
__bpf_kfunc_end_defs();
-BTF_SET8_START(bpf_map_iter_kfunc_ids)
+BTF_SET8_START(bpf_map_iter_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_map_sum_elem_count, KF_TRUSTED_ARGS)
BTF_SET8_END(bpf_map_iter_kfunc_ids)
diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index c0adb7254b45..732df57b7c1d 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -520,7 +520,7 @@ void cgroup_base_stat_cputime_show(struct seq_file *seq)
}
/* Add bpf kfuncs for cgroup_rstat_updated() and cgroup_rstat_flush() */
-BTF_SET8_START(bpf_rstat_kfunc_ids)
+BTF_SET8_START(bpf_rstat_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, cgroup_rstat_updated)
BTF_ID_FLAGS(func, cgroup_rstat_flush, KF_SLEEPABLE)
BTF_SET8_END(bpf_rstat_kfunc_ids)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 7ac6c52b25eb..bf1ef600e4d2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1412,7 +1412,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,
__bpf_kfunc_end_defs();
-BTF_SET8_START(key_sig_kfunc_set)
+BTF_SET8_START(key_sig_kfunc_set, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
BTF_ID_FLAGS(func, bpf_lookup_system_key, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_key_put, KF_RELEASE)
@@ -1475,7 +1475,7 @@ __bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
__bpf_kfunc_end_defs();
-BTF_SET8_START(fs_kfunc_set_ids)
+BTF_SET8_START(fs_kfunc_set_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_SET8_END(fs_kfunc_set_ids)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index dfd919374017..18b91612931d 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -617,7 +617,7 @@ CFI_NOSEAL(bpf_kfunc_call_memb_release_dtor);
__bpf_kfunc_end_defs();
-BTF_SET8_START(bpf_test_modify_return_ids)
+BTF_SET8_START(bpf_test_modify_return_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_modify_return_test)
BTF_ID_FLAGS(func, bpf_modify_return_test2)
BTF_ID_FLAGS(func, bpf_fentry_test1, KF_SLEEPABLE)
@@ -628,7 +628,7 @@ static const struct btf_kfunc_id_set bpf_test_modify_return_set = {
.set = &bpf_test_modify_return_ids,
};
-BTF_SET8_START(test_sk_check_kfunc_ids)
+BTF_SET8_START(test_sk_check_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_release, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_kfunc_call_memb_release, KF_RELEASE)
BTF_SET8_END(test_sk_check_kfunc_ids)
diff --git a/net/core/filter.c b/net/core/filter.c
index 24061f29c9dd..f278a3976f61 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11853,15 +11853,15 @@ int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags,
return 0;
}
-BTF_SET8_START(bpf_kfunc_check_set_skb)
+BTF_SET8_START(bpf_kfunc_check_set_skb, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_dynptr_from_skb)
BTF_SET8_END(bpf_kfunc_check_set_skb)
-BTF_SET8_START(bpf_kfunc_check_set_xdp)
+BTF_SET8_START(bpf_kfunc_check_set_xdp, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_dynptr_from_xdp)
BTF_SET8_END(bpf_kfunc_check_set_xdp)
-BTF_SET8_START(bpf_kfunc_check_set_sock_addr)
+BTF_SET8_START(bpf_kfunc_check_set_sock_addr, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_sock_addr_set_sun_path)
BTF_SET8_END(bpf_kfunc_check_set_sock_addr)
@@ -11936,7 +11936,7 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
__bpf_kfunc_end_defs();
-BTF_SET8_START(bpf_sk_iter_kfunc_ids)
+BTF_SET8_START(bpf_sk_iter_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_sock_destroy, KF_TRUSTED_ARGS)
BTF_SET8_END(bpf_sk_iter_kfunc_ids)
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 4869c1c2d8f3..84a01fb26f41 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -771,7 +771,7 @@ __bpf_kfunc int bpf_xdp_metadata_rx_vlan_tag(const struct xdp_md *ctx,
__bpf_kfunc_end_defs();
-BTF_SET8_START(xdp_metadata_kfunc_ids)
+BTF_SET8_START(xdp_metadata_kfunc_ids, BTF_SET8_KFUNC)
#define XDP_METADATA_KFUNC(_, __, name, ___) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS)
XDP_METADATA_KFUNC_xxx
#undef XDP_METADATA_KFUNC
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
index ae8b15e6896f..096918e9f93f 100644
--- a/net/ipv4/bpf_tcp_ca.c
+++ b/net/ipv4/bpf_tcp_ca.c
@@ -195,7 +195,7 @@ bpf_tcp_ca_get_func_proto(enum bpf_func_id func_id,
}
}
-BTF_SET8_START(bpf_tcp_ca_check_kfunc_ids)
+BTF_SET8_START(bpf_tcp_ca_check_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, tcp_reno_ssthresh)
BTF_ID_FLAGS(func, tcp_reno_cong_avoid)
BTF_ID_FLAGS(func, tcp_reno_undo_cwnd)
diff --git a/net/ipv4/fou_bpf.c b/net/ipv4/fou_bpf.c
index 4da03bf45c9b..e527be123f79 100644
--- a/net/ipv4/fou_bpf.c
+++ b/net/ipv4/fou_bpf.c
@@ -100,7 +100,7 @@ __bpf_kfunc int bpf_skb_get_fou_encap(struct __sk_buff *skb_ctx,
__bpf_kfunc_end_defs();
-BTF_SET8_START(fou_kfunc_set)
+BTF_SET8_START(fou_kfunc_set, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_skb_set_fou_encap)
BTF_ID_FLAGS(func, bpf_skb_get_fou_encap)
BTF_SET8_END(fou_kfunc_set)
diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
index 22358032dd48..dbf904312292 100644
--- a/net/ipv4/tcp_bbr.c
+++ b/net/ipv4/tcp_bbr.c
@@ -1155,7 +1155,7 @@ static struct tcp_congestion_ops tcp_bbr_cong_ops __read_mostly = {
.set_state = bbr_set_state,
};
-BTF_SET8_START(tcp_bbr_check_kfunc_ids)
+BTF_SET8_START(tcp_bbr_check_kfunc_ids, BTF_SET8_KFUNC)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
BTF_ID_FLAGS(func, bbr_init)
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 0fd78ecb67e7..b10bbc1cbc14 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -485,7 +485,7 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
.name = "cubic",
};
-BTF_SET8_START(tcp_cubic_check_kfunc_ids)
+BTF_SET8_START(tcp_cubic_check_kfunc_ids, BTF_SET8_KFUNC)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
BTF_ID_FLAGS(func, cubictcp_init)
diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
index bb23bb5b387a..2d2a8ba3e19e 100644
--- a/net/ipv4/tcp_dctcp.c
+++ b/net/ipv4/tcp_dctcp.c
@@ -260,7 +260,7 @@ static struct tcp_congestion_ops dctcp_reno __read_mostly = {
.name = "dctcp-reno",
};
-BTF_SET8_START(tcp_dctcp_check_kfunc_ids)
+BTF_SET8_START(tcp_dctcp_check_kfunc_ids, BTF_SET8_KFUNC)
#ifdef CONFIG_X86
#ifdef CONFIG_DYNAMIC_FTRACE
BTF_ID_FLAGS(func, dctcp_init)
diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
index 475358ec8212..329b3f2a68d0 100644
--- a/net/netfilter/nf_conntrack_bpf.c
+++ b/net/netfilter/nf_conntrack_bpf.c
@@ -467,7 +467,7 @@ __bpf_kfunc int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
__bpf_kfunc_end_defs();
-BTF_SET8_START(nf_ct_kfunc_set)
+BTF_SET8_START(nf_ct_kfunc_set, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_xdp_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_xdp_ct_lookup, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_skb_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
index 6e3b2f58855f..22237dda59df 100644
--- a/net/netfilter/nf_nat_bpf.c
+++ b/net/netfilter/nf_nat_bpf.c
@@ -54,7 +54,7 @@ __bpf_kfunc int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
__bpf_kfunc_end_defs();
-BTF_SET8_START(nf_nat_kfunc_set)
+BTF_SET8_START(nf_nat_kfunc_set, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
BTF_SET8_END(nf_nat_kfunc_set)
diff --git a/net/xfrm/xfrm_interface_bpf.c b/net/xfrm/xfrm_interface_bpf.c
index 7d5e920141e9..484cc727a253 100644
--- a/net/xfrm/xfrm_interface_bpf.c
+++ b/net/xfrm/xfrm_interface_bpf.c
@@ -93,7 +93,7 @@ __bpf_kfunc int bpf_skb_set_xfrm_info(struct __sk_buff *skb_ctx, const struct bp
__bpf_kfunc_end_defs();
-BTF_SET8_START(xfrm_ifc_kfunc_set)
+BTF_SET8_START(xfrm_ifc_kfunc_set, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_skb_get_xfrm_info)
BTF_ID_FLAGS(func, bpf_skb_set_xfrm_info)
BTF_SET8_END(xfrm_ifc_kfunc_set)
diff --git a/net/xfrm/xfrm_state_bpf.c b/net/xfrm/xfrm_state_bpf.c
index 9e20d4a377f7..d22276b003b4 100644
--- a/net/xfrm/xfrm_state_bpf.c
+++ b/net/xfrm/xfrm_state_bpf.c
@@ -117,7 +117,7 @@ __bpf_kfunc void bpf_xdp_xfrm_state_release(struct xfrm_state *x)
__bpf_kfunc_end_defs();
-BTF_SET8_START(xfrm_state_kfunc_set)
+BTF_SET8_START(xfrm_state_kfunc_set, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_xdp_get_xfrm_state, KF_RET_NULL | KF_ACQUIRE)
BTF_ID_FLAGS(func, bpf_xdp_xfrm_state_release, KF_RELEASE)
BTF_SET8_END(xfrm_state_kfunc_set)
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 91907b321f91..32972334cd50 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -341,7 +341,7 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
.write = bpf_testmod_test_write,
};
-BTF_SET8_START(bpf_testmod_common_kfunc_ids)
+BTF_SET8_START(bpf_testmod_common_kfunc_ids, BTF_SET8_KFUNC)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
--
2.42.1
On Wed, Jan 03, 2024 at 04:31:55PM -0700, Daniel Xu wrote:
> This commit adds support for optional flags on BTF_SET8s.
> struct btf_id_set8 already supported 32 bits worth of flags, but was
> only used for alignment purposes before.
>
> We now use these bits to encode flags. The next commit will tag all
> kfunc sets with a flag so that pahole can recognize which
> BTF_ID_FLAGS(func, ..) are actual kfuncs.
>
> Signed-off-by: Daniel Xu <[email protected]>
> ---
> include/linux/btf_ids.h | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index a9cb10b0e2e9..88f914579fa1 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -183,17 +183,21 @@ extern struct btf_id_set name;
> * .word (1 << 3) | (1 << 1) | (1 << 2)
> *
> */
> -#define __BTF_SET8_START(name, scope) \
> +#define ___BTF_SET8_START(name, scope, flags) \
> asm( \
> ".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
> "." #scope " __BTF_ID__set8__" #name "; \n" \
> "__BTF_ID__set8__" #name ":; \n" \
> -".zero 8 \n" \
> +".zero 4 \n" \
> +".long " #flags "\n" \
> ".popsection; \n");
>
> -#define BTF_SET8_START(name) \
> +#define __BTF_SET8_START(name, scope, flags, ...) \
> +___BTF_SET8_START(name, scope, flags)
> +
> +#define BTF_SET8_START(name, ...) \
> __BTF_ID_LIST(name, local) \
> -__BTF_SET8_START(name, local)
> +__BTF_SET8_START(name, local, ##__VA_ARGS__, 0)
I think it'd better to use something like:
BTF_SET8_KFUNCS_START(fsverity_set_ids)
instead of:
BTF_SET8_START(fsverity_set_ids, BTF_SET8_KFUNC)
and to keep current BTF_SET8_START without flags argument, like:
#define BTF_SET8_START(name) \
__BTF_SET8_START(... , 0, ...
#define BTF_SET8_KFUNCS_START(name) \
__BTF_SET8_START(... , BTF_SET8_KFUNC, ...
also I'd rename BTF_SET8_KFUNC to BTF_SET8_KFUNCS (with S)
do you have the pahole changes somewhere? would be great to
see all the related changes and try the whole thing
jirka
>
> #define BTF_SET8_END(name) \
> asm( \
> @@ -214,7 +218,7 @@ extern struct btf_id_set8 name;
> #define BTF_SET_START(name) static struct btf_id_set __maybe_unused name = { 0 };
> #define BTF_SET_START_GLOBAL(name) static struct btf_id_set __maybe_unused name = { 0 };
> #define BTF_SET_END(name)
> -#define BTF_SET8_START(name) static struct btf_id_set8 __maybe_unused name = { 0 };
> +#define BTF_SET8_START(name, ...) static struct btf_id_set8 __maybe_unused name = { 0 };
> #define BTF_SET8_END(name)
>
> #endif /* CONFIG_DEBUG_INFO_BTF */
> --
> 2.42.1
>
On Wed, Jan 03, 2024 at 04:31:56PM -0700, Daniel Xu wrote:
SNIP
> diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> index 88f914579fa1..771e29762a2d 100644
> --- a/include/linux/btf_ids.h
> +++ b/include/linux/btf_ids.h
> @@ -8,6 +8,9 @@ struct btf_id_set {
> u32 ids[];
> };
>
> +/* This flag implies BTF_SET8 holds kfunc(s) */
> +#define BTF_SET8_KFUNC (1 << 0)
> +
> struct btf_id_set8 {
> u32 cnt;
> u32 flags;
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 51e8b4bee0c8..b8ba00a4179f 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7769,6 +7769,9 @@ static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
> struct btf *btf;
> int ret, i;
>
> + /* All kfuncs need to be tagged as such in BTF */
> + WARN_ON(!(kset->set->flags & BTF_SET8_KFUNC));
__register_btf_kfunc_id_set gets called also from the 'hooks' path:
bpf_mptcp_kfunc_init
register_btf_fmodret_id_set
__register_btf_kfunc_id_set
so it will hit the warn.. it should be probably in the register_btf_kfunc_id_set ?
also given that we can have modules calling register_btf_kfunc_id_set,
should we just return error instead of the warn?
SNIP
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 91907b321f91..32972334cd50 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -341,7 +341,7 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
> .write = bpf_testmod_test_write,
> };
>
> -BTF_SET8_START(bpf_testmod_common_kfunc_ids)
> +BTF_SET8_START(bpf_testmod_common_kfunc_ids, BTF_SET8_KFUNC)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
> BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
we need to change also bpf_testmod_check_kfunc_ids set
jirka
> --
> 2.42.1
>
On Thu, Jan 4, 2024 at 3:23 AM Jiri Olsa <[email protected]> wrote:
>
> On Wed, Jan 03, 2024 at 04:31:55PM -0700, Daniel Xu wrote:
> > This commit adds support for optional flags on BTF_SET8s.
> > struct btf_id_set8 already supported 32 bits worth of flags, but was
> > only used for alignment purposes before.
> >
> > We now use these bits to encode flags. The next commit will tag all
> > kfunc sets with a flag so that pahole can recognize which
> > BTF_ID_FLAGS(func, ..) are actual kfuncs.
> >
> > Signed-off-by: Daniel Xu <[email protected]>
> > ---
> > include/linux/btf_ids.h | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > index a9cb10b0e2e9..88f914579fa1 100644
> > --- a/include/linux/btf_ids.h
> > +++ b/include/linux/btf_ids.h
> > @@ -183,17 +183,21 @@ extern struct btf_id_set name;
> > * .word (1 << 3) | (1 << 1) | (1 << 2)
> > *
> > */
> > -#define __BTF_SET8_START(name, scope) \
> > +#define ___BTF_SET8_START(name, scope, flags) \
> > asm( \
> > ".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
> > "." #scope " __BTF_ID__set8__" #name "; \n" \
> > "__BTF_ID__set8__" #name ":; \n" \
> > -".zero 8 \n" \
> > +".zero 4 \n" \
> > +".long " #flags "\n" \
> > ".popsection; \n");
> >
> > -#define BTF_SET8_START(name) \
> > +#define __BTF_SET8_START(name, scope, flags, ...) \
> > +___BTF_SET8_START(name, scope, flags)
> > +
> > +#define BTF_SET8_START(name, ...) \
> > __BTF_ID_LIST(name, local) \
> > -__BTF_SET8_START(name, local)
> > +__BTF_SET8_START(name, local, ##__VA_ARGS__, 0)
>
> I think it'd better to use something like:
>
> BTF_SET8_KFUNCS_START(fsverity_set_ids)
>
> instead of:
>
> BTF_SET8_START(fsverity_set_ids, BTF_SET8_KFUNC)
>
> and to keep current BTF_SET8_START without flags argument, like:
>
> #define BTF_SET8_START(name) \
> __BTF_SET8_START(... , 0, ...
>
> #define BTF_SET8_KFUNCS_START(name) \
> __BTF_SET8_START(... , BTF_SET8_KFUNC, ...
I was about to suggest the same :)
We can drop SET8 part as well, since it's implementation detail.
Just BTF_KFUNCS_START and pair it with BTF_KFUNCS_END
that will be the same as BTF_SET8_END.
Until we need to do something else with these macros.
>
> also I'd rename BTF_SET8_KFUNC to BTF_SET8_KFUNCS (with S)
>
> do you have the pahole changes somewhere? would be great to
> see all the related changes and try the whole thing
+1
without corresponding pahole changes it's not clear whether
it actually helps.
Hi Jiri,
On Thu, Jan 04, 2024 at 12:41:51PM +0100, Jiri Olsa wrote:
> On Wed, Jan 03, 2024 at 04:31:56PM -0700, Daniel Xu wrote:
>
> SNIP
>
> > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > index 88f914579fa1..771e29762a2d 100644
> > --- a/include/linux/btf_ids.h
> > +++ b/include/linux/btf_ids.h
> > @@ -8,6 +8,9 @@ struct btf_id_set {
> > u32 ids[];
> > };
> >
> > +/* This flag implies BTF_SET8 holds kfunc(s) */
> > +#define BTF_SET8_KFUNC (1 << 0)
> > +
> > struct btf_id_set8 {
> > u32 cnt;
> > u32 flags;
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 51e8b4bee0c8..b8ba00a4179f 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -7769,6 +7769,9 @@ static int __register_btf_kfunc_id_set(enum btf_kfunc_hook hook,
> > struct btf *btf;
> > int ret, i;
> >
> > + /* All kfuncs need to be tagged as such in BTF */
> > + WARN_ON(!(kset->set->flags & BTF_SET8_KFUNC));
>
> __register_btf_kfunc_id_set gets called also from the 'hooks' path:
>
> bpf_mptcp_kfunc_init
> register_btf_fmodret_id_set
> __register_btf_kfunc_id_set
>
> so it will hit the warn.. it should be probably in the register_btf_kfunc_id_set ?
Yeah, good catch.
>
> also given that we can have modules calling register_btf_kfunc_id_set,
> should we just return error instead of the warn?
It looks like quite a few registrations go through late_initcall(),
in which error codes are thrown away. I'm looking at
init/main.c:do_initcall_level:
for (fn = initcall_levels[level]; fn < initcall_levels[level+1]; fn++)
do_one_initcall(initcall_from_entry(fn));
Higher level question: if out of tree module does not follow convention,
it would still make sense to WARN(), right?
>
> SNIP
>
> > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > index 91907b321f91..32972334cd50 100644
> > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> > @@ -341,7 +341,7 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
> > .write = bpf_testmod_test_write,
> > };
> >
> > -BTF_SET8_START(bpf_testmod_common_kfunc_ids)
> > +BTF_SET8_START(bpf_testmod_common_kfunc_ids, BTF_SET8_KFUNC)
> > BTF_ID_FLAGS(func, bpf_iter_testmod_seq_new, KF_ITER_NEW)
> > BTF_ID_FLAGS(func, bpf_iter_testmod_seq_next, KF_ITER_NEXT | KF_RET_NULL)
> > BTF_ID_FLAGS(func, bpf_iter_testmod_seq_destroy, KF_ITER_DESTROY)
>
> we need to change also bpf_testmod_check_kfunc_ids set
Good catch, thanks.
Daniel
On Thu, Jan 04, 2024 at 09:11:56AM -0800, Alexei Starovoitov wrote:
> On Thu, Jan 4, 2024 at 3:23 AM Jiri Olsa <[email protected]> wrote:
> >
> > On Wed, Jan 03, 2024 at 04:31:55PM -0700, Daniel Xu wrote:
> > > This commit adds support for optional flags on BTF_SET8s.
> > > struct btf_id_set8 already supported 32 bits worth of flags, but was
> > > only used for alignment purposes before.
> > >
> > > We now use these bits to encode flags. The next commit will tag all
> > > kfunc sets with a flag so that pahole can recognize which
> > > BTF_ID_FLAGS(func, ..) are actual kfuncs.
> > >
> > > Signed-off-by: Daniel Xu <[email protected]>
> > > ---
> > > include/linux/btf_ids.h | 14 +++++++++-----
> > > 1 file changed, 9 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
> > > index a9cb10b0e2e9..88f914579fa1 100644
> > > --- a/include/linux/btf_ids.h
> > > +++ b/include/linux/btf_ids.h
> > > @@ -183,17 +183,21 @@ extern struct btf_id_set name;
> > > * .word (1 << 3) | (1 << 1) | (1 << 2)
> > > *
> > > */
> > > -#define __BTF_SET8_START(name, scope) \
> > > +#define ___BTF_SET8_START(name, scope, flags) \
> > > asm( \
> > > ".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
> > > "." #scope " __BTF_ID__set8__" #name "; \n" \
> > > "__BTF_ID__set8__" #name ":; \n" \
> > > -".zero 8 \n" \
> > > +".zero 4 \n" \
> > > +".long " #flags "\n" \
> > > ".popsection; \n");
> > >
> > > -#define BTF_SET8_START(name) \
> > > +#define __BTF_SET8_START(name, scope, flags, ...) \
> > > +___BTF_SET8_START(name, scope, flags)
> > > +
> > > +#define BTF_SET8_START(name, ...) \
> > > __BTF_ID_LIST(name, local) \
> > > -__BTF_SET8_START(name, local)
> > > +__BTF_SET8_START(name, local, ##__VA_ARGS__, 0)
> >
> > I think it'd better to use something like:
> >
> > BTF_SET8_KFUNCS_START(fsverity_set_ids)
> >
> > instead of:
> >
> > BTF_SET8_START(fsverity_set_ids, BTF_SET8_KFUNC)
> >
> > and to keep current BTF_SET8_START without flags argument, like:
> >
> > #define BTF_SET8_START(name) \
> > __BTF_SET8_START(... , 0, ...
> >
> > #define BTF_SET8_KFUNCS_START(name) \
> > __BTF_SET8_START(... , BTF_SET8_KFUNC, ...
>
> I was about to suggest the same :)
>
> We can drop SET8 part as well, since it's implementation detail.
> Just BTF_KFUNCS_START and pair it with BTF_KFUNCS_END
> that will be the same as BTF_SET8_END.
> Until we need to do something else with these macros.
Ack, will change.
>
> >
> > also I'd rename BTF_SET8_KFUNC to BTF_SET8_KFUNCS (with S)
> >
> > do you have the pahole changes somewhere? would be great to
> > see all the related changes and try the whole thing
>
> +1
> without corresponding pahole changes it's not clear whether
> it actually helps.
Here's a checkpointed branch: https://github.com/danobi/pahole/tree/kfunc_btf-mailed .
I won't force push to it.
It should work against this patchset. I might need to clean it up a bit still.
Thanks,
Daniel
On Thu, Jan 04, 2024 at 06:17:50PM -0700, Daniel Xu wrote:
[...]
> >
> > also given that we can have modules calling register_btf_kfunc_id_set,
> > should we just return error instead of the warn?
>
> It looks like quite a few registrations go through late_initcall(),
> in which error codes are thrown away. I'm looking at
> init/main.c:do_initcall_level:
>
> for (fn = initcall_levels[level]; fn < initcall_levels[level+1]; fn++)
> do_one_initcall(initcall_from_entry(fn));
>
> Higher level question: if out of tree module does not follow convention,
> it would still make sense to WARN(), right?
Ah, I got what you meant now. I'd say returning error makes sense but
WARN() is also useful. I'll send v2 with both.
[...]