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

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Tue, Jan 24, 2023 at 5:18 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> 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.

Schrodinger's kfuncs :)

> [...]
>
> > 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?

It depends.
Obviously we're not going to change/remove/deprecate kfuncs unless
there is a need to do so.
And when we do we will consider all users.

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

That's not something we can promise.
Take conntrack kfuncs. If netfilter folks decide to sunset
conntrack tomorrow we won't be standing in their way.

On the other side the dynptr kfuncs are going to stay as-is for
foreseeable future because they don't rely on other kernel
subsystems to do the job.
Both cases may still change if users themselves
(after using it in prod) come back with reasons to change it.

In the past the kernel devs were dictating the helper uapi to
users and users had no option, but to shut up and use what's available.
Now they will be able to use new apis and request changes.
At that point it will be a set of users X vs a set of users Y.
If ten users say that this kfunc sucks while one user
wants to keep it as-is we will introduce another kfunc and
will start deprecating the one that lost the vote.
The deprecation time window will depend on case by case
considering maintenance cost, etc.

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

Of course. That has been brought up many times.
Just do it. Whoever has time to do it.