Re: [bpf-next 08/11] bpf/selftests: convert tcp_hdr_options test to ASSERT_* macros

[Martin KaFai Lau <martin.lau@xxxxxxxxx>]

On 9/25/22 10:12 PM, Wang Yufen wrote:
> @@ -497,26 +481,20 @@ static void misc(void)
>   		/* MSG_EOR to ensure skb will not be combined */
>   		ret = send(sk_fds.active_fd, send_msg, sizeof(send_msg),
>   			   MSG_EOR);
> -		if (CHECK(ret != sizeof(send_msg), "send(msg)", "ret:%d\n",
> -			  ret))
> +		if (!ASSERT_EQ(ret, sizeof(send_msg), "send(msg)"))
>   			goto check_linum;
>   
>   		ret = read(sk_fds.passive_fd, recv_msg, sizeof(recv_msg));
> -		if (CHECK(ret != sizeof(send_msg), "read(msg)", "ret:%d\n",
> -			  ret))
> +		if (ASSERT_EQ(ret, sizeof(send_msg), "read(msg)"))

Thanks for the clean up.  However, this looks wrong...

>   			goto check_linum;
>   	}
>   
>   	if (sk_fds_shutdown(&sk_fds))
>   		goto check_linum;
>   
> -	CHECK(misc_skel->bss->nr_syn != 1, "unexpected nr_syn",
> -	      "expected (1) != actual (%u)\n",
> -		misc_skel->bss->nr_syn);
> +	ASSERT_EQ(misc_skel->bss->nr_syn, 1, "unexpected nr_syn");
>   
> -	CHECK(misc_skel->bss->nr_data != nr_data, "unexpected nr_data",
> -	      "expected (%u) != actual (%u)\n",
> -	      nr_data, misc_skel->bss->nr_data);
> +	ASSERT_EQ(misc_skel->bss->nr_data, nr_data, "unexpected nr_data");
>   
>   	/* The last ACK may have been delayed, so it is either 1 or 2. */
>   	CHECK(misc_skel->bss->nr_pure_ack != 1 &&
> @@ -525,12 +503,10 @@ static void misc(void)
>   	      "expected (1 or 2) != actual (%u)\n",
>   		misc_skel->bss->nr_pure_ack);
>   
> -	CHECK(misc_skel->bss->nr_fin != 1, "unexpected nr_fin",
> -	      "expected (1) != actual (%u)\n",
> -	      misc_skel->bss->nr_fin);
> +	ASSERT_EQ(misc_skel->bss->nr_fin, 1, "unexpected nr_fin");
>   
>   check_linum:
> -	CHECK_FAIL(check_error_linum(&sk_fds));
> +	ASSERT_FALSE(check_error_linum(&sk_fds), "check_error_linum");
>   	sk_fds_close(&sk_fds);
>   	bpf_link__destroy(link);
>   }
> @@ -555,15 +531,15 @@ void test_tcp_hdr_options(void)
>   	int i;
>   
>   	skel = test_tcp_hdr_options__open_and_load();
> -	if (CHECK(!skel, "open and load skel", "failed"))
> +	if (!ASSERT_OK_PTR(skel, "open and load skel"))
>   		return;
>   
>   	misc_skel = test_misc_tcp_hdr_options__open_and_load();
> -	if (CHECK(!misc_skel, "open and load misc test skel", "failed"))
> +	if (!ASSERT_OK_PTR(misc_skel, "open and load misc test skel"))
>   		goto skel_destroy;
>   
>   	cg_fd = test__join_cgroup(CG_NAME);
> -	if (CHECK_FAIL(cg_fd < 0))
> +	if (ASSERT_GE(cg_fd, 0, "join_cgroup"))

...and at least this one also.  Have your tested it?

Here is some quick checks that will be useful.

Before this patch:
#> ./test_progs -t tcp_hdr_options
#197/1   tcp_hdr_options/simple_estab:OK
#197/2   tcp_hdr_options/no_exprm_estab:OK
#197/3   tcp_hdr_options/syncookie_estab:OK
#197/4   tcp_hdr_options/fastopen_estab:OK
#197/5   tcp_hdr_options/fin:OK
#197/6   tcp_hdr_options/misc:OK
#197     tcp_hdr_options:OK
Summary: 1/6 PASSED, 0 SKIPPED, 0 FAILED

#> ./test_progs -v -t tcp_hdr_options |& egrep PASS | wc -l
123

After this patch:
#> ./test_progs -t tcp_hdr_options
#197     tcp_hdr_options:OK
Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED

#> ./test_progs -v -t tcp_hdr_options |& egrep PASS | wc -l
4

Please check all the changes in this set.