On Wed, Jan 4, 2023 at 11:51 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Wed, Jan 04, 2023 at 10:43:52AM -0800, Andrii Nakryiko wrote:
> >
> > struct bpf_dynptr dptr = ...;
> > bool is_null = false;
> >
> > if (bpf_core_value_exists(enum bpf_func_id, BPF_FUNC_dynptr_is_null)) {
> > is_null = bpf_dynptr_is_null(&dptr);
> > } else {
> > struct bpf_dynptr_kern *kdptr = (void*)&dptr;
> > is_null = !!BPF_CORE_READ(kdptr, data);
> > }
> >
> > How do you detect the existence of kfunc today? Preferably without
> > doing extra work in user-space.
> >
> > Now, let's say kfunc changes its signature. Show me a short example on
> > how you deal with that in BPF C code?
>
> Didn't we add bpf_core_type_matches for func protos specifically
> to deal with function signature changes in the kernel after tracepoint
> args got swapped?
> I'm assuming the same mechanism will work for kfuncs.
> If not we can come up with a new one.
It would be good if someone actually try that and see if it works, and
if it doesn't, to come up with an approach that does. Right now I just
see hand-wavy arguments that BPF helpers and BPF kfuncs are equivalent
in this regard. Which currently I'm afraid they are not.
>
> >
> > Think about sched_ext. Right now it's so bleeding edge that you have
> > to assume the very latest and freshest kernel code. So you know all
> > the kfuncs that you need should exist otherwise sched_ext doesn't work
> > at all. Ok, happy place.
> >
> > Now a year or two passes by. Some kfuncs are added, some are changed.
> > We still believe that BPF CO-RE (compile once - run everywhere) is
> > good and we don't want to compile and distribute multiple versions of
> > BPF application, right? You'll want to do some extra (or more
> > performant) stuff if kernel is recent and has some new kfunc, but
> > fallback to some default suboptimal behavior otherwise. How do you do
> > that in a simple and straightforward way?
>
> with a help of CORE, of course.
> If it doesn't exist today we can add it.
>
> > But even worse is what if
> > some critical kfunc is changed between kernel versions and you do
How about this one? I'm honestly curious to see someone try and figure
out what works and what doesn't.
> > *need* to support both versions. Think about those aspects, because
> > sched_ext will run into them almost inevitably soon after its
> > inclusion into kernel.
> >
> >
> > One way or another there are some technical solution of various
> > degrees of creativity. And I'm actually not sure if I have a solution
> > for kfunc signature change at all. Without BTF we could use two
> > separate .c files and statically link them together, which would work
> > because extern is untyped in pure C. But with BPF static linking we do
> > have BTF information for each extern, and those BTF types will be
> > incompatible for the same extern func.
> >
> > We can probably come up with some hacks and conventions, as usual, but
> > better start thinking about them now.
> >
> > But hopefully you can empathize a bit more with poor end users that
> > have to do hack like this and why having bpf_dynptr API defined as
> > stable BPF helpers, with no extra dependencies on BTF in kernel,
>
> BTF is a reasonable dependency.
> You've just used it to detect whether helper exists or not.
> So it's fine to use the same to check whether kfunc exists or not.
BTFGen doesn't require kernel to be built with BTF, and yet I get BPF
CO-RE stuff. But you are jumbling everything together. I don't need
BPF CO-RE to build a useful BPF application that needs to use
ringbuf+dynptr (think uprobe'ing of some app, USDTs, etc), yet we will
require BTF for no reason.
Just as you are afraid of not getting UAPI right because we can't
anticipate possible changes, let's be just as much afraid of
unnecessary dependencies, which can be a blocker or pain for some
users in some situations. Isn't that fair?
>
> >
> > Depends on perspective. If I was some humble dev trying to build
> > BPF-based tool that should work on x86, arm64, s390x, and riscv (or
> > whatever other architecture), and dynptr API is only based on kfuncs,
> > I'm screwed. I can't sponsor or do kfunc support for my favorite
> > architecture, I'm stuck waiting for this to be done by someone some
> > time, if ever.
>
> If kfuncs and bpf trampoline don't work on a particular architecture
> that developer is likely screwed anyway. Dynptr is the last thing they
> would worry about.
uprobe+dynptr+ringbuf is all I need for useful apps. Likely or not can
be argued to the end of times.
