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

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 1/17/23 1:22 PM, Toke Høiland-Jørgensen wrote:
>> Daniel Borkmann <daniel@iogearbox.net> writes:
>>> On 1/17/23 12:30 PM, Toke Høiland-Jørgensen wrote:
>>>> Daniel Borkmann <daniel@iogearbox.net> writes:
>>>>> On 1/16/23 11:57 PM, Toke Høiland-Jørgensen 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 added a
>>>>>> placeholder reference indicating that this (TBD) functionality exists.
>>>>>
>>>>> Thanks for the writeup.. I did some edits to your sections to make some parts
>>>>> more clear and to leave out other parts (e.g. libbpf-related bits which are not
>>>>> relevant in here and it's one of many libs). I also edited some parts to leave
>>>>> us more flexibility. Here would be my take mixed in:
>>>>
>>>> Edits LGTM, with just one nit, below:
>>>>
>>>>> 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. In the extreme case that
>>>>> could also include removal of a kfunc. This means that BPF programs using
>>>>> kfuncs might need to adapt to changes between kernel versions. 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.
>>>>>
>>>>> 3.1 Promotion to "stable"
>>>>> -------------------------
>>>>>
>>>>> While kfuncs are by default considered unstable as described above, some kfuncs
>>>>> may warrant a stronger stability guarantee and could be marked as *stable*. The
>>>>> decision to move a kfunc to *stable* is taken on a case-by-case basis and has
>>>>> a high barrier, taking into account its usefulness under longer-term production
>>>>> deployment without any unforeseen API issues or limitations. In general, it is
>>>
>>> Forgot, we should probably also add after "[...] or limitations.":
>>>
>>>     Such promotion request along with aforementioned argumentation on why a kfunc
>>>     is ready to be stabilized must be driven from developer-side.
>> 
>> What does "driven from developer-side" mean, exactly? And what kind of
>> developers (BPF app developers, or kernel devs)?
>
> Mainly to denote that this needs to be an explicit request from the community
> rather than something that would happen automagically after some time (e.g.
> where maintainers would just put the KF_STABLE stamp to it). 'kfunc xyz has
> been used in our fleet in production in the context of project abc for two
> years now and its API is sufficient to cover all foreseeable needs. The
> kfunc didn't need to get extended since it was added [...]', for example.
> The developer-hat can be both as long as there is a concrete relation to
> usage of the kfunc that can be provided to then make the case.

Right, makes sense! So how about:

"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."

-Toke