Re: [PATCH mptcp-next 3/4] selftests: mptcp: enable io thread mode

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 01/08/2024 11:21, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> This patch adds a new parameter "listensock" for the client main_loop()
> to support io thread mode tests. In it, invoke accept() to get the peer
> socket and pass it to copyfd_io(), then to copyfd_io_thread().

Please add the reason why this is needed.

Also, it is not clear that this will cause the same app to send and
receives data. Is it really required to reproduce the bug? Can we not do
the usual client-server thing? e.g.

  ./mptcp_connect.sh -m thread

Or at least an explicit option.

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../selftests/net/mptcp/mptcp_connect.c       | 25 ++++++++++++++++---
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 477969ba9653..bd61cf203501 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -1372,9 +1372,9 @@ void xdisconnect(int fd, int addrlen)
>  		xerror("can't disconnect: %d", errno);
>  }
>  
> -int main_loop(void)
> +int main_loop(int listensock)
>  {
> -	int fd = 0, ret, fd_in = 0;
> +	int fd = 0, ret, fd_in = 0, peerfd = 1;
>  	struct addrinfo *peer;
>  	struct wstate winfo;
>  
> @@ -1389,6 +1389,19 @@ int main_loop(void)
>  	if (fd < 0)
>  		return 2;
>  
> +	if (cfg_mode == CFG_MODE_THREAD &&
> +	    listensock >= 0) {

(detail: can it not go to the previous line?)

> +		peerfd = accept(listensock, NULL, NULL);
> +		while (peerfd == -1) {
> +			if (errno == EINTR)
> +				continue;

Mmh, do you not need to call 'accept()' again?

> +			return -errno;
> +		}
> +
> +		if (cfg_timeo)
> +			settimeo(peerfd, cfg_timeo);
> +	}
> +
>  again:
>  	check_getpeername_connect(fd);
>  
> @@ -1407,7 +1420,7 @@ int main_loop(void)
>  			xerror("can't open %s:%d", cfg_input, errno);
>  	}
>  
> -	ret = copyfd_io(fd_in, fd, 1, 0, &winfo);
> +	ret = copyfd_io(fd_in, fd, peerfd, 0, &winfo);
>  	if (ret)
>  		return ret;
>  
> @@ -1430,6 +1443,8 @@ int main_loop(void)
>  		close(fd);
>  	}
>  
> +	if (listensock >= 0)
> +		close(listensock);
>  	return 0;
>  }
>  
> @@ -1630,9 +1645,11 @@ int main(int argc, char *argv[])
>  			set_mark(fd, cfg_mark);
>  		if (cfg_cmsg_types.cmsg_enabled)
>  			apply_cmsg_types(fd, &cfg_cmsg_types);
> +		if (cfg_mode == CFG_MODE_THREAD)
> +			return main_loop(fd);
>  
>  		return main_loop_s(fd);
>  	}
>  
> -	return main_loop();
> +	return main_loop(-1);
>  }

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.