* [PATCH net] selftests/net: report etf errors correctly
@ 2020-06-18 14:55 Willem de Bruijn
2020-06-18 15:54 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Willem de Bruijn @ 2020-06-18 14:55 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
The ETF qdisc can queue skbs that it could not pace on the errqueue.
Address a few issues in the selftest
- recv buffer size was too small, and incorrectly calculated
- compared errno to ee_code instead of ee_errno
- missed error type invalid request
Fixes: ea6a547669b3 ("selftests/net: make so_txtime more robust to timer variance")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
tools/testing/selftests/net/so_txtime.c | 33 +++++++++++++++++++------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/net/so_txtime.c b/tools/testing/selftests/net/so_txtime.c
index 383bac05ac32..3400afff0c3c 100644
--- a/tools/testing/selftests/net/so_txtime.c
+++ b/tools/testing/selftests/net/so_txtime.c
@@ -15,8 +15,9 @@
#include <inttypes.h>
#include <linux/net_tstamp.h>
#include <linux/errqueue.h>
+#include <linux/if_ether.h>
#include <linux/ipv6.h>
-#include <linux/tcp.h>
+#include <linux/udp.h>
#include <stdbool.h>
#include <stdlib.h>
#include <stdio.h>
@@ -140,8 +141,8 @@ static void do_recv_errqueue_timeout(int fdt)
{
char control[CMSG_SPACE(sizeof(struct sock_extended_err)) +
CMSG_SPACE(sizeof(struct sockaddr_in6))] = {0};
- char data[sizeof(struct ipv6hdr) +
- sizeof(struct tcphdr) + 1];
+ char data[sizeof(struct ethhdr) + sizeof(struct ipv6hdr) +
+ sizeof(struct udphdr) + 1];
struct sock_extended_err *err;
struct msghdr msg = {0};
struct iovec iov = {0};
@@ -159,6 +160,8 @@ static void do_recv_errqueue_timeout(int fdt)
msg.msg_controllen = sizeof(control);
while (1) {
+ const char *reason;
+
ret = recvmsg(fdt, &msg, MSG_ERRQUEUE);
if (ret == -1 && errno == EAGAIN)
break;
@@ -176,14 +179,30 @@ static void do_recv_errqueue_timeout(int fdt)
err = (struct sock_extended_err *)CMSG_DATA(cm);
if (err->ee_origin != SO_EE_ORIGIN_TXTIME)
error(1, 0, "errqueue: origin 0x%x\n", err->ee_origin);
- if (err->ee_code != ECANCELED)
- error(1, 0, "errqueue: code 0x%x\n", err->ee_code);
+
+ switch (err->ee_errno) {
+ case ECANCELED:
+ if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
+ error(1, 0, "errqueue: unknown ECANCELED %u\n",
+ err->ee_code);
+ reason = "missed txtime";
+ break;
+ case EINVAL:
+ if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
+ error(1, 0, "errqueue: unknown EINVAL %u\n",
+ err->ee_code);
+ reason = "invalid txtime";
+ break;
+ default:
+ error(1, 0, "errqueue: errno %u code %u\n",
+ err->ee_errno, err->ee_code);
+ };
tstamp = ((int64_t) err->ee_data) << 32 | err->ee_info;
tstamp -= (int64_t) glob_tstart;
tstamp /= 1000 * 1000;
- fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped\n",
- data[ret - 1], tstamp);
+ fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
+ data[ret - 1], tstamp, reason);
msg.msg_flags = 0;
msg.msg_controllen = sizeof(control);
--
2.27.0.290.gba653c62da-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH net] selftests/net: report etf errors correctly
2020-06-18 14:55 [PATCH net] selftests/net: report etf errors correctly Willem de Bruijn
@ 2020-06-18 15:54 ` Jakub Kicinski
2020-06-18 16:18 ` Willem de Bruijn
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2020-06-18 15:54 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, Willem de Bruijn
On Thu, 18 Jun 2020 10:55:49 -0400 Willem de Bruijn wrote:
> + switch (err->ee_errno) {
> + case ECANCELED:
> + if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
> + error(1, 0, "errqueue: unknown ECANCELED %u\n",
> + err->ee_code);
> + reason = "missed txtime";
> + break;
> + case EINVAL:
> + if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
> + error(1, 0, "errqueue: unknown EINVAL %u\n",
> + err->ee_code);
> + reason = "invalid txtime";
> + break;
> + default:
> + error(1, 0, "errqueue: errno %u code %u\n",
> + err->ee_errno, err->ee_code);
> + };
>
> tstamp = ((int64_t) err->ee_data) << 32 | err->ee_info;
> tstamp -= (int64_t) glob_tstart;
> tstamp /= 1000 * 1000;
> - fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped\n",
> - data[ret - 1], tstamp);
> + fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
> + data[ret - 1], tstamp, reason);
Hi Willem! Checkpatch is grumpy about some misalignment here:
CHECK: Alignment should match open parenthesis
#67: FILE: tools/testing/selftests/net/so_txtime.c:187:
+ error(1, 0, "errqueue: unknown ECANCELED %u\n",
+ err->ee_code);
CHECK: Alignment should match open parenthesis
#73: FILE: tools/testing/selftests/net/so_txtime.c:193:
+ error(1, 0, "errqueue: unknown EINVAL %u\n",
+ err->ee_code);
CHECK: Alignment should match open parenthesis
#87: FILE: tools/testing/selftests/net/so_txtime.c:205:
+ fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
+ data[ret - 1], tstamp, reason);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] selftests/net: report etf errors correctly
2020-06-18 15:54 ` Jakub Kicinski
@ 2020-06-18 16:18 ` Willem de Bruijn
2020-06-18 16:36 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Willem de Bruijn @ 2020-06-18 16:18 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Willem de Bruijn, Network Development, David Miller
On Thu, Jun 18, 2020 at 11:54 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 18 Jun 2020 10:55:49 -0400 Willem de Bruijn wrote:
> > + switch (err->ee_errno) {
> > + case ECANCELED:
> > + if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
> > + error(1, 0, "errqueue: unknown ECANCELED %u\n",
> > + err->ee_code);
> > + reason = "missed txtime";
> > + break;
> > + case EINVAL:
> > + if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
> > + error(1, 0, "errqueue: unknown EINVAL %u\n",
> > + err->ee_code);
> > + reason = "invalid txtime";
> > + break;
> > + default:
> > + error(1, 0, "errqueue: errno %u code %u\n",
> > + err->ee_errno, err->ee_code);
> > + };
> >
> > tstamp = ((int64_t) err->ee_data) << 32 | err->ee_info;
> > tstamp -= (int64_t) glob_tstart;
> > tstamp /= 1000 * 1000;
> > - fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped\n",
> > - data[ret - 1], tstamp);
> > + fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
> > + data[ret - 1], tstamp, reason);
>
> Hi Willem! Checkpatch is grumpy about some misalignment here:
>
> CHECK: Alignment should match open parenthesis
> #67: FILE: tools/testing/selftests/net/so_txtime.c:187:
> + error(1, 0, "errqueue: unknown ECANCELED %u\n",
> + err->ee_code);
>
> CHECK: Alignment should match open parenthesis
> #73: FILE: tools/testing/selftests/net/so_txtime.c:193:
> + error(1, 0, "errqueue: unknown EINVAL %u\n",
> + err->ee_code);
>
> CHECK: Alignment should match open parenthesis
> #87: FILE: tools/testing/selftests/net/so_txtime.c:205:
> + fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
> + data[ret - 1], tstamp, reason);
Thanks for the heads-up, Jakub.
I decided to follow the convention in the file, which is to align with
the start of the string.
Given that, do you want me to resubmit with the revised offset? I'm
fine either way, of course.
Also, which incantation of checkpatch do you use? I did run
checkpatch, without extra args, and it did not warn me about this.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] selftests/net: report etf errors correctly
2020-06-18 16:18 ` Willem de Bruijn
@ 2020-06-18 16:36 ` Jakub Kicinski
2020-06-18 16:43 ` Willem de Bruijn
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2020-06-18 16:36 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, David Miller
On Thu, 18 Jun 2020 12:18:01 -0400 Willem de Bruijn wrote:
> On Thu, Jun 18, 2020 at 11:54 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Thu, 18 Jun 2020 10:55:49 -0400 Willem de Bruijn wrote:
> > > + switch (err->ee_errno) {
> > > + case ECANCELED:
> > > + if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
> > > + error(1, 0, "errqueue: unknown ECANCELED %u\n",
> > > + err->ee_code);
> > > + reason = "missed txtime";
> > > + break;
> > > + case EINVAL:
> > > + if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
> > > + error(1, 0, "errqueue: unknown EINVAL %u\n",
> > > + err->ee_code);
> > > + reason = "invalid txtime";
> > > + break;
> > > + default:
> > > + error(1, 0, "errqueue: errno %u code %u\n",
> > > + err->ee_errno, err->ee_code);
> > > + };
> > >
> > > tstamp = ((int64_t) err->ee_data) << 32 | err->ee_info;
> > > tstamp -= (int64_t) glob_tstart;
> > > tstamp /= 1000 * 1000;
> > > - fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped\n",
> > > - data[ret - 1], tstamp);
> > > + fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
> > > + data[ret - 1], tstamp, reason);
> >
> > Hi Willem! Checkpatch is grumpy about some misalignment here:
> >
> > CHECK: Alignment should match open parenthesis
> > #67: FILE: tools/testing/selftests/net/so_txtime.c:187:
> > + error(1, 0, "errqueue: unknown ECANCELED %u\n",
> > + err->ee_code);
> >
> > CHECK: Alignment should match open parenthesis
> > #73: FILE: tools/testing/selftests/net/so_txtime.c:193:
> > + error(1, 0, "errqueue: unknown EINVAL %u\n",
> > + err->ee_code);
> >
> > CHECK: Alignment should match open parenthesis
> > #87: FILE: tools/testing/selftests/net/so_txtime.c:205:
> > + fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
> > + data[ret - 1], tstamp, reason);
>
> Thanks for the heads-up, Jakub.
>
> I decided to follow the convention in the file, which is to align with
> the start of the string.
Ack, I remember the selftest was added with a larger series so I didn't
want to complain about minutia :)
> Given that, do you want me to resubmit with the revised offset? I'm
> fine either way, of course.
No strong feelings, perhaps it's fine if the rest of the file is
like that already.
> Also, which incantation of checkpatch do you use? I did run
> checkpatch, without extra args, and it did not warn me about this.
I run with --strict, and pick the warnings which make sense.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net] selftests/net: report etf errors correctly
2020-06-18 16:36 ` Jakub Kicinski
@ 2020-06-18 16:43 ` Willem de Bruijn
0 siblings, 0 replies; 5+ messages in thread
From: Willem de Bruijn @ 2020-06-18 16:43 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Willem de Bruijn, Network Development, David Miller
On Thu, Jun 18, 2020 at 12:36 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 18 Jun 2020 12:18:01 -0400 Willem de Bruijn wrote:
> > On Thu, Jun 18, 2020 at 11:54 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Thu, 18 Jun 2020 10:55:49 -0400 Willem de Bruijn wrote:
> > > > + switch (err->ee_errno) {
> > > > + case ECANCELED:
> > > > + if (err->ee_code != SO_EE_CODE_TXTIME_MISSED)
> > > > + error(1, 0, "errqueue: unknown ECANCELED %u\n",
> > > > + err->ee_code);
> > > > + reason = "missed txtime";
> > > > + break;
> > > > + case EINVAL:
> > > > + if (err->ee_code != SO_EE_CODE_TXTIME_INVALID_PARAM)
> > > > + error(1, 0, "errqueue: unknown EINVAL %u\n",
> > > > + err->ee_code);
> > > > + reason = "invalid txtime";
> > > > + break;
> > > > + default:
> > > > + error(1, 0, "errqueue: errno %u code %u\n",
> > > > + err->ee_errno, err->ee_code);
> > > > + };
> > > >
> > > > tstamp = ((int64_t) err->ee_data) << 32 | err->ee_info;
> > > > tstamp -= (int64_t) glob_tstart;
> > > > tstamp /= 1000 * 1000;
> > > > - fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped\n",
> > > > - data[ret - 1], tstamp);
> > > > + fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
> > > > + data[ret - 1], tstamp, reason);
> > >
> > > Hi Willem! Checkpatch is grumpy about some misalignment here:
> > >
> > > CHECK: Alignment should match open parenthesis
> > > #67: FILE: tools/testing/selftests/net/so_txtime.c:187:
> > > + error(1, 0, "errqueue: unknown ECANCELED %u\n",
> > > + err->ee_code);
> > >
> > > CHECK: Alignment should match open parenthesis
> > > #73: FILE: tools/testing/selftests/net/so_txtime.c:193:
> > > + error(1, 0, "errqueue: unknown EINVAL %u\n",
> > > + err->ee_code);
> > >
> > > CHECK: Alignment should match open parenthesis
> > > #87: FILE: tools/testing/selftests/net/so_txtime.c:205:
> > > + fprintf(stderr, "send: pkt %c at %" PRId64 "ms dropped: %s\n",
> > > + data[ret - 1], tstamp, reason);
> >
> > Thanks for the heads-up, Jakub.
> >
> > I decided to follow the convention in the file, which is to align with
> > the start of the string.
>
> Ack, I remember the selftest was added with a larger series so I didn't
> want to complain about minutia :)
>
> > Given that, do you want me to resubmit with the revised offset? I'm
> > fine either way, of course.
>
> No strong feelings, perhaps it's fine if the rest of the file is
> like that already.
We'll have to standardize at some point anyway. Sent a v2.
>
> > Also, which incantation of checkpatch do you use? I did run
> > checkpatch, without extra args, and it did not warn me about this.
>
> I run with --strict, and pick the warnings which make sense.
Ah, thanks. I've updated my bash alias to do the same from now on. The
PRId64 CamelCase warning is a false positive I'll have to leave as is.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-18 16:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-18 14:55 [PATCH net] selftests/net: report etf errors correctly Willem de Bruijn
2020-06-18 15:54 ` Jakub Kicinski
2020-06-18 16:18 ` Willem de Bruijn
2020-06-18 16:36 ` Jakub Kicinski
2020-06-18 16:43 ` Willem de Bruijn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox