Re: [PATCH mptcp-net v2 6/6] selftests: mptcp: connect: trunc: read all recv data

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

On Sun, 2025-11-02 at 12:30 +0100, Matthieu Baerts (NGI0) wrote:
> MPTCP Join "fastclose server" selftest is sometimes failing because
> the
> client output file doesn't have the expected size, e.g. 296B instead
> of
> 1024B.

Thanks for this fix. After repeatedly running the fastclose tests,
patches 5 and 6 have indeed eliminated the flaky failures.

> 
> When looking at a packet trace when this happens, the server sent the
> expected 1024B in two parts -- 100B, then 924B -- then the
> MP_FASTCLOSE.
> It is then strange to see the client only receiving 296B, which would
> mean it only got a part of the second packet. The problem is then not
> on
> the networking side, but rather on the data reception side.
> 
> When mptcp_connect is launched with '-f -1', it means the connection
> might stop before having sent everything, because a reset has been
> received. When this happens, the program was directly stopped. But it
> is
> also possible there are still some data to read, simply because the
> previous 'read' step was done with a buffer smaller than the pending
> data, see do_rnd_read(). In this case, it is important to read what's
> left in the kernel buffers before stopping without error like before.
> 
> SIGPIPE is now ignored, not to quit the app before having read
> everything.
> 
> Fixes: 6bf41020b72b ("selftests: mptcp: update and extend fastclose
> test-cases")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> v2:
>  - Also catch EPIPE, and ignore SIGPIPE
> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.c | 18
> +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index c030b08a7195..404a77bf366a 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -710,8 +710,14 @@ static int copyfd_io_poll(int infd, int peerfd,
> int outfd,
>  
>  				bw = do_rnd_write(peerfd, winfo->buf
> + winfo->off, winfo->len);
>  				if (bw < 0) {
> -					if (cfg_rcv_trunc)
> -						return 0;
> +					/* expected reset, continue
> to read */
> +					if (cfg_rcv_trunc &&
> +					    (errno == ECONNRESET ||
> +					     errno == EPIPE)) {
> +						fds.events &=
> ~POLLOUT;
> +						continue;
> +					}

We're adjusting the behavior of the falseclose tests here and need to
ensure it doesn't affect other tests. I'm considering adding a new
config like cfg_rcv_ign to encapsulate these changes within an if
(cfg_rcv_ign) block, and only enable this new config in the falseclose
tests.

WDYT?

Thanks,
-Geliang

> +
>  					perror("write");
>  					return 111;
>  				}
> @@ -737,8 +743,10 @@ static int copyfd_io_poll(int infd, int peerfd,
> int outfd,
>  		}
>  
>  		if (fds.revents & (POLLERR | POLLNVAL)) {
> -			if (cfg_rcv_trunc)
> -				return 0;
> +			if (cfg_rcv_trunc) {
> +				fds.events &= ~(POLLERR | POLLNVAL);
> +				continue;
> +			}
>  			fprintf(stderr, "Unexpected revents: "
>  				"POLLERR/POLLNVAL(%x)\n",
> fds.revents);
>  			return 5;
> @@ -1441,7 +1449,7 @@ static void parse_opts(int argc, char **argv)
>  			 */
>  			if (cfg_truncate < 0) {
>  				cfg_rcv_trunc = true;
> -				signal(SIGPIPE, handle_signal);
> +				signal(SIGPIPE, SIG_IGN);
>  			}
>  			break;
>  		case 'j':
>