On Fri, Oct 9, 2020 at 11:41 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> On 10/9/20 8:35 PM, Andrii Nakryiko wrote:
> > On Fri, Oct 9, 2020 at 11:21 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
> >> On 10/9/20 8:09 PM, Yaniv Agman wrote:
> >>> בתאריך יום ו׳, 9 באוק׳ 2020 ב-20:39 מאת Daniel Borkmann
> >>> <daniel@xxxxxxxxxxxxx>:
> >>>>
> >>>> On 10/9/20 6:56 PM, Yaniv Agman wrote:
> >>>>> בתאריך יום ו׳, 9 באוק׳ 2020 ב-19:27 מאת Daniel Borkmann
> >>>>> <daniel@xxxxxxxxxxxxx>:
> >>>>>>
> >>>>>> [ Cc +Yonghong ]
> >>>>>>
> >>>>>> On 10/9/20 6:05 PM, Yaniv Agman wrote:
> >>>>>>> Pulling the latest changes of libbpf and compiling my application with it,
> >>>>>>> I see the following error:
> >>>>>>>
> >>>>>>> ../libbpf/src//root/usr/include/bpf/bpf_helpers.h:99:10: error:
> >>>>>>> unknown register name 'r0' in asm
> >>>>>>> : "r0", "r1", "r2", "r3", "r4", "r5");
> >>>>>>>
> >>>>>>> The commit which introduced this change is:
> >>>>>>> 80c7838600d39891f274e2f7508b95a75e4227c1
> >>>>>>>
> >>>>>>> I'm not sure if I'm doing something wrong (missing include?), or this
> >>>>>>> is a genuine error
> >>>>>>
> >>>>>> Seems like your clang/llvm version might be too old.
> >>>>>
> >>>>> I'm using clang 10.0.1
> >>>>
> >>>> Ah, okay, I see. Would this diff do the trick for you?
> >>>
> >>> Yes! Now it compiles without any problems!
> >>
> >> Great, thx, I'll cook proper fix and check with clang6 as Yonghong mentioned.
> >
> > Am I the only one confused here?... Yonghong said it should be
> > supported as early as clang 6, Yaniv is using Clang 10 and is still
> > getting this error. Let's figure out what's the problem before adding
> > unnecessary checks.
> >
> > I think it's not the clang_major check that helped, rather __bpf__
> > check. So please hold off on the fix, let's get to the bottom of this
> > first.
>
> I don't see confusion here (maybe other than which minimal clang/llvm version
> libbpf should support). If we do `#if __clang_major__ >= 6 && defined(__bpf__)`
> for the final patch, then this means that user passed clang -target bpf and
> the min supported version for inline assembly was there, otherwise we fall back
> to bpf_tail_call. In Yaniv's case, he probably had native target with -emit-llvm
> and then used llc invocation.
The "-emit-llvm" was the part that we were missing and had to figure
it out, before we could discuss the fix.
>
> >>>> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> >>>> index 2bdb7d6dbad2..31e356831fcf 100644
> >>>> --- a/tools/lib/bpf/bpf_helpers.h
> >>>> +++ b/tools/lib/bpf/bpf_helpers.h
> >>>> @@ -72,6 +72,7 @@
> >>>> /*
> >>>> * Helper function to perform a tail call with a constant/immediate map slot.
> >>>> */
> >>>> +#if __clang_major__ >= 10 && defined(__bpf__)
> >>>> static __always_inline void
> >>>> bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
> >>>> {
> >>>> @@ -98,6 +99,9 @@ bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
> >>>> :: [ctx]"r"(ctx), [map]"r"(map), [slot]"i"(slot)
> >>>> : "r0", "r1", "r2", "r3", "r4", "r5");
> >>>> }
> >>>> +#else
> >>>> +# define bpf_tail_call_static bpf_tail_call
bpf_tail_call_static has very specific guarantees, so in cases where
we can't use inline assembly to satisfy those guarantees, I think we
should not just silently redefine bpf_tail_call_static as
bpf_tail_call, rather make compilation fail if someone is attempting
to use bpf_tail_call_static. _Static_assert could be used to provide a
better error message here, probably.
> >>>> +#endif /* __clang_major__ >= 10 && __bpf__ */
> >>>>
> >>>> /*
> >>>> * Helper structure used by eBPF C program
> >>
>
