* [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking
@ 2023-01-31 21:00 Andrei Gherzan
2023-01-31 21:47 ` Willem de Bruijn
2023-01-31 21:51 ` Willem de Bruijn
0 siblings, 2 replies; 6+ messages in thread
From: Andrei Gherzan @ 2023-01-31 21:00 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Fred Klassen
Cc: Willem de Bruijn, Andrei Gherzan, netdev, linux-kselftest,
linux-kernel
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 <andrei.gherzan@canonical.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Paolo Abeni <pabeni@redhat.com>
---
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..ef887842522a 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking
2023-01-31 21:00 [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking Andrei Gherzan
@ 2023-01-31 21:47 ` Willem de Bruijn
2023-01-31 21:51 ` Willem de Bruijn
1 sibling, 0 replies; 6+ messages in thread
From: Willem de Bruijn @ 2023-01-31 21:47 UTC (permalink / raw)
To: Andrei Gherzan
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Fred Klassen, netdev, linux-kselftest, linux-kernel
On Tue, Jan 31, 2023 at 4:01 PM Andrei Gherzan
<andrei.gherzan@canonical.com> 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 <andrei.gherzan@canonical.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Paolo Abeni <pabeni@redhat.com>
Fixes should go to net, instead of net-next.
But the code itself looks good to me.
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking
2023-01-31 21:00 [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking Andrei Gherzan
2023-01-31 21:47 ` Willem de Bruijn
@ 2023-01-31 21:51 ` Willem de Bruijn
2023-01-31 22:16 ` Andrei Gherzan
1 sibling, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2023-01-31 21:51 UTC (permalink / raw)
To: Andrei Gherzan
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Fred Klassen, netdev, linux-kselftest, linux-kernel
On Tue, Jan 31, 2023 at 4:01 PM Andrei Gherzan
<andrei.gherzan@canonical.com> 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 <andrei.gherzan@canonical.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Paolo Abeni <pabeni@redhat.com>
> ---
> 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..ef887842522a 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)
nit: his indentation looks off though
> {
> 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);
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking
2023-01-31 21:51 ` Willem de Bruijn
@ 2023-01-31 22:16 ` Andrei Gherzan
2023-01-31 22:28 ` Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Gherzan @ 2023-01-31 22:16 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Fred Klassen, netdev, linux-kselftest, linux-kernel
On 23/01/31 04:51PM, Willem de Bruijn wrote:
> On Tue, Jan 31, 2023 at 4:01 PM Andrei Gherzan
> <andrei.gherzan@canonical.com> 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 <andrei.gherzan@canonical.com>
> > Cc: Willem de Bruijn <willemb@google.com>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > ---
> > 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..ef887842522a 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)
>
> nit: his indentation looks off though
This one I've missed but I couldn't find any guidelines on it. Could you
clarify to me what this should be or point me to soem docs? Happy to fix
otherwise. I'm currently using vim smartindent but it is definitely not
in line with what is here already.
>
> > {
> > 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);
> > }
--
Andrei Gherzan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking
2023-01-31 22:16 ` Andrei Gherzan
@ 2023-01-31 22:28 ` Willem de Bruijn
2023-01-31 23:11 ` Andrei Gherzan
0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2023-01-31 22:28 UTC (permalink / raw)
To: Andrei Gherzan
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Fred Klassen, netdev, linux-kselftest, linux-kernel
On Tue, Jan 31, 2023 at 5:16 PM Andrei Gherzan
<andrei.gherzan@canonical.com> wrote:
>
> On 23/01/31 04:51PM, Willem de Bruijn wrote:
> > On Tue, Jan 31, 2023 at 4:01 PM Andrei Gherzan
> > <andrei.gherzan@canonical.com> 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 <andrei.gherzan@canonical.com>
> > > Cc: Willem de Bruijn <willemb@google.com>
> > > Cc: Paolo Abeni <pabeni@redhat.com>
> > > ---
> > > 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..ef887842522a 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)
> >
> > nit: his indentation looks off though
>
> This one I've missed but I couldn't find any guidelines on it. Could you
> clarify to me what this should be or point me to soem docs? Happy to fix
> otherwise. I'm currently using vim smartindent but it is definitely not
> in line with what is here already.
It should align with the parameter above.
https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking
2023-01-31 22:28 ` Willem de Bruijn
@ 2023-01-31 23:11 ` Andrei Gherzan
0 siblings, 0 replies; 6+ messages in thread
From: Andrei Gherzan @ 2023-01-31 23:11 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Fred Klassen, netdev, linux-kselftest, linux-kernel
On 23/01/31 05:28PM, Willem de Bruijn wrote:
> On Tue, Jan 31, 2023 at 5:16 PM Andrei Gherzan
> <andrei.gherzan@canonical.com> wrote:
> >
> > On 23/01/31 04:51PM, Willem de Bruijn wrote:
> > > On Tue, Jan 31, 2023 at 4:01 PM Andrei Gherzan
> > > <andrei.gherzan@canonical.com> 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 <andrei.gherzan@canonical.com>
> > > > Cc: Willem de Bruijn <willemb@google.com>
> > > > Cc: Paolo Abeni <pabeni@redhat.com>
> > > > ---
> > > > 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..ef887842522a 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)
> > >
> > > nit: his indentation looks off though
> >
> > This one I've missed but I couldn't find any guidelines on it. Could you
> > clarify to me what this should be or point me to soem docs? Happy to fix
> > otherwise. I'm currently using vim smartindent but it is definitely not
> > in line with what is here already.
>
> It should align with the parameter above.
Found the roots of the issue - tab stop was 4 so it was rendered
confusing for me. I'll fix and resend including email prefix change (net
vs net next) and the CC footers (they should be Cc: not CC:).
>
> https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
--
Andrei Gherzan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-31 23:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-31 21:00 [PATCH net-next v3 4/4] selftests: net: udpgso_bench_tx: Cater for pending datagrams zerocopy benchmarking Andrei Gherzan
2023-01-31 21:47 ` Willem de Bruijn
2023-01-31 21:51 ` Willem de Bruijn
2023-01-31 22:16 ` Andrei Gherzan
2023-01-31 22:28 ` Willem de Bruijn
2023-01-31 23:11 ` Andrei Gherzan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).