[PATCH] bpf_helpers.h: define bpf_tail_call_static when building with GCC

[PATCH] bpf_helpers.h: define bpf_tail_call_static when building with GCC

[Jose E. Marchesi]

The definition of bpf_tail_call_static in tools/lib/bpf/bpf_helpers.h
is guarded by a preprocessor check to assure that clang is recent
enough to support it.  This patch updates the guard so the function is
compiled when using GCC as well.

Tested in bpf-next master.
No regressions.

Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
Cc: Yonghong Song <yhs@meta.com>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: david.faust@oracle.com
Cc: cupertino.miranda@oracle.com
---
 tools/lib/bpf/bpf_helpers.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 2324cc42b017..3306f50c5081 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -136,7 +136,7 @@
 /*
  * Helper function to perform a tail call with a constant/immediate map slot.
  */
-#if __clang_major__ >= 8 && defined(__bpf__)
+#if (!defined(__clang__) || __clang_major__ >= 8) && defined(__bpf__)
 static __always_inline void
 bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
 {
-- 
2.30.2

Re: [PATCH] bpf_helpers.h: define bpf_tail_call_static when building with GCC

[Yonghong Song]

On 1/23/24 10:59 AM, Jose E. Marchesi wrote:
> The definition of bpf_tail_call_static in tools/lib/bpf/bpf_helpers.h
> is guarded by a preprocessor check to assure that clang is recent
> enough to support it.  This patch updates the guard so the function is
> compiled when using GCC as well.
>
> Tested in bpf-next master.
> No regressions.
>
> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
> Cc: Yonghong Song <yhs@meta.com>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: david.faust@oracle.com
> Cc: cupertino.miranda@oracle.com
> ---
>   tools/lib/bpf/bpf_helpers.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index 2324cc42b017..3306f50c5081 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -136,7 +136,7 @@
>   /*
>    * Helper function to perform a tail call with a constant/immediate map slot.
>    */
> -#if __clang_major__ >= 8 && defined(__bpf__)
> +#if (!defined(__clang__) || __clang_major__ >= 8) && defined(__bpf__)

Do you want to guard with a gcc version as well here or you assume any gcc which supports bpf
should be okay here?

>   static __always_inline void
>   bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
>   {

Re: [PATCH] bpf_helpers.h: define bpf_tail_call_static when building with GCC

[Jose E. Marchesi]

> On 1/23/24 10:59 AM, Jose E. Marchesi wrote:
>> The definition of bpf_tail_call_static in tools/lib/bpf/bpf_helpers.h
>> is guarded by a preprocessor check to assure that clang is recent
>> enough to support it.  This patch updates the guard so the function is
>> compiled when using GCC as well.
>>
>> Tested in bpf-next master.
>> No regressions.
>>
>> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
>> Cc: Yonghong Song <yhs@meta.com>
>> Cc: Eduard Zingerman <eddyz87@gmail.com>
>> Cc: david.faust@oracle.com
>> Cc: cupertino.miranda@oracle.com
>> ---
>>   tools/lib/bpf/bpf_helpers.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
>> index 2324cc42b017..3306f50c5081 100644
>> --- a/tools/lib/bpf/bpf_helpers.h
>> +++ b/tools/lib/bpf/bpf_helpers.h
>> @@ -136,7 +136,7 @@
>>   /*
>>    * Helper function to perform a tail call with a constant/immediate map slot.
>>    */
>> -#if __clang_major__ >= 8 && defined(__bpf__)
>> +#if (!defined(__clang__) || __clang_major__ >= 8) && defined(__bpf__)
>
> Do you want to guard with a gcc version as well here or you assume any gcc which supports bpf
> should be okay here?

The second, because GCC versions that do not support
bpf_tail_call_static are not capable of building the selftests for many
other reasons, so there is little point to support them.

>
>>   static __always_inline void
>>   bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
>>   {

Re: [PATCH] bpf_helpers.h: define bpf_tail_call_static when building with GCC

[Andrii Nakryiko]

On Wed, Jan 24, 2024 at 12:25 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> > On 1/23/24 10:59 AM, Jose E. Marchesi wrote:
> >> The definition of bpf_tail_call_static in tools/lib/bpf/bpf_helpers.h
> >> is guarded by a preprocessor check to assure that clang is recent
> >> enough to support it.  This patch updates the guard so the function is
> >> compiled when using GCC as well.
> >>
> >> Tested in bpf-next master.
> >> No regressions.
> >>
> >> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
> >> Cc: Yonghong Song <yhs@meta.com>
> >> Cc: Eduard Zingerman <eddyz87@gmail.com>
> >> Cc: david.faust@oracle.com
> >> Cc: cupertino.miranda@oracle.com
> >> ---
> >>   tools/lib/bpf/bpf_helpers.h | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> >> index 2324cc42b017..3306f50c5081 100644
> >> --- a/tools/lib/bpf/bpf_helpers.h
> >> +++ b/tools/lib/bpf/bpf_helpers.h
> >> @@ -136,7 +136,7 @@
> >>   /*
> >>    * Helper function to perform a tail call with a constant/immediate map slot.
> >>    */
> >> -#if __clang_major__ >= 8 && defined(__bpf__)
> >> +#if (!defined(__clang__) || __clang_major__ >= 8) && defined(__bpf__)
> >
> > Do you want to guard with a gcc version as well here or you assume any gcc which supports bpf
> > should be okay here?
>
> The second, because GCC versions that do not support
> bpf_tail_call_static are not capable of building the selftests for many
> other reasons, so there is little point to support them.

bpf_helpers.h is part of libbpf-provided API, and so it's going to be
used way beyond just BPF selftests. So I think it's prudent to guard
with version check just like we do it for clang. I assume you do know
GCC version that is meant to support this?

>
> >
> >>   static __always_inline void
> >>   bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
> >>   {
>