Re: [PATCH bpf-next] selftests/bpf: Remove "&>" usage in the selftests

[Yonghong Song <yonghong.song@linux.dev>]

On 1/26/24 6:50 PM, Martin KaFai Lau wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> In s390, CI reported that the sock_iter_batch selftest
> hits this error very often:
>
> 2024-01-26T16:56:49.3091804Z Bind /proc/self/ns/net -> /run/netns/sock_iter_batch_netns failed: No such file or directory
> 2024-01-26T16:56:49.3149524Z Cannot remove namespace file "/run/netns/sock_iter_batch_netns": No such file or directory
> 2024-01-26T16:56:49.3772213Z test_sock_iter_batch:FAIL:ip netns add sock_iter_batch_netns unexpected error: 256 (errno 0)
>
> It happens very often in s390 but Manu also noticed it happens very
> sparsely in other arch also.
>
> It turns out the default dash shell does not recognize "&>"

Not sure whether it is feasible or not. But is it possible
for all our test VMs we run '/bin/bash' before everyting else
so we have a uniform bash environment so we do not need to
worry about other shells?

> as a redirection operator, so the command went to the background.
> In the sock_iter_batch selftest, the "ip netns delete" went
> into background and then race with the following "ip netns add"
> command.
>
> This patch replaces the "&> /dev/null" usage with ">/dev/null 2>&1"
> and does this redirection in the SYS_NOFAIL macro instead of doing
> it individually by its caller. The SYS_NOFAIL callers do not care
> about failure, so it is no harm to do this redirection even if
> some of the existing callers do not redirect to /dev/null now.
>
> It touches different test files, so I skipped the Fixes tags
> in this patch. Some of the changed tests do not use "&>"
> but they use the SYS_NOFAIL, so these tests are also
> changed to avoid doing its own redirection because
> SYS_NOFAIL does it internally now.
>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> ---
>   .../selftests/bpf/prog_tests/decap_sanity.c    |  2 +-
>   .../selftests/bpf/prog_tests/fib_lookup.c      |  2 +-
>   .../selftests/bpf/prog_tests/ip_check_defrag.c |  4 ++--
>   .../selftests/bpf/prog_tests/lwt_redirect.c    |  2 +-
>   .../selftests/bpf/prog_tests/lwt_reroute.c     |  2 +-
>   tools/testing/selftests/bpf/prog_tests/mptcp.c |  2 +-
>   .../selftests/bpf/prog_tests/sock_destroy.c    |  2 +-
>   .../selftests/bpf/prog_tests/sock_iter_batch.c |  4 ++--
>   .../selftests/bpf/prog_tests/test_tunnel.c     | 18 +++++++++---------
>   tools/testing/selftests/bpf/test_progs.h       |  7 ++++++-
>   10 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/decap_sanity.c b/tools/testing/selftests/bpf/prog_tests/decap_sanity.c
> index 5c0ebe6ba866..dcb9e5070cc3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/decap_sanity.c
> +++ b/tools/testing/selftests/bpf/prog_tests/decap_sanity.c
> @@ -72,6 +72,6 @@ void test_decap_sanity(void)
>   		bpf_tc_hook_destroy(&qdisc_hook);
>   		close_netns(nstoken);
>   	}
> -	SYS_NOFAIL("ip netns del " NS_TEST " &> /dev/null");
> +	SYS_NOFAIL("ip netns del " NS_TEST);
>   	decap_sanity__destroy(skel);
>   }

[...]

> diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> index 2f9f6f250f17..80df51244886 100644
> --- a/tools/testing/selftests/bpf/test_progs.h
> +++ b/tools/testing/selftests/bpf/test_progs.h
> @@ -385,10 +385,15 @@ int test__join_cgroup(const char *path);
>   			goto goto_label;				\
>   	})
>   
> +#define ALL_TO_DEV_NULL " >/dev/null 2>&1"
> +
>   #define SYS_NOFAIL(fmt, ...)						\
>   	({								\
>   		char cmd[1024];						\
> -		snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__);		\
> +		int n;							\
> +		n = snprintf(cmd, sizeof(cmd), fmt, ##__VA_ARGS__);	\
> +		if (n < sizeof(cmd) && sizeof(cmd) - n >= sizeof(ALL_TO_DEV_NULL)) \
> +			strcat(cmd, ALL_TO_DEV_NULL);			\
>   		system(cmd);						\
>   	})
>