[bpf-next,1/3] selftests/bpf: Export send_byte helper

Commit Message

Geliang Tang April 2, 2024, 6:38 a.m. UTC

From: Geliang Tang <[e-mail]>

The helper send_byte is defined in mptcp.c, sk_lookup.c and tcp_rtt.c.
It makes sense to export it into network_helpers.h to let it can be
used for all BPF selftests.

Signed-off-by: Geliang Tang <[e-mail]>
---
 tools/testing/selftests/bpf/network_helpers.c      |  9 +++++++++
 tools/testing/selftests/bpf/network_helpers.h      |  1 +
 tools/testing/selftests/bpf/prog_tests/mptcp.c     |  7 -------
 tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 13 -------------
 tools/testing/selftests/bpf/prog_tests/tcp_rtt.c   |  7 -------
 5 files changed, 10 insertions(+), 27 deletions(-)

Comments

Martin KaFai Lau April 4, 2024, 6:03 a.m. UTC | #1

On 4/1/24 11:38 PM, Geliang Tang wrote:
> From: Geliang Tang <[e-mail]>
> 
> The helper send_byte is defined in mptcp.c, sk_lookup.c and tcp_rtt.c.
> It makes sense to export it into network_helpers.h to let it can be
> used for all BPF selftests.
> 
> Signed-off-by: Geliang Tang <[e-mail]>
> ---
>   tools/testing/selftests/bpf/network_helpers.c      |  9 +++++++++
>   tools/testing/selftests/bpf/network_helpers.h      |  1 +
>   tools/testing/selftests/bpf/prog_tests/mptcp.c     |  7 -------
>   tools/testing/selftests/bpf/prog_tests/sk_lookup.c | 13 -------------
>   tools/testing/selftests/bpf/prog_tests/tcp_rtt.c   |  7 -------
>   5 files changed, 10 insertions(+), 27 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index 6db27a9088e9..f2f8f7388d7c 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -497,3 +497,12 @@ int get_socket_local_port(int sock_fd)
>   
>   	return -1;
>   }
> +
> +int send_byte(int fd)
> +{
> +	char b = 0x55;
> +
> +	if (!ASSERT_EQ(write(fd, &b, sizeof(b)), 1, "send single byte"))
> +		return -1;
> +	return 0;
> +}

I think it is overkill to have a dedicated helper that only does the error 
checking on the single syscall send/write. I would drop this patch and directly 
use the syscall send/write instead.

Patch

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index 6db27a9088e9..f2f8f7388d7c 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -497,3 +497,12 @@  int get_socket_local_port(int sock_fd)
 
 	return -1;
 }
+
+int send_byte(int fd)
+{
+	char b = 0x55;
+
+	if (!ASSERT_EQ(write(fd, &b, sizeof(b)), 1, "send single byte"))
+		return -1;
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h
index 94b9be24e39b..d7ee2857093c 100644
--- a/tools/testing/selftests/bpf/network_helpers.h
+++ b/tools/testing/selftests/bpf/network_helpers.h
@@ -71,6 +71,7 @@  struct nstoken;
  */
 struct nstoken *open_netns(const char *name);
 void close_netns(struct nstoken *token);
+int send_byte(int fd);
 
 static __u16 csum_fold(__u32 csum)
 {
diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
index 8f8d792307c1..bff13ecec64c 100644
--- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
+++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
@@ -225,13 +225,6 @@  static void test_base(void)
 	close(cgroup_fd);
 }
 
-static void send_byte(int fd)
-{
-	char b = 0x55;
-
-	ASSERT_EQ(write(fd, &b, sizeof(b)), 1, "send single byte");
-}
-
 static int verify_mptcpify(int server_fd, int client_fd)
 {
 	struct __mptcp_info info;
diff --git a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
index 597d0467a926..b4310a5ec1a4 100644
--- a/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
+++ b/tools/testing/selftests/bpf/prog_tests/sk_lookup.c
@@ -282,19 +282,6 @@  static int fill_sk_lookup_ctx(struct bpf_sk_lookup *ctx, const char *local_ip, _
 	return 0;
 }
 
-static int send_byte(int fd)
-{
-	ssize_t n;
-
-	errno = 0;
-	n = send(fd, "a", 1, 0);
-	if (CHECK(n <= 0, "send_byte", "send")) {
-		log_err("failed/partial send");
-		return -1;
-	}
-	return 0;
-}
-
 static int recv_byte(int fd)
 {
 	char buf[1];
diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c
index 8fe84da1b9b4..dcb2d5e9c8db 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcp_rtt.c
@@ -12,13 +12,6 @@  struct tcp_rtt_storage {
 	__u32 icsk_retransmits;
 };
 
-static void send_byte(int fd)
-{
-	char b = 0x55;
-
-	ASSERT_EQ(write(fd, &b, sizeof(b)), 1, "send single byte");
-}
-
 static int wait_for_ack(int fd, int retries)
 {
 	struct tcp_info info;