BPF List
 help / color / mirror / Atom feed
From: Martin KaFai Lau <martin.lau@linux.dev>
To: Wang Yufen <wangyufen@huawei.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	andrii@kernel.org, song@kernel.org, yhs@fb.com,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org, mykolal@fb.com,
	shuah@kernel.org, delyank@fb.com, zhudi2@huawei.com,
	jakub@cloudflare.com, kuba@kernel.org, kuifeng@fb.com,
	deso@posteo.net, zhuyifei@google.com, hengqi.chen@gmail.com
Subject: Re: [bpf-next 08/11] bpf/selftests: convert tcp_hdr_options test to ASSERT_* macros
Date: Thu, 3 Nov 2022 18:29:47 -0700	[thread overview]
Message-ID: <0dc6d45a-8e0c-46d6-511e-d15eebe684c9@linux.dev> (raw)
In-Reply-To: <1664169131-32405-9-git-send-email-wangyufen@huawei.com>

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.

  reply	other threads:[~2022-11-04  1:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-26  5:12 [bpf-next 00/11] bpf/selftests: convert some tests to ASSERT_* macros Wang Yufen
2022-09-26  5:12 ` [bpf-next 01/11] bpf/selftests: convert sockmap_basic test " Wang Yufen
2022-09-26  5:12 ` [bpf-next 02/11] bpf/selftests: convert sockmap_ktls " Wang Yufen
2022-09-26  5:12 ` [bpf-next 03/11] bpf/selftests: convert sockopt " Wang Yufen
2022-09-26  5:12 ` [bpf-next 04/11] bpf/selftests: convert sockopt_inherit " Wang Yufen
2022-09-26  5:12 ` [bpf-next 05/11] bpf/selftests: convert sockopt_multi " Wang Yufen
2022-09-26  5:12 ` [bpf-next 06/11] bpf/selftests: convert sockopt_sk " Wang Yufen
2022-09-26  5:12 ` [bpf-next 07/11] bpf/selftests: convert tcp_estats " Wang Yufen
2022-09-26  5:12 ` [bpf-next 08/11] bpf/selftests: convert tcp_hdr_options " Wang Yufen
2022-11-04  1:29   ` Martin KaFai Lau [this message]
2022-09-26  5:12 ` [bpf-next 09/11] bpf/selftests: convert tcp_rtt " Wang Yufen
2022-09-26  5:12 ` [bpf-next 10/11] bpf/selftests: convert tcpbpf_user " Wang Yufen
2022-09-26  5:12 ` [bpf-next 11/11] bpf/selftests: convert udp_limit " Wang Yufen
2022-09-29  0:37 ` [bpf-next 00/11] bpf/selftests: convert some tests " Andrii Nakryiko
2022-09-29  3:37   ` wangyufen
2022-09-29  0:40 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0dc6d45a-8e0c-46d6-511e-d15eebe684c9@linux.dev \
    --to=martin.lau@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=delyank@fb.com \
    --cc=deso@posteo.net \
    --cc=haoluo@google.com \
    --cc=hengqi.chen@gmail.com \
    --cc=jakub@cloudflare.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuifeng@fb.com \
    --cc=mykolal@fb.com \
    --cc=sdf@google.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=wangyufen@huawei.com \
    --cc=yhs@fb.com \
    --cc=zhudi2@huawei.com \
    --cc=zhuyifei@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox