On Wed, Jan 18, 2023 at 11:48:59AM +0100, Daniel Borkmann wrote:
>
> My $0.02 is that I don't think we need to make a hard-cut ban as part of this.
The hard-cut is easier to enforce otherwise every developer will be arguing that
their new feature is special and it requires a new discussion.
This thread has been going for too long. We need to finish it now and
don't come back to it again every now and then.
imo this is the summary of the thread:
bpf folks fall into two categories: kernel maintainers and bpf developers/users.
- developers add new bpf features. They obviously want to use them and want bpf users
to know that the feature they added is not going to disappear in the next kernel.
They want stability.
- maintainers want to make sure that the kernel development doesn't suffer because
developers keep adding new apis. They want freedom to innovate and change apis.
Maintainers also know that developers make mistakes and might leave the community.
The kernel is huge and core infra changes all the time.
bpf apis must never be a reason not to change something in the kernel.
Freedom to change and stability just don't overlap.
These two camps can never agree on what is more important.
But we can make them co-exist.
The bpf developers adding new kfunc should assume that it's stable and proceed
to use it in bpf progs and production applications.
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.
The longer the kfunc is present the harder it will be for maintainers to justify
removing it. The developers have to stick around and demonstrate that their
kfunc is actually being used. The better developers do it the bigger the effort
maintainers will put into keeping the kfunc perfectly intact.
Some kfunc might be perfect on the first try and it will be stable from the
first kernel release it appeared in.
Other kfuncs might be questionable. Like what happened with conntrack kfunc.
It looked good first, but then the same developers who added it came back to change it.
The approach of 'assume stable, but fix it if you like' worked in this case.
Take bpf_obj_new kfunc. I think it's great and I hope the interface will stick.
But we have an option to change it.
Take Andrii's upcoming 'open coded iterators'. The concept and api look great.
I hope we will do it right the first time and
bpf progs will start to use it immediately.
In such case why would anyone think of changing it?
If api works well and progs are using we will keep it this way.
But imagine we decide to replace the verifier with something better.
It will give us much better flexibility, but sadly bounded loops and
iterators will be in the way.
What we most likely going to do in such case we'll keep two verifiers for
several years and deprecate the old one along with kfuncs that we couldn't keep.
That would be a scheme for deprecation of kfunc.
Maybe we will use KF_DEPRECATE mechansim in such case or something else.
I think we need to cross that bridge when we get there.
Introducing KF_STABLE and KF_DEPRECATED right now looks premature.
We can discuss it, but adding it to a doc and committing to it is too early.
We don't have any kfuncs to mark as KF_STABLE or as KF_DEPRECATED.
No one presented any data on usage of existing kfuncs.
So we're not going to change or remove any one of them.
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.
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.
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.
I think it's more important to decide how we document kfuncs and
how equivalent of bpf_helper_defs.h can be done.
> The 'All new BPF kernel helper-like functionality must initially start out as
> kfuncs.' is pretty clear where things would need to start out with, and we could
> leave the option on the table if really needed to go BPF helper route when
> promoting kfunc to stable at the same time. I had that in the text suggestion
> earlier, it's more corner case and maybe we'll never need it but we also don't
> drive ourselves into a corner where we close the door on it. Lets let the infra
> around kfuncs evolve further first.
Going kfunc->helper for stability was discussed already. It probably got lost
in the noise. The summary was that it's not an option for the following reason:
kfuncs and helpers are done through different mechanisms on prog and kernel side.
The prog either sees = (void *)1 hack or normal call to extern func.
The generated code is different.
Say, we convert a kfunc to helper. Immediately the existing bpf prog that uses
that kfunc will fail to load. That's the opposite of stability.
We're going to require the developer to demonstrate the real world use of kfunc
before promoting to stable, but with such 'promotion' we will break bpf progs.
Say, we keep kfunc and introduce a new helper that does exactly the same.
But it won't help bpf prog. The prog was using kfunc in production,
why would it do some kind of CO-RE thing to compile 'call foo' differently
depending whether 'foo' is kfunc or helper in the given kernel. And so on.
