[PATCH bpf-next 1/2] selftests/bpf: Run reuseport tests only with supported socket types

[PATCH bpf-next 1/2] selftests/bpf: Run reuseport tests only with supported socket types

[Jakub Sitnicki]

SOCKMAP and SOCKHASH map types can be used with reuseport BPF programs but
don't support yet storing UDP sockets. Instead of marking UDP tests with
SOCK{MAP,HASH} as skipped, don't run them at all.

Skipped test might signal that the test environment is not suitable for
running the test, while in reality the functionality is not implemented in
the kernel yet.

Before:

  sh# ./test_progs -t select_reuseport
  …
  #40 select_reuseport:OK
  Summary: 1/126 PASSED, 30 SKIPPED, 0 FAILED

After:

  sh# ./test_progs  -t select_reuseport
  …
  #40 select_reuseport:OK
  Summary: 1/98 PASSED, 2 SKIPPED, 0 FAILED

The remaining two skipped tests are SYN cookies tests, which will be
addressed in the subsequent patch.

Fixes: 11318ba8cafd ("selftests/bpf: Extend SK_REUSEPORT tests to cover SOCKMAP/SOCKHASH")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
 .../selftests/bpf/prog_tests/select_reuseport.c     | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
index 68d452bb9fd9..8c41d6d63fcf 100644
--- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
+++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
@@ -807,6 +807,12 @@ static void test_config(int sotype, sa_family_t family, bool inany)
 	char s[MAX_TEST_NAME];
 	const struct test *t;
 
+	/* SOCKMAP/SOCKHASH don't support UDP yet */
+	if (sotype == SOCK_DGRAM &&
+	    (inner_map_type == BPF_MAP_TYPE_SOCKMAP ||
+	     inner_map_type == BPF_MAP_TYPE_SOCKHASH))
+		return;
+
 	for (t = tests; t < tests + ARRAY_SIZE(tests); t++) {
 		snprintf(s, sizeof(s), "%s %s/%s %s %s",
 			 maptype_str(inner_map_type),
@@ -816,13 +822,6 @@ static void test_config(int sotype, sa_family_t family, bool inany)
 		if (!test__start_subtest(s))
 			continue;
 
-		if (sotype == SOCK_DGRAM &&
-		    inner_map_type != BPF_MAP_TYPE_REUSEPORT_SOCKARRAY) {
-			/* SOCKMAP/SOCKHASH don't support UDP yet */
-			test__skip();
-			continue;
-		}
-
 		setup_per_test(sotype, family, inany, t->no_inner_map);
 		t->fn(sotype, family);
 		cleanup_per_test(t->no_inner_map);
-- 
2.24.1

[PATCH bpf-next 2/2] selftests/bpf: Run SYN cookies with reuseport BPF test only for TCP

[Jakub Sitnicki]

Currently we run SYN cookies test for all socket types and mark the test as
skipped if socket type is not compatible. This causes confusion because
skipped test might indicate a problem with the testing environment.

Instead, run the test only for the socket type which supports SYN cookies.

Also, switch to using designated initializers when setting up tests, so
that we can tweak only some test parameters, leaving the rest initialized
to default values.

Fixes: eecd618b4516 ("selftests/bpf: Mark SYN cookie test skipped for UDP sockets")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
 .../selftests/bpf/prog_tests/select_reuseport.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
index 8c41d6d63fcf..a1dd13b34d4b 100644
--- a/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
+++ b/tools/testing/selftests/bpf/prog_tests/select_reuseport.c
@@ -509,11 +509,6 @@ static void test_syncookie(int type, sa_family_t family)
 		.pass_on_failure = 0,
 	};
 
-	if (type != SOCK_STREAM) {
-		test__skip();
-		return;
-	}
-
 	/*
 	 * +1 for TCP-SYN and
 	 * +1 for the TCP-ACK (ack the syncookie)
@@ -787,7 +782,7 @@ static const char *sotype_str(int sotype)
 	}
 }
 
-#define TEST_INIT(fn, ...) { fn, #fn, __VA_ARGS__ }
+#define TEST_INIT(fn_, ...) { .fn = fn_, .name = #fn_, __VA_ARGS__ }
 
 static void test_config(int sotype, sa_family_t family, bool inany)
 {
@@ -795,12 +790,15 @@ static void test_config(int sotype, sa_family_t family, bool inany)
 		void (*fn)(int sotype, sa_family_t family);
 		const char *name;
 		bool no_inner_map;
+		int need_sotype;
 	} tests[] = {
-		TEST_INIT(test_err_inner_map, true /* no_inner_map */),
+		TEST_INIT(test_err_inner_map,
+			  .no_inner_map = true),
 		TEST_INIT(test_err_skb_data),
 		TEST_INIT(test_err_sk_select_port),
 		TEST_INIT(test_pass),
-		TEST_INIT(test_syncookie),
+		TEST_INIT(test_syncookie,
+			  .need_sotype = SOCK_STREAM),
 		TEST_INIT(test_pass_on_err),
 		TEST_INIT(test_detach_bpf),
 	};
@@ -814,6 +812,9 @@ static void test_config(int sotype, sa_family_t family, bool inany)
 		return;
 
 	for (t = tests; t < tests + ARRAY_SIZE(tests); t++) {
+		if (t->need_sotype && t->need_sotype != sotype)
+			continue; /* test not compatible with socket type */
+
 		snprintf(s, sizeof(s), "%s %s/%s %s %s",
 			 maptype_str(inner_map_type),
 			 family_str(family), sotype_str(sotype),
-- 
2.24.1

Re: [PATCH bpf-next 1/2] selftests/bpf: Run reuseport tests only with supported socket types

[Martin KaFai Lau]

On Mon, Feb 24, 2020 at 02:53:26PM +0100, Jakub Sitnicki wrote:
> SOCKMAP and SOCKHASH map types can be used with reuseport BPF programs but
> don't support yet storing UDP sockets. Instead of marking UDP tests with
> SOCK{MAP,HASH} as skipped, don't run them at all.
> 
> Skipped test might signal that the test environment is not suitable for
> running the test, while in reality the functionality is not implemented in
> the kernel yet.
> 
> Before:
> 
>   sh# ./test_progs -t select_reuseport
>   …
>   #40 select_reuseport:OK
>   Summary: 1/126 PASSED, 30 SKIPPED, 0 FAILED
> 
> After:
> 
>   sh# ./test_progs  -t select_reuseport
>   …
>   #40 select_reuseport:OK
>   Summary: 1/98 PASSED, 2 SKIPPED, 0 FAILED
Acked-by: Martin KaFai Lau <kafai@fb.com>

Re: [PATCH bpf-next 2/2] selftests/bpf: Run SYN cookies with reuseport BPF test only for TCP

[Martin KaFai Lau]

On Mon, Feb 24, 2020 at 02:53:27PM +0100, Jakub Sitnicki wrote:
> Currently we run SYN cookies test for all socket types and mark the test as
> skipped if socket type is not compatible. This causes confusion because
> skipped test might indicate a problem with the testing environment.
> 
> Instead, run the test only for the socket type which supports SYN cookies.
Acked-by: Martin KaFai Lau <kafai@fb.com>

Re: [PATCH bpf-next 1/2] selftests/bpf: Run reuseport tests only with supported socket types

[Alexei Starovoitov]

On Mon, Feb 24, 2020 at 5:53 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> SOCKMAP and SOCKHASH map types can be used with reuseport BPF programs but
> don't support yet storing UDP sockets. Instead of marking UDP tests with
> SOCK{MAP,HASH} as skipped, don't run them at all.
>
> Skipped test might signal that the test environment is not suitable for
> running the test, while in reality the functionality is not implemented in
> the kernel yet.
>
> Before:
>
>   sh# ./test_progs -t select_reuseport
>   …
>   #40 select_reuseport:OK
>   Summary: 1/126 PASSED, 30 SKIPPED, 0 FAILED
>
> After:
>
>   sh# ./test_progs  -t select_reuseport
>   …
>   #40 select_reuseport:OK
>   Summary: 1/98 PASSED, 2 SKIPPED, 0 FAILED
>
> The remaining two skipped tests are SYN cookies tests, which will be
> addressed in the subsequent patch.
>
> Fixes: 11318ba8cafd ("selftests/bpf: Extend SK_REUSEPORT tests to cover SOCKMAP/SOCKHASH")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>

Applied. Thanks