On Tue, Dec 20, 2022 at 11:31:25AM -0800, Andrii Nakryiko wrote: > On Fri, Dec 16, 2022 at 9:35 AM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Mon, Dec 12, 2022 at 12:12:09PM -0800, Andrii Nakryiko wrote: > > > > > > There is no clean way to ever move from unstable kfunc to a stable helper. > > > > No clean way? Yet in the other email you proposed a way. > > Not pretty, but workable. > > I'm sure if ever there will be a need to stabilize the kfunc we will > > find a clean way to do it. > > You can't have stable and unstable helper definition in the same .c > file, of course we can. uapi helpers vs kfuncs argument is not a black and white comparison. It's not just stable vs unstable. uapi has strict rules and helpers in uapi/bpf.h have to follow those rules. While kfuncs in terms of stability are equivalent to EXPORT_SYMBOL_GPL. Meaning they are largely unstable. The upsteam kernel keeps changing those EXPORT_SYMBOL* functions, but distros can apply their own "stability rules". See Redhat's kABI, for example. A distro can guarantee a stability of certain EXPORT_SYMBOL* for their customers, but that doesn't bind upstream development. With uapi bpf helpers we have to guarantee their stability, while with kfuncs we can do whatever we want. Right now all kfuncs are unstable and to prove the point we changed them couple times already (nf_conn*). We also have bpf_obj_new_impl() kfunc which is equivalent to EXPORT_SYMBOL(__kmalloc). Hard to imagine more stable and more fundamental function. Of course we want bpf programs to use bpf_obj_new() and assume that it's going to be available in all future kernel releases. But at the same time we're not bound by uapi rules. bpf_obj_new() will likely be stable, but not uapi stable. If we screw up (or find better way to allocate memory in the future) we can change it. We can invent our own deprecation rules for stable-ish kfuncs and invent our more-unstable-than-current-unstable rules for kfuncs that are too much kernel release dependent. > But regardless, dynptr is modeled as black box with hidden state, and > its API surface area is bigger (offset, size, is null or not, > manipulations over those aspects; then there is skb/xdp abstraction to > be taken care of for generic read/write). It has a wider *generic* API > surface to be useful and effectively used. tbh dynptr as an abstraction of skb/xdp is not convincing. cilium created their own abstraction on top of skb and xdp and it's zero cost. While dynptr is not free, so xdp users unlikely to use dynptr(xdp) for perf reasons. So I suspect it won't be a success story in the long run, but we can certainly try it out since they will be kfuncs and can be deprecated if maintenance outweighs the number of users. > All *two* of them, bpf_get_current_task() and > bpf_get_current_task_btf(), right? They are 2 years apart. > bpf_get_current_task() was added before BTF era. It is still actively > used today and there is nothing wrong with it. It works on older > kernels just fine, even with BPF CO-RE (as backporting a few simple > patches to generate BTF is simple and easy; not so much with BPF > verifier changes to add native BTF support). I don't see much problem > having both, they are not maintenance burden. bpf_get_current_pid_tgid bpf_get_current_uid_gid bpf_get_current_comm bpf_get_current_task bpf_get_current_task_btf bpf_get_current_cgroup_id bpf_get_current_ancestor_cgroup_id bpf_skb_ancestor_cgroup_id bpf_sk_cgroup_id bpf_sk_ancestor_cgroup_id _are_ a maintenance burden. The verifier got smarter and we could have removed all of them, but uapi rules makes it impossible. The bpf prog could have been enabled to access all these task_struct and cgroup fields directly. Likely without any kfuncs. bpf_send_signal vs bpf_send_signal_thread bpf_jiffies64 vs bpf_this_cpu_ptr etc there are plenty examples where uapi bpf helpers became a burden. They are working and will keep working, but we could have done much better job if not for uapi. These are the examples where uapi rules are too strong for bpf development. Our pace of adding new features is high. The kernel uapi rules are too strict for us. At one point DaveM declared freeze on sizeof(struct sk_buff). It was a difficult, but correct decision. We have to declare freeze on bpf helpers. 211 helpers that have to be maintained forever is a huge burden. All new features should use kfuncs and we need to figure out a deprecation and stability story for them. How to document kfuncs cleanly, how to discover them, etc.
