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(-)
--
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 <[email protected]>
---
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;
}
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 <[email protected]>
---
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);
}
}
On Mon, Jul 6, 2020 at 10:00 AM Jesper Dangaard Brouer
<[email protected]> 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 <[email protected]>
> ---
> 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?
[...]
On Mon, 6 Jul 2020 15:17:57 -0700
Andrii Nakryiko <[email protected]> wrote:
> On Mon, Jul 6, 2020 at 10:00 AM Jesper Dangaard Brouer
> <[email protected]> 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 <[email protected]>
> > ---
> > 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