| Message ID | 20230131210051.475983-4-andrei.gherzan@canonical.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net-next,v3,1/4] selftests: net: udpgso_bench_rx: Fix 'used uninitialized' compiler warning | expand |
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>
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
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");
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(-)