On Thu, Jan 5, 2023 at 1:14 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 1/4/23 11:37 AM, Alexei Starovoitov wrote: > > Would you invest in developing application against unstable syscall API? Absolutely. > > People develop all tons of stuff on top of fuse-fs. People develop apps that interact > > with tracing bpf progs that are clearly unstable. They do suffer when kernel side > > changes and people accept that cost. BPF and tracing in general contributed to that mind change. > > In a datacenter quite a few user apps are tied to kernel internals. > > > >> Imho, it's one of BPF's strengths and > >> we should keep the door open, not close it. > > The strength of BPF was and still is that it has both stable and unstable interfaces. > > Roughly: networking is stable, tracing is unstable. > > The point is that to be stable one doesn't need to use helpers. > > We can make kfuncs stable too if we focus all our efforts this way and > > for that we need to abandon adding helpers though it's a pain short term. > > > >>>> to actual BPF helpers by then where we go and say, that kfunc has proven itself in production > >>>> and from an API PoV that it is ready to be a proper BPF helper, and until this point > >>> "Proper BPF helper" model is broken. > >>> static void *(*bpf_map_lookup_elem)(void *map, const void *key) = (void *) 1; > >>> > >>> is a hack that works only when compiler optimizes the code. > >>> See gcc's attr(kernel_helper) workaround. > >>> This 'proper helper' hack is the reason we cannot compile bpf programs with -O0. > >>> And because it's uapi we cannot even fix this > >>> With kfuncs we will be able to compile with -O0 and debug bpf programs with better tools. > >>> These tools don't exist yet, but we have a way forward whereas with helpers > >>> we are stuck with -O2. > >> Better debugging tools are needed either way, independent of -O0 or -O2. I don't > >> think -O0 is a requirement or barrier for that. It may open up possibilities for > >> new tools, but production is still running with -O2. Proper BPF helper model is > >> broken, but everyone relies on it, and will be for a very very long time to come, > >> whether we like it or not. There is a larger ecosystem around BPF devs outside of > >> kernel, and developers will use the existing means today. There are recommendations / > >> guidelines that we can provide but we also don't have control over what developers > >> are doing. Yet we should make their life easier, not harder. > > Fully fleshed out kfunc infra will make developers job easier. No one is advocating > > to make users suffer. > > It is a long discussion. I am replying on a thread with points that I have also > been thinking about kfunc and helper. > > I think bpf helper is a kernel function but helpers need to be defined in a more > tedious form. It requires to define bpf_func_proto and then wrap into > BPF_CALL_x. It was not obvious for me to get around to understand the reason > behind it. With kfunc, it is a more natural way for other kernel developers to > expose subsystem features to bpf prog. In time, I believe we will be able to > make kfunc has a similar experience as EXPORT_SYMBOL_*. > > Thus, for subsystem (hid, fuse, netdev...etc) exposing functions to bpf prog, I > think it makes sense to stay with kfunc from now on. The subsystem is not > exposing something like syscall as an uapi. bpf prog is part of the kernel in > the sense that it extends that subsystem code. I don't think bpf needs to > provide extra and more guarantee than the EXPORT_SYMBOL_* in term of api. That > said, we should still review kfunc in a way that ensuring it is competent to the > best of our knowledge at that point with the limited initial use cases at hand. > I won't be surprised some of the existing EXPORT_SYMBOL_* kernel functions will > be exposed to the bpf prog as kfunc as-is without any change in the future. For > example, a few tcp cc kfuncs such as tcp_slow_start. They are likely stable > without much change for a long time. It can be directly exposed as bpf kfunc. > kfunc is a way to expose subsystem function without needing the bpf_func_proto > and BPF_CALL_x quirks. When the function can be dual compiled later, the kfunc > can also be inlined. > > If kfunc will be used for subsystem, it is very likely the number of kfunc will > grow and exceed the bpf helpers soon. This seems to be a stronger need to work > on the user experience problems about kfunc that have mentioned in this thread > sooner than later. They have to be solved regardless. May be start with stable > kfunc first. If the new helper is guaranteed stable, then why it cannot be kfunc > but instead needs to go through the bpf_func_proto and BPF_CALL_x? In time, I > hope the bpf helper support in the verifier can be quieted down (eg. > check_helper_call vs check_kfunc_call) and focus energy into kfunc like inlining > kfunc...etc. Sorry, I am late to this discussion. The way I read this is that kfuncs and helpers are implementation details and the real question is about the stability and mutability of the helper methods. I think there are two kinds of BPF program developers, and I might be oversimplifying to convey a point here: [1] Tracing people: They craft tracing programs and are more accustomed to probing deeper into kernel internals, handling variable renames and consequently will tolerate a kfunc changing its signature, being renamed or disappearing. [2] Network people: They are not accustomed to mutability the same way as the tracing people. If there is mutability here, these users will face a change in developer experience. I see two paths forward here: [a] We want to somewhat preserve the developer experience of [2] and we find a way to do somewhat stable APIs. kfuncs have the benefit that they are eventually mutable, but a longer stability guarantee for helpers used by [2] could ameliorate the pains of mutability. e.g. something we could do for certain helpers is a deprecation story, e.g. a kfunc won't change for X kernel versions, or when we annotate kfuncs as deprecated, libbpf can warn users "this kfunc is going away in kernel version Z"). If this would be difficult to guarantee and we do care about developer experience, we might need to have some helpers exposed as UAPI. [b] We accept the fact the user experience will change more for [2] and that's a trade-off we accept. IMHO, this is not ideal and while tracing folks have found a way to cope, it would be yet another thing to worry about for folks who are not used to it. There are things we can do to make it slightly less burdensome for the user by adding a shim in BPF headers (however, it won't solve problems for everyone though e.g. inline BPF, other languages but will give them a template for their respective "shims"). Another thing to consider if there are use-cases where some users disable BTF (for whatever reason, like running BPF in a pacemaker :P or in extremely low memory cases).
