| Message ID | 1634f4575a4a2c8e7621941b633a0be50e65e195.1596815971.git.pabeni@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [net] mptcp: more stable diag self-tests | expand |
Hi Paolo, On 07/08/2020 17:59, Paolo Abeni wrote: > During diag self-tests we introduce long wait in the mptcp test > program to give the script enough time to access the sockets > dump. > > Such wait is introduced after shutting down one sockets end. Since > commit 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine") > if boths sides shutdown the socket is correctly tranistioned (small typo: s/boths/both/ and s/tranistioned/transitioned/ > into CLOSED status. > > As a side effect some sockets are not dumped via the diag interface, > because the socket state (CLOSED) does not match the default filter, and > this cause self-tests instability. > > Address the issue moving the above mentioned wait before shutting > down the socket. I just did a quick test and it seems it fixes the issue on my VM, thanks! I guess it should fix the issue on my CI as well. ===== 8< ===== # selftests: net/mptcp: diag.sh # no msk on netns creation [ ok ] # after MPC handshake [ ok ] # ....chk remote_key [ ok ] # ....chk no fallback [ ok ] # check fallback [ ok ] # many msk socket present [ ok ] ok 4 selftests: net/mptcp: diag.sh ===== 8< ===== > Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests") Your SoB is missing ;-) Also do not hesitate to add: Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/68 > --- > tools/testing/selftests/net/mptcp/mptcp_connect.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c > index cad6f73a5fd0..090620c3e10c 100644 > --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c > +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c > @@ -406,10 +406,11 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd) > > /* ... but we still receive. > * Close our write side, ev. give some time > - * for address notification > + * for address notification and/or checking > + * the current status > */ > - if (cfg_join) > - usleep(400000); > + if (cfg_wait) > + usleep(cfg_wait); Would it be OK for the stability to keep a wait? I guess it would be hard to wait for an event (check counters, etc.) to happen. (if it is possible, it can also be done in other patch, good to have a fix here :-) ) > shutdown(peerfd, SHUT_WR); > } else { > if (errno == EINTR) > @@ -427,7 +428,7 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd) > } > > /* leave some time for late join/announce */ > - if (cfg_wait) > + if (cfg_join) > usleep(cfg_wait); (same here) Cheers, Matt > > close(peerfd); >
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c index cad6f73a5fd0..090620c3e10c 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c @@ -406,10 +406,11 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd) /* ... but we still receive. * Close our write side, ev. give some time - * for address notification + * for address notification and/or checking + * the current status */ - if (cfg_join) - usleep(400000); + if (cfg_wait) + usleep(cfg_wait); shutdown(peerfd, SHUT_WR); } else { if (errno == EINTR) @@ -427,7 +428,7 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd) } /* leave some time for late join/announce */ - if (cfg_wait) + if (cfg_join) usleep(cfg_wait); close(peerfd);
During diag self-tests we introduce long wait in the mptcp test program to give the script enough time to access the sockets dump. Such wait is introduced after shutting down one sockets end. Since commit 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine") if boths sides shutdown the socket is correctly tranistioned into CLOSED status. As a side effect some sockets are not dumped via the diag interface, because the socket state (CLOSED) does not match the default filter, and this cause self-tests instability. Address the issue moving the above mentioned wait before shutting down the socket. Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests") --- tools/testing/selftests/net/mptcp/mptcp_connect.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)