2020-07-07 07:14:11

by Jesper Dangaard Brouer

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

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

--


2020-07-07 07:14:52

by Jesper Dangaard Brouer

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

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 <[email protected]>
---
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);
}
}



2020-07-07 07:26:39

by Andrii Nakryiko

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

On Tue, Jul 7, 2020 at 12:12 AM Jesper Dangaard Brouer
<[email protected]> 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 <[email protected]>

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

2020-07-08 18:17:46

by Jesper Dangaard Brouer

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

On Tue, 7 Jul 2020 00:23:48 -0700
Andrii Nakryiko <[email protected]> wrote:

> On Tue, Jul 7, 2020 at 12:12 AM Jesper Dangaard Brouer
> <[email protected]> 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 <[email protected]>
>
> 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

2020-07-08 20:04:02

by Andrii Nakryiko

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

On Wed, Jul 8, 2020 at 11:16 AM Jesper Dangaard Brouer
<[email protected]> wrote:
>
> On Tue, 7 Jul 2020 00:23:48 -0700
> Andrii Nakryiko <[email protected]> wrote:
>
> > On Tue, Jul 7, 2020 at 12:12 AM Jesper Dangaard Brouer
> > <[email protected]> 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 <[email protected]>
> >
> > 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
>

2020-07-08 23:00:30

by Daniel Borkmann

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

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!