[PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Jose E. Marchesi]

This little patch modifies selftests/bpf/Makefile so it passes the
following extra options when invoking gcc-bpf:

 -gbtf
   This makes GCC to emit BTF debug info in .BTF and .BTF.ext.

 -mco-re
   This tells GCC to generate CO-RE relocations in .BTF.ext.

 -masm=pseudoc
   This tells GCC to emit BPF assembler using the pseudo-c syntax.

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/testing/selftests/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index edc73f8f5aef..702428021132 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -442,7 +442,7 @@ endef
 # Build BPF object using GCC
 define GCC_BPF_BUILD_RULE
 	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
-	$(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
+	$(Q)$(BPF_GCC) $3 -O2 -gbtf -mco-re -masm=pseudoc -c $1 -o $2
 endef
 
 SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
-- 
2.30.2

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Eduard Zingerman]

On Wed, 2024-04-24 at 10:41 +0200, Jose E. Marchesi wrote:
> This little patch modifies selftests/bpf/Makefile so it passes the
> following extra options when invoking gcc-bpf:
> 
>  -gbtf
>    This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
> 
>  -mco-re
>    This tells GCC to generate CO-RE relocations in .BTF.ext.
> 
>  -masm=pseudoc
>    This tells GCC to emit BPF assembler using the pseudo-c syntax.
> 
> 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
> ---

I tried this patch with regular LLVM build and everything works as expected.
Unfortunately, can't test for GCC build.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Yonghong Song]

On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
> This little patch modifies selftests/bpf/Makefile so it passes the
> following extra options when invoking gcc-bpf:
>
>   -gbtf
>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.

Could we do if '-g' is specified, for bpf program,
btf will be automatically generated?

>
>   -mco-re
>     This tells GCC to generate CO-RE relocations in .BTF.ext.

Can we make this default? That is, remove -mco-re option. I
can imagine for any serious bpf program, co-re is a must.

>
>   -masm=pseudoc
>     This tells GCC to emit BPF assembler using the pseudo-c syntax.

Can we make it the other way round such that -masm=pseudoc is
the default? You can have an option e.g., -masm=non-pseudoc,
for the other format?

>
> 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/testing/selftests/bpf/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index edc73f8f5aef..702428021132 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -442,7 +442,7 @@ endef
>   # Build BPF object using GCC
>   define GCC_BPF_BUILD_RULE
>   	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
> -	$(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
> +	$(Q)$(BPF_GCC) $3 -O2 -gbtf -mco-re -masm=pseudoc -c $1 -o $2
>   endef
>   
>   SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Jose E. Marchesi]

Hi Yonghong.

> On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
>> This little patch modifies selftests/bpf/Makefile so it passes the
>> following extra options when invoking gcc-bpf:
>>
>>   -gbtf
>>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
>
> Could we do if '-g' is specified, for bpf program,
> btf will be automatically generated?

Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
-gdwarf.  DWARF can always be generated by using -gdwarf.

Faust, Indu, WDYT?

>>
>>   -mco-re
>>     This tells GCC to generate CO-RE relocations in .BTF.ext.
>
> Can we make this default? That is, remove -mco-re option. I
> can imagine for any serious bpf program, co-re is a must.

CO-RE depends on BTF.  So I understand the above as making -mco-re the
default if BTF is generated, i.e. if -gbtf (or -g with the modification
above) are specified.  Isn't that what clang does?  Am I interpreting
correctly?

>>
>>   -masm=pseudoc
>>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
>
> Can we make it the other way round such that -masm=pseudoc is
> the default? You can have an option e.g., -masm=non-pseudoc,
> for the other format?

We could add a configure-time build option:

  --with-bpf-default-asm-syntax={pseudoc,normal}

so that GCC can be built to use whatever selected syntax as default.
Distros and people can then decide what to do.

>>
>> 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/testing/selftests/bpf/Makefile | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index edc73f8f5aef..702428021132 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -442,7 +442,7 @@ endef
>>   # Build BPF object using GCC
>>   define GCC_BPF_BUILD_RULE
>>   	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
>> -	$(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
>> +	$(Q)$(BPF_GCC) $3 -O2 -gbtf -mco-re -masm=pseudoc -c $1 -o $2
>>   endef
>>     SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[David Faust]

On 4/24/24 14:24, Jose E. Marchesi wrote:
> 
> Hi Yonghong.
> 
>> On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
>>> This little patch modifies selftests/bpf/Makefile so it passes the
>>> following extra options when invoking gcc-bpf:
>>>
>>>   -gbtf
>>>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
>>
>> Could we do if '-g' is specified, for bpf program,
>> btf will be automatically generated?
> 
> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
> -gdwarf.  DWARF can always be generated by using -gdwarf.
> 
> Faust, Indu, WDYT?

I agree it makes sense. Will need to look for the appropriate way
to set this in gcc, but I am happy to prepare a patch.

> 
>>>
>>>   -mco-re
>>>     This tells GCC to generate CO-RE relocations in .BTF.ext.
>>
>> Can we make this default? That is, remove -mco-re option. I
>> can imagine for any serious bpf program, co-re is a must.
> 
> CO-RE depends on BTF.  So I understand the above as making -mco-re the
> default if BTF is generated, i.e. if -gbtf (or -g with the modification
> above) are specified.  Isn't that what clang does?  Am I interpreting
> correctly?

This is already the default. We enable -mco-re automatically in the BPF
backend if also generating BTF, unless user explicitly disables via
-mno-co-re.

This should work the same way with plain -g, once -g means BTF for the
target.

> 
>>>
>>>   -masm=pseudoc
>>>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
>>
>> Can we make it the other way round such that -masm=pseudoc is
>> the default? You can have an option e.g., -masm=non-pseudoc,
>> for the other format?
> 
> We could add a configure-time build option:
> 
>   --with-bpf-default-asm-syntax={pseudoc,normal}
> 
> so that GCC can be built to use whatever selected syntax as default.
> Distros and people can then decide what to do.
> 
>>>
>>> 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/testing/selftests/bpf/Makefile | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>>> index edc73f8f5aef..702428021132 100644
>>> --- a/tools/testing/selftests/bpf/Makefile
>>> +++ b/tools/testing/selftests/bpf/Makefile
>>> @@ -442,7 +442,7 @@ endef
>>>   # Build BPF object using GCC
>>>   define GCC_BPF_BUILD_RULE
>>>   	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
>>> -	$(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
>>> +	$(Q)$(BPF_GCC) $3 -O2 -gbtf -mco-re -masm=pseudoc -c $1 -o $2
>>>   endef
>>>     SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Alexei Starovoitov]

On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> Hi Yonghong.
>
> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
> >> This little patch modifies selftests/bpf/Makefile so it passes the
> >> following extra options when invoking gcc-bpf:
> >>
> >>   -gbtf
> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
> >
> > Could we do if '-g' is specified, for bpf program,
> > btf will be automatically generated?
>
> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
> -gdwarf.  DWARF can always be generated by using -gdwarf.
>
> Faust, Indu, WDYT?
>
> >>
> >>   -mco-re
> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
> >
> > Can we make this default? That is, remove -mco-re option. I
> > can imagine for any serious bpf program, co-re is a must.
>
> CO-RE depends on BTF.  So I understand the above as making -mco-re the
> default if BTF is generated, i.e. if -gbtf (or -g with the modification
> above) are specified.  Isn't that what clang does?  Am I interpreting
> correctly?
>
> >>
> >>   -masm=pseudoc
> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
> >
> > Can we make it the other way round such that -masm=pseudoc is
> > the default? You can have an option e.g., -masm=non-pseudoc,
> > for the other format?
>
> We could add a configure-time build option:
>
>   --with-bpf-default-asm-syntax={pseudoc,normal}
>
> so that GCC can be built to use whatever selected syntax as default.
> Distros and people can then decide what to do.

distros just ship stuff.
It's our job to pick good defaults.

I agree with Yonghong that -g should imply -gbtf for bpf target
and -mco-re doesn't need to be a flag at all.
Compiler should do it when it sees those special attributes in C code.
-masm=pseudoc is a good default as well, since that's what
everyone in bpf community is used to.

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Jose E. Marchesi]

> On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>>
>> Hi Yonghong.
>>
>> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
>> >> This little patch modifies selftests/bpf/Makefile so it passes the
>> >> following extra options when invoking gcc-bpf:
>> >>
>> >>   -gbtf
>> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
>> >
>> > Could we do if '-g' is specified, for bpf program,
>> > btf will be automatically generated?
>>
>> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
>> -gdwarf.  DWARF can always be generated by using -gdwarf.
>>
>> Faust, Indu, WDYT?
>>
>> >>
>> >>   -mco-re
>> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
>> >
>> > Can we make this default? That is, remove -mco-re option. I
>> > can imagine for any serious bpf program, co-re is a must.
>>
>> CO-RE depends on BTF.  So I understand the above as making -mco-re the
>> default if BTF is generated, i.e. if -gbtf (or -g with the modification
>> above) are specified.  Isn't that what clang does?  Am I interpreting
>> correctly?
>>
>> >>
>> >>   -masm=pseudoc
>> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
>> >
>> > Can we make it the other way round such that -masm=pseudoc is
>> > the default? You can have an option e.g., -masm=non-pseudoc,
>> > for the other format?
>>
>> We could add a configure-time build option:
>>
>>   --with-bpf-default-asm-syntax={pseudoc,normal}
>>
>> so that GCC can be built to use whatever selected syntax as default.
>> Distros and people can then decide what to do.
>
> distros just ship stuff.
> It's our job to pick good defaults.

Yeah it was a rather dumb idea that would only complicate things for no
good reason.

The unfortunate fact is that at this point the kernel headers that
almost all BPF programs use contain pseudo-C inline assembly and having
the toolchain using the conventional assembly syntax by default would
force users to specify the command-line option explicitly, which is a
great PITA.  So I guess this is one of these situations where the worse
option is indeed the best default, in practical terms.

So ok, as much as it sucks we will make -masm=pseudoc the default in GCC
for the sake of practicality.

> I agree with Yonghong that -g should imply -gbtf for bpf target
> and -mco-re doesn't need to be a flag at all.

We like the idea of -g implying -gbtf rather than -gdwarf for the BPF
target.  It makes sense.  Faust is already working on it.

As for -mco-re, it is already the default with -gbtf, and now it will be
the default for -g.

> Compiler should do it when it sees those special attributes in C code.
> -masm=pseudoc is a good default as well, since that's what
> everyone in bpf community is used to.

We will try to get all the changes above upstream before GCC 14 gets
branched, which shall happen any day now.  Once they are in GCC the only
extra option to be added to GCC_BPF_BUILD_RULE will be -g.  Will send an
updated patch then.

Salud!

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Alexei Starovoitov]

On Thu, Apr 25, 2024 at 5:32 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> > On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
> > <jose.marchesi@oracle.com> wrote:
> >>
> >>
> >> Hi Yonghong.
> >>
> >> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
> >> >> This little patch modifies selftests/bpf/Makefile so it passes the
> >> >> following extra options when invoking gcc-bpf:
> >> >>
> >> >>   -gbtf
> >> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
> >> >
> >> > Could we do if '-g' is specified, for bpf program,
> >> > btf will be automatically generated?
> >>
> >> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
> >> -gdwarf.  DWARF can always be generated by using -gdwarf.
> >>
> >> Faust, Indu, WDYT?
> >>
> >> >>
> >> >>   -mco-re
> >> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
> >> >
> >> > Can we make this default? That is, remove -mco-re option. I
> >> > can imagine for any serious bpf program, co-re is a must.
> >>
> >> CO-RE depends on BTF.  So I understand the above as making -mco-re the
> >> default if BTF is generated, i.e. if -gbtf (or -g with the modification
> >> above) are specified.  Isn't that what clang does?  Am I interpreting
> >> correctly?
> >>
> >> >>
> >> >>   -masm=pseudoc
> >> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
> >> >
> >> > Can we make it the other way round such that -masm=pseudoc is
> >> > the default? You can have an option e.g., -masm=non-pseudoc,
> >> > for the other format?
> >>
> >> We could add a configure-time build option:
> >>
> >>   --with-bpf-default-asm-syntax={pseudoc,normal}
> >>
> >> so that GCC can be built to use whatever selected syntax as default.
> >> Distros and people can then decide what to do.
> >
> > distros just ship stuff.
> > It's our job to pick good defaults.
>
> Yeah it was a rather dumb idea that would only complicate things for no
> good reason.
>
> The unfortunate fact is that at this point the kernel headers that
> almost all BPF programs use contain pseudo-C inline assembly and having
> the toolchain using the conventional assembly syntax by default would
> force users to specify the command-line option explicitly, which is a
> great PITA.  So I guess this is one of these situations where the worse
> option is indeed the best default, in practical terms.
>
> So ok, as much as it sucks we will make -masm=pseudoc the default in GCC
> for the sake of practicality.
>
> > I agree with Yonghong that -g should imply -gbtf for bpf target
> > and -mco-re doesn't need to be a flag at all.
>
> We like the idea of -g implying -gbtf rather than -gdwarf for the BPF
> target.  It makes sense.  Faust is already working on it.
>
> As for -mco-re, it is already the default with -gbtf, and now it will be
> the default for -g.
>
> > Compiler should do it when it sees those special attributes in C code.
> > -masm=pseudoc is a good default as well, since that's what
> > everyone in bpf community is used to.
>
> We will try to get all the changes above upstream before GCC 14 gets
> branched, which shall happen any day now.  Once they are in GCC the only
> extra option to be added to GCC_BPF_BUILD_RULE will be -g.  Will send an
> updated patch then.

Awesome. This is all great to hear.

Thanks!

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Andrii Nakryiko]

On Thu, Apr 25, 2024 at 5:33 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> > On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
> > <jose.marchesi@oracle.com> wrote:
> >>
> >>
> >> Hi Yonghong.
> >>
> >> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
> >> >> This little patch modifies selftests/bpf/Makefile so it passes the
> >> >> following extra options when invoking gcc-bpf:
> >> >>
> >> >>   -gbtf
> >> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
> >> >
> >> > Could we do if '-g' is specified, for bpf program,
> >> > btf will be automatically generated?
> >>
> >> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
> >> -gdwarf.  DWARF can always be generated by using -gdwarf.
> >>
> >> Faust, Indu, WDYT?
> >>
> >> >>
> >> >>   -mco-re
> >> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
> >> >
> >> > Can we make this default? That is, remove -mco-re option. I
> >> > can imagine for any serious bpf program, co-re is a must.
> >>
> >> CO-RE depends on BTF.  So I understand the above as making -mco-re the
> >> default if BTF is generated, i.e. if -gbtf (or -g with the modification
> >> above) are specified.  Isn't that what clang does?  Am I interpreting
> >> correctly?
> >>
> >> >>
> >> >>   -masm=pseudoc
> >> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
> >> >
> >> > Can we make it the other way round such that -masm=pseudoc is
> >> > the default? You can have an option e.g., -masm=non-pseudoc,
> >> > for the other format?
> >>
> >> We could add a configure-time build option:
> >>
> >>   --with-bpf-default-asm-syntax={pseudoc,normal}
> >>
> >> so that GCC can be built to use whatever selected syntax as default.
> >> Distros and people can then decide what to do.
> >
> > distros just ship stuff.
> > It's our job to pick good defaults.
>
> Yeah it was a rather dumb idea that would only complicate things for no
> good reason.
>
> The unfortunate fact is that at this point the kernel headers that
> almost all BPF programs use contain pseudo-C inline assembly and having
> the toolchain using the conventional assembly syntax by default would
> force users to specify the command-line option explicitly, which is a
> great PITA.  So I guess this is one of these situations where the worse
> option is indeed the best default, in practical terms.
>
> So ok, as much as it sucks we will make -masm=pseudoc the default in GCC
> for the sake of practicality.
>
> > I agree with Yonghong that -g should imply -gbtf for bpf target
> > and -mco-re doesn't need to be a flag at all.
>
> We like the idea of -g implying -gbtf rather than -gdwarf for the BPF
> target.  It makes sense.  Faust is already working on it.
>
> As for -mco-re, it is already the default with -gbtf, and now it will be
> the default for -g.
>
> > Compiler should do it when it sees those special attributes in C code.
> > -masm=pseudoc is a good default as well, since that's what
> > everyone in bpf community is used to.
>
> We will try to get all the changes above upstream before GCC 14 gets
> branched, which shall happen any day now.  Once they are in GCC the only
> extra option to be added to GCC_BPF_BUILD_RULE will be -g.  Will send an
> updated patch then.

-g is already passed through common BPF_CFLAGS, see Clang rules, you
won't see explicit -g, but it's there (all those flags are passed as
$3 argument)

>
> Salud!
>

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Jose E. Marchesi]

> On Thu, Apr 25, 2024 at 5:33 AM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>>
>> > On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
>> > <jose.marchesi@oracle.com> wrote:
>> >>
>> >>
>> >> Hi Yonghong.
>> >>
>> >> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
>> >> >> This little patch modifies selftests/bpf/Makefile so it passes the
>> >> >> following extra options when invoking gcc-bpf:
>> >> >>
>> >> >>   -gbtf
>> >> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
>> >> >
>> >> > Could we do if '-g' is specified, for bpf program,
>> >> > btf will be automatically generated?
>> >>
>> >> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
>> >> -gdwarf.  DWARF can always be generated by using -gdwarf.
>> >>
>> >> Faust, Indu, WDYT?
>> >>
>> >> >>
>> >> >>   -mco-re
>> >> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
>> >> >
>> >> > Can we make this default? That is, remove -mco-re option. I
>> >> > can imagine for any serious bpf program, co-re is a must.
>> >>
>> >> CO-RE depends on BTF.  So I understand the above as making -mco-re the
>> >> default if BTF is generated, i.e. if -gbtf (or -g with the modification
>> >> above) are specified.  Isn't that what clang does?  Am I interpreting
>> >> correctly?
>> >>
>> >> >>
>> >> >>   -masm=pseudoc
>> >> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
>> >> >
>> >> > Can we make it the other way round such that -masm=pseudoc is
>> >> > the default? You can have an option e.g., -masm=non-pseudoc,
>> >> > for the other format?
>> >>
>> >> We could add a configure-time build option:
>> >>
>> >>   --with-bpf-default-asm-syntax={pseudoc,normal}
>> >>
>> >> so that GCC can be built to use whatever selected syntax as default.
>> >> Distros and people can then decide what to do.
>> >
>> > distros just ship stuff.
>> > It's our job to pick good defaults.
>>
>> Yeah it was a rather dumb idea that would only complicate things for no
>> good reason.
>>
>> The unfortunate fact is that at this point the kernel headers that
>> almost all BPF programs use contain pseudo-C inline assembly and having
>> the toolchain using the conventional assembly syntax by default would
>> force users to specify the command-line option explicitly, which is a
>> great PITA.  So I guess this is one of these situations where the worse
>> option is indeed the best default, in practical terms.
>>
>> So ok, as much as it sucks we will make -masm=pseudoc the default in GCC
>> for the sake of practicality.
>>
>> > I agree with Yonghong that -g should imply -gbtf for bpf target
>> > and -mco-re doesn't need to be a flag at all.
>>
>> We like the idea of -g implying -gbtf rather than -gdwarf for the BPF
>> target.  It makes sense.  Faust is already working on it.
>>
>> As for -mco-re, it is already the default with -gbtf, and now it will be
>> the default for -g.
>>
>> > Compiler should do it when it sees those special attributes in C code.
>> > -masm=pseudoc is a good default as well, since that's what
>> > everyone in bpf community is used to.
>>
>> We will try to get all the changes above upstream before GCC 14 gets
>> branched, which shall happen any day now.  Once they are in GCC the only
>> extra option to be added to GCC_BPF_BUILD_RULE will be -g.  Will send an
>> updated patch then.
>
> -g is already passed through common BPF_CFLAGS, see Clang rules, you
> won't see explicit -g, but it's there (all those flags are passed as
> $3 argument)

Didn't notice that.  Even better :)
Thanks for the hint.

>>
>> Salud!
>>

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Jose E. Marchesi]

> On Thu, Apr 25, 2024 at 5:32 AM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>>
>>
>> > On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
>> > <jose.marchesi@oracle.com> wrote:
>> >>
>> >>
>> >> Hi Yonghong.
>> >>
>> >> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
>> >> >> This little patch modifies selftests/bpf/Makefile so it passes the
>> >> >> following extra options when invoking gcc-bpf:
>> >> >>
>> >> >>   -gbtf
>> >> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
>> >> >
>> >> > Could we do if '-g' is specified, for bpf program,
>> >> > btf will be automatically generated?
>> >>
>> >> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
>> >> -gdwarf.  DWARF can always be generated by using -gdwarf.
>> >>
>> >> Faust, Indu, WDYT?
>> >>
>> >> >>
>> >> >>   -mco-re
>> >> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
>> >> >
>> >> > Can we make this default? That is, remove -mco-re option. I
>> >> > can imagine for any serious bpf program, co-re is a must.
>> >>
>> >> CO-RE depends on BTF.  So I understand the above as making -mco-re the
>> >> default if BTF is generated, i.e. if -gbtf (or -g with the modification
>> >> above) are specified.  Isn't that what clang does?  Am I interpreting
>> >> correctly?
>> >>
>> >> >>
>> >> >>   -masm=pseudoc
>> >> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
>> >> >
>> >> > Can we make it the other way round such that -masm=pseudoc is
>> >> > the default? You can have an option e.g., -masm=non-pseudoc,
>> >> > for the other format?
>> >>
>> >> We could add a configure-time build option:
>> >>
>> >>   --with-bpf-default-asm-syntax={pseudoc,normal}
>> >>
>> >> so that GCC can be built to use whatever selected syntax as default.
>> >> Distros and people can then decide what to do.
>> >
>> > distros just ship stuff.
>> > It's our job to pick good defaults.
>>
>> Yeah it was a rather dumb idea that would only complicate things for no
>> good reason.
>>
>> The unfortunate fact is that at this point the kernel headers that
>> almost all BPF programs use contain pseudo-C inline assembly and having
>> the toolchain using the conventional assembly syntax by default would
>> force users to specify the command-line option explicitly, which is a
>> great PITA.  So I guess this is one of these situations where the worse
>> option is indeed the best default, in practical terms.
>>
>> So ok, as much as it sucks we will make -masm=pseudoc the default in GCC
>> for the sake of practicality.
>>
>> > I agree with Yonghong that -g should imply -gbtf for bpf target
>> > and -mco-re doesn't need to be a flag at all.
>>
>> We like the idea of -g implying -gbtf rather than -gdwarf for the BPF
>> target.  It makes sense.  Faust is already working on it.
>>
>> As for -mco-re, it is already the default with -gbtf, and now it will be
>> the default for -g.
>>
>> > Compiler should do it when it sees those special attributes in C code.
>> > -masm=pseudoc is a good default as well, since that's what
>> > everyone in bpf community is used to.
>>
>> We will try to get all the changes above upstream before GCC 14 gets
>> branched, which shall happen any day now.  Once they are in GCC the only
>> extra option to be added to GCC_BPF_BUILD_RULE will be -g.  Will send an
>> updated patch then.
>
> Awesome. This is all great to hear.

The GCC 14 release branch was created today, but we managed to get the
changes for -g and default to pseudo-C just in time.

Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile

[Alexei Starovoitov]

On Fri, Apr 26, 2024 at 7:41 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> > On Thu, Apr 25, 2024 at 5:32 AM Jose E. Marchesi
> > <jose.marchesi@oracle.com> wrote:
> >>
> >>
> >> > On Wed, Apr 24, 2024 at 2:30 PM Jose E. Marchesi
> >> > <jose.marchesi@oracle.com> wrote:
> >> >>
> >> >>
> >> >> Hi Yonghong.
> >> >>
> >> >> > On 4/24/24 1:41 AM, Jose E. Marchesi wrote:
> >> >> >> This little patch modifies selftests/bpf/Makefile so it passes the
> >> >> >> following extra options when invoking gcc-bpf:
> >> >> >>
> >> >> >>   -gbtf
> >> >> >>     This makes GCC to emit BTF debug info in .BTF and .BTF.ext.
> >> >> >
> >> >> > Could we do if '-g' is specified, for bpf program,
> >> >> > btf will be automatically generated?
> >> >>
> >> >> Hmm, in principle I wouldn't oppose for -g to mean -gbtf instead of
> >> >> -gdwarf.  DWARF can always be generated by using -gdwarf.
> >> >>
> >> >> Faust, Indu, WDYT?
> >> >>
> >> >> >>
> >> >> >>   -mco-re
> >> >> >>     This tells GCC to generate CO-RE relocations in .BTF.ext.
> >> >> >
> >> >> > Can we make this default? That is, remove -mco-re option. I
> >> >> > can imagine for any serious bpf program, co-re is a must.
> >> >>
> >> >> CO-RE depends on BTF.  So I understand the above as making -mco-re the
> >> >> default if BTF is generated, i.e. if -gbtf (or -g with the modification
> >> >> above) are specified.  Isn't that what clang does?  Am I interpreting
> >> >> correctly?
> >> >>
> >> >> >>
> >> >> >>   -masm=pseudoc
> >> >> >>     This tells GCC to emit BPF assembler using the pseudo-c syntax.
> >> >> >
> >> >> > Can we make it the other way round such that -masm=pseudoc is
> >> >> > the default? You can have an option e.g., -masm=non-pseudoc,
> >> >> > for the other format?
> >> >>
> >> >> We could add a configure-time build option:
> >> >>
> >> >>   --with-bpf-default-asm-syntax={pseudoc,normal}
> >> >>
> >> >> so that GCC can be built to use whatever selected syntax as default.
> >> >> Distros and people can then decide what to do.
> >> >
> >> > distros just ship stuff.
> >> > It's our job to pick good defaults.
> >>
> >> Yeah it was a rather dumb idea that would only complicate things for no
> >> good reason.
> >>
> >> The unfortunate fact is that at this point the kernel headers that
> >> almost all BPF programs use contain pseudo-C inline assembly and having
> >> the toolchain using the conventional assembly syntax by default would
> >> force users to specify the command-line option explicitly, which is a
> >> great PITA.  So I guess this is one of these situations where the worse
> >> option is indeed the best default, in practical terms.
> >>
> >> So ok, as much as it sucks we will make -masm=pseudoc the default in GCC
> >> for the sake of practicality.
> >>
> >> > I agree with Yonghong that -g should imply -gbtf for bpf target
> >> > and -mco-re doesn't need to be a flag at all.
> >>
> >> We like the idea of -g implying -gbtf rather than -gdwarf for the BPF
> >> target.  It makes sense.  Faust is already working on it.
> >>
> >> As for -mco-re, it is already the default with -gbtf, and now it will be
> >> the default for -g.
> >>
> >> > Compiler should do it when it sees those special attributes in C code.
> >> > -masm=pseudoc is a good default as well, since that's what
> >> > everyone in bpf community is used to.
> >>
> >> We will try to get all the changes above upstream before GCC 14 gets
> >> branched, which shall happen any day now.  Once they are in GCC the only
> >> extra option to be added to GCC_BPF_BUILD_RULE will be -g.  Will send an
> >> updated patch then.
> >
> > Awesome. This is all great to hear.
>
> The GCC 14 release branch was created today, but we managed to get the
> changes for -g and default to pseudo-C just in time.

Nice. Pls let us know when mirrors/packages are ready, so we teach CI
to start using gcc-bpf.