* [PATCH bpf-next 0/2] selftests/bpf: Fix number of arguments in test
@ 2024-05-06 15:18 Cupertino Miranda
2024-05-06 15:18 ` [PATCH bpf-next 1/2] selftests/bpf: Add CFLAGS per source file and runner Cupertino Miranda
2024-05-06 15:18 ` [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC Cupertino Miranda
0 siblings, 2 replies; 6+ messages in thread
From: Cupertino Miranda @ 2024-05-06 15:18 UTC (permalink / raw
To: bpf
Cc: Cupertino Miranda, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Yonghong Song, David Faust, Jose Marchesi,
Elena Zannoni
Hi everyone,
This patch series is in the context of GCC support.
GCC errors when number of arguments does not fit within the
requirements of BPF.
These patches fixes the functions that contain 6 arguments by
combining those in an array.
Best regards,
Cupertino
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
Cupertino Miranda (2):
selftests/bpf: Add CFLAGS per source file and runner
selftests/bpf: Change functions definitions to support GCC
tools/testing/selftests/bpf/Makefile | 16 +++++++++-------
.../selftests/bpf/progs/test_xdp_noinline.c | 15 +++++++++------
2 files changed, 18 insertions(+), 13 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 1/2] selftests/bpf: Add CFLAGS per source file and runner
2024-05-06 15:18 [PATCH bpf-next 0/2] selftests/bpf: Fix number of arguments in test Cupertino Miranda
@ 2024-05-06 15:18 ` Cupertino Miranda
2024-05-06 19:56 ` Yonghong Song
2024-05-06 15:18 ` [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC Cupertino Miranda
1 sibling, 1 reply; 6+ messages in thread
From: Cupertino Miranda @ 2024-05-06 15:18 UTC (permalink / raw
To: bpf
Cc: Cupertino Miranda, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Yonghong Song, David Faust, Jose Marchesi,
Elena Zannoni
This patch adds support to specify CFLAGS per source file and per test
runner.
Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
---
tools/testing/selftests/bpf/Makefile | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index ba28d42b74db..e506a5948cc2 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -81,11 +81,11 @@ TEST_INST_SUBDIRS += bpf_gcc
# The following tests contain C code that, although technically legal,
# triggers GCC warnings that cannot be disabled: declaration of
# anonymous struct types in function parameter lists.
-progs/btf_dump_test_case_bitfields.c-CFLAGS := -Wno-error
-progs/btf_dump_test_case_namespacing.c-CFLAGS := -Wno-error
-progs/btf_dump_test_case_packing.c-CFLAGS := -Wno-error
-progs/btf_dump_test_case_padding.c-CFLAGS := -Wno-error
-progs/btf_dump_test_case_syntax.c-CFLAGS := -Wno-error
+progs/btf_dump_test_case_bitfields.c-bpf_gcc-CFLAGS := -Wno-error
+progs/btf_dump_test_case_namespacing.c-bpf_gcc-CFLAGS := -Wno-error
+progs/btf_dump_test_case_packing.c-bpf_gcc-CFLAGS := -Wno-error
+progs/btf_dump_test_case_padding.c-bpf_gcc-CFLAGS := -Wno-error
+progs/btf_dump_test_case_syntax.c-bpf_gcc-CFLAGS := -Wno-error
endif
ifneq ($(CLANG_CPUV4),)
@@ -498,7 +498,7 @@ endef
# Using TRUNNER_XXX variables, provided by callers of DEFINE_TEST_RUNNER and
# set up by DEFINE_TEST_RUNNER itself, create test runner build rules with:
# $1 - test runner base binary name (e.g., test_progs)
-# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, gcc-bpf, etc)
+# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, bpf_gcc, etc)
define DEFINE_TEST_RUNNER_RULES
ifeq ($($(TRUNNER_OUTPUT)-dir),)
@@ -521,7 +521,8 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o: \
| $(TRUNNER_OUTPUT) $$(BPFOBJ)
$$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@, \
$(TRUNNER_BPF_CFLAGS) \
- $$($$<-CFLAGS))
+ $$($$<-CFLAGS) \
+ $$($$<-$2-CFLAGS))
$(TRUNNER_BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC
2024-05-06 15:18 [PATCH bpf-next 0/2] selftests/bpf: Fix number of arguments in test Cupertino Miranda
2024-05-06 15:18 ` [PATCH bpf-next 1/2] selftests/bpf: Add CFLAGS per source file and runner Cupertino Miranda
@ 2024-05-06 15:18 ` Cupertino Miranda
2024-05-06 19:57 ` Yonghong Song
2024-05-06 21:44 ` Andrii Nakryiko
1 sibling, 2 replies; 6+ messages in thread
From: Cupertino Miranda @ 2024-05-06 15:18 UTC (permalink / raw
To: bpf
Cc: Cupertino Miranda, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Yonghong Song, David Faust, Jose Marchesi,
Elena Zannoni
The test_xdp_noinline.c contains 2 functions that use more then 5
arguments. This patch collapses the 2 last arguments in an array.
Also in GCC and ipa_sra optimization increases the number of arguments
used in function encap_v4. This pass disables the optimization for that
particular file.
Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: David Faust <david.faust@oracle.com>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: Elena Zannoni <elena.zannoni@oracle.com>
---
tools/testing/selftests/bpf/Makefile | 1 +
.../selftests/bpf/progs/test_xdp_noinline.c | 15 +++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index e506a5948cc2..6fe9b0dd2ea0 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -86,6 +86,7 @@ progs/btf_dump_test_case_namespacing.c-bpf_gcc-CFLAGS := -Wno-error
progs/btf_dump_test_case_packing.c-bpf_gcc-CFLAGS := -Wno-error
progs/btf_dump_test_case_padding.c-bpf_gcc-CFLAGS := -Wno-error
progs/btf_dump_test_case_syntax.c-bpf_gcc-CFLAGS := -Wno-error
+progs/test_xdp_noinline.c-bpf_gcc-CFLAGS := -fno-ipa-sra
endif
ifneq ($(CLANG_CPUV4),)
diff --git a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
index 5c7e4758a0ca..a38199f900ec 100644
--- a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
+++ b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
@@ -588,12 +588,13 @@ static void connection_table_lookup(struct real_definition **real,
__attribute__ ((noinline))
static int process_l3_headers_v6(struct packet_description *pckt,
__u8 *protocol, __u64 off,
- __u16 *pkt_bytes, void *data,
- void *data_end)
+ __u16 *pkt_bytes, void *extra_args[2])
{
struct ipv6hdr *ip6h;
__u64 iph_len;
int action;
+ void *data = extra_args[0];
+ void *data_end = extra_args[1];
ip6h = data + off;
if (ip6h + 1 > data_end)
@@ -619,11 +620,12 @@ static int process_l3_headers_v6(struct packet_description *pckt,
__attribute__ ((noinline))
static int process_l3_headers_v4(struct packet_description *pckt,
__u8 *protocol, __u64 off,
- __u16 *pkt_bytes, void *data,
- void *data_end)
+ __u16 *pkt_bytes, void *extra_args[2])
{
struct iphdr *iph;
int action;
+ void *data = extra_args[0];
+ void *data_end = extra_args[1];
iph = data + off;
if (iph + 1 > data_end)
@@ -666,13 +668,14 @@ static int process_packet(void *data, __u64 off, void *data_end,
__u8 protocol;
__u32 vip_num;
int action;
+ void *extra_args[2] = { data, data_end };
if (is_ipv6)
action = process_l3_headers_v6(&pckt, &protocol, off,
- &pkt_bytes, data, data_end);
+ &pkt_bytes, extra_args);
else
action = process_l3_headers_v4(&pckt, &protocol, off,
- &pkt_bytes, data, data_end);
+ &pkt_bytes, extra_args);
if (action >= 0)
return action;
protocol = pckt.flow.proto;
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] selftests/bpf: Add CFLAGS per source file and runner
2024-05-06 15:18 ` [PATCH bpf-next 1/2] selftests/bpf: Add CFLAGS per source file and runner Cupertino Miranda
@ 2024-05-06 19:56 ` Yonghong Song
0 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2024-05-06 19:56 UTC (permalink / raw
To: Cupertino Miranda, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman,
David Faust, Jose Marchesi, Elena Zannoni
On 5/6/24 8:18 AM, Cupertino Miranda wrote:
> This patch adds support to specify CFLAGS per source file and per test
> runner.
>
> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
> Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: David Faust <david.faust@oracle.com>
> Cc: Jose Marchesi <jose.marchesi@oracle.com>
> Cc: Elena Zannoni <elena.zannoni@oracle.com>
Ack with a nit below.
Acked-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> tools/testing/selftests/bpf/Makefile | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index ba28d42b74db..e506a5948cc2 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -81,11 +81,11 @@ TEST_INST_SUBDIRS += bpf_gcc
> # The following tests contain C code that, although technically legal,
> # triggers GCC warnings that cannot be disabled: declaration of
> # anonymous struct types in function parameter lists.
> -progs/btf_dump_test_case_bitfields.c-CFLAGS := -Wno-error
> -progs/btf_dump_test_case_namespacing.c-CFLAGS := -Wno-error
> -progs/btf_dump_test_case_packing.c-CFLAGS := -Wno-error
> -progs/btf_dump_test_case_padding.c-CFLAGS := -Wno-error
> -progs/btf_dump_test_case_syntax.c-CFLAGS := -Wno-error
> +progs/btf_dump_test_case_bitfields.c-bpf_gcc-CFLAGS := -Wno-error
> +progs/btf_dump_test_case_namespacing.c-bpf_gcc-CFLAGS := -Wno-error
> +progs/btf_dump_test_case_packing.c-bpf_gcc-CFLAGS := -Wno-error
> +progs/btf_dump_test_case_padding.c-bpf_gcc-CFLAGS := -Wno-error
> +progs/btf_dump_test_case_syntax.c-bpf_gcc-CFLAGS := -Wno-error
> endif
>
> ifneq ($(CLANG_CPUV4),)
> @@ -498,7 +498,7 @@ endef
> # Using TRUNNER_XXX variables, provided by callers of DEFINE_TEST_RUNNER and
> # set up by DEFINE_TEST_RUNNER itself, create test runner build rules with:
> # $1 - test runner base binary name (e.g., test_progs)
> -# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, gcc-bpf, etc)
> +# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, bpf_gcc, etc)
> define DEFINE_TEST_RUNNER_RULES
The gcc-bpf below also needs an update.
# $2 - test runner extra "flavor" (e.g., no_alu32, cpuv4, gcc-bpf, etc)
define DEFINE_TEST_RUNNER
>
> ifeq ($($(TRUNNER_OUTPUT)-dir),)
> @@ -521,7 +521,8 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o: \
> | $(TRUNNER_OUTPUT) $$(BPFOBJ)
> $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@, \
> $(TRUNNER_BPF_CFLAGS) \
> - $$($$<-CFLAGS))
> + $$($$<-CFLAGS) \
> + $$($$<-$2-CFLAGS))
>
> $(TRUNNER_BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
> $$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC
2024-05-06 15:18 ` [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC Cupertino Miranda
@ 2024-05-06 19:57 ` Yonghong Song
2024-05-06 21:44 ` Andrii Nakryiko
1 sibling, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2024-05-06 19:57 UTC (permalink / raw
To: Cupertino Miranda, bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman,
David Faust, Jose Marchesi, Elena Zannoni
On 5/6/24 8:18 AM, Cupertino Miranda wrote:
> The test_xdp_noinline.c contains 2 functions that use more then 5
> arguments. This patch collapses the 2 last arguments in an array.
> Also in GCC and ipa_sra optimization increases the number of arguments
> used in function encap_v4. This pass disables the optimization for that
> particular file.
>
> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
> Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: David Faust <david.faust@oracle.com>
> Cc: Jose Marchesi <jose.marchesi@oracle.com>
> Cc: Elena Zannoni <elena.zannoni@oracle.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC
2024-05-06 15:18 ` [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC Cupertino Miranda
2024-05-06 19:57 ` Yonghong Song
@ 2024-05-06 21:44 ` Andrii Nakryiko
1 sibling, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2024-05-06 21:44 UTC (permalink / raw
To: Cupertino Miranda
Cc: bpf, Alexei Starovoitov, Eduard Zingerman, Yonghong Song,
David Faust, Jose Marchesi, Elena Zannoni
On Mon, May 6, 2024 at 8:19 AM Cupertino Miranda
<cupertino.miranda@oracle.com> wrote:
>
> The test_xdp_noinline.c contains 2 functions that use more then 5
> arguments. This patch collapses the 2 last arguments in an array.
> Also in GCC and ipa_sra optimization increases the number of arguments
> used in function encap_v4. This pass disables the optimization for that
> particular file.
>
> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
> Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: David Faust <david.faust@oracle.com>
> Cc: Jose Marchesi <jose.marchesi@oracle.com>
> Cc: Elena Zannoni <elena.zannoni@oracle.com>
> ---
> tools/testing/selftests/bpf/Makefile | 1 +
> .../selftests/bpf/progs/test_xdp_noinline.c | 15 +++++++++------
> 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index e506a5948cc2..6fe9b0dd2ea0 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -86,6 +86,7 @@ progs/btf_dump_test_case_namespacing.c-bpf_gcc-CFLAGS := -Wno-error
> progs/btf_dump_test_case_packing.c-bpf_gcc-CFLAGS := -Wno-error
> progs/btf_dump_test_case_padding.c-bpf_gcc-CFLAGS := -Wno-error
> progs/btf_dump_test_case_syntax.c-bpf_gcc-CFLAGS := -Wno-error
> +progs/test_xdp_noinline.c-bpf_gcc-CFLAGS := -fno-ipa-sra
Can this be done through `#pragma GCC optimize`? If yes, let's do that instead?
> endif
>
> ifneq ($(CLANG_CPUV4),)
> diff --git a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> index 5c7e4758a0ca..a38199f900ec 100644
> --- a/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> +++ b/tools/testing/selftests/bpf/progs/test_xdp_noinline.c
> @@ -588,12 +588,13 @@ static void connection_table_lookup(struct real_definition **real,
> __attribute__ ((noinline))
> static int process_l3_headers_v6(struct packet_description *pckt,
> __u8 *protocol, __u64 off,
> - __u16 *pkt_bytes, void *data,
> - void *data_end)
> + __u16 *pkt_bytes, void *extra_args[2])
> {
> struct ipv6hdr *ip6h;
> __u64 iph_len;
> int action;
> + void *data = extra_args[0];
> + void *data_end = extra_args[1];
>
> ip6h = data + off;
> if (ip6h + 1 > data_end)
> @@ -619,11 +620,12 @@ static int process_l3_headers_v6(struct packet_description *pckt,
> __attribute__ ((noinline))
> static int process_l3_headers_v4(struct packet_description *pckt,
> __u8 *protocol, __u64 off,
> - __u16 *pkt_bytes, void *data,
> - void *data_end)
> + __u16 *pkt_bytes, void *extra_args[2])
> {
> struct iphdr *iph;
> int action;
> + void *data = extra_args[0];
> + void *data_end = extra_args[1];
>
> iph = data + off;
> if (iph + 1 > data_end)
> @@ -666,13 +668,14 @@ static int process_packet(void *data, __u64 off, void *data_end,
> __u8 protocol;
> __u32 vip_num;
> int action;
> + void *extra_args[2] = { data, data_end };
>
> if (is_ipv6)
> action = process_l3_headers_v6(&pckt, &protocol, off,
> - &pkt_bytes, data, data_end);
> + &pkt_bytes, extra_args);
> else
> action = process_l3_headers_v4(&pckt, &protocol, off,
> - &pkt_bytes, data, data_end);
> + &pkt_bytes, extra_args);
> if (action >= 0)
> return action;
> protocol = pckt.flow.proto;
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-06 21:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-06 15:18 [PATCH bpf-next 0/2] selftests/bpf: Fix number of arguments in test Cupertino Miranda
2024-05-06 15:18 ` [PATCH bpf-next 1/2] selftests/bpf: Add CFLAGS per source file and runner Cupertino Miranda
2024-05-06 19:56 ` Yonghong Song
2024-05-06 15:18 ` [PATCH bpf-next 2/2] selftests/bpf: Change functions definitions to support GCC Cupertino Miranda
2024-05-06 19:57 ` Yonghong Song
2024-05-06 21:44 ` Andrii Nakryiko
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 read-only IMAP folder(s) and NNTP newsgroup(s).