[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 2 Feb 2023 13:21:19 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: David Vernet <void@...ifault.com>
Cc: bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Song Liu <song@...nel.org>, Yonghong Song <yhs@...a.com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...gle.com>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
Kernel Team <kernel-team@...a.com>,
Toke Høiland-Jørgensen <toke@...hat.com>,
Jesper Dangaard Brouer <brouer@...hat.com>,
Jonathan Corbet <corbet@....net>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH bpf-next 2/3] bpf: Add KF_DEPRECATED kfunc flag
On Thu, Feb 2, 2023 at 8:31 AM David Vernet <void@...ifault.com> wrote:
>
> Now that we have our kfunc lifecycle expectations clearly documented,
> and that KF_DEPRECATED is documented as an optional method for kfunc
> developers and maintainers to provide a deprecation story to BPF users,
> we need to actually implement the flag.
>
> This patch adds KF_DEPRECATED, and updates the verifier to issue a
> verifier log message if a deprecated kfunc is called. Currently, a BPF
> program either has to fail to verify, or be loaded with log level 2 in
> order to see the message. We could eventually enhance this to always
> be logged regardless of log level or verification status, or we could
> instead emit a warning to dmesg. This seems like the least controversial
> option for now.
>
> A subsequent patch will add a selftest that verifies this behavior.
>
> Signed-off-by: David Vernet <void@...ifault.com>
> ---
> include/linux/btf.h | 1 +
> kernel/bpf/verifier.c | 8 ++++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 49e0fe6d8274..a0ea788ee9b0 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -71,6 +71,7 @@
> #define KF_SLEEPABLE (1 << 5) /* kfunc may sleep */
> #define KF_DESTRUCTIVE (1 << 6) /* kfunc performs destructive actions */
> #define KF_RCU (1 << 7) /* kfunc only takes rcu pointer arguments */
> +#define KF_DEPRECATED (1 << 8) /* kfunc is slated to be removed or deprecated */
>
> /*
> * Tag marking a kernel function as a kfunc. This is meant to minimize the
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 4cc0e70ee71e..22adcf24f9e1 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8511,6 +8511,11 @@ static bool is_kfunc_rcu(struct bpf_kfunc_call_arg_meta *meta)
> return meta->kfunc_flags & KF_RCU;
> }
>
> +static bool is_kfunc_deprecated(const struct bpf_kfunc_call_arg_meta *meta)
> +{
> + return meta->kfunc_flags & KF_DEPRECATED;
> +}
> +
> static bool is_kfunc_arg_kptr_get(struct bpf_kfunc_call_arg_meta *meta, int arg)
> {
> return arg == 0 && (meta->kfunc_flags & KF_KPTR_GET);
> @@ -9646,6 +9651,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> mark_btf_func_reg_size(env, regno, t->size);
> }
>
> + if (is_kfunc_deprecated(&meta))
> + verbose(env, "calling deprecated kfunc %s\n", func_name);
> +
Since prog will successfully load, no one will notice this message.
I think we can skip patches 2 and 3 for now.
Powered by blists - more mailing lists