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 04, 2023 at 10:59:15AM -0800, Andrii Nakryiko wrote:
> 
> > >> 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.

Right and that's what gcc did with attribute((kernel_helper(1)),
but we didn't like it because gcc and clang would diverge.
Now you're arguing it's just a bpf_helper_defs.h change and we should
have allowed it?

Also consider that 'call <some constant>' or more precise 'call absolute_address'
as an instruction exist in only one CPU architecture. It's BPF ISA.
It's a mistake that I made 8 years ago and inability to fix it bothers me.
Now we have 100 times more developers than we had 8 years ago.
I expect 100 time more UAPI and ABI mistakes.
Minimizing unfixable mistakes is what I'm after.



[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