* [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile @ 2024-04-24 8:41 Jose E. Marchesi 2024-04-24 16:47 ` Eduard Zingerman 2024-04-24 21:05 ` Yonghong Song 0 siblings, 2 replies; 12+ messages in thread From: Jose E. Marchesi @ 2024-04-24 8:41 UTC (permalink / raw) To: bpf Cc: Jose E . Marchesi, Yonghong Song, Eduard Zingerman, david.faust, cupertino.miranda 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-24 8:41 [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile Jose E. Marchesi @ 2024-04-24 16:47 ` Eduard Zingerman 2024-04-24 21:05 ` Yonghong Song 1 sibling, 0 replies; 12+ messages in thread From: Eduard Zingerman @ 2024-04-24 16:47 UTC (permalink / raw) To: Jose E. Marchesi, bpf; +Cc: Yonghong Song, david.faust, cupertino.miranda 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> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-24 8:41 [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile Jose E. Marchesi 2024-04-24 16:47 ` Eduard Zingerman @ 2024-04-24 21:05 ` Yonghong Song 2024-04-24 21:24 ` Jose E. Marchesi 1 sibling, 1 reply; 12+ messages in thread From: Yonghong Song @ 2024-04-24 21:05 UTC (permalink / raw) To: Jose E. Marchesi, bpf Cc: Yonghong Song, Eduard Zingerman, david.faust, cupertino.miranda 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-24 21:05 ` Yonghong Song @ 2024-04-24 21:24 ` Jose E. Marchesi 2024-04-24 21:47 ` David Faust 2024-04-24 21:48 ` Alexei Starovoitov 0 siblings, 2 replies; 12+ messages in thread From: Jose E. Marchesi @ 2024-04-24 21:24 UTC (permalink / raw) To: Yonghong Song Cc: bpf, Yonghong Song, Eduard Zingerman, david.faust, cupertino.miranda, indu.bhagat 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-24 21:24 ` Jose E. Marchesi @ 2024-04-24 21:47 ` David Faust 2024-04-24 21:48 ` Alexei Starovoitov 1 sibling, 0 replies; 12+ messages in thread From: David Faust @ 2024-04-24 21:47 UTC (permalink / raw) To: Jose E. Marchesi, Yonghong Song Cc: bpf, Yonghong Song, Eduard Zingerman, cupertino.miranda, indu.bhagat 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-24 21:24 ` Jose E. Marchesi 2024-04-24 21:47 ` David Faust @ 2024-04-24 21:48 ` Alexei Starovoitov 2024-04-25 12:32 ` Jose E. Marchesi 1 sibling, 1 reply; 12+ messages in thread From: Alexei Starovoitov @ 2024-04-24 21:48 UTC (permalink / raw) To: Jose E. Marchesi Cc: Yonghong Song, bpf, Yonghong Song, Eduard Zingerman, David Faust, Cupertino Miranda, indu.bhagat 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-24 21:48 ` Alexei Starovoitov @ 2024-04-25 12:32 ` Jose E. Marchesi 2024-04-25 15:40 ` Alexei Starovoitov 2024-04-25 18:20 ` Andrii Nakryiko 0 siblings, 2 replies; 12+ messages in thread From: Jose E. Marchesi @ 2024-04-25 12:32 UTC (permalink / raw) To: Alexei Starovoitov Cc: Yonghong Song, bpf, Yonghong Song, Eduard Zingerman, David Faust, Cupertino Miranda, indu.bhagat > 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! ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-25 12:32 ` Jose E. Marchesi @ 2024-04-25 15:40 ` Alexei Starovoitov 2024-04-26 14:41 ` Jose E. Marchesi 2024-04-25 18:20 ` Andrii Nakryiko 1 sibling, 1 reply; 12+ messages in thread From: Alexei Starovoitov @ 2024-04-25 15:40 UTC (permalink / raw) To: Jose E. Marchesi Cc: Yonghong Song, bpf, Yonghong Song, Eduard Zingerman, David Faust, Cupertino Miranda, indu.bhagat 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! ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-25 15:40 ` Alexei Starovoitov @ 2024-04-26 14:41 ` Jose E. Marchesi 2024-04-26 14:47 ` Alexei Starovoitov 0 siblings, 1 reply; 12+ messages in thread From: Jose E. Marchesi @ 2024-04-26 14:41 UTC (permalink / raw) To: Alexei Starovoitov Cc: Yonghong Song, bpf, Yonghong Song, Eduard Zingerman, David Faust, Cupertino Miranda, indu.bhagat > 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-26 14:41 ` Jose E. Marchesi @ 2024-04-26 14:47 ` Alexei Starovoitov 0 siblings, 0 replies; 12+ messages in thread From: Alexei Starovoitov @ 2024-04-26 14:47 UTC (permalink / raw) To: Jose E. Marchesi, Manu Bretelle, Mykola Lysenko Cc: Yonghong Song, bpf, Yonghong Song, Eduard Zingerman, David Faust, Cupertino Miranda, indu.bhagat 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-25 12:32 ` Jose E. Marchesi 2024-04-25 15:40 ` Alexei Starovoitov @ 2024-04-25 18:20 ` Andrii Nakryiko 2024-04-25 18:48 ` Jose E. Marchesi 1 sibling, 1 reply; 12+ messages in thread From: Andrii Nakryiko @ 2024-04-25 18:20 UTC (permalink / raw) To: Jose E. Marchesi Cc: Alexei Starovoitov, Yonghong Song, bpf, Yonghong Song, Eduard Zingerman, David Faust, Cupertino Miranda, indu.bhagat 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! > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile 2024-04-25 18:20 ` Andrii Nakryiko @ 2024-04-25 18:48 ` Jose E. Marchesi 0 siblings, 0 replies; 12+ messages in thread From: Jose E. Marchesi @ 2024-04-25 18:48 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Yonghong Song, bpf, Yonghong Song, Eduard Zingerman, David Faust, Cupertino Miranda, indu.bhagat > 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! >> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-04-26 14:47 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-24 8:41 [PATCH bpf-next] bpf: add a few more options for GCC_BPF in selftests/bpf/Makefile Jose E. Marchesi 2024-04-24 16:47 ` Eduard Zingerman 2024-04-24 21:05 ` Yonghong Song 2024-04-24 21:24 ` Jose E. Marchesi 2024-04-24 21:47 ` David Faust 2024-04-24 21:48 ` Alexei Starovoitov 2024-04-25 12:32 ` Jose E. Marchesi 2024-04-25 15:40 ` Alexei Starovoitov 2024-04-26 14:41 ` Jose E. Marchesi 2024-04-26 14:47 ` Alexei Starovoitov 2024-04-25 18:20 ` Andrii Nakryiko 2024-04-25 18:48 ` Jose E. Marchesi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).