linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next V2 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes
@ 2020-07-06 17:00 Jesper Dangaard Brouer
  2020-07-06 17:00 ` [PATCH bpf-next V2 1/2] selftests/bpf: test_progs use another shell exit on non-actions Jesper Dangaard Brouer
  2020-07-06 17:00 ` [PATCH bpf-next V2 2/2] selftests/bpf: test_progs avoid minus shell exit codes Jesper Dangaard Brouer
  0 siblings, 2 replies; 5+ messages in thread
From: Jesper Dangaard Brouer @ 2020-07-06 17:00 UTC (permalink / raw)
  To: bpf, Andrii Nakryiko
  Cc: Jesper Dangaard Brouer, Hangbin Liu, Daniel Borkmann,
	Alexei Starovoitov, vkabatov, jbenc, yhs, kafai, netdev,
	linux-kernel

This patchset makes it easier to use test_progs from shell scripts, by using
proper shell exit codes. The process's exit status should be a number
between 0 and 255 as defined in man exit(3) else it will be masked to comply.

Shell exit codes used by programs should be below 127. As 127 and above are
used for indicating signals. E.g. 139 means 11=SIGSEGV $((139 & 127))=11.
POSIX defines in man wait(3p) signal check if WIFSIGNALED(STATUS) and
WTERMSIG(139)=11. (Hint: cmd 'kill -l' list signals and their numbers).

Using Segmentation fault as an example, as these have happened before with
different tests (that are part of test_progs). CI people writing these
shell-scripts could pickup these hints and report them, if that makes sense.

---

Jesper Dangaard Brouer (2):
      selftests/bpf: test_progs use another shell exit on non-actions
      selftests/bpf: test_progs avoid minus shell exit codes


 tools/testing/selftests/bpf/test_progs.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

--


^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH bpf-next V2 1/2] selftests/bpf: test_progs use another shell exit on non-actions
  2020-07-06 17:00 [PATCH bpf-next V2 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes Jesper Dangaard Brouer
@ 2020-07-06 17:00 ` Jesper Dangaard Brouer
  2020-07-06 17:00 ` [PATCH bpf-next V2 2/2] selftests/bpf: test_progs avoid minus shell exit codes Jesper Dangaard Brouer
  1 sibling, 0 replies; 5+ messages in thread
From: Jesper Dangaard Brouer @ 2020-07-06 17:00 UTC (permalink / raw)
  To: bpf, Andrii Nakryiko
  Cc: Jesper Dangaard Brouer, Hangbin Liu, Daniel Borkmann,
	Alexei Starovoitov, vkabatov, jbenc, yhs, kafai, netdev,
	linux-kernel

This is a follow up adjustment to commit 6c92bd5cd465 ("selftests/bpf:
Test_progs indicate to shell on non-actions"), that returns shell exit
indication EXIT_FAILURE (value 1) when user selects a non-existing test.

The problem with using EXIT_FAILURE is that a shell script cannot tell
the difference between a non-existing test and the test failing.

This patch uses value 2 as shell exit indication.
(Aside note unrecognized option parameters use value 64).

Fixes: 6c92bd5cd465 ("selftests/bpf: Test_progs indicate to shell on non-actions")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 tools/testing/selftests/bpf/test_progs.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 104e833d0087..e8f7cd5dbae4 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -12,6 +12,8 @@
 #include <string.h>
 #include <execinfo.h> /* backtrace */
 
+#define EXIT_NO_TEST 2
+
 /* defined in test_progs.h */
 struct test_env env = {};
 
@@ -740,7 +742,7 @@ int main(int argc, char **argv)
 	close(env.saved_netns_fd);
 
 	if (env.succ_cnt + env.fail_cnt + env.skip_cnt == 0)
-		return EXIT_FAILURE;
+		return EXIT_NO_TEST;
 
 	return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
 }



^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH bpf-next V2 2/2] selftests/bpf: test_progs avoid minus shell exit codes
  2020-07-06 17:00 [PATCH bpf-next V2 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes Jesper Dangaard Brouer
  2020-07-06 17:00 ` [PATCH bpf-next V2 1/2] selftests/bpf: test_progs use another shell exit on non-actions Jesper Dangaard Brouer
@ 2020-07-06 17:00 ` Jesper Dangaard Brouer
  2020-07-06 22:17   ` Andrii Nakryiko
  1 sibling, 1 reply; 5+ messages in thread
From: Jesper Dangaard Brouer @ 2020-07-06 17:00 UTC (permalink / raw)
  To: bpf, Andrii Nakryiko
  Cc: Jesper Dangaard Brouer, Hangbin Liu, Daniel Borkmann,
	Alexei Starovoitov, vkabatov, jbenc, yhs, kafai, netdev,
	linux-kernel

There are a number of places in test_progs that use minus-1 as the argument
to exit(). This improper use as a process exit status is masked to be a
number between 0 and 255 as defined in man exit(3).

This patch use two different positive exit codes instead, to allow a shell
script to tell the two error cases apart.

Fixes: fd27b1835e70 ("selftests/bpf: Reset process and thread affinity after each test/sub-test")
Fixes: 811d7e375d08 ("bpf: selftests: Restore netns after each test")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 tools/testing/selftests/bpf/test_progs.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index e8f7cd5dbae4..50803b080593 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -12,7 +12,9 @@
 #include <string.h>
 #include <execinfo.h> /* backtrace */
 
-#define EXIT_NO_TEST 2
+#define EXIT_NO_TEST		2
+#define EXIT_ERR_NETNS		3
+#define EXIT_ERR_RESET_AFFINITY	4
 
 /* defined in test_progs.h */
 struct test_env env = {};
@@ -113,13 +115,13 @@ static void reset_affinity() {
 	if (err < 0) {
 		stdio_restore();
 		fprintf(stderr, "Failed to reset process affinity: %d!\n", err);
-		exit(-1);
+		exit(EXIT_ERR_RESET_AFFINITY);
 	}
 	err = pthread_setaffinity_np(pthread_self(), sizeof(cpuset), &cpuset);
 	if (err < 0) {
 		stdio_restore();
 		fprintf(stderr, "Failed to reset thread affinity: %d!\n", err);
-		exit(-1);
+		exit(EXIT_ERR_RESET_AFFINITY);
 	}
 }
 
@@ -128,7 +130,7 @@ static void save_netns(void)
 	env.saved_netns_fd = open("/proc/self/ns/net", O_RDONLY);
 	if (env.saved_netns_fd == -1) {
 		perror("open(/proc/self/ns/net)");
-		exit(-1);
+		exit(EXIT_ERR_NETNS);
 	}
 }
 
@@ -137,7 +139,7 @@ static void restore_netns(void)
 	if (setns(env.saved_netns_fd, CLONE_NEWNET) == -1) {
 		stdio_restore();
 		perror("setns(CLONE_NEWNS)");
-		exit(-1);
+		exit(EXIT_ERR_NETNS);
 	}
 }
 



^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next V2 2/2] selftests/bpf: test_progs avoid minus shell exit codes
  2020-07-06 17:00 ` [PATCH bpf-next V2 2/2] selftests/bpf: test_progs avoid minus shell exit codes Jesper Dangaard Brouer
@ 2020-07-06 22:17   ` Andrii Nakryiko
  2020-07-07  6:08     ` Jesper Dangaard Brouer
  0 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2020-07-06 22:17 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: bpf, Hangbin Liu, Daniel Borkmann, Alexei Starovoitov,
	Veronika Kabatova, Jiri Benc, Yonghong Song, Martin Lau,
	Networking, open list

On Mon, Jul 6, 2020 at 10:00 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> There are a number of places in test_progs that use minus-1 as the argument
> to exit(). This improper use as a process exit status is masked to be a
> number between 0 and 255 as defined in man exit(3).

nit: I wouldn't call it improper use, as it's a well defined behavior
(lower byte of returned integer).

>
> This patch use two different positive exit codes instead, to allow a shell

typo: uses

> script to tell the two error cases apart.
>
> Fixes: fd27b1835e70 ("selftests/bpf: Reset process and thread affinity after each test/sub-test")
> Fixes: 811d7e375d08 ("bpf: selftests: Restore netns after each test")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
>  tools/testing/selftests/bpf/test_progs.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index e8f7cd5dbae4..50803b080593 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -12,7 +12,9 @@
>  #include <string.h>
>  #include <execinfo.h> /* backtrace */
>
> -#define EXIT_NO_TEST 2
> +#define EXIT_NO_TEST           2
> +#define EXIT_ERR_NETNS         3
> +#define EXIT_ERR_RESET_AFFINITY        4

Let's not overdo this with too granular error codes? All of those seem
to be just a failure, is there any practical need to differentiate
between NETNS vs RESET_AFFINITY failure?

I'd go with 3 values:

1 - at least one test failed
2 - no tests were selected
3 - "infra" (not a test-specific failure) error (like netns or affinity failed).

Thoughts?


[...]

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next V2 2/2] selftests/bpf: test_progs avoid minus shell exit codes
  2020-07-06 22:17   ` Andrii Nakryiko
@ 2020-07-07  6:08     ` Jesper Dangaard Brouer
  0 siblings, 0 replies; 5+ messages in thread
From: Jesper Dangaard Brouer @ 2020-07-07  6:08 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Hangbin Liu, Daniel Borkmann, Alexei Starovoitov,
	Veronika Kabatova, Jiri Benc, Yonghong Song, Martin Lau,
	Networking, open list, brouer

On Mon, 6 Jul 2020 15:17:57 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Mon, Jul 6, 2020 at 10:00 AM Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> >
> > There are a number of places in test_progs that use minus-1 as the argument
> > to exit(). This improper use as a process exit status is masked to be a
> > number between 0 and 255 as defined in man exit(3).  
> 
> nit: I wouldn't call it improper use, as it's a well defined behavior
> (lower byte of returned integer).
> 
> >
> > This patch use two different positive exit codes instead, to allow a shell  
> 
> typo: uses
> 
> > script to tell the two error cases apart.
> >
> > Fixes: fd27b1835e70 ("selftests/bpf: Reset process and thread affinity after each test/sub-test")
> > Fixes: 811d7e375d08 ("bpf: selftests: Restore netns after each test")
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > ---
> >  tools/testing/selftests/bpf/test_progs.c |   12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index e8f7cd5dbae4..50803b080593 100644
> > --- a/tools/testing/selftests/bpf/test_progs.c
> > +++ b/tools/testing/selftests/bpf/test_progs.c
> > @@ -12,7 +12,9 @@
> >  #include <string.h>
> >  #include <execinfo.h> /* backtrace */
> >
> > -#define EXIT_NO_TEST 2
> > +#define EXIT_NO_TEST           2
> > +#define EXIT_ERR_NETNS         3
> > +#define EXIT_ERR_RESET_AFFINITY        4  
> 
> Let's not overdo this with too granular error codes? All of those seem
> to be just a failure, is there any practical need to differentiate
> between NETNS vs RESET_AFFINITY failure?

I agree, because both cases (NETNS vs RESET_AFFINITY) print to stderr,
which makes it possible to troubleshoot for a human afterwards.  The
shell script just need to differentiate that is an "infra" setup issue,
as we e.g. might want to allow the RPM build to continue in those cases.


> I'd go with 3 values:
> 
> 1 - at least one test failed
> 2 - no tests were selected
> 3 - "infra" (not a test-specific failure) error (like netns or affinity failed).
> 
> Thoughts?

Sure, I can do this.

What define name reflect this best:
 EXIT_ERR_SETUP ?
 EXIT_ERR_INFRA ?
 EXIT_ERR_SETUP_INFRA ?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-07  6:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 17:00 [PATCH bpf-next V2 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes Jesper Dangaard Brouer
2020-07-06 17:00 ` [PATCH bpf-next V2 1/2] selftests/bpf: test_progs use another shell exit on non-actions Jesper Dangaard Brouer
2020-07-06 17:00 ` [PATCH bpf-next V2 2/2] selftests/bpf: test_progs avoid minus shell exit codes Jesper Dangaard Brouer
2020-07-06 22:17   ` Andrii Nakryiko
2020-07-07  6:08     ` Jesper Dangaard Brouer

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