Re: bpf helpers freeze. Was: [PATCH v2 bpf-next 0/6] Dynptr convenience helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 4, 2023 at 4:13 PM 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

This is subjective and there is no point in arguing about that. I find
BPF helper definitions more obvious and more discoverable, for
example. But it doesn't matter what I prefer personally.

Whatever the case might be, this is purely internal implementation
detail that can be improved and unified much more between helpers and
kfuncs, and it's way less important compared to stability and
usability issues brought up in this thread, as it has no bearing on
user's experience.

> 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_*.

The original goal for kfuncs was to just directly expose kernel
functions as is, but then we ended up adding allowlists, tuning them,
fixing them, reworking them. We are talking about different lists per
different program types, etc. But again, this is internal matters.

There is fundamentally no difference between how kfunc and helpers
can/should be defined, they are both kernel functions with additional
annotations. If we put work into it we can converge the mechanics of
how they are defined.


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



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux