[PATCH bpf-next 1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result

[PATCH bpf-next 1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result

[Martin KaFai Lau]

From: Martin KaFai Lau <martin.lau@kernel.org>

In tcp_hdr_options test, it ensures the received tcp hdr option
and the sk local storage have the expected values. It uses memcmp
to check that. Testing the memcmp result with ASSERT_OK is confusing
because ASSERT_OK will print out the errno which is not set.
This patch uses ASSERT_EQ to check for 0 instead.

Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
---
 tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
index 5cf85d0f9827..13bcaeb028b8 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
@@ -151,7 +151,7 @@ static int check_hdr_opt(const struct bpf_test_option *exp,
 			 const struct bpf_test_option *act,
 			 const char *hdr_desc)
 {
-	if (!ASSERT_OK(memcmp(exp, act, sizeof(*exp)), hdr_desc)) {
+	if (!ASSERT_EQ(memcmp(exp, act, sizeof(*exp)), 0, hdr_desc)) {
 		print_option(exp, "expected: ");
 		print_option(act, "  actual: ");
 		return -1;
@@ -169,7 +169,7 @@ static int check_hdr_stg(const struct hdr_stg *exp, int fd,
 		  "map_lookup(hdr_stg_map_fd)"))
 		return -1;
 
-	if (!ASSERT_OK(memcmp(exp, &act, sizeof(*exp)), stg_desc)) {
+	if (!ASSERT_EQ(memcmp(exp, &act, sizeof(*exp)), 0, stg_desc)) {
 		print_hdr_stg(exp, "expected: ");
 		print_hdr_stg(&act, "  actual: ");
 		return -1;
-- 
2.34.1

[PATCH bpf-next 2/2] selftests/bpf: Fix a fd leak in an error path in network_helpers.c

[Martin KaFai Lau]

From: Martin KaFai Lau <martin.lau@kernel.org>

In __start_server, it leaks a fd when setsockopt(SO_REUSEPORT) fails.
This patch fixes it.

Fixes: eed92afdd14c ("bpf: selftest: Test batching and bpf_(get|set)sockopt in bpf tcp iter")
Reported-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
---
 tools/testing/selftests/bpf/network_helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 01de33191226..596caa176582 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -95,7 +95,7 @@ static int __start_server(int type, int protocol, const struct sockaddr *addr,
 	if (reuseport &&
 	    setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &on, sizeof(on))) {
 		log_err("Failed to set SO_REUSEPORT");
-		return -1;
+		goto error_close;
 	}
 
 	if (bind(fd, addr, addrlen) < 0) {
-- 
2.34.1

Re: [PATCH bpf-next 1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result

[Yonghong Song]

On 3/15/23 5:07 PM, Martin KaFai Lau wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
> 
> In tcp_hdr_options test, it ensures the received tcp hdr option
> and the sk local storage have the expected values. It uses memcmp
> to check that. Testing the memcmp result with ASSERT_OK is confusing
> because ASSERT_OK will print out the errno which is not set.
> This patch uses ASSERT_EQ to check for 0 instead.
> 
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 2/2] selftests/bpf: Fix a fd leak in an error path in network_helpers.c

[Yonghong Song]

On 3/15/23 5:07 PM, Martin KaFai Lau wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
> 
> In __start_server, it leaks a fd when setsockopt(SO_REUSEPORT) fails.
> This patch fixes it.
> 
> Fixes: eed92afdd14c ("bpf: selftest: Test batching and bpf_(get|set)sockopt in bpf tcp iter")
> Reported-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result

[John Fastabend]

Yonghong Song wrote:
> 
> 
> On 3/15/23 5:07 PM, Martin KaFai Lau wrote:
> > From: Martin KaFai Lau <martin.lau@kernel.org>
> > 
> > In tcp_hdr_options test, it ensures the received tcp hdr option
> > and the sk local storage have the expected values. It uses memcmp
> > to check that. Testing the memcmp result with ASSERT_OK is confusing
> > because ASSERT_OK will print out the errno which is not set.
> > This patch uses ASSERT_EQ to check for 0 instead.
> > 
> > Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> 
> Acked-by: Yonghong Song <yhs@fb.com>

Acked-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf-next 2/2] selftests/bpf: Fix a fd leak in an error path in network_helpers.c

[John Fastabend]

Yonghong Song wrote:
> 
> 
> On 3/15/23 5:07 PM, Martin KaFai Lau wrote:
> > From: Martin KaFai Lau <martin.lau@kernel.org>
> > 
> > In __start_server, it leaks a fd when setsockopt(SO_REUSEPORT) fails.
> > This patch fixes it.
> > 
> > Fixes: eed92afdd14c ("bpf: selftest: Test batching and bpf_(get|set)sockopt in bpf tcp iter")
> > Reported-by: Andrii Nakryiko <andrii@kernel.org>
> > Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> 
> Acked-by: Yonghong Song <yhs@fb.com>

Acked-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf-next 1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Wed, 15 Mar 2023 17:07:25 -0700 you wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
> 
> In tcp_hdr_options test, it ensures the received tcp hdr option
> and the sk local storage have the expected values. It uses memcmp
> to check that. Testing the memcmp result with ASSERT_OK is confusing
> because ASSERT_OK will print out the errno which is not set.
> This patch uses ASSERT_EQ to check for 0 instead.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] selftests/bpf: Use ASSERT_EQ instead ASSERT_OK for testing memcmp result
    https://git.kernel.org/bpf/bpf-next/c/ed01385c0d78
  - [bpf-next,2/2] selftests/bpf: Fix a fd leak in an error path in network_helpers.c
    https://git.kernel.org/bpf/bpf-next/c/226efec2b0ef

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html