Daniel Borkmann <daniel@xxxxxxxxxxxxx> writes: > On 1/17/23 3:38 PM, Toke Høiland-Jørgensen wrote: >> Daniel Borkmann <daniel@xxxxxxxxxxxxx> writes: >>> On 1/17/23 1:22 PM, Toke Høiland-Jørgensen wrote: >>>> Daniel Borkmann <daniel@xxxxxxxxxxxxx> writes: >>>>> On 1/17/23 12:30 PM, Toke Høiland-Jørgensen wrote: >>>>>> Daniel Borkmann <daniel@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx 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