On Mon, Nov 2, 2020 at 5:33 PM Martin KaFai Lau [off-list ref] wrote:

On Mon, Nov 02, 2020 at 04:49:42PM -0800, Alexander Duyck wrote:

quoted

On Mon, Nov 2, 2020 at 4:38 PM Martin KaFai Lau [off-list ref] wrote:

quoted

On Sat, Oct 31, 2020 at 11:52:18AM -0700, Alexander Duyck wrote:

quoted

From: Alexander Duyck <alexanderduyck@fb.com>

Drop the tcp_client/server.py files in favor of using a client and server
thread within the test case. Specifically we spawn a new thread to play the

The thread comment may be outdated in v2.

quoted

role of the server, and the main testing thread plays the role of client.

Add logic to the end of the run_test function to guarantee that the sockets
are closed when we begin verifying results.

Doing this we are able to reduce overhead since we don't have two python
workers possibly floating around. In addition we don't have to worry about
synchronization issues and as such the retry loop waiting for the threads
to close the sockets can be dropped as we will have already closed the
sockets in the local executable and synchronized the server thread.

Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
 .../testing/selftests/bpf/prog_tests/tcpbpf_user.c |   96 ++++++++++++++++----
 tools/testing/selftests/bpf/tcp_client.py          |   50 ----------
 tools/testing/selftests/bpf/tcp_server.py          |   80 -----------------
 3 files changed, 78 insertions(+), 148 deletions(-)
 delete mode 100755 tools/testing/selftests/bpf/tcp_client.py
 delete mode 100755 tools/testing/selftests/bpf/tcp_server.py

diff --git a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
index 54f1dce97729..17d4299435df 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcpbpf_user.c

@@ -1,13 +1,14 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <inttypes.h>
 #include <test_progs.h>
+#include <network_helpers.h>

 #include "test_tcpbpf.h"

+#define LO_ADDR6 "::1"
 #define CG_NAME "/tcpbpf-user-test"

-/* 3 comes from one listening socket + both ends of the connection */
-#define EXPECTED_CLOSE_EVENTS                3
+static __u32 duration;

 #define EXPECT_EQ(expected, actual, fmt)                     \
      do {                                                    \

@@ -42,7 +43,9 @@ int verify_result(const struct tcpbpf_globals *result)
      EXPECT_EQ(0x80, result->bad_cb_test_rv, PRIu32);
      EXPECT_EQ(0, result->good_cb_test_rv, PRIu32);
      EXPECT_EQ(1, result->num_listen, PRIu32);
-     EXPECT_EQ(EXPECTED_CLOSE_EVENTS, result->num_close_events, PRIu32);
+
+     /* 3 comes from one listening socket + both ends of the connection */
+     EXPECT_EQ(3, result->num_close_events, PRIu32);

      return ret;
 }

@@ -66,6 +69,75 @@ int verify_sockopt_result(int sock_map_fd)
      return ret;
 }

+static int run_test(void)
+{
+     int listen_fd = -1, cli_fd = -1, accept_fd = -1;
+     char buf[1000];
+     int err = -1;
+     int i;
+
+     listen_fd = start_server(AF_INET6, SOCK_STREAM, LO_ADDR6, 0, 0);
+     if (CHECK(listen_fd == -1, "start_server", "listen_fd:%d errno:%d\n",
+               listen_fd, errno))
+             goto done;
+
+     cli_fd = connect_to_fd(listen_fd, 0);
+     if (CHECK(cli_fd == -1, "connect_to_fd(listen_fd)",
+               "cli_fd:%d errno:%d\n", cli_fd, errno))
+             goto done;
+
+     accept_fd = accept(listen_fd, NULL, NULL);
+     if (CHECK(accept_fd == -1, "accept(listen_fd)",
+               "accept_fd:%d errno:%d\n", accept_fd, errno))
+             goto done;
+
+     /* Send 1000B of '+'s from cli_fd -> accept_fd */
+     for (i = 0; i < 1000; i++)
+             buf[i] = '+';
+
+     err = send(cli_fd, buf, 1000, 0);
+     if (CHECK(err != 1000, "send(cli_fd)", "err:%d errno:%d\n", err, errno))
+             goto done;
+
+     err = recv(accept_fd, buf, 1000, 0);
+     if (CHECK(err != 1000, "recv(accept_fd)", "err:%d errno:%d\n", err, errno))
+             goto done;
+
+     /* Send 500B of '.'s from accept_fd ->cli_fd */
+     for (i = 0; i < 500; i++)
+             buf[i] = '.';
+
+     err = send(accept_fd, buf, 500, 0);
+     if (CHECK(err != 500, "send(accept_fd)", "err:%d errno:%d\n", err, errno))
+             goto done;
+
+     err = recv(cli_fd, buf, 500, 0);

Unlikely, but err from the above send()/recv() could be 0.

Is that an issue? It would still trigger the check below as that is not 500.

Mostly for consistency.  "err" will be returned and tested for non-zero
in test_tcpbpf_user().

Okay that makes sense. Now that I have looked it over more it does
lead to an error in the final product since it will attempt to verify
data on a failed connection so I will probably just replaced err with
a new variable such as rv for the send/recv part of the function so
that err stays at -1 until we are closing the sockets.