[PATCH v6 mptcp-next 6/6] selftests: mptcp: add MPTCP_FULL_INFO testcase

Paolo Abeni posted 6 patches 1 year, 3 months ago
Maintainers: Matthieu Baerts <matthieu.baerts@tessares.net>, Mat Martineau <martineau@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
[PATCH v6 mptcp-next 6/6] selftests: mptcp: add MPTCP_FULL_INFO testcase
Posted by Paolo Abeni 1 year, 3 months ago
Add a testcase explicitly triggering the newly introduce
MPTCP_FULL_INFO getsockopt.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 .../selftests/net/mptcp/mptcp_sockopt.c       | 91 ++++++++++++++++++-
 1 file changed, 89 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
index ff8fcdfccf76..568e95599355 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -86,9 +86,38 @@ struct mptcp_subflow_addrs {
 #define MPTCP_SUBFLOW_ADDRS	3
 #endif
 
+#ifndef MPTCP_FULL_INFO
+struct mptcp_subflow_info {
+	__u32				id;
+	struct mptcp_subflow_addrs	addrs;
+};
+
+struct mptcp_full_info {
+	__u32		size_tcpinfo_kernel;	/* must be 0, set by kernel */
+	__u32		size_tcpinfo_user;
+	__u32		size_sfinfo_kernel;	/* must be 0, set by kernel */
+	__u32		size_sfinfo_user;
+	__u32		num_subflows;		/* must be 0, set by kernel (real subflow count) */
+	__u32		size_arrays_user;	/* max subflows that userspace is interested in;
+						 * the buffers at subflow_info/tcp_info
+						 * are respectively at least:
+						 *  size_arrays * size_sfinfo_user
+						 *  size_arrays * size_tcpinfo_user
+						 * bytes wide
+						 */
+	__aligned_u64		subflow_info;
+	__aligned_u64		tcp_info;
+	struct mptcp_info	mptcp_info;
+};
+
+#define MPTCP_FULL_INFO		4
+#endif
+
 struct so_state {
 	struct mptcp_info mi;
 	struct mptcp_info last_sample;
+	struct tcp_info tcp_info;
+	struct mptcp_subflow_addrs addrs;
 	uint64_t mptcpi_rcv_delta;
 	uint64_t tcpi_rcv_delta;
 	bool pkt_stats_avail;
@@ -365,6 +394,8 @@ static void do_getsockopt_tcp_info(struct so_state *s, int fd, size_t r, size_t
 		olen -= sizeof(struct mptcp_subflow_data);
 		assert(olen == sizeof(struct tcp_info));
 
+		s->tcp_info = ti.ti[0];
+
 		if (ti.ti[0].tcpi_bytes_sent == w &&
 		    ti.ti[0].tcpi_bytes_received == r)
 			goto done;
@@ -386,7 +417,7 @@ static void do_getsockopt_tcp_info(struct so_state *s, int fd, size_t r, size_t
 	do_getsockopt_bogus_sf_data(fd, MPTCP_TCPINFO);
 }
 
-static void do_getsockopt_subflow_addrs(int fd)
+static void do_getsockopt_subflow_addrs(struct so_state *s, int fd)
 {
 	struct sockaddr_storage remote, local;
 	socklen_t olen, rlen, llen;
@@ -433,6 +464,7 @@ static void do_getsockopt_subflow_addrs(int fd)
 
 	assert(memcmp(&local, &addrs.addr[0].ss_local, sizeof(local)) == 0);
 	assert(memcmp(&remote, &addrs.addr[0].ss_remote, sizeof(remote)) == 0);
+	s->addrs = addrs.addr[0];
 
 	memset(&addrs, 0, sizeof(addrs));
 
@@ -453,13 +485,68 @@ static void do_getsockopt_subflow_addrs(int fd)
 	do_getsockopt_bogus_sf_data(fd, MPTCP_SUBFLOW_ADDRS);
 }
 
+static void do_getsockopt_mptcp_full_info(struct so_state *s, int fd)
+{
+	size_t data_size = sizeof(struct mptcp_full_info);
+	struct mptcp_subflow_info sfinfo[2];
+	struct tcp_info tcp_info[2];
+	struct mptcp_full_info mfi;
+	socklen_t olen;
+	int ret;
+
+	memset(&mfi, 0, data_size);
+	memset(tcp_info, 0, sizeof(tcp_info));
+	memset(sfinfo, 0, sizeof(sfinfo));
+
+	mfi.size_tcpinfo_user = sizeof(struct tcp_info);
+	mfi.size_sfinfo_user = sizeof(struct mptcp_subflow_info);
+	mfi.size_arrays_user = 2;
+	mfi.subflow_info = (unsigned long) &sfinfo[0];
+	mfi.tcp_info = (unsigned long) &tcp_info[0];
+	olen = data_size;
+
+	ret = getsockopt(fd, SOL_MPTCP, MPTCP_FULL_INFO, &mfi, &olen);
+	if (ret < 0) {
+		if (errno == EOPNOTSUPP) {
+			perror("MPTCP_FULL_INFO test skipped");
+			return;
+		}
+		xerror("getsockopt MPTCP_FULL_INFO");
+	}
+
+	assert(olen <= data_size);
+	assert(mfi.size_tcpinfo_user == mfi.size_tcpinfo_kernel);
+	assert(mfi.size_tcpinfo_user == sizeof(struct tcp_info));
+	assert(mfi.size_sfinfo_user == mfi.size_sfinfo_kernel);
+	assert(mfi.size_sfinfo_user == sizeof(struct mptcp_subflow_info));
+	assert(mfi.num_subflows == 1);
+
+	/* Tolerate future extension to mptcp_info struct and running newer
+	 * test on top of older kernel.
+	 * Anyway any kernel supporting MPTCP_FULL_INFO must at least include
+	 * the following in mptcp_info.
+	 */
+	assert(olen > (socklen_t)__builtin_offsetof(struct mptcp_full_info, tcp_info));
+	assert(mfi.mptcp_info.mptcpi_subflows == 0);
+	assert(mfi.mptcp_info.mptcpi_bytes_sent == s->last_sample.mptcpi_bytes_sent);
+	assert(mfi.mptcp_info.mptcpi_bytes_received == s->last_sample.mptcpi_bytes_received);
+
+	assert(sfinfo[0].id == 1);
+	assert(tcp_info[0].tcpi_bytes_sent == s->tcp_info.tcpi_bytes_sent);
+	assert(tcp_info[0].tcpi_bytes_received == s->tcp_info.tcpi_bytes_received);
+	assert(!memcmp(&sfinfo->addrs, &s->addrs, sizeof(struct mptcp_subflow_addrs)));
+}
+
 static void do_getsockopts(struct so_state *s, int fd, size_t r, size_t w)
 {
 	do_getsockopt_mptcp_info(s, fd, w);
 
 	do_getsockopt_tcp_info(s, fd, r, w);
 
-	do_getsockopt_subflow_addrs(fd);
+	do_getsockopt_subflow_addrs(s, fd);
+
+	if (r)
+		do_getsockopt_mptcp_full_info(s, fd);
 }
 
 static void connect_one_server(int fd, int pipefd)
-- 
2.40.1
Re: [PATCH v6 mptcp-next 6/6] selftests: mptcp: add MPTCP_FULL_INFO testcase
Posted by Matthieu Baerts 1 year, 3 months ago
Hi Paolo,

On 25/05/2023 09:17, Paolo Abeni wrote:
> Add a testcase explicitly triggering the newly introduce
> MPTCP_FULL_INFO getsockopt.

(...)

> +static void do_getsockopt_mptcp_full_info(struct so_state *s, int fd)
> +{
> +	size_t data_size = sizeof(struct mptcp_full_info);
> +	struct mptcp_subflow_info sfinfo[2];
> +	struct tcp_info tcp_info[2];
> +	struct mptcp_full_info mfi;
> +	socklen_t olen;
> +	int ret;
> +
> +	memset(&mfi, 0, data_size);
> +	memset(tcp_info, 0, sizeof(tcp_info));
> +	memset(sfinfo, 0, sizeof(sfinfo));
> +
> +	mfi.size_tcpinfo_user = sizeof(struct tcp_info);
> +	mfi.size_sfinfo_user = sizeof(struct mptcp_subflow_info);
> +	mfi.size_arrays_user = 2;
> +	mfi.subflow_info = (unsigned long) &sfinfo[0];
> +	mfi.tcp_info = (unsigned long) &tcp_info[0];
> +	olen = data_size;
> +
> +	ret = getsockopt(fd, SOL_MPTCP, MPTCP_FULL_INFO, &mfi, &olen);
> +	if (ret < 0) {
> +		if (errno == EOPNOTSUPP) {
> +			perror("MPTCP_FULL_INFO test skipped");
> +			return;
> +		}
> +		xerror("getsockopt MPTCP_FULL_INFO");
> +	}
> +
> +	assert(olen <= data_size);
> +	assert(mfi.size_tcpinfo_user == mfi.size_tcpinfo_kernel);
> +	assert(mfi.size_tcpinfo_user == sizeof(struct tcp_info));
> +	assert(mfi.size_sfinfo_user == mfi.size_sfinfo_kernel);
> +	assert(mfi.size_sfinfo_user == sizeof(struct mptcp_subflow_info));

Similar to the patch I sent to support old kernels, we cannot do these 4
last assert(), see:

https://patchwork.kernel.org/project/mptcp/patch/20230406-mptcp-issue-368-selftests-old-kernels-v2-17-50313e4f83ab@tessares.net/

Because we cannot guess the size of these two structures in the kernel,
I suggest to do this:

  assert(mfi.size_tcpinfo_user > 0);
  assert(mfi.size_tcpinfo_kernel > 0);
  assert(mfi.size_sfinfo_user > 0);
  assert(mfi.size_sfinfo_kernel > 0);

Better than nothing... In case of error, one would be set to 0 or we
would not get info from these structures below.

> +	assert(mfi.num_subflows == 1);
> +
> +	/* Tolerate future extension to mptcp_info struct and running newer
> +	 * test on top of older kernel.
> +	 * Anyway any kernel supporting MPTCP_FULL_INFO must at least include
> +	 * the following in mptcp_info.
> +	 */
> +	assert(olen > (socklen_t)__builtin_offsetof(struct mptcp_full_info, tcp_info));

Likely, this should always be correct because 'mptcp_info' is after
tcp_info, no? On the other hand, hard to do better  if we need to
support old kernels... So fine to keep it I suppose.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH v6 mptcp-next 6/6] selftests: mptcp: add MPTCP_FULL_INFO testcase
Posted by Paolo Abeni 1 year, 3 months ago
On Thu, 2023-05-25 at 17:22 +0200, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 25/05/2023 09:17, Paolo Abeni wrote:
> > Add a testcase explicitly triggering the newly introduce
> > MPTCP_FULL_INFO getsockopt.
> 
> (...)
> 
> > +static void do_getsockopt_mptcp_full_info(struct so_state *s, int fd)
> > +{
> > +	size_t data_size = sizeof(struct mptcp_full_info);
> > +	struct mptcp_subflow_info sfinfo[2];
> > +	struct tcp_info tcp_info[2];
> > +	struct mptcp_full_info mfi;
> > +	socklen_t olen;
> > +	int ret;
> > +
> > +	memset(&mfi, 0, data_size);
> > +	memset(tcp_info, 0, sizeof(tcp_info));
> > +	memset(sfinfo, 0, sizeof(sfinfo));
> > +
> > +	mfi.size_tcpinfo_user = sizeof(struct tcp_info);
> > +	mfi.size_sfinfo_user = sizeof(struct mptcp_subflow_info);
> > +	mfi.size_arrays_user = 2;
> > +	mfi.subflow_info = (unsigned long) &sfinfo[0];
> > +	mfi.tcp_info = (unsigned long) &tcp_info[0];
> > +	olen = data_size;
> > +
> > +	ret = getsockopt(fd, SOL_MPTCP, MPTCP_FULL_INFO, &mfi, &olen);
> > +	if (ret < 0) {
> > +		if (errno == EOPNOTSUPP) {
> > +			perror("MPTCP_FULL_INFO test skipped");
> > +			return;
> > +		}
> > +		xerror("getsockopt MPTCP_FULL_INFO");
> > +	}
> > +
> > +	assert(olen <= data_size);
> > +	assert(mfi.size_tcpinfo_user == mfi.size_tcpinfo_kernel);
> > +	assert(mfi.size_tcpinfo_user == sizeof(struct tcp_info));
> > +	assert(mfi.size_sfinfo_user == mfi.size_sfinfo_kernel);
> > +	assert(mfi.size_sfinfo_user == sizeof(struct mptcp_subflow_info));
> 
> Similar to the patch I sent to support old kernels, we cannot do these 4
> last assert(), see:
> 
> https://patchwork.kernel.org/project/mptcp/patch/20230406-mptcp-issue-368-selftests-old-kernels-v2-17-50313e4f83ab@tessares.net/
> 
> Because we cannot guess the size of these two structures in the kernel,

Regarding mptcp_subflow_info we can infer it must be >=
sizeof(mptcp_subflow_addrs) + 4...



> I suggest to do this:
> 
>   assert(mfi.size_tcpinfo_user > 0);
>   assert(mfi.size_tcpinfo_kernel > 0);
>   assert(mfi.size_sfinfo_user > 0);
>   assert(mfi.size_sfinfo_kernel > 0);
> 
> Better than nothing... In case of error, one would be set to 0 or we
> would not get info from these structures below.

... so we could check:

  int min_sfinfo_size = 4 + sizeof(struct mptcp_subflow_addrs);

  // ...

  assert(mfi.size_tcpinfo_kernel > 0);
  assert(mfi.size_tcpinfo_user ==
         min(mfi.size_tcpinfo_kernel, sizeof(struct tcp_info));
  assert(mfi.size_sfinfo_kernel >= min_sfinfo_size);    
  assert(mfi.size_sfinfo_user ==
         min(mfi.size_sfinfo_kernel, sizeof(struct mptcp_subflow_info));


/P
Re: [PATCH v6 mptcp-next 6/6] selftests: mptcp: add MPTCP_FULL_INFO testcase
Posted by Matthieu Baerts 1 year, 3 months ago
Hi Paolo,

On 25/05/2023 18:39, Paolo Abeni wrote:
> On Thu, 2023-05-25 at 17:22 +0200, Matthieu Baerts wrote:
>> Hi Paolo,
>>
>> On 25/05/2023 09:17, Paolo Abeni wrote:
>>> Add a testcase explicitly triggering the newly introduce
>>> MPTCP_FULL_INFO getsockopt.
>>
>> (...)
>>
>>> +static void do_getsockopt_mptcp_full_info(struct so_state *s, int fd)
>>> +{
>>> +	size_t data_size = sizeof(struct mptcp_full_info);
>>> +	struct mptcp_subflow_info sfinfo[2];
>>> +	struct tcp_info tcp_info[2];
>>> +	struct mptcp_full_info mfi;
>>> +	socklen_t olen;
>>> +	int ret;
>>> +
>>> +	memset(&mfi, 0, data_size);
>>> +	memset(tcp_info, 0, sizeof(tcp_info));
>>> +	memset(sfinfo, 0, sizeof(sfinfo));
>>> +
>>> +	mfi.size_tcpinfo_user = sizeof(struct tcp_info);
>>> +	mfi.size_sfinfo_user = sizeof(struct mptcp_subflow_info);
>>> +	mfi.size_arrays_user = 2;
>>> +	mfi.subflow_info = (unsigned long) &sfinfo[0];
>>> +	mfi.tcp_info = (unsigned long) &tcp_info[0];
>>> +	olen = data_size;
>>> +
>>> +	ret = getsockopt(fd, SOL_MPTCP, MPTCP_FULL_INFO, &mfi, &olen);
>>> +	if (ret < 0) {
>>> +		if (errno == EOPNOTSUPP) {
>>> +			perror("MPTCP_FULL_INFO test skipped");
>>> +			return;
>>> +		}
>>> +		xerror("getsockopt MPTCP_FULL_INFO");
>>> +	}
>>> +
>>> +	assert(olen <= data_size);
>>> +	assert(mfi.size_tcpinfo_user == mfi.size_tcpinfo_kernel);
>>> +	assert(mfi.size_tcpinfo_user == sizeof(struct tcp_info));
>>> +	assert(mfi.size_sfinfo_user == mfi.size_sfinfo_kernel);
>>> +	assert(mfi.size_sfinfo_user == sizeof(struct mptcp_subflow_info));
>>
>> Similar to the patch I sent to support old kernels, we cannot do these 4
>> last assert(), see:
>>
>> https://patchwork.kernel.org/project/mptcp/patch/20230406-mptcp-issue-368-selftests-old-kernels-v2-17-50313e4f83ab@tessares.net/
>>
>> Because we cannot guess the size of these two structures in the kernel,
> 
> Regarding mptcp_subflow_info we can infer it must be >=
> sizeof(mptcp_subflow_addrs) + 4...

Does that mean sizeof(mptcp_subflow_addrs) can never be modified?

>> I suggest to do this:
>>
>>   assert(mfi.size_tcpinfo_user > 0);
>>   assert(mfi.size_tcpinfo_kernel > 0);
>>   assert(mfi.size_sfinfo_user > 0);
>>   assert(mfi.size_sfinfo_kernel > 0);
>>
>> Better than nothing... In case of error, one would be set to 0 or we
>> would not get info from these structures below.
> 
> ... so we could check:
> 
>   int min_sfinfo_size = 4 + sizeof(struct mptcp_subflow_addrs);
> 
>   // ...
> 
>   assert(mfi.size_tcpinfo_kernel > 0);
>   assert(mfi.size_tcpinfo_user ==
>          min(mfi.size_tcpinfo_kernel, sizeof(struct tcp_info));

Good idea to use 'min' here!

>   assert(mfi.size_sfinfo_kernel >= min_sfinfo_size);    
>   assert(mfi.size_sfinfo_user ==
>          min(mfi.size_sfinfo_kernel, sizeof(struct mptcp_subflow_info));

I guess I should do the same in:

https://patchwork.kernel.org/project/mptcp/patch/20230406-mptcp-issue-368-selftests-old-kernels-v2-17-50313e4f83ab@tessares.net/

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net