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

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

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> The bpf developers adding new kfunc should assume that it's stable and proceed
> to use it in bpf progs and production applications.

"Assume all kfuncs are stable" is fine by me, but that is emphatically
not what we have been saying thus far, quite the opposite...

> The bpf maintainers will keep this stability promise. They obviously will not
> reap it out of the kernel on the whim, but they will nuke it if this kfunc
> will be in the way of the kernel innovation.

...and it is contradicted by this last bit. I mean "it's stable, but
we'll remove it if it's in the way" is not, well, stable.

[...]

> bpf developers and users should assume that all kfuncs are stable and use them.
> When somebody comes to argue that a particular kfunc needs to change
> the developer who added that kfunc better to be around to argue that the kfunc is
> perfect the way it is. If developer is gone the maintainers will make a call.
> It's a self regulating system.
> kfuncs will be stable if developers/users are around.
> Yet the maintainers will have a freedom to change if absolutely necessary.

This assumes users (i.e., BPF program authors) are around during the
development phase, which they are generally not. Except for the users
who are also BPF devs, but that's a minority (if not now, hopefully in
the future). So I really think we need to document some expectations
here.

For instance, what happens if we change a kfunc, and a user shows up
during the -rc phase saying it broke their application? Are we going to
revert that change?

> Back to deprecation...
> I think KF_DEPRECATED is a good idea.
> When kfunc will be auto emitted into vmlinux.h (or whatever other file)
> or shipped in libbpf header we can emit
> __attribute__((deprecated("reason", "replacement")));
> to that header file (so it's seen during bpf prog build) and
> start dmesg warn on them in the verifier.
> Kernel splats do get noticed. The users would have to act quickly.

So how about documenting that bit? Something like:

"We promise that kfuncs will not be removed without going through a
deprecation phase. The length of the deprecation will be proportional to
how long that kfunc has existed in the kernel, but will be no shorter
than XX kernel releases." ?

> As far as KF_STABLE... I think it hurts the system in the long run.
> The developer can argue at one point in time that kfunc has to be KF_STABLE.
> The patch will be applied, but the developer is off the hook and can disappear.
> The maintainers would have to argue on behalf of the developer
> and keep maintaining it? The maintainers won't have a signal whether
> kfunc is still useful after initial KF_STABLE patch.

Doing the above wrt deprecation without having an explicit stable tag
would be OK with me.

> I think it's more important to decide how we document kfuncs and
> how equivalent of bpf_helper_defs.h can be done.

I agree we (also) need to do this. As well as have some support for
querying for them from userspace on a running kernel (for CO-RE
purposes).

-Toke