Re: [RFC PATCH v2] Documentation/bpf: Add a description of "stable kfuncs"

[Stanislav Fomichev <sdf@google.com>]

On Tue, Jan 17, 2023 at 1:27 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Following up on the discussion at the BPF office hours, this patch adds a
> description of the (new) concept of "stable kfuncs", which are kfuncs that
> offer a "more stable" interface than what we have now, but is still not
> part of UAPI.
>
> This is mostly meant as a straw man proposal to focus discussions around
> stability guarantees. From the discussion, it seemed clear that there were
> at least some people (myself included) who felt that there needs to be some
> way to export functionality that we consider "stable" (in the sense of
> "applications can rely on its continuing existence").
>
> One option is to keep BPF helpers as the stable interface and implement
> some technical solution for moving functionality from kfuncs to helpers
> once it has stood the test of time and we're comfortable committing to it
> as a stable API. Another is to freeze the helper definitions, and instead
> use kfuncs for this purpose as well, by marking a subset of them as
> "stable" in some way. Or we can do both and have multiple levels of
> "stable", I suppose.
>
> This patch is an attempt to describe what the "stable kfuncs" idea might
> look like, as well as to formulate some criteria for what we mean by
> "stable", and describe an explicit deprecation procedure. Feel free to
> critique any part of this (including rejecting the notion entirely).
>
> Some people mentioned (in the office hours) that should we decide to go in
> this direction, there's some work that needs to be done in libbpf (and
> probably the kernel too?) to bring the kfunc developer experience up to par
> with helpers. Things like exporting kfunc definitions to vmlinux.h (to make
> them discoverable), and having CO-RE support for using them, etc. I kinda
> consider that orthogonal to what's described here, but I do think we should
> fix those issues before implementing the procedures described here.
>
> v2:
> - Incorporate Daniel's changes
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
>  Documentation/bpf/kfuncs.rst | 87 +++++++++++++++++++++++++++++++++---
>  1 file changed, 81 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
> index 9fd7fb539f85..dd40a4ee35f2 100644
> --- a/Documentation/bpf/kfuncs.rst
> +++ b/Documentation/bpf/kfuncs.rst
> @@ -7,9 +7,9 @@ BPF Kernel Functions (kfuncs)
>
>  BPF Kernel Functions or more commonly known as kfuncs are functions in the Linux
>  kernel which are exposed for use by BPF programs. Unlike normal BPF helpers,
> -kfuncs do not have a stable interface and can change from one kernel release to
> -another. Hence, BPF programs need to be updated in response to changes in the
> -kernel.
> +kfuncs by default do not have a stable interface and can change from one kernel
> +release to another. Hence, BPF programs may need to be updated in response to
> +changes in the kernel. See :ref:`BPF_kfunc_stability`.
>
>  2. Defining a kfunc
>  ===================
> @@ -223,14 +223,89 @@ type. An example is shown below::
>          }
>          late_initcall(init_subsystem);
>
> -3. Core kfuncs
> +
> +.. _BPF_kfunc_stability:
> +
> +3. API (in)stability of kfuncs
> +==============================
> +
> +By default, kfuncs exported to BPF programs are considered a kernel-internal
> +interface that can change between kernel versions. This means that BPF programs
> +using kfuncs may need to adapt to changes between kernel versions. In the
> +extreme case that could also include removal of a kfunc. In other words, kfuncs
> +are _not_ part of the kernel UAPI! Rather, these kfuncs can be thought of as
> +being similar to internal kernel API functions exported using the

[..]

> +``EXPORT_SYMBOL_GPL`` macro. All new BPF kernel helper-like functionality must
> +initially start out as kfuncs.

To clarify, as part of this proposal, are we making a decision here
that we ban new helpers going forward?

(also left some spelling nits below)

> +
> +3.1 Promotion to "stable" kfuncs
> +--------------------------------
> +
> +While kfuncs are by default considered unstable as described above, some kfuncs
> +may warrant a stronger stability guarantee and can be marked as *stable*. The
> +decision to move a kfunc to *stable* is taken on a case-by-case basis and must
> +clear a high bar, taking into account the functions' usefulness under
> +longer-term production deployment without any unforeseen API issues or
> +limitations. In general, it is not expected that every kfunc will turn into a
> +stable one - think of it as an exception rather than the norm.
> +
> +Those kfuncs which have been promoted to stable are then marked using the
> +``KF_STABLE`` tag. The process for requesting a kfunc be marked as stable
> +consists of submitting a patch to the bpf@vger.kernel.org mailing list adding
> +the ``KF_STABLE`` tag to that kfunc's definition. The patch description must
> +include the rationale for why the kfunc should be promoted to stable, including
> +references to existing production uses, etc. The patch will be considered the
> +same was as any other patch, and ultimately the decision on whether a kfunc

nit: most likely s/same was/same way/ here?

> +should be promoted to stable is taken by the BPF maintainers.
> +
> +Stable kfuncs provide the following stability guarantees:
> +
> +1. Stable kfuncs will not change their function signature or functionality in a
> +   way that may cause incompatibilities for BPF programs calling the function.
> +
> +2. The BPF community will make every reasonable effort to keep stable kfuncs
> +   around as long as they continue to be useful to real-world BPF applications.
> +
> +3. Should a stable kfunc turn out to be no longer useful, the BPF community may
> +   decide to eventually remove it. In this case, before being removed that kfunc
> +   will go through a deprecation procedure as outlined below.
> +
> +3.2 Deprecation of kfuncs
> +-------------------------
> +
> +As described above, the community will make every reasonable effort to keep
> +kfuncs available through future kernel versions once they are marked as stable.
> +However, it may happen case that BPF development moves in an unforeseen

'may happen case' -> 'may happen in case' ?

> +direction so that even a stable kfunc ceases to be useful for program
> +development.
> +
> +In this case, stable kfuncs can be marked as *deprecated* using the
> +``KF_DEPRECATED`` tag. Such a deprecation request cannot be arbitrary and must
> +explain why a given stable kfunc should be deprecated. Once a kfunc is marked as
> +deprecated, the following procedure will be followed for removal:
> +
> +1. A kfunc marked as deprecated will be kept in the kernel for a conservatively
> +   chosen period of time after it was first marked as deprecated (usually
> +   corresponding to a span of multiple years).
> +
> +2. Deprecated functions will be documented in the kernel docs along with their
> +   remaining lifespan and including a recommendation for new functionality that
> +   can replace the usage of the deprecated function (or an explanation for why
> +   no such replacement exists).
> +
> +3. After the deprecation period, the kfunc will be removed and the function name
> +   will be marked as invalid inside the kernel (to ensure that no new kfunc is
> +   accidentally introduced with the same name in the future). After this
> +   happens, BPF programs calling the kfunc will be rejected by the verifier.
> +
> +4. Core kfuncs
>  ==============
>
>  The BPF subsystem provides a number of "core" kfuncs that are potentially
>  applicable to a wide variety of different possible use cases and programs.
>  Those kfuncs are documented here.
>
> -3.1 struct task_struct * kfuncs
> +4.1 struct task_struct * kfuncs
>  -------------------------------
>
>  There are a number of kfuncs that allow ``struct task_struct *`` objects to be
> @@ -306,7 +381,7 @@ Here is an example of it being used:
>                 return 0;
>         }
>
> -3.2 struct cgroup * kfuncs
> +4.2 struct cgroup * kfuncs
>  --------------------------
>
>  ``struct cgroup *`` objects also have acquire and release functions:
> --
> 2.39.0
>