Re: [PATCH v2 bpf-next 1/2] bpf: Add __bpf_kfunc_{start,end}_defs macros

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux