From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: "sunyucong@gmail.com" <sunyucong@gmail.com>
Cc: Jiang Wang <jiang.wang@bytedance.com>,
Cong Wang <cong.wang@bytedance.com>,
Yucong Sun <fallentree@fb.com>,
Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: reduce more flakyness in sockmap_listen
Date: Thu, 26 Aug 2021 14:05:30 -0700 [thread overview]
Message-ID: <CAADnVQ+u_vzMmftV4YoTs42HSia4L6DjDc++wP9Bd03n8PVtKw@mail.gmail.com> (raw)
In-Reply-To: <CAJygYd2aK_s6x4KO71G0KQLdMr5z07hAPqu5fsj+cQpxUw+7tw@mail.gmail.com>
On Thu, Aug 26, 2021 at 12:24 PM sunyucong@gmail.com
<sunyucong@gmail.com> wrote:
>
> I don't think it's AF_UNIX alone, I'm getting select() timeout for all family:
>
> ./test_progs:udp_redir_to_connected:1775: ingress: read: Timer expired
> udp_redir_to_connected:FAIL:1775
> #120/36 sockmap_listen/sockmap IPv4 test_udp_redir:FAIL
> ./test_progs:inet_unix_redir_to_connected:1865: ingress: read: Timer expired
> inet_unix_redir_to_connected:FAIL:1865
That's something different. It's ETIME and not EAGAIN.
Do you see IO_TIMEOUT_SEC==30 seconds elapsed between these lines?
No matter how slow the qemu setup is, the test shouldn't wait that long.
> ./test_progs:inet_unix_redir_to_connected:1865: ingress: read: Timer expired
> inet_unix_redir_to_connected:FAIL:1865
> ./test_progs:unix_inet_redir_to_connected:1947: ingress: read: Timer expired
> unix_inet_redir_to_connected:FAIL:1947
> ./test_progs:unix_inet_redir_to_connected:1947: ingress: read: Timer expired
> unix_inet_redir_to_connected:FAIL:1947
> ...
> ./test_progs:udp_redir_to_connected:1775: ingress: read: Timer expired
> udp_redir_to_connected:FAIL:1775
> #120/73 sockmap_listen/sockmap IPv6 test_udp_redir:FAIL
> ./test_progs:inet_unix_redir_to_connected:1865: ingress: read: Timer expired
> inet_unix_redir_to_connected:FAIL:1865
> ./test_progs:inet_unix_redir_to_connected:1865: ingress: read: Timer expired
> inet_unix_redir_to_connected:FAIL:1865
> ./test_progs:unix_inet_redir_to_connected:1947: ingress: read: Timer expired
> unix_inet_redir_to_connected:FAIL:1947
> ./test_progs:unix_inet_redir_to_connected:1947: ingress: read: Timer expired
> unix_inet_redir_to_connected:FAIL:1947
> #120/74 sockmap_listen/sockmap IPv6 test_udp_unix_redir:FAIL
> ./test_progs:unix_redir_to_connected:1605: ingress: read: Timer expired
> unix_redir_to_connected:FAIL:1605
> #120/75 sockmap_listen/sockmap Unix test_unix_redir:FAIL
> ./test_progs:unix_redir_to_connected:1605: ingress: read: Timer expired
> unix_redir_to_connected:FAIL:1605
>
> On Thu, Aug 26, 2021 at 12:07 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Thu, Aug 26, 2021 at 11:18 AM sunyucong@gmail.com
> > <sunyucong@gmail.com> wrote:
> > >
> > > Reporting back: I tried a select() based approach, (as attached below)
> > > but unfortunately it doesn't seem to work. During testing, I am
> > > always getting full timeout errors as the socket never seems to become
> > > ready to read(). My guess is that this has something to do with the
> > > sockets being created through sockpair() , but I am unable to confirm.
> > >
> > > On the other hand, the previous patch approach works perfectly fine, I
> > > would still like to request to apply that instead.
> >
> > Ok. Applied your earlier patch, but it's a short term workaround.
> > select() should work for af_unix.
> > I suspect something got broken with the redirect.
> > Cong, Jiang,
> > could you please take a look ?
> >
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> > > b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> > > index 5c5979046523..247e8b7a6911 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> > > @@ -949,7 +949,6 @@ static void redir_to_connected(int family, int
> > > sotype, int sock_mapfd,
> > > int err, n;
> > > u32 key;
> > > char b;
> > > - int retries = 100;
> > >
> > > zero_verdict_count(verd_mapfd);
> > >
> > > @@ -1002,15 +1001,12 @@ static void redir_to_connected(int family, int
> > > sotype, int sock_mapfd,
> > > goto close_peer1;
> > > if (pass != 1)
> > > FAIL("%s: want pass count 1, have %d", log_prefix, pass);
> > > -again:
> > > +
> > > + if (poll_read(c0, IO_TIMEOUT_SEC))
> > > + FAIL_ERRNO("%s: read", log_prefix);
> > > n = read(c0, &b, 1);
> > > - if (n < 0) {
> > > - if (errno == EAGAIN && retries--) {
> > > - usleep(1000);
> > > - goto again;
> > > - }
> > > + if (n < 0)
> > > FAIL_ERRNO("%s: read", log_prefix);
> > > - }
> > > if (n == 0)
> > > FAIL("%s: incomplete read", log_prefix);
> > >
> > > @@ -1571,7 +1567,6 @@ static void unix_redir_to_connected(int sotype,
> > > int sock_mapfd,
> > > const char *log_prefix = redir_mode_str(mode);
> > > int c0, c1, p0, p1;
> > > unsigned int pass;
> > > - int retries = 100;
> > > int err, n;
> > > int sfd[2];
> > > u32 key;
> > > @@ -1606,15 +1601,11 @@ static void unix_redir_to_connected(int
> > > sotype, int sock_mapfd,
> > > if (pass != 1)
> > > FAIL("%s: want pass count 1, have %d", log_prefix, pass);
> > >
> > > -again:
> > > + if (poll_read(mode == REDIR_INGRESS ? p0 : c0, IO_TIMEOUT_SEC))
> > > + FAIL_ERRNO("%s: read", log_prefix);
> > > n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);
> > > - if (n < 0) {
> > > - if (errno == EAGAIN && retries--) {
> > > - usleep(1000);
> > > - goto again;
> > > - }
> > > + if (n < 0)
> > > FAIL_ERRNO("%s: read", log_prefix);
> > > - }
> > > if (n == 0)
> > > FAIL("%s: incomplete read", log_prefix);
> > >
> > > @@ -1748,7 +1739,6 @@ static void udp_redir_to_connected(int family,
> > > int sock_mapfd, int verd_mapfd,
> > > const char *log_prefix = redir_mode_str(mode);
> > > int c0, c1, p0, p1;
> > > unsigned int pass;
> > > - int retries = 100;
> > > int err, n;
> > > u32 key;
> > > char b;
> > > @@ -1781,15 +1771,11 @@ static void udp_redir_to_connected(int family,
> > > int sock_mapfd, int verd_mapfd,
> > > if (pass != 1)
> > > FAIL("%s: want pass count 1, have %d", log_prefix, pass);
> > >
> > > -again:
> > > + if (poll_read(mode == REDIR_INGRESS ? p0 : c0, IO_TIMEOUT_SEC * 10))
> > > + FAIL_ERRNO("%s: read", log_prefix);
> > > n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);
> > > - if (n < 0) {
> > > - if (errno == EAGAIN && retries--) {
> > > - usleep(1000);
> > > - goto again;
> > > - }
> > > + if (n < 0)
> > > FAIL_ERRNO("%s: read", log_prefix);
> > > - }
> > > if (n == 0)
> > > FAIL("%s: incomplete read", log_prefix);
> > >
> > > @@ -1841,7 +1827,6 @@ static void inet_unix_redir_to_connected(int
> > > family, int type, int sock_mapfd,
> > > const char *log_prefix = redir_mode_str(mode);
> > > int c0, c1, p0, p1;
> > > unsigned int pass;
> > > - int retries = 100;
> > > int err, n;
> > > int sfd[2];
> > > u32 key;
> > > @@ -1876,15 +1861,11 @@ static void inet_unix_redir_to_connected(int
> > > family, int type, int sock_mapfd,
> > > if (pass != 1)
> > > FAIL("%s: want pass count 1, have %d", log_prefix, pass);
> > >
> > > -again:
> > > + if (poll_read(mode == REDIR_INGRESS ? p0 : c0, IO_TIMEOUT_SEC))
> > > + FAIL_ERRNO("%s: read", log_prefix);
> > > n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);
> > > - if (n < 0) {
> > > - if (errno == EAGAIN && retries--) {
> > > - usleep(1000);
> > > - goto again;
> > > - }
> > > + if (n < 0)
> > > FAIL_ERRNO("%s: read", log_prefix);
> > > - }
> > > if (n == 0)
> > > FAIL("%s: incomplete read", log_prefix);
> > >
> > > @@ -1932,7 +1913,6 @@ static void unix_inet_redir_to_connected(int
> > > family, int type, int sock_mapfd,
> > > int sfd[2];
> > > u32 key;
> > > char b;
> > > - int retries = 100;
> > >
> > > zero_verdict_count(verd_mapfd);
> > >
> > > @@ -1963,15 +1943,11 @@ static void unix_inet_redir_to_connected(int
> > > family, int type, int sock_mapfd,
> > > if (pass != 1)
> > > FAIL("%s: want pass count 1, have %d", log_prefix, pass);
> > >
> > > -again:
> > > + if (poll_read(mode == REDIR_INGRESS ? p0 : c0, IO_TIMEOUT_SEC))
> > > + FAIL_ERRNO("%s: read", log_prefix);
> > > n = read(mode == REDIR_INGRESS ? p0 : c0, &b, 1);
> > > - if (n < 0) {
> > > - if (errno == EAGAIN && retries--) {
> > > - usleep(1000);
> > > - goto again;
> > > - }
> > > + if (n < 0)
> > > FAIL_ERRNO("%s: read", log_prefix);
> > > - }
> > > if (n == 0)
> > > FAIL("%s: incomplete read", log_prefix);
next prev parent reply other threads:[~2021-08-26 21:05 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-25 18:47 [PATCH bpf-next] selftests/bpf: reduce more flakyness in sockmap_listen Yucong Sun
2021-08-26 1:04 ` Alexei Starovoitov
2021-08-26 2:37 ` sunyucong
2021-08-26 18:18 ` sunyucong
2021-08-26 19:07 ` Alexei Starovoitov
2021-08-26 19:23 ` sunyucong
2021-08-26 21:05 ` Alexei Starovoitov [this message]
2021-08-26 21:07 ` sunyucong
2021-08-31 19:33 ` Cong Wang
2021-09-02 1:33 ` Cong Wang
2021-09-02 3:34 ` sunyucong
2021-09-03 23:44 ` Cong Wang
2021-09-07 2:24 ` sunyucong
2021-09-15 14:13 ` sunyucong
2021-09-15 16:54 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAADnVQ+u_vzMmftV4YoTs42HSia4L6DjDc++wP9Bd03n8PVtKw@mail.gmail.com \
--to=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=cong.wang@bytedance.com \
--cc=fallentree@fb.com \
--cc=jiang.wang@bytedance.com \
--cc=sunyucong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).