Re: [PATCH mptcp-next v22 2/8] selftests: mptcp: connect: check TFO SIOCINQ overflow

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] selftests: mptcp: wait_for_remote_key() fails on TCP and
  fallback sockets
- [Low] selftests: mptcp: missing SOL_MPTCP fallback macro in
  mptcp_connect.c
--

commit 4c8b1264330b7ada03e819d1d1820de00d3fda02
Author: Geliang Tang <geliang@kernel.org>

selftests: mptcp: connect: check TFO SIOCINQ overflow

This commit adds a check in mptcp_connect's server loop to wait for the
remote key on MPTFO connections and verify that SIOCINQ does not return
INT_MAX.

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 178d98d91fea2..41eff818e950e 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c

[ ... ]

> @@ -1176,6 +1177,24 @@ static void maybe_close(int fd)
>  		close(fd);
>  }
>  
> +static bool wait_for_remote_key(int fd)
> +{
> +	struct mptcp_info info;
> +	int retries = 10;
> +	socklen_t olen;
> +
> +	while (retries > 0) {
> +		olen = sizeof(info);
> +		if (getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &info, &olen) < 0)

[Severity: Low]
Does SOL_MPTCP need a fallback definition in mptcp_connect.c?  SOL_MPTCP is
not defined in the UAPI headers, so building against older libc headers
(prior to glibc 2.34) will fail with an undeclared identifier error unless
it is guarded with #ifndef SOL_MPTCP like IPPROTO_MPTCP and TCP_ULP at the
top of this file (and SOL_MPTCP in mptcp_inq.c and mptcp_sockopt.c).

> +			return false;
> +		if (info.mptcpi_flags & MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED)
> +			return true;
> +		usleep(10000);
> +		retries--;
> +	}
> +	return false;
> +}
> +
>  int main_loop_s(int listensock)
>  {
>  	struct sockaddr_storage ss;
> @@ -1207,6 +1226,24 @@ int main_loop_s(int listensock)
>  		check_sockaddr(pf, &ss, salen);
>  		check_getpeername(remotesock, &ss, salen);
>  
> +		if (cfg_sockopt_types.mptfo) {
> +			int avail;
> +
> +			if (!wait_for_remote_key(remotesock)) {
> +				fprintf(stderr,
> +					"TFO: timeout waiting for third ACK\n");
> +				close(remotesock);
> +				return 1;
> +			}

[Severity: Low]
Will wait_for_remote_key() fail when remotesock is a plain TCP socket or an
MPTCP socket that fell back to TCP?

In mptcp_connect.sh, run_tests_mptfo() passes "-o MPTFO" to run_tests_lo(),
which also runs MPTCP->TCP, TCP->MPTCP, and TCP->TCP transfers when invoked
with -t:

tools/testing/selftests/net/mptcp/mptcp_connect.sh:run_tests_lo() {
    ...
	do_transfer ${listener_ns} ${connector_ns} MPTCP TCP \
		    ${connect_addr} ${local_addr} "${extra_args}"
    ...
	do_transfer ${listener_ns} ${connector_ns} TCP MPTCP \
		    ${connect_addr} ${local_addr} "${extra_args}"
    ...
}

On a TCP listener (-s TCP) or an MPTCP listener accepting a connection from
a TCP client, getsockopt(fd, SOL_MPTCP, MPTCP_INFO, ...) fails immediately
with ENOPROTOOPT, causing wait_for_remote_key() to return false and abort
the test with "TFO: timeout waiting for third ACK".

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793964796.git.tanggeliang@kylinos.cn?part=2