bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static
@ 2020-03-02 14:53 Toke Høiland-Jørgensen
  2020-03-02 16:58 ` Andrey Ignatov
  2020-03-03  1:03 ` Alexei Starovoitov
  0 siblings, 2 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-03-02 14:53 UTC (permalink / raw)
  To: daniel, ast; +Cc: Toke Høiland-Jørgensen, bpf, netdev, Andrey Ignatov

The cgroup selftests did not declare the bpf_log_buf variable as static, leading
to a linker error with GCC 10 (which defaults to -fno-common). Fix this by
adding the missing static declarations.

Fixes: 257c88559f36 ("selftests/bpf: Convert test_cgroup_attach to prog_tests")
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 .../testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c | 2 +-
 tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c    | 2 +-
 tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
index 5b13f2c6c402..70e94e783070 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
@@ -6,7 +6,7 @@
 
 #define PING_CMD	"ping -q -c1 -w1 127.0.0.1 > /dev/null"
 
-char bpf_log_buf[BPF_LOG_BUF_SIZE];
+static char bpf_log_buf[BPF_LOG_BUF_SIZE];
 
 static int prog_load(void)
 {
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
index 2ff21dbce179..139f8e82c7c6 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
@@ -6,7 +6,7 @@
 
 #define PING_CMD	"ping -q -c1 -w1 127.0.0.1 > /dev/null"
 
-char bpf_log_buf[BPF_LOG_BUF_SIZE];
+static char bpf_log_buf[BPF_LOG_BUF_SIZE];
 
 static int map_fd = -1;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
index 9d8cb48b99de..9e96f8d87fea 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
@@ -8,7 +8,7 @@
 #define BAR		"/foo/bar/"
 #define PING_CMD	"ping -q -c1 -w1 127.0.0.1 > /dev/null"
 
-char bpf_log_buf[BPF_LOG_BUF_SIZE];
+static char bpf_log_buf[BPF_LOG_BUF_SIZE];
 
 static int prog_load(int verdict)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static
  2020-03-02 14:53 [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static Toke Høiland-Jørgensen
@ 2020-03-02 16:58 ` Andrey Ignatov
  2020-03-02 17:48   ` Toke Høiland-Jørgensen
  2020-03-03  1:03 ` Alexei Starovoitov
  1 sibling, 1 reply; 6+ messages in thread
From: Andrey Ignatov @ 2020-03-02 16:58 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: daniel, ast, bpf, netdev

Toke Høiland-Jørgensen <toke@redhat.com> [Mon, 2020-03-02 06:54 -0800]:
> The cgroup selftests did not declare the bpf_log_buf variable as static, leading
> to a linker error with GCC 10 (which defaults to -fno-common). Fix this by
> adding the missing static declarations.
> 
> Fixes: 257c88559f36 ("selftests/bpf: Convert test_cgroup_attach to prog_tests")

Hi Toke,

Thanks for the fix.

My 257c88559f36 commit was just a split that simply moved this
bpf_log_buf from tools/testing/selftests/bpf/test_cgroup_attach.c to all
new three files as is among many other things. Before that it was moved
as is from samples/ in
ba0c0cc05dda ("selftests/bpf: convert test_cgrp2_attach2 example into kselftest")
and before that it was introduced in
d40fc181ebec ("samples/bpf: Make samples more libbpf-centric")

Though since these are new files I guess having just 257c88559f36 in the
tag should be fine(?) so:

Acked-by: Andrey Ignatov <rdna@fb.com>


> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
>  .../testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c | 2 +-
>  tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c    | 2 +-
>  tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
> index 5b13f2c6c402..70e94e783070 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_autodetach.c
> @@ -6,7 +6,7 @@
>  
>  #define PING_CMD	"ping -q -c1 -w1 127.0.0.1 > /dev/null"
>  
> -char bpf_log_buf[BPF_LOG_BUF_SIZE];
> +static char bpf_log_buf[BPF_LOG_BUF_SIZE];
>  
>  static int prog_load(void)
>  {
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
> index 2ff21dbce179..139f8e82c7c6 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_multi.c
> @@ -6,7 +6,7 @@
>  
>  #define PING_CMD	"ping -q -c1 -w1 127.0.0.1 > /dev/null"
>  
> -char bpf_log_buf[BPF_LOG_BUF_SIZE];
> +static char bpf_log_buf[BPF_LOG_BUF_SIZE];
>  
>  static int map_fd = -1;
>  
> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
> index 9d8cb48b99de..9e96f8d87fea 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_attach_override.c
> @@ -8,7 +8,7 @@
>  #define BAR		"/foo/bar/"
>  #define PING_CMD	"ping -q -c1 -w1 127.0.0.1 > /dev/null"
>  
> -char bpf_log_buf[BPF_LOG_BUF_SIZE];
> +static char bpf_log_buf[BPF_LOG_BUF_SIZE];
>  
>  static int prog_load(int verdict)
>  {
> -- 
> 2.25.1
> 

-- 
Andrey Ignatov

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static
  2020-03-02 16:58 ` Andrey Ignatov
@ 2020-03-02 17:48   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-03-02 17:48 UTC (permalink / raw)
  To: Andrey Ignatov; +Cc: daniel, ast, bpf, netdev

Andrey Ignatov <rdna@fb.com> writes:

> Toke Høiland-Jørgensen <toke@redhat.com> [Mon, 2020-03-02 06:54 -0800]:
>> The cgroup selftests did not declare the bpf_log_buf variable as static, leading
>> to a linker error with GCC 10 (which defaults to -fno-common). Fix this by
>> adding the missing static declarations.
>> 
>> Fixes: 257c88559f36 ("selftests/bpf: Convert test_cgroup_attach to prog_tests")
>
> Hi Toke,
>
> Thanks for the fix.
>
> My 257c88559f36 commit was just a split that simply moved this
> bpf_log_buf from tools/testing/selftests/bpf/test_cgroup_attach.c to all
> new three files as is among many other things. Before that it was moved
> as is from samples/ in
> ba0c0cc05dda ("selftests/bpf: convert test_cgrp2_attach2 example into kselftest")
> and before that it was introduced in
> d40fc181ebec ("samples/bpf: Make samples more libbpf-centric")
>
> Though since these are new files I guess having just 257c88559f36 in the
> tag should be fine(?) so:

Yeah, I did realise you didn't write the original code, but this Fixes
tag should at least make the patch be picked up by any stable trees
after you moved things around, so I guess that's good enough :)

-Toke


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static
  2020-03-02 14:53 [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static Toke Høiland-Jørgensen
  2020-03-02 16:58 ` Andrey Ignatov
@ 2020-03-03  1:03 ` Alexei Starovoitov
  2020-03-03  8:09   ` Toke Høiland-Jørgensen
  1 sibling, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2020-03-03  1:03 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: daniel, ast, bpf, netdev, Andrey Ignatov

On Mon, Mar 02, 2020 at 03:53:48PM +0100, Toke Høiland-Jørgensen wrote:
> The cgroup selftests did not declare the bpf_log_buf variable as static, leading
> to a linker error with GCC 10 (which defaults to -fno-common). Fix this by
> adding the missing static declarations.
> 
> Fixes: 257c88559f36 ("selftests/bpf: Convert test_cgroup_attach to prog_tests")
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

Applied to bpf-next.
It's hardly a fix. Fixes tag doesn't make it a fix in my mind.
I really see no point rushing it into bpf->net->Linus's tree at this point.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static
  2020-03-03  1:03 ` Alexei Starovoitov
@ 2020-03-03  8:09   ` Toke Høiland-Jørgensen
  2020-03-03 16:27     ` Alexei Starovoitov
  0 siblings, 1 reply; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-03-03  8:09 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: daniel, ast, bpf, netdev, Andrey Ignatov

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Mon, Mar 02, 2020 at 03:53:48PM +0100, Toke Høiland-Jørgensen wrote:
>> The cgroup selftests did not declare the bpf_log_buf variable as static, leading
>> to a linker error with GCC 10 (which defaults to -fno-common). Fix this by
>> adding the missing static declarations.
>> 
>> Fixes: 257c88559f36 ("selftests/bpf: Convert test_cgroup_attach to prog_tests")
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>
> Applied to bpf-next.
> It's hardly a fix. Fixes tag doesn't make it a fix in my mind.

It fixes a compile error of selftests with GCC 10; how is that not a
fix? We found it while setting up a CI test compiling Linus' tree on
Fedora rawhide, so it does happen in the wild.

> I really see no point rushing it into bpf->net->Linus's tree at this point.

Well if you're not pushing any other fixes then OK, sure, no reason to
go through the whole process just for this. But if you end up pushing
another round of fixes anyway, please include this as well. If not, I
guess we can wait :)

-Toke


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static
  2020-03-03  8:09   ` Toke Høiland-Jørgensen
@ 2020-03-03 16:27     ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2020-03-03 16:27 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Daniel Borkmann, Alexei Starovoitov, bpf, Network Development,
	Andrey Ignatov

On Tue, Mar 3, 2020 at 12:10 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>
> > On Mon, Mar 02, 2020 at 03:53:48PM +0100, Toke Høiland-Jørgensen wrote:
> >> The cgroup selftests did not declare the bpf_log_buf variable as static, leading
> >> to a linker error with GCC 10 (which defaults to -fno-common). Fix this by
> >> adding the missing static declarations.
> >>
> >> Fixes: 257c88559f36 ("selftests/bpf: Convert test_cgroup_attach to prog_tests")
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> >
> > Applied to bpf-next.
> > It's hardly a fix. Fixes tag doesn't make it a fix in my mind.
>
> It fixes a compile error of selftests with GCC 10; how is that not a
> fix? We found it while setting up a CI test compiling Linus' tree on
> Fedora rawhide, so it does happen in the wild.
>
> > I really see no point rushing it into bpf->net->Linus's tree at this point.
>
> Well if you're not pushing any other fixes then OK, sure, no reason to
> go through the whole process just for this. But if you end up pushing
> another round of fixes anyway, please include this as well. If not, I
> guess we can wait :)

CI stands for Continuous Integration == development.
stable tree is not for development.
If you want to develop anything or accommodate the tree
for external development you need to use development tree.
Which is bpf-next.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-03 16:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-02 14:53 [PATCH bpf] selftests/bpf: Declare bpf_log_buf variables as static Toke Høiland-Jørgensen
2020-03-02 16:58 ` Andrey Ignatov
2020-03-02 17:48   ` Toke Høiland-Jørgensen
2020-03-03  1:03 ` Alexei Starovoitov
2020-03-03  8:09   ` Toke Høiland-Jørgensen
2020-03-03 16:27     ` Alexei Starovoitov

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).