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 3:38 PM, Toke Høiland-Jørgensen wrote:
>> 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."
>
> Sounds good to me!

Cool. I'll incorporate your changes (+ what we discussed) and send a v2
to make it easier for others to chime in...

-Toke