On Fri, Feb 03, 2023 at 08:47:21AM -0600, David Vernet wrote: > On Fri, Feb 03, 2023 at 02:04:10PM +0100, Toke Høiland-Jørgensen wrote: > > David Vernet <void@xxxxxxxxxxxxx> writes: > > > > > BPF kernel <-> kernel API stability has been discussed at length over > > > the last several weeks and months. Now that we've largely aligned over > > > kfuncs being the way forward, and BPF helpers being considered frozen, > > > it's time to document the expectations for kfunc lifecycles and > > > stability so that everyone (BPF users, kfunc developers, and > > > maintainers) are all aligned, and have a crystal-clear understanding of > > > the expectations surrounding kfuncs. > > > > > > To do that, this patch adds that documentation to the main kfuncs > > > documentation page via a new 'kfunc lifecycle expectations' section. The > > > patch describes how decisions are made in the kernel regarding whether > > > to include, keep, deprecate, or change / remove a kfunc. As described > > > very overtly in the patch itself, but likely worth highlighting here: > > > > > > "kfunc stability" does not mean, nor ever will mean, "BPF APIs may block > > > development elsewhere in the kernel". > > > > > > Rather, the intention and expectation is for kfuncs to be treated like > > > EXPORT_SYMBOL_GPL symbols in the kernel. The goal is for kfuncs to be a > > > safe and valuable option for maintainers and kfunc developers to extend > > > the kernel, without tying anyone's hands, or imposing any kind of > > > restrictions on maintainers in the same way that UAPI changes do. > > > > > > In addition to the 'kfunc lifecycle expectations' section, this patch > > > also adds documentation for a new KF_DEPRECATED kfunc flag which kfunc > > > authors or maintainers can choose to add to kfuncs if and when they > > > decide to deprecate them. Note that as described in the patch itself, a > > > kfunc need not be deprecated before being changed or removed -- this > > > flag is simply provided as an available deprecation mechanism for those > > > that want to provide a deprecation story / timeline to their users. > > > When necessary, kfuncs may be changed or removed to accommodate changes > > > elsewhere in the kernel without any deprecation at all. > > > > > > Signed-off-by: David Vernet <void@xxxxxxxxxxxxx> > > > > Some comments below, but otherwise please add my: > > > > Co-developed-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx> > > > > should we Cc the next version to linux-api@vger as well just to get a > > bit more visibility in case others have comments? > > Yes, good idea, will do. > > > > > > --- > > > Documentation/bpf/kfuncs.rst | 138 +++++++++++++++++++++++++++++++++-- > > > 1 file changed, 133 insertions(+), 5 deletions(-) > > > > > > diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst > > > index 0bd07b39c2a4..4135f3111b67 100644 > > > --- a/Documentation/bpf/kfuncs.rst > > > +++ b/Documentation/bpf/kfuncs.rst > > > @@ -13,7 +13,7 @@ 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. > > > +kernel. See :ref:`BPF_kfunc_lifecycle_expectations` for more information. > > > > > > 2. Defining a kfunc > > > =================== > > > @@ -238,6 +238,32 @@ single argument which must be a trusted argument or a MEM_RCU pointer. > > > The argument may have reference count of 0 and the kfunc must take this > > > into consideration. > > > > > > +.. _KF_deprecated_flag: > > > + > > > +2.4.9 KF_DEPRECATED flag > > > +------------------------ > > > + > > > +The KF_DEPRECATED flag is used for kfuncs which are expected to be changed or > > > +removed in a subsequent kernel release. Deprecated kfuncs may be removed at any > > > +time, though if possible (and when applicable), developers are encouraged to > > > +provide users with a deprecation window to ease the burden of migrating off of > > > +the kfunc. > > > > > > > > I think the "may be removed at any time" is a bit odd here. If someone > > wants to just remove a kfunc, why bother with the deprecation flag at > > all? Besides, that whole "deprecation is optional" bit is explained > > I definitely agree that the phrasing could be improved, but my intention > with that phrase was actually to answer the exact nuanced question you > posed. I think we need to clarify that KF_DEPRECATED is an optional tool > that developers can use to provide a softer deprecation story to their > users, rather than a flag that exists as part of a larger, strict > deprecation policy. Otherwise, I think it would be unclear to someone > reading this when and why they would need to use the flag. I liked your > suggestion below, and proposed a bit of extra text to try and capture > this point. Lmk what you think. > > > below, in this section we're just explaining the flag. So I'd just drop > > this bit and combine the first two paragraphs as: > > > > "The KF_DEPRECATED flag is used for kfuncs which are scheduled to be > > changed or removed in a subsequent kernel release. A kfunc that is > > marked with KF_DEPRECATED should also have any relevant information > > captured in its kernel doc. Such information typically includes the > > kfunc's expected remaining lifespan, a recommendation for new > > functionality that can replace it if any is available, and possibly a > > rationale for why it is being removed." > > I like this -- are you OK with adding this in a small subsequent > paragraph to address the point I made above? > > Note that KF_DEPRECATED is simply a tool that developers can choose to > use to ease their users' burden of migrating off of a kfunc. While > developers are encouraged to use KF_DEPRECATED to provide a > transitionary deprecation period to users of their kfunc, doing so is > not strictly required, and the flag does not exist as part of any larger > kfunc deprecation policy. Nevermind, after reading this a few more times, I think what you proposed above is sufficient. Will not be including this extra paragraph in v3. [...]
