[PATCH bpf-next V3 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes

[PATCH bpf-next V3 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes

[Jesper Dangaard Brouer]

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 |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

--

[PATCH bpf-next V3 1/2] selftests/bpf: test_progs use another shell exit on non-actions

[Jesper Dangaard Brouer]

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..65d3f8686e29 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;
 }

[PATCH bpf-next V3 2/2] selftests/bpf: test_progs avoid minus shell exit codes

[Jesper Dangaard Brouer]

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

This patch use positive exit code 3 instead of minus-1. These cases are put
in the same group of infrastructure setup errors.

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 |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 65d3f8686e29..b1e4dadacd9b 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -13,6 +13,7 @@
 #include <execinfo.h> /* backtrace */
 
 #define EXIT_NO_TEST		2
+#define EXIT_ERR_SETUP_INFRA	3
 
 /* defined in test_progs.h */
 struct test_env env = {};
@@ -113,13 +114,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_SETUP_INFRA);
 	}
 	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_SETUP_INFRA);
 	}
 }
 
@@ -128,7 +129,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_SETUP_INFRA);
 	}
 }
 
@@ -137,7 +138,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_SETUP_INFRA);
 	}
 }
 

Re: [PATCH bpf-next V3 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes

[Andrii Nakryiko]

On Tue, Jul 7, 2020 at 12:12 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> 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 |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> --
>

For the series:

Acked-by: Andrii Nakryiko <andriin@fb.com>

My preference was shorter EXIT_ERR_SETUP, but it doesn't matter.

Re: [PATCH bpf-next V3 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes

[Jesper Dangaard Brouer]

On Tue, 7 Jul 2020 00:23:48 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Tue, Jul 7, 2020 at 12:12 AM Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> >
> > 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 |   13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > --
> >  
> 
> For the series:
> 
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> 
> My preference was shorter EXIT_ERR_SETUP, but it doesn't matter.

I can just resend the patchset, if you prefer?

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

Re: [PATCH bpf-next V3 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes

[Andrii Nakryiko]

On Wed, Jul 8, 2020 at 11:16 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> On Tue, 7 Jul 2020 00:23:48 -0700
> Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> > On Tue, Jul 7, 2020 at 12:12 AM Jesper Dangaard Brouer
> > <brouer@redhat.com> wrote:
> > >
> > > 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 |   13 ++++++++-----
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > >
> > > --
> > >
> >
> > For the series:
> >
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> >
> > My preference was shorter EXIT_ERR_SETUP, but it doesn't matter.
>
> I can just resend the patchset, if you prefer?

Doesn't matter to me, you can keep it as is.

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

Re: [PATCH bpf-next V3 0/2] BPF selftests test runner 'test_progs' use proper shell exit codes

[Daniel Borkmann]

On 7/7/20 9:12 AM, Jesper Dangaard Brouer wrote:
> 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

Applied, thanks!