| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 3 Nov 2020 08:01:08 -0800
From: Alexander Duyck <alexander.duyck@...il.com>
To: Martin KaFai Lau <kafai@...com>
Cc: bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
John Fastabend <john.fastabend@...il.com>,
Kernel Team <kernel-team@...com>,
Netdev <netdev@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Lawrence Brakmo <brakmo@...com>,
Andrii Nakryiko <andrii.nakryiko@...il.com>,
alexanderduyck@...com
Subject: Re: [bpf-next PATCH v2 2/5] selftests/bpf: Drop python client/server
in favor of threads
On Mon, Nov 2, 2020 at 5:33 PM Martin KaFai Lau <kafai@...com> wrote:
>
> On Mon, Nov 02, 2020 at 04:49:42PM -0800, Alexander Duyck wrote:
> > On Mon, Nov 2, 2020 at 4:38 PM Martin KaFai Lau <kafai@...com> wrote:
> > >
> > > On Sat, Oct 31, 2020 at 11:52:18AM -0700, Alexander Duyck wrote:
> > > > From: Alexander Duyck <alexanderduyck@...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.
> > >
> > > > 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@...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.
Powered by blists - more mailing lists