diff mbox series

[bpf-next] selftests/bpf: Fix flaky fib_lookup test

Message ID 20230309054749.3143752-1-martin.lau@linux.dev (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf-next] selftests/bpf: Fix flaky fib_lookup test | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR pending PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 pending Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 pending Logs for build for aarch64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-4 pending Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-7 success Logs for llvm-toolchain
bpf/vmtest-bpf-next-VM_Test-8 success Logs for set-matrix
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 20 this patch: 20
netdev/cc_maintainers warning 11 maintainers not CCed: mykolal@fb.com song@kernel.org shuah@kernel.org sdf@google.com haoluo@google.com yhs@fb.com john.fastabend@gmail.com liuhangbin@gmail.com kpsingh@kernel.org jolsa@kernel.org linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 20 this patch: 20
netdev/checkpatch warning WARNING: Reported-by: should be immediately followed by Link: with a URL to the report WARNING: line length of 82 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Martin KaFai Lau March 9, 2023, 5:47 a.m. UTC 
From: Martin KaFai Lau <martin.lau@kernel.org>

There is a report that fib_lookup test is flaky when running in parallel.
A symptom of slowness or delay. An example:

Testing IPv6 stale neigh
set_lookup_params:PASS:inet_pton(IPV6_IFACE_ADDR) 0 nsec
test_fib_lookup:PASS:bpf_prog_test_run_opts 0 nsec
test_fib_lookup:FAIL:fib_lookup_ret unexpected fib_lookup_ret: actual 0 != expected 7
test_fib_lookup:FAIL:dmac not match unexpected dmac not match: actual 1 != expected 0
dmac expected 11:11:11:11:11:11 actual 00:00:00:00:00:00

[ Note that the "fib_lookup_ret unexpected fib_lookup_ret actual 0 ..."
  is reversed in terms of expected and actual value. Fixing in this
  patch also. ]

One possibility is the testing stale neigh entry was marked dead by the
gc (in neigh_periodic_work). The default gc_stale_time sysctl is 60s.
This patch increases it to 5 mins.

It also:
- fixes the reversed arg (actual vs expected) in one of the
  ASSERT_EQ test
- removes the nodad command arg when adding v4 neigh entry which
  currently has a warning.

Fixes: 168de0233586 ("selftests/bpf: Add bpf_fib_lookup test")
Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/fib_lookup.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Martin KaFai Lau March 9, 2023, 5:51 a.m. UTC | #1 
On 3/8/23 9:47 PM, Martin KaFai Lau wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
> 
> There is a report that fib_lookup test is flaky when running in parallel.
> A symptom of slowness or delay. An example:
> 
> Testing IPv6 stale neigh
> set_lookup_params:PASS:inet_pton(IPV6_IFACE_ADDR) 0 nsec
> test_fib_lookup:PASS:bpf_prog_test_run_opts 0 nsec
> test_fib_lookup:FAIL:fib_lookup_ret unexpected fib_lookup_ret: actual 0 != expected 7
> test_fib_lookup:FAIL:dmac not match unexpected dmac not match: actual 1 != expected 0
> dmac expected 11:11:11:11:11:11 actual 00:00:00:00:00:00
> 
> [ Note that the "fib_lookup_ret unexpected fib_lookup_ret actual 0 ..."
>    is reversed in terms of expected and actual value. Fixing in this
>    patch also. ]
> 
> One possibility is the testing stale neigh entry was marked dead by the
> gc (in neigh_periodic_work). The default gc_stale_time sysctl is 60s.
> This patch increases it to 5 mins.

typo... should be 15 mins. I will re-spin shortly.

> 
> It also:
> - fixes the reversed arg (actual vs expected) in one of the
>    ASSERT_EQ test
> - removes the nodad command arg when adding v4 neigh entry which
>    currently has a warning.
> 
> Fixes: 168de0233586 ("selftests/bpf: Add bpf_fib_lookup test")
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> ---
>   tools/testing/selftests/bpf/prog_tests/fib_lookup.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c
> index 429393caf612..0d8b90d21184 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c
> @@ -54,11 +54,19 @@ static int setup_netns(void)
>   	SYS(fail, "ip link add veth1 type veth peer name veth2");
>   	SYS(fail, "ip link set dev veth1 up");
>   
> +	err = write_sysctl("/proc/sys/net/ipv4/neigh/veth1/gc_stale_time", "900");
> +	if (!ASSERT_OK(err, "write_sysctl(net.ipv4.conf.veth1.forwarding)"))
> +		goto fail;
> +
> +	err = write_sysctl("/proc/sys/net/ipv6/neigh/veth1/gc_stale_time", "900");
> +	if (!ASSERT_OK(err, "write_sysctl(net.ipv4.conf.veth1.forwarding)"))
> +		goto fail;
> +
>   	SYS(fail, "ip addr add %s/64 dev veth1 nodad", IPV6_IFACE_ADDR);
>   	SYS(fail, "ip neigh add %s dev veth1 nud failed", IPV6_NUD_FAILED_ADDR);
>   	SYS(fail, "ip neigh add %s dev veth1 lladdr %s nud stale", IPV6_NUD_STALE_ADDR, DMAC);
>   
> -	SYS(fail, "ip addr add %s/24 dev veth1 nodad", IPV4_IFACE_ADDR);
> +	SYS(fail, "ip addr add %s/24 dev veth1", IPV4_IFACE_ADDR);
>   	SYS(fail, "ip neigh add %s dev veth1 nud failed", IPV4_NUD_FAILED_ADDR);
>   	SYS(fail, "ip neigh add %s dev veth1 lladdr %s nud stale", IPV4_NUD_STALE_ADDR, DMAC);
>   
> @@ -158,7 +166,7 @@ void test_fib_lookup(void)
>   		if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
>   			continue;
>   
> -		ASSERT_EQ(tests[i].expected_ret, skel->bss->fib_lookup_ret,
> +		ASSERT_EQ(skel->bss->fib_lookup_ret, tests[i].expected_ret,
>   			  "fib_lookup_ret");
>   
>   		ret = memcmp(tests[i].dmac, fib_params->dmac, sizeof(tests[i].dmac));
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c
index 429393caf612..0d8b90d21184 100644
--- a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c
+++ b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c
@@ -54,11 +54,19 @@  static int setup_netns(void)
 	SYS(fail, "ip link add veth1 type veth peer name veth2");
 	SYS(fail, "ip link set dev veth1 up");
 
+	err = write_sysctl("/proc/sys/net/ipv4/neigh/veth1/gc_stale_time", "900");
+	if (!ASSERT_OK(err, "write_sysctl(net.ipv4.conf.veth1.forwarding)"))
+		goto fail;
+
+	err = write_sysctl("/proc/sys/net/ipv6/neigh/veth1/gc_stale_time", "900");
+	if (!ASSERT_OK(err, "write_sysctl(net.ipv4.conf.veth1.forwarding)"))
+		goto fail;
+
 	SYS(fail, "ip addr add %s/64 dev veth1 nodad", IPV6_IFACE_ADDR);
 	SYS(fail, "ip neigh add %s dev veth1 nud failed", IPV6_NUD_FAILED_ADDR);
 	SYS(fail, "ip neigh add %s dev veth1 lladdr %s nud stale", IPV6_NUD_STALE_ADDR, DMAC);
 
-	SYS(fail, "ip addr add %s/24 dev veth1 nodad", IPV4_IFACE_ADDR);
+	SYS(fail, "ip addr add %s/24 dev veth1", IPV4_IFACE_ADDR);
 	SYS(fail, "ip neigh add %s dev veth1 nud failed", IPV4_NUD_FAILED_ADDR);
 	SYS(fail, "ip neigh add %s dev veth1 lladdr %s nud stale", IPV4_NUD_STALE_ADDR, DMAC);
 
@@ -158,7 +166,7 @@  void test_fib_lookup(void)
 		if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
 			continue;
 
-		ASSERT_EQ(tests[i].expected_ret, skel->bss->fib_lookup_ret,
+		ASSERT_EQ(skel->bss->fib_lookup_ret, tests[i].expected_ret,
 			  "fib_lookup_ret");
 
 		ret = memcmp(tests[i].dmac, fib_params->dmac, sizeof(tests[i].dmac));