On Wed, Nov 1, 2023 at 5:56 AM Dave Marchevsky <davemarchevsky@xxxxxx> wrote:
>
> BPF kfuncs are meant to be called from BPF programs. Accordingly, most
> kfuncs are not called from anywhere in the kernel, which the
> -Wmissing-prototypes warning is unhappy about. We've peppered
> __diag_ignore_all("-Wmissing-prototypes", ... everywhere kfuncs are
> defined in the codebase to suppress this warning.
>
> This patch adds two macros meant to bound one or many kfunc definitions.
> All existing kfunc definitions which use these __diag calls to suppress
> -Wmissing-prototypes are migrated to use the newly-introduced macros.
> A new __diag_ignore_all - for "-Wmissing-declarations" - is added to the
> __bpf_kfunc_start_defs macro based on feedback from Andrii on an earlier
> version of this patch [0] and another recent mailing list thread [1].
>
> In the future we might need to ignore different warnings or do other
> kfunc-specific things. This change will make it easier to make such
> modifications for all kfunc defs.
>
> [0]: https://lore.kernel.org/bpf/CAEf4BzaE5dRWtK6RPLnjTW-MW9sx9K3Fn6uwqCTChK2Dcb1Xig@xxxxxxxxxxxxxx/
> [1]: https://lore.kernel.org/bpf/ZT+2qCc%2FaXep0%2FLf@krava/
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Cc: Jiri Olsa <olsajiri@xxxxxxxxx>
Acked-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
>
> v1 -> v2: https://lore.kernel.org/bpf/20231030210638.2415306-1-davemarchevsky@xxxxxx/
> * Update Documentation/bpf/kfuncs.rst to use new macros (Yafang, Andrii)
> * Update recently-added open-coded {task,cgroup} iters to use new
> macros (Yafang, Andrii)
> * Add Andrii ack
>
> Documentation/bpf/kfuncs.rst | 6 ++----
> include/linux/btf.h | 9 +++++++++
> kernel/bpf/bpf_iter.c | 6 ++----
> kernel/bpf/cgroup_iter.c | 6 ++----
> kernel/bpf/cpumask.c | 6 ++----
> kernel/bpf/helpers.c | 6 ++----
> kernel/bpf/map_iter.c | 6 ++----
> kernel/bpf/task_iter.c | 18 ++++++------------
> kernel/trace/bpf_trace.c | 6 ++----
> net/bpf/test_run.c | 7 +++----
> net/core/filter.c | 13 ++++---------
> net/core/xdp.c | 6 ++----
> net/ipv4/fou_bpf.c | 6 ++----
> net/netfilter/nf_conntrack_bpf.c | 6 ++----
> net/netfilter/nf_nat_bpf.c | 6 ++----
> net/xfrm/xfrm_interface_bpf.c | 6 ++----
> 16 files changed, 46 insertions(+), 73 deletions(-)
>
> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index 0d2647fb358d..723408e399ab 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst
> @@ -37,16 +37,14 @@ prototype in a header for the wrapper kfunc.
> An example is given below::
>
> /* Disables missing prototype warnings */
> - __diag_push();
> - __diag_ignore_all("-Wmissing-prototypes",
> - "Global kfuncs as their definitions will be in BTF");
> + __bpf_kfunc_start_defs();
>
> __bpf_kfunc struct task_struct *bpf_find_get_task_by_vpid(pid_t nr)
> {
> return find_get_task_by_vpid(nr);
> }
>
> - __diag_pop();
> + __bpf_kfunc_end_defs();
>
> A wrapper kfunc is often needed when we need to annotate parameters of the
> kfunc. Otherwise one may directly make the kfunc visible to the BPF program by
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index c2231c64d60b..dc5ce962f600 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -84,6 +84,15 @@
> */
> #define __bpf_kfunc __used noinline
>
> +#define __bpf_kfunc_start_defs() \
> + __diag_push(); \
> + __diag_ignore_all("-Wmissing-declarations", \
> + "Global kfuncs as their definitions will be in BTF");\
> + __diag_ignore_all("-Wmissing-prototypes", \
> + "Global kfuncs as their definitions will be in BTF")
> +
> +#define __bpf_kfunc_end_defs() __diag_pop()
> +
> /*
> * Return the name of the passed struct, if exists, or halt the build if for
> * example the structure gets renamed. In this way, developers have to revisit
> diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
> index 833faa04461b..0fae79164187 100644
> --- a/kernel/bpf/bpf_iter.c
> +++ b/kernel/bpf/bpf_iter.c
> @@ -782,9 +782,7 @@ struct bpf_iter_num_kern {
> int end; /* final value, exclusive */
> } __aligned(8);
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> __bpf_kfunc int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end)
> {
> @@ -843,4 +841,4 @@ __bpf_kfunc void bpf_iter_num_destroy(struct bpf_iter_num *it)
> s->cur = s->end = 0;
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
> diff --git a/kernel/bpf/cgroup_iter.c b/kernel/bpf/cgroup_iter.c
> index 209e5135f9fb..d1b5c5618dd7 100644
> --- a/kernel/bpf/cgroup_iter.c
> +++ b/kernel/bpf/cgroup_iter.c
> @@ -305,9 +305,7 @@ struct bpf_iter_css_kern {
> unsigned int flags;
> } __attribute__((aligned(8)));
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> __bpf_kfunc int bpf_iter_css_new(struct bpf_iter_css *it,
> struct cgroup_subsys_state *start, unsigned int flags)
> @@ -358,4 +356,4 @@ __bpf_kfunc void bpf_iter_css_destroy(struct bpf_iter_css *it)
> {
> }
>
> -__diag_pop();
> \ No newline at end of file
> +__bpf_kfunc_end_defs();
> diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
> index 6983af8e093c..e01c741e54e7 100644
> --- a/kernel/bpf/cpumask.c
> +++ b/kernel/bpf/cpumask.c
> @@ -34,9 +34,7 @@ static bool cpu_valid(u32 cpu)
> return cpu < nr_cpu_ids;
> }
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global kfuncs as their definitions will be in BTF");
> +__bpf_kfunc_start_defs();
>
> /**
> * bpf_cpumask_create() - Create a mutable BPF cpumask.
> @@ -407,7 +405,7 @@ __bpf_kfunc u32 bpf_cpumask_any_and_distribute(const struct cpumask *src1,
> return cpumask_any_and_distribute(src1, src2);
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(cpumask_kfunc_btf_ids)
> BTF_ID_FLAGS(func, bpf_cpumask_create, KF_ACQUIRE | KF_RET_NULL)
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index e46ac288a108..0e4657606eaa 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1886,9 +1886,7 @@ void bpf_rb_root_free(const struct btf_field *field, void *rb_root,
> }
> }
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> __bpf_kfunc void *bpf_obj_new_impl(u64 local_type_id__k, void *meta__ign)
> {
> @@ -2505,7 +2503,7 @@ __bpf_kfunc void bpf_throw(u64 cookie)
> WARN(1, "A call to BPF exception callback should never return\n");
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(generic_btf_ids)
> #ifdef CONFIG_KEXEC_CORE
> diff --git a/kernel/bpf/map_iter.c b/kernel/bpf/map_iter.c
> index 6fc9dae9edc8..6abd7c5df4b3 100644
> --- a/kernel/bpf/map_iter.c
> +++ b/kernel/bpf/map_iter.c
> @@ -193,9 +193,7 @@ static int __init bpf_map_iter_init(void)
>
> late_initcall(bpf_map_iter_init);
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> __bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map)
> {
> @@ -213,7 +211,7 @@ __bpf_kfunc s64 bpf_map_sum_elem_count(const struct bpf_map *map)
> return ret;
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(bpf_map_iter_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_map_sum_elem_count, KF_TRUSTED_ARGS)
> diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
> index 59e747938bdb..6cd8295c9683 100644
> --- a/kernel/bpf/task_iter.c
> +++ b/kernel/bpf/task_iter.c
> @@ -824,9 +824,7 @@ struct bpf_iter_task_vma_kern {
> struct bpf_iter_task_vma_kern_data *data;
> } __attribute__((aligned(8)));
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
> struct task_struct *task, u64 addr)
> @@ -892,7 +890,7 @@ __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
> }
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> struct bpf_iter_css_task {
> __u64 __opaque[1];
> @@ -902,9 +900,7 @@ struct bpf_iter_css_task_kern {
> struct css_task_iter *css_it;
> } __attribute__((aligned(8)));
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> __bpf_kfunc int bpf_iter_css_task_new(struct bpf_iter_css_task *it,
> struct cgroup_subsys_state *css, unsigned int flags)
> @@ -950,7 +946,7 @@ __bpf_kfunc void bpf_iter_css_task_destroy(struct bpf_iter_css_task *it)
> bpf_mem_free(&bpf_global_ma, kit->css_it);
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> struct bpf_iter_task {
> __u64 __opaque[3];
> @@ -971,9 +967,7 @@ enum {
> BPF_TASK_ITER_PROC_THREADS
> };
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> __bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
> struct task_struct *task__nullable, unsigned int flags)
> @@ -1043,7 +1037,7 @@ __bpf_kfunc void bpf_iter_task_destroy(struct bpf_iter_task *it)
> {
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index df697c74d519..84e8a0f6e4e0 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1252,9 +1252,7 @@ static const struct bpf_func_proto bpf_get_func_arg_cnt_proto = {
> };
>
> #ifdef CONFIG_KEYS
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "kfuncs which will be used in BPF programs");
> +__bpf_kfunc_start_defs();
>
> /**
> * bpf_lookup_user_key - lookup a key by its serial
> @@ -1404,7 +1402,7 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr,
> }
> #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(key_sig_kfunc_set)
> BTF_ID_FLAGS(func, bpf_lookup_user_key, KF_ACQUIRE | KF_RET_NULL | KF_SLEEPABLE)
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 0841f8d82419..c9fdcc5cdce1 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -503,9 +503,8 @@ static int bpf_test_finish(const union bpf_attr *kattr,
> * architecture dependent calling conventions. 7+ can be supported in the
> * future.
> */
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
> +
> __bpf_kfunc int bpf_fentry_test1(int a)
> {
> return a + 1;
> @@ -605,7 +604,7 @@ __bpf_kfunc void bpf_kfunc_call_memb_release(struct prog_test_member *p)
> {
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(bpf_test_modify_return_ids)
> BTF_ID_FLAGS(func, bpf_modify_return_test)
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 21d75108c2e9..383f96b0a1c7 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -11767,9 +11767,7 @@ bpf_sk_base_func_proto(enum bpf_func_id func_id)
> return func;
> }
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
> __bpf_kfunc int bpf_dynptr_from_skb(struct sk_buff *skb, u64 flags,
> struct bpf_dynptr_kern *ptr__uninit)
> {
> @@ -11816,7 +11814,7 @@ __bpf_kfunc int bpf_sock_addr_set_sun_path(struct bpf_sock_addr_kern *sa_kern,
>
> return 0;
> }
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags,
> struct bpf_dynptr_kern *ptr__uninit)
> @@ -11879,10 +11877,7 @@ static int __init bpf_kfunc_init(void)
> }
> late_initcall(bpf_kfunc_init);
>
> -/* Disables missing prototype warnings */
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> /* bpf_sock_destroy: Destroy the given socket with ECONNABORTED error code.
> *
> @@ -11916,7 +11911,7 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
> return sk->sk_prot->diag_destroy(sk, ECONNABORTED);
> }
>
> -__diag_pop()
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(bpf_sk_iter_kfunc_ids)
> BTF_ID_FLAGS(func, bpf_sock_destroy, KF_TRUSTED_ARGS)
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index df4789ab512d..b6f1d6dab3f2 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -696,9 +696,7 @@ struct xdp_frame *xdpf_clone(struct xdp_frame *xdpf)
> return nxdpf;
> }
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in vmlinux BTF");
> +__bpf_kfunc_start_defs();
>
> /**
> * bpf_xdp_metadata_rx_timestamp - Read XDP frame RX timestamp.
> @@ -738,7 +736,7 @@ __bpf_kfunc int bpf_xdp_metadata_rx_hash(const struct xdp_md *ctx, u32 *hash,
> return -EOPNOTSUPP;
> }
>
> -__diag_pop();
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(xdp_metadata_kfunc_ids)
> #define XDP_METADATA_KFUNC(_, __, name, ___) BTF_ID_FLAGS(func, name, KF_TRUSTED_ARGS)
> diff --git a/net/ipv4/fou_bpf.c b/net/ipv4/fou_bpf.c
> index 3760a14b6b57..4da03bf45c9b 100644
> --- a/net/ipv4/fou_bpf.c
> +++ b/net/ipv4/fou_bpf.c
> @@ -22,9 +22,7 @@ enum bpf_fou_encap_type {
> FOU_BPF_ENCAP_GUE,
> };
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in BTF");
> +__bpf_kfunc_start_defs();
>
> /* bpf_skb_set_fou_encap - Set FOU encap parameters
> *
> @@ -100,7 +98,7 @@ __bpf_kfunc int bpf_skb_get_fou_encap(struct __sk_buff *skb_ctx,
> return 0;
> }
>
> -__diag_pop()
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(fou_kfunc_set)
> BTF_ID_FLAGS(func, bpf_skb_set_fou_encap)
> diff --git a/net/netfilter/nf_conntrack_bpf.c b/net/netfilter/nf_conntrack_bpf.c
> index b21799d468d2..475358ec8212 100644
> --- a/net/netfilter/nf_conntrack_bpf.c
> +++ b/net/netfilter/nf_conntrack_bpf.c
> @@ -230,9 +230,7 @@ static int _nf_conntrack_btf_struct_access(struct bpf_verifier_log *log,
> return 0;
> }
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in nf_conntrack BTF");
> +__bpf_kfunc_start_defs();
>
> /* bpf_xdp_ct_alloc - Allocate a new CT entry
> *
> @@ -467,7 +465,7 @@ __bpf_kfunc int bpf_ct_change_status(struct nf_conn *nfct, u32 status)
> return nf_ct_change_status_common(nfct, status);
> }
>
> -__diag_pop()
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(nf_ct_kfunc_set)
> BTF_ID_FLAGS(func, bpf_xdp_ct_alloc, KF_ACQUIRE | KF_RET_NULL)
> diff --git a/net/netfilter/nf_nat_bpf.c b/net/netfilter/nf_nat_bpf.c
> index 141ee7783223..6e3b2f58855f 100644
> --- a/net/netfilter/nf_nat_bpf.c
> +++ b/net/netfilter/nf_nat_bpf.c
> @@ -12,9 +12,7 @@
> #include <net/netfilter/nf_conntrack_core.h>
> #include <net/netfilter/nf_nat.h>
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in nf_nat BTF");
> +__bpf_kfunc_start_defs();
>
> /* bpf_ct_set_nat_info - Set source or destination nat address
> *
> @@ -54,7 +52,7 @@ __bpf_kfunc int bpf_ct_set_nat_info(struct nf_conn___init *nfct,
> return nf_nat_setup_info(ct, &range, manip) == NF_DROP ? -ENOMEM : 0;
> }
>
> -__diag_pop()
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(nf_nat_kfunc_set)
> BTF_ID_FLAGS(func, bpf_ct_set_nat_info, KF_TRUSTED_ARGS)
> diff --git a/net/xfrm/xfrm_interface_bpf.c b/net/xfrm/xfrm_interface_bpf.c
> index d74f3fd20f2b..7d5e920141e9 100644
> --- a/net/xfrm/xfrm_interface_bpf.c
> +++ b/net/xfrm/xfrm_interface_bpf.c
> @@ -27,9 +27,7 @@ struct bpf_xfrm_info {
> int link;
> };
>
> -__diag_push();
> -__diag_ignore_all("-Wmissing-prototypes",
> - "Global functions as their definitions will be in xfrm_interface BTF");
> +__bpf_kfunc_start_defs();
>
> /* bpf_skb_get_xfrm_info - Get XFRM metadata
> *
> @@ -93,7 +91,7 @@ __bpf_kfunc int bpf_skb_set_xfrm_info(struct __sk_buff *skb_ctx, const struct bp
> return 0;
> }
>
> -__diag_pop()
> +__bpf_kfunc_end_defs();
>
> BTF_SET8_START(xfrm_ifc_kfunc_set)
> BTF_ID_FLAGS(func, bpf_skb_get_xfrm_info)
> --
> 2.34.1
>
--
Regards
Yafang
