Check that value x can be set and retrieved.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../selftests/net/mptcp/mptcp_sockopt.c       | 41 ++++++++++++++++++-
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index ac9a4d9c1764..12aa2d921fd5 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -133,10 +133,43 @@ static void xgetaddrinfo(const char *node, const char *service,
 	}
 }
 
+static void check_tcp_defer(int sock, unsigned int value)
+{
+	unsigned int rv;
+	socklen_t rvl;
+	int err;
+
+	rvl = sizeof(rv);
+	err = getsockopt(sock, IPPROTO_TCP, TCP_DEFER_ACCEPT, &rv, &rvl);
+	if (err) {
+		perror("getsockopt TCP_DEFER_ACCEPT");
+		exit(1);
+	}
+
+	assert(rvl == (int)sizeof(rv));
+	assert(rv == value);
+}
+
+static unsigned int set_tcp_defer(int sock)
+{
+	unsigned int v = rand();
+	int err;
+
+	v &= 0x1f;
+
+	err = setsockopt(sock, IPPROTO_TCP, TCP_DEFER_ACCEPT, &v, sizeof(v));
+	if (err) {
+		perror("setsockopt TCP_DEFER_ACCEPT");
+		exit(1);
+	}
+
+	return v;
+}
+
 static int sock_listen_mptcp(const char * const listenaddr,
 			     const char * const port)
 {
-	int sock;
+	int sock, value;
 	struct addrinfo hints = {
 		.ai_protocol = IPPROTO_TCP,
 		.ai_socktype = SOCK_STREAM,
@@ -173,9 +206,13 @@ static int sock_listen_mptcp(const char * const listenaddr,
 	if (sock < 0)
 		xerror("could not create listen socket");
 
+	value = set_tcp_defer(sock);
+
 	if (listen(sock, 20))
 		die_perror("listen");
 
+	check_tcp_defer(sock, value);
+
 	return sock;
 }
 
@@ -630,7 +667,7 @@ static void test_ip_tos_sockopt(int fd)
 	r = getsockopt(fd, SOL_IP, IP_TOS, &tos_out, &s);
 	if (r != -1 && errno != EINVAL)
 		die_perror("getsockopt IP_TOS did not indicate -EINVAL");
-	if (s != -1)
+	if ((int)s != -1)
 		xerror("expect socklen_t == -1");
 }
 
-- 
2.35.1

On Mon, 2 May 2022, Florian Westphal wrote:

> Check that value x can be set and retrieved.
>

Hi Florian -

I see the same error in this test with both my local test VM and when 
running Matthieu's CI script on a different machine:

# mptcp_sockopt: mptcp_sockopt.c:150: check_tcp_defer: Assertion `rv == value' failed.
# mptcp_sockopt: mptcp_sockopt.c:765: main: Assertion `e1 == 4' failed.
# ./mptcp_sockopt.sh: line 243: 16800 Aborted                 ./mptcp_sockopt
# FAIL: SOL_MPTCP getsockopt
# PASS: TCP_INQ cmsg/ioctl -t tcp
# PASS: TCP_INQ cmsg/ioctl -6 -t tcp
# PASS: TCP_INQ cmsg/ioctl -r tcp
# PASS: TCP_INQ cmsg/ioctl -6 -r tcp
# PASS: TCP_INQ cmsg/ioctl -r tcp -t tcp

I've attached the .config (debug).

- Mat


> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> .../selftests/net/mptcp/mptcp_sockopt.c       | 41 ++++++++++++++++++-
> 1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> index ac9a4d9c1764..12aa2d921fd5 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> @@ -133,10 +133,43 @@ static void xgetaddrinfo(const char *node, const char *service,
> 	}
> }
>
> +static void check_tcp_defer(int sock, unsigned int value)
> +{
> +	unsigned int rv;
> +	socklen_t rvl;
> +	int err;
> +
> +	rvl = sizeof(rv);
> +	err = getsockopt(sock, IPPROTO_TCP, TCP_DEFER_ACCEPT, &rv, &rvl);
> +	if (err) {
> +		perror("getsockopt TCP_DEFER_ACCEPT");
> +		exit(1);
> +	}
> +
> +	assert(rvl == (int)sizeof(rv));
> +	assert(rv == value);
> +}
> +
> +static unsigned int set_tcp_defer(int sock)
> +{
> +	unsigned int v = rand();
> +	int err;
> +
> +	v &= 0x1f;
> +
> +	err = setsockopt(sock, IPPROTO_TCP, TCP_DEFER_ACCEPT, &v, sizeof(v));
> +	if (err) {
> +		perror("setsockopt TCP_DEFER_ACCEPT");
> +		exit(1);
> +	}
> +
> +	return v;
> +}
> +
> static int sock_listen_mptcp(const char * const listenaddr,
> 			     const char * const port)
> {
> -	int sock;
> +	int sock, value;
> 	struct addrinfo hints = {
> 		.ai_protocol = IPPROTO_TCP,
> 		.ai_socktype = SOCK_STREAM,
> @@ -173,9 +206,13 @@ static int sock_listen_mptcp(const char * const listenaddr,
> 	if (sock < 0)
> 		xerror("could not create listen socket");
>
> +	value = set_tcp_defer(sock);
> +
> 	if (listen(sock, 20))
> 		die_perror("listen");
>
> +	check_tcp_defer(sock, value);
> +
> 	return sock;
> }
>
> @@ -630,7 +667,7 @@ static void test_ip_tos_sockopt(int fd)
> 	r = getsockopt(fd, SOL_IP, IP_TOS, &tos_out, &s);
> 	if (r != -1 && errno != EINVAL)
> 		die_perror("getsockopt IP_TOS did not indicate -EINVAL");
> -	if (s != -1)
> +	if ((int)s != -1)
> 		xerror("expect socklen_t == -1");
> }
>
> -- 
> 2.35.1
>
>
>

--
Mat Martineau
Intel

Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
> On Mon, 2 May 2022, Florian Westphal wrote:
> 
> > Check that value x can be set and retrieved.
> > 
> 
> Hi Florian -
> 
> I see the same error in this test with both my local test VM and when
> running Matthieu's CI script on a different machine:
> 
> # mptcp_sockopt: mptcp_sockopt.c:150: check_tcp_defer: Assertion `rv == value' failed.
> # mptcp_sockopt: mptcp_sockopt.c:765: main: Assertion `e1 == 4' failed.

seems like fix is to remove this test, kernel xlates value to
number-of-retrainsmits so set and get may return different values.

On Tue, 3 May 2022, Florian Westphal wrote:

> Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
>> On Mon, 2 May 2022, Florian Westphal wrote:
>>
>>> Check that value x can be set and retrieved.
>>>
>>
>> Hi Florian -
>>
>> I see the same error in this test with both my local test VM and when
>> running Matthieu's CI script on a different machine:
>>
>> # mptcp_sockopt: mptcp_sockopt.c:150: check_tcp_defer: Assertion `rv == value' failed.
>> # mptcp_sockopt: mptcp_sockopt.c:765: main: Assertion `e1 == 4' failed.
>
> seems like fix is to remove this test, kernel xlates value to
> number-of-retrainsmits so set and get may return different values.
>

Yeah, similar pass-through socket options aren't covered in the self tests 
either. Ok with me to drop this commit.

--
Mat Martineau
Intel

Hi Florian,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): selftest_mptcp_sockopt 🔴:
  - Task: https://cirrus-ci.com/task/5785434943913984
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5785434943913984/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 2 failed test(s): selftest_diag selftest_mptcp_sockopt - Critical: 1 Call Trace(s) ❌:
  - Task: https://cirrus-ci.com/task/5457720147968000
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5457720147968000/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/da7722c87e4c


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)