2018-03-06 15:11:27

by Anders Roxell

[permalink] [raw]
Subject: [PATCH] selftest: net: reuseport_bpf_numa: don't fail if no numa support

The reuseport_bpf_numa test case fails there's no numa support. The
test shouldn't fail if there's no support it should be skipped with a
pass.

Fixes: 3c2c3c16aaf6 ("reuseport, bpf: add test case for bpf_get_numa_node_id")
Signed-off-by: Anders Roxell <[email protected]>
---
tools/testing/selftests/net/reuseport_bpf_numa.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/reuseport_bpf_numa.c b/tools/testing/selftests/net/reuseport_bpf_numa.c
index 365c32e84189..9245c14165b7 100644
--- a/tools/testing/selftests/net/reuseport_bpf_numa.c
+++ b/tools/testing/selftests/net/reuseport_bpf_numa.c
@@ -228,8 +228,10 @@ int main(void)
{
int *rcv_fd, nodes;

- if (numa_available() < 0)
- error(1, errno, "no numa api support");
+ if (numa_available() < 0) {
+ fprintf(stderr, "no numa api support");
+ return 0;
+ }

nodes = numa_max_node() + 1;

--
2.11.0



2018-03-07 18:27:17

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] selftest: net: reuseport_bpf_numa: don't fail if no numa support

From: Anders Roxell <[email protected]>
Date: Tue, 6 Mar 2018 16:10:04 +0100

> The reuseport_bpf_numa test case fails there's no numa support. The
> test shouldn't fail if there's no support it should be skipped with a
> pass.
>
> Fixes: 3c2c3c16aaf6 ("reuseport, bpf: add test case for bpf_get_numa_node_id")
> Signed-off-by: Anders Roxell <[email protected]>

I don't know about this.

The test did not pass. So it should not be "skipped with a pass".

We were unable to run it at all, which means we don't know if it
would pass or fail. This means there is a third state besides
pass or fail which we must acknowledge and implement.



2018-03-07 19:52:56

by Shuah Khan

[permalink] [raw]
Subject: Re: [PATCH] selftest: net: reuseport_bpf_numa: don't fail if no numa support

On 03/07/2018 11:25 AM, David Miller wrote:
> From: Anders Roxell <[email protected]>
> Date: Tue, 6 Mar 2018 16:10:04 +0100
>
>> The reuseport_bpf_numa test case fails there's no numa support. The
>> test shouldn't fail if there's no support it should be skipped with a
>> pass.
>>
>> Fixes: 3c2c3c16aaf6 ("reuseport, bpf: add test case for bpf_get_numa_node_id")
>> Signed-off-by: Anders Roxell <[email protected]>
>
> I don't know about this.
>
> The test did not pass. So it should not be "skipped with a pass".
>

You are right that "skipped with pass" is ambiguous. Don't we wish
we could have done that in college :)

> We were unable to run it at all, which means we don't know if it
> would pass or fail. This means there is a third state besides
> pass or fail which we must acknowledge and implement.
>

Several tests and test scripts treated test that can't be run as errors
which is also ambiguous. Test user community requested that they would
like Skips to be treated as pass. I think the reasoning is that on ARM
systems, several tests get skipped - NUMA happens to be one and there are
others. Also the configs. Not all config options might be enabled
and tests need to be skipped.

This seems to causing problems for test result analysis.

So I made a call to say - okay let's treat Skip as pass to make it easier
for analysis. Some test authors are okay with treating skip as pass. Some
test authors don't like doing so. So we are currently in an inconsistent
state. Maybe networking tests are the only ones that don't treat skip as
pass at the moment. In any case, dependency checks are made from test shell
scripts mainly and in some cases from test programs such as bpf_numa and when
dependency isn't met, some tests treat it as an error and sone as skip-pass.

Lots of test scripts check for !pass and call it fail. So changing skip as a
distinct state requires some work in several tests. Not impossible, just
effort.

Kselftest infrastructure on the other hand maintains skip count and for
tests skipped, also prints a distinct message for it. Originally skip was
treated by the framework as distinct state implying that the test can't be
run which makes more sense. Based on the request as stated above, I made
a change to map KSFT_SKIP to KSFT_PASS. Several tests use the infrastructure.

That said, I am open to adding a distinct state for skipped because can't
be run.

thanks,
-- Shuah





2018-03-07 20:00:57

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] selftest: net: reuseport_bpf_numa: don't fail if no numa support

From: Shuah Khan <[email protected]>
Date: Wed, 7 Mar 2018 12:51:06 -0700

> That said, I am open to adding a distinct state for skipped because
> can't be run.

That is what I think should happen.

2018-05-18 22:28:41

by Anders Roxell

[permalink] [raw]
Subject: [PATCH v2] selftests: net: reuseport_bpf_numa: don't fail if no numa support

The reuseport_bpf_numa test case fails there's no numa support. The
test shouldn't fail if there's no support it should be skipped.

Fixes: 3c2c3c16aaf6 ("reuseport, bpf: add test case for bpf_get_numa_node_id")
Signed-off-by: Anders Roxell <[email protected]>
---
tools/testing/selftests/net/reuseport_bpf_numa.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/reuseport_bpf_numa.c b/tools/testing/selftests/net/reuseport_bpf_numa.c
index 365c32e84189..c9f478b40996 100644
--- a/tools/testing/selftests/net/reuseport_bpf_numa.c
+++ b/tools/testing/selftests/net/reuseport_bpf_numa.c
@@ -23,6 +23,8 @@
#include <unistd.h>
#include <numa.h>

+#include "../kselftest.h"
+
static const int PORT = 8888;

static void build_rcv_group(int *rcv_fd, size_t len, int family, int proto)
@@ -229,7 +231,7 @@ int main(void)
int *rcv_fd, nodes;

if (numa_available() < 0)
- error(1, errno, "no numa api support");
+ ksft_exit_skip("no numa api support\n");

nodes = numa_max_node() + 1;

--
2.17.0


2018-05-23 10:30:03

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH v2] selftests: net: reuseport_bpf_numa: don't fail if no numa support

On 05/19/2018 12:27 AM, Anders Roxell wrote:
> The reuseport_bpf_numa test case fails there's no numa support. The
> test shouldn't fail if there's no support it should be skipped.
>
> Fixes: 3c2c3c16aaf6 ("reuseport, bpf: add test case for bpf_get_numa_node_id")
> Signed-off-by: Anders Roxell <[email protected]>

Applied to bpf tree, thanks Anders!