2023-02-01 00:17:55

by Andrei Gherzan

[permalink] [raw]
Subject: [PATCH net v4 1/4] selftests: net: udpgso_bench_rx: Fix 'used uninitialized' compiler warning

This change fixes the following compiler warning:

/usr/include/x86_64-linux-gnu/bits/error.h:40:5: warning: ‘gso_size’ may
be used uninitialized [-Wmaybe-uninitialized]
40 | __error_noreturn (__status, __errnum, __format,
__va_arg_pack ());
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
udpgso_bench_rx.c: In function ‘main’:
udpgso_bench_rx.c:253:23: note: ‘gso_size’ was declared here
253 | int ret, len, gso_size, budget = 256;

Fixes: 3327a9c46352 ("selftests: add functionals test for UDP GRO")
Signed-off-by: Andrei Gherzan <[email protected]>
---
tools/testing/selftests/net/udpgso_bench_rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c
index 6a193425c367..d0895bd1933f 100644
--- a/tools/testing/selftests/net/udpgso_bench_rx.c
+++ b/tools/testing/selftests/net/udpgso_bench_rx.c
@@ -250,7 +250,7 @@ static int recv_msg(int fd, char *buf, int len, int *gso_size)
static void do_flush_udp(int fd)
{
static char rbuf[ETH_MAX_MTU];
- int ret, len, gso_size, budget = 256;
+ int ret, len, gso_size = 0, budget = 256;

len = cfg_read_all ? sizeof(rbuf) : 0;
while (budget--) {
--
2.34.1



2023-02-01 00:18:22

by Andrei Gherzan

[permalink] [raw]
Subject: [PATCH net v4 2/4] selftests: net: udpgso_bench_rx/tx: Stop when wrong CLI args are provided

Leaving unrecognized arguments buried in the output, can easily hide a
CLI/script typo. Avoid this by exiting when wrong arguments are provided to
the udpgso_bench test programs.

Fixes: 3a687bef148d ("selftests: udp gso benchmark")
Signed-off-by: Andrei Gherzan <[email protected]>
Cc: Willem de Bruijn <[email protected]>
---
tools/testing/selftests/net/udpgso_bench_rx.c | 2 ++
tools/testing/selftests/net/udpgso_bench_tx.c | 2 ++
2 files changed, 4 insertions(+)

diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c b/tools/testing/selftests/net/udpgso_bench_rx.c
index d0895bd1933f..4058c7451e70 100644
--- a/tools/testing/selftests/net/udpgso_bench_rx.c
+++ b/tools/testing/selftests/net/udpgso_bench_rx.c
@@ -336,6 +336,8 @@ static void parse_opts(int argc, char **argv)
cfg_verify = true;
cfg_read_all = true;
break;
+ default:
+ exit(1);
}
}

diff --git a/tools/testing/selftests/net/udpgso_bench_tx.c b/tools/testing/selftests/net/udpgso_bench_tx.c
index f1fdaa270291..b47b5c32039f 100644
--- a/tools/testing/selftests/net/udpgso_bench_tx.c
+++ b/tools/testing/selftests/net/udpgso_bench_tx.c
@@ -490,6 +490,8 @@ static void parse_opts(int argc, char **argv)
case 'z':
cfg_zerocopy = true;
break;
+ default:
+ exit(1);
}
}

--
2.34.1


2023-02-01 00:19:02

by Andrei Gherzan

[permalink] [raw]
Subject: [PATCH net v4 3/4] selftests: net: udpgso_bench: Fix racing bug between the rx/tx programs

"udpgro_bench.sh" invokes udpgso_bench_rx/udpgso_bench_tx programs
subsequently and while doing so, there is a chance that the rx one is not
ready to accept socket connections. This racing bug could fail the test
with at least one of the following:

./udpgso_bench_tx: connect: Connection refused
./udpgso_bench_tx: sendmsg: Connection refused
./udpgso_bench_tx: write: Connection refused

This change addresses this by making udpgro_bench.sh wait for the rx
program to be ready before firing off the tx one - up to a 10s timeout.

Fixes: 3a687bef148d ("selftests: udp gso benchmark")
Signed-off-by: Andrei Gherzan <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Willem de Bruijn <[email protected]>
---
tools/testing/selftests/net/udpgso_bench.sh | 24 +++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/udpgso_bench.sh b/tools/testing/selftests/net/udpgso_bench.sh
index dc932fd65363..640bc43452fa 100755
--- a/tools/testing/selftests/net/udpgso_bench.sh
+++ b/tools/testing/selftests/net/udpgso_bench.sh
@@ -7,6 +7,7 @@ readonly GREEN='\033[0;92m'
readonly YELLOW='\033[0;33m'
readonly RED='\033[0;31m'
readonly NC='\033[0m' # No Color
+readonly TESTPORT=8000

readonly KSFT_PASS=0
readonly KSFT_FAIL=1
@@ -56,11 +57,26 @@ trap wake_children EXIT

run_one() {
local -r args=$@
+ local nr_socks=0
+ local i=0
+ local -r timeout=10
+
+ ./udpgso_bench_rx -p "$TESTPORT" &
+ ./udpgso_bench_rx -p "$TESTPORT" -t &
+
+ # Wait for the above test program to get ready to receive connections.
+ while [ "$i" -lt "$timeout" ]; do
+ nr_socks="$(ss -lnHi | grep -c "\*:${TESTPORT}")"
+ [ "$nr_socks" -eq 2 ] && break
+ i=$((i + 1))
+ sleep 1
+ done
+ if [ "$nr_socks" -ne 2 ]; then
+ echo "timed out while waiting for udpgso_bench_rx"
+ exit 1
+ fi

- ./udpgso_bench_rx &
- ./udpgso_bench_rx -t &
-
- ./udpgso_bench_tx ${args}
+ ./udpgso_bench_tx -p "$TESTPORT" ${args}
}

run_in_netns() {
--
2.34.1


2023-02-01 00:19:28

by Andrei Gherzan

[permalink] [raw]
Subject: [PATCH net v4 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking

The test tool can check that the zerocopy number of completions value is
valid taking into consideration the number of datagram send calls. This can
catch the system into a state where the datagrams are still in the system
(for example in a qdisk, waiting for the network interface to return a
completion notification, etc).

This change adds a retry logic of computing the number of completions up to
a configurable (via CLI) timeout (default: 2 seconds).

Fixes: 79ebc3c26010 ("net/udpgso_bench_tx: options to exercise TX CMSG")
Signed-off-by: Andrei Gherzan <[email protected]>
Cc: Willem de Bruijn <[email protected]>
Cc: Paolo Abeni <[email protected]>
---
tools/testing/selftests/net/udpgso_bench_tx.c | 34 +++++++++++++++----
1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/udpgso_bench_tx.c b/tools/testing/selftests/net/udpgso_bench_tx.c
index b47b5c32039f..477392715a9a 100644
--- a/tools/testing/selftests/net/udpgso_bench_tx.c
+++ b/tools/testing/selftests/net/udpgso_bench_tx.c
@@ -62,6 +62,7 @@ static int cfg_payload_len = (1472 * 42);
static int cfg_port = 8000;
static int cfg_runtime_ms = -1;
static bool cfg_poll;
+static int cfg_poll_loop_timeout_ms = 2000;
static bool cfg_segment;
static bool cfg_sendmmsg;
static bool cfg_tcp;
@@ -235,16 +236,17 @@ static void flush_errqueue_recv(int fd)
}
}

-static void flush_errqueue(int fd, const bool do_poll)
+static void flush_errqueue(int fd, const bool do_poll,
+ unsigned long poll_timeout, const bool poll_err)
{
if (do_poll) {
struct pollfd fds = {0};
int ret;

fds.fd = fd;
- ret = poll(&fds, 1, 500);
+ ret = poll(&fds, 1, poll_timeout);
if (ret == 0) {
- if (cfg_verbose)
+ if ((cfg_verbose) && (poll_err))
fprintf(stderr, "poll timeout\n");
} else if (ret < 0) {
error(1, errno, "poll");
@@ -254,6 +256,20 @@ static void flush_errqueue(int fd, const bool do_poll)
flush_errqueue_recv(fd);
}

+static void flush_errqueue_retry(int fd, unsigned long num_sends)
+{
+ unsigned long tnow, tstop;
+ bool first_try = true;
+
+ tnow = gettimeofday_ms();
+ tstop = tnow + cfg_poll_loop_timeout_ms;
+ do {
+ flush_errqueue(fd, true, tstop - tnow, first_try);
+ first_try = false;
+ tnow = gettimeofday_ms();
+ } while ((stat_zcopies != num_sends) && (tnow < tstop));
+}
+
static int send_tcp(int fd, char *data)
{
int ret, done = 0, count = 0;
@@ -413,7 +429,8 @@ static int send_udp_segment(int fd, char *data)

static void usage(const char *filepath)
{
- error(1, 0, "Usage: %s [-46acmHPtTuvz] [-C cpu] [-D dst ip] [-l secs] [-M messagenr] [-p port] [-s sendsize] [-S gsosize]",
+ error(1, 0, "Usage: %s [-46acmHPtTuvz] [-C cpu] [-D dst ip] [-l secs] "
+ "[-L secs] [-M messagenr] [-p port] [-s sendsize] [-S gsosize]",
filepath);
}

@@ -423,7 +440,7 @@ static void parse_opts(int argc, char **argv)
int max_len, hdrlen;
int c;

- while ((c = getopt(argc, argv, "46acC:D:Hl:mM:p:s:PS:tTuvz")) != -1) {
+ while ((c = getopt(argc, argv, "46acC:D:Hl:L:mM:p:s:PS:tTuvz")) != -1) {
switch (c) {
case '4':
if (cfg_family != PF_UNSPEC)
@@ -452,6 +469,9 @@ static void parse_opts(int argc, char **argv)
case 'l':
cfg_runtime_ms = strtoul(optarg, NULL, 10) * 1000;
break;
+ case 'L':
+ cfg_poll_loop_timeout_ms = strtoul(optarg, NULL, 10) * 1000;
+ break;
case 'm':
cfg_sendmmsg = true;
break;
@@ -679,7 +699,7 @@ int main(int argc, char **argv)
num_sends += send_udp(fd, buf[i]);
num_msgs++;
if ((cfg_zerocopy && ((num_msgs & 0xF) == 0)) || cfg_tx_tstamp)
- flush_errqueue(fd, cfg_poll);
+ flush_errqueue(fd, cfg_poll, 500, true);

if (cfg_msg_nr && num_msgs >= cfg_msg_nr)
break;
@@ -698,7 +718,7 @@ int main(int argc, char **argv)
} while (!interrupted && (cfg_runtime_ms == -1 || tnow < tstop));

if (cfg_zerocopy || cfg_tx_tstamp)
- flush_errqueue(fd, true);
+ flush_errqueue_retry(fd, num_sends);

if (close(fd))
error(1, errno, "close");
--
2.34.1


2023-02-01 14:55:20

by Willem de Bruijn

[permalink] [raw]
Subject: Re: [PATCH net v4 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking

On Tue, Jan 31, 2023 at 7:19 PM Andrei Gherzan
<[email protected]> wrote:
>
> The test tool can check that the zerocopy number of completions value is
> valid taking into consideration the number of datagram send calls. This can
> catch the system into a state where the datagrams are still in the system
> (for example in a qdisk, waiting for the network interface to return a
> completion notification, etc).
>
> This change adds a retry logic of computing the number of completions up to
> a configurable (via CLI) timeout (default: 2 seconds).
>
> Fixes: 79ebc3c26010 ("net/udpgso_bench_tx: options to exercise TX CMSG")
> Signed-off-by: Andrei Gherzan <[email protected]>
> Cc: Willem de Bruijn <[email protected]>
> Cc: Paolo Abeni <[email protected]>

Reviewed-by: Willem de Bruijn <[email protected]>

> ---
> tools/testing/selftests/net/udpgso_bench_tx.c | 34 +++++++++++++++----
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/net/udpgso_bench_tx.c b/tools/testing/selftests/net/udpgso_bench_tx.c
> index b47b5c32039f..477392715a9a 100644
> --- a/tools/testing/selftests/net/udpgso_bench_tx.c
> +++ b/tools/testing/selftests/net/udpgso_bench_tx.c
> @@ -62,6 +62,7 @@ static int cfg_payload_len = (1472 * 42);
> static int cfg_port = 8000;
> static int cfg_runtime_ms = -1;
> static bool cfg_poll;
> +static int cfg_poll_loop_timeout_ms = 2000;
> static bool cfg_segment;
> static bool cfg_sendmmsg;
> static bool cfg_tcp;
> @@ -235,16 +236,17 @@ static void flush_errqueue_recv(int fd)
> }
> }
>
> -static void flush_errqueue(int fd, const bool do_poll)
> +static void flush_errqueue(int fd, const bool do_poll,
> + unsigned long poll_timeout, const bool poll_err)
> {
> if (do_poll) {
> struct pollfd fds = {0};
> int ret;
>
> fds.fd = fd;
> - ret = poll(&fds, 1, 500);
> + ret = poll(&fds, 1, poll_timeout);
> if (ret == 0) {
> - if (cfg_verbose)
> + if ((cfg_verbose) && (poll_err))

small nit: unnecessary parentheses

2023-02-01 14:55:35

by Willem de Bruijn

[permalink] [raw]
Subject: Re: [PATCH net v4 3/4] selftests: net: udpgso_bench: Fix racing bug between the rx/tx programs

On Tue, Jan 31, 2023 at 7:18 PM Andrei Gherzan
<[email protected]> wrote:
>
> "udpgro_bench.sh" invokes udpgso_bench_rx/udpgso_bench_tx programs
> subsequently and while doing so, there is a chance that the rx one is not
> ready to accept socket connections. This racing bug could fail the test
> with at least one of the following:
>
> ./udpgso_bench_tx: connect: Connection refused
> ./udpgso_bench_tx: sendmsg: Connection refused
> ./udpgso_bench_tx: write: Connection refused
>
> This change addresses this by making udpgro_bench.sh wait for the rx
> program to be ready before firing off the tx one - up to a 10s timeout.
>
> Fixes: 3a687bef148d ("selftests: udp gso benchmark")
> Signed-off-by: Andrei Gherzan <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: Willem de Bruijn <[email protected]>

Reviewed-by: Willem de Bruijn <[email protected]>

2023-02-01 14:56:04

by Willem de Bruijn

[permalink] [raw]
Subject: Re: [PATCH net v4 2/4] selftests: net: udpgso_bench_rx/tx: Stop when wrong CLI args are provided

On Tue, Jan 31, 2023 at 7:18 PM Andrei Gherzan
<[email protected]> wrote:
>
> Leaving unrecognized arguments buried in the output, can easily hide a
> CLI/script typo. Avoid this by exiting when wrong arguments are provided to
> the udpgso_bench test programs.
>
> Fixes: 3a687bef148d ("selftests: udp gso benchmark")
> Signed-off-by: Andrei Gherzan <[email protected]>
> Cc: Willem de Bruijn <[email protected]>

Reviewed-by: Willem de Bruijn <[email protected]>

2023-02-01 14:56:26

by Willem de Bruijn

[permalink] [raw]
Subject: Re: [PATCH net v4 1/4] selftests: net: udpgso_bench_rx: Fix 'used uninitialized' compiler warning

On Tue, Jan 31, 2023 at 7:18 PM Andrei Gherzan
<[email protected]> wrote:
>
> This change fixes the following compiler warning:
>
> /usr/include/x86_64-linux-gnu/bits/error.h:40:5: warning: ‘gso_size’ may
> be used uninitialized [-Wmaybe-uninitialized]
> 40 | __error_noreturn (__status, __errnum, __format,
> __va_arg_pack ());
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> udpgso_bench_rx.c: In function ‘main’:
> udpgso_bench_rx.c:253:23: note: ‘gso_size’ was declared here
> 253 | int ret, len, gso_size, budget = 256;
>
> Fixes: 3327a9c46352 ("selftests: add functionals test for UDP GRO")
> Signed-off-by: Andrei Gherzan <[email protected]>

Reviewed-by: Willem de Bruijn <[email protected]>

2023-02-02 12:45:48

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v4 1/4] selftests: net: udpgso_bench_rx: Fix 'used uninitialized' compiler warning

Hello:

This series was applied to netdev/net.git (master)
by Paolo Abeni <[email protected]>:

On Wed, 1 Feb 2023 00:16:10 +0000 you wrote:
> This change fixes the following compiler warning:
>
> /usr/include/x86_64-linux-gnu/bits/error.h:40:5: warning: ‘gso_size’ may
> be used uninitialized [-Wmaybe-uninitialized]
> 40 | __error_noreturn (__status, __errnum, __format,
> __va_arg_pack ());
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> udpgso_bench_rx.c: In function ‘main’:
> udpgso_bench_rx.c:253:23: note: ‘gso_size’ was declared here
> 253 | int ret, len, gso_size, budget = 256;
>
> [...]

Here is the summary with links:
- [net,v4,1/4] selftests: net: udpgso_bench_rx: Fix 'used uninitialized' compiler warning
https://git.kernel.org/netdev/net/c/c03c80e3a03f
- [net,v4,2/4] selftests: net: udpgso_bench_rx/tx: Stop when wrong CLI args are provided
https://git.kernel.org/netdev/net/c/db9b47ee9f5f
- [net,v4,3/4] selftests: net: udpgso_bench: Fix racing bug between the rx/tx programs
https://git.kernel.org/netdev/net/c/dafe93b9ee21
- [net,v4,4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking
https://git.kernel.org/netdev/net/c/329c9cd769c2

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