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 6:25 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> On 1/4/23 12:51 AM, Alexei Starovoitov wrote:
> > On Tue, Jan 03, 2023 at 12:43:58PM +0100, Daniel Borkmann wrote:
> >> On 12/31/22 1:42 AM, Alexei Starovoitov wrote:
> >>> On Fri, Dec 30, 2022 at 03:00:21PM -0600, David Vernet wrote:
> >>>>>>
> >>>>>> Taking bpf_get_current_task() as an example, I think it's better to have
> >>>>>> the debate be "should we keep supporting this / are users still using
> >>>>>> it?" rather than, "it's UAPI, there's nothing to even discuss". The
> >>>>>> point being that even if bpf_get_current_task() is still used, there may
> >>>>>> (and inevitably will) be other UAPI helpers that are useless and that we
> >>>>>> just can't remove.
> >>>

+1 to all the things Daniel said about end user pains and barriers for
adoption, glad I'm not the only one arguing this anymore.

[...]

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

But specifically about how the BPF helper model is broken, that's at
least an exaggeration. BPF helper call is defined at BPF ISA level, it
has to be a `call <some constant>;`, and as long as compiler generates
such code, it's all good. From C standpoint UAPI is just a function
call:

bpf_map_lookup_elem(&map, ...);

As long as this compiles and generates proper `call 1;` assembly
instruction, we are good. If/when both Clang and GCC support an
alternative way to define helper and not as a static func pointer, -O0
builds (at least in the aspect of calling BPF helpers, I suspect other
stuff will break still) will just work. And what's better,
bpf_helper_defs.h would be able to pick the best option based on
compiler's support with end users not having to care or notice the
difference.

This is not an UAPI problem at all.


> 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. Better debugging
> possibilities should cater to everyone.
>
> Thanks,
> Daniel



[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