[PATCH mptcp-next v2 2/6] mptcp: introduce MPTCP_FULL_INFO getsockopt

Paolo Abeni posted 6 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH mptcp-next v2 2/6] mptcp: introduce MPTCP_FULL_INFO getsockopt
Posted by Paolo Abeni 2 years, 7 months ago
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/uapi/linux/mptcp.h | 12 ++++++
 net/mptcp/sockopt.c        | 75 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 32af2d278cb4..483d4d2a3646 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -12,6 +12,7 @@
 #include <linux/in.h>		/* for sockaddr_in			*/
 #include <linux/in6.h>		/* for sockaddr_in6			*/
 #include <linux/socket.h>	/* for sockaddr_storage and sa_family	*/
+#include <linux/tcp.h>		/* for tcp_info				*/
 
 #define MPTCP_SUBFLOW_FLAG_MCAP_REM		_BITUL(0)
 #define MPTCP_SUBFLOW_FLAG_MCAP_LOC		_BITUL(1)
@@ -244,9 +245,20 @@ struct mptcp_subflow_addrs {
 	};
 };
 
+struct mptcp_subflow_info {
+	__u32				id;
+	struct mptcp_subflow_addrs	addrs;
+};
+
+struct mptcp_subflow_full_info {
+	struct mptcp_subflow_info	subflow_info;
+	struct tcp_info			tcp_info;
+};
+
 /* MPTCP socket options */
 #define MPTCP_INFO		1
 #define MPTCP_TCPINFO		2
 #define MPTCP_SUBFLOW_ADDRS	3
+#define MPTCP_FULL_INFO		4
 
 #endif /* _UAPI_MPTCP_H */
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index d4258869ac48..b4d04d5dc1f7 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1146,6 +1146,79 @@ static int mptcp_getsockopt_subflow_addrs(struct mptcp_sock *msk, char __user *o
 	return 0;
 }
 
+static int mptcp_getsockopt_full_info(struct mptcp_sock *msk, char __user *optval,
+				      int __user *optlen)
+{
+	struct mptcp_subflow_context *subflow;
+	struct sock *sk = (struct sock *)msk;
+	unsigned int sfcount = 0, copied = 0;
+	int mptcp_info_len, len, size_user;
+	struct mptcp_subflow_data sfd;
+	char __user *infoptr;
+
+	len = mptcp_get_subflow_data(&sfd, optval, optlen);
+	if (len < 0)
+		return len;
+
+	/* don't bother filling the mptcp info if there is not enough
+	 * user-space-provided storage
+	 */
+	infoptr = optval + sfd.size_subflow_data;
+	if (len > sizeof(struct mptcp_subflow_data)) {
+		struct mptcp_info mptcp_info;
+
+		memset(&mptcp_info, 0, sizeof(mptcp_info));
+		mptcp_info_len = min_t(unsigned int, len, sizeof(struct mptcp_info));
+
+		mptcp_diag_fill_info(msk, &mptcp_info);
+
+		if (copy_to_user(infoptr, &mptcp_info, mptcp_info_len))
+			return -EFAULT;
+
+		len -= mptcp_info_len;
+		copied += mptcp_info_len;
+		infoptr += mptcp_info_len;
+	}
+
+	sfd.size_kernel = sizeof(struct mptcp_subflow_full_info);
+	sfd.size_user = min_t(unsigned int, sfd.size_user,
+			      sizeof(struct mptcp_subflow_full_info));
+	lock_sock(sk);
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+		++sfcount;
+		if (len >= size_user) {
+			struct mptcp_subflow_full_info sinfo;
+
+			memset(&sinfo, 0, sizeof(sinfo));
+			sinfo.subflow_info.id = subflow->subflow_id;
+			mptcp_get_sub_addrs(ssk, &sinfo.subflow_info.addrs);
+
+			/* don't bother filling the tcp info if the storage is too small */
+			if (sfd.size_user > sizeof(struct mptcp_subflow_info))
+				tcp_get_info(ssk, &sinfo.tcp_info);
+
+			if (copy_to_user(infoptr, &sinfo, sfd.size_user)) {
+				release_sock(sk);
+				return -EFAULT;
+			}
+
+			infoptr += sfd.size_user;
+			copied += sfd.size_user;
+			len -= sfd.size_user;
+		}
+	}
+	release_sock(sk);
+
+	sfd.num_subflows = sfcount;
+	if (mptcp_put_subflow_data(&sfd, optval, copied, optlen))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int mptcp_put_int_option(struct mptcp_sock *msk, char __user *optval,
 				int __user *optlen, int val)
 {
@@ -1219,6 +1292,8 @@ static int mptcp_getsockopt_sol_mptcp(struct mptcp_sock *msk, int optname,
 	switch (optname) {
 	case MPTCP_INFO:
 		return mptcp_getsockopt_info(msk, optval, optlen);
+	case MPTCP_FULL_INFO:
+		return mptcp_getsockopt_full_info(msk, optval, optlen);
 	case MPTCP_TCPINFO:
 		return mptcp_getsockopt_tcpinfo(msk, optval, optlen);
 	case MPTCP_SUBFLOW_ADDRS:
-- 
2.40.1
Re: [PATCH mptcp-next v2 2/6] mptcp: introduce MPTCP_FULL_INFO getsockopt
Posted by Matthieu Baerts 2 years, 6 months ago
Hi Paolo,

On 22/05/2023 15:56, Paolo Abeni wrote:
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Do you mind motivating the addition of this new socket option? :)

Also, it is not clear how we manage the different size for struct
tcp_info and struct mptcp_subflow_addrs but I will look at that in more
details later.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-next v2 2/6] mptcp: introduce MPTCP_FULL_INFO getsockopt
Posted by Paolo Abeni 2 years, 6 months ago
On Tue, 2023-05-23 at 17:31 +0200, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 22/05/2023 15:56, Paolo Abeni wrote:
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> Do you mind motivating the addition of this new socket option? :)

Sure, I will do.

> Also, it is not clear how we manage the different size for struct
> tcp_info and struct mptcp_subflow_addrs but I will look at that in more
> details later.

This patch does not allow for different mptcp_subflow_addrs size. I
hope it's not a problem??!

/P
Re: [PATCH mptcp-next v2 2/6] mptcp: introduce MPTCP_FULL_INFO getsockopt
Posted by Matthieu Baerts 2 years, 6 months ago
Hi Paolo,

On 23/05/2023 18:46, Paolo Abeni wrote:
> On Tue, 2023-05-23 at 17:31 +0200, Matthieu Baerts wrote:
>> Hi Paolo,
>>
>> On 22/05/2023 15:56, Paolo Abeni wrote:
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>
>> Do you mind motivating the addition of this new socket option? :)
> 
> Sure, I will do.
> 
>> Also, it is not clear how we manage the different size for struct
>> tcp_info and struct mptcp_subflow_addrs but I will look at that in more
>> details later.
> 
> This patch does not allow for different mptcp_subflow_addrs size. I
> hope it's not a problem??!

I guess it is fine, it should not change. But then it is probably better
to add a note in mptcp.h, no?

The alternative is write something like:

 - header with the features we want + number of subflows per feature
(and the kernel can write how many were available)
 - feature 1 (tcp_info):
   - mptcp_subflow_data structure
   - tcp_info for each subflow
 - feature 2 (addr):
   - mptcp_subflow_data structure
   - addr for each subflow

So we can accept multiple features and we can pick what we want.

Just an idea, maybe it is not useful or too complex to manage.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-next v2 2/6] mptcp: introduce MPTCP_FULL_INFO getsockopt
Posted by Paolo Abeni 2 years, 6 months ago
On Tue, 2023-05-23 at 18:55 +0200, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 23/05/2023 18:46, Paolo Abeni wrote:
> > On Tue, 2023-05-23 at 17:31 +0200, Matthieu Baerts wrote:
> > > Hi Paolo,
> > > 
> > > On 22/05/2023 15:56, Paolo Abeni wrote:
> > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > 
> > > Do you mind motivating the addition of this new socket option? :)
> > 
> > Sure, I will do.
> > 
> > > Also, it is not clear how we manage the different size for struct
> > > tcp_info and struct mptcp_subflow_addrs but I will look at that in more
> > > details later.
> > 
> > This patch does not allow for different mptcp_subflow_addrs size. I
> > hope it's not a problem??!
> 
> I guess it is fine, it should not change. But then it is probably better
> to add a note in mptcp.h, no?
> 
> The alternative is write something like:
> 
>  - header with the features we want + number of subflows per feature
> (and the kernel can write how many were available)
>  - feature 1 (tcp_info):
>    - mptcp_subflow_data structure
>    - tcp_info for each subflow
>  - feature 2 (addr):
>    - mptcp_subflow_data structure
>    - addr for each subflow
> 
> So we can accept multiple features and we can pick what we want.
> 
> Just an idea, maybe it is not useful or too complex to manage.

mptcp_subflow_addrs includes 2 __kernel_sockaddr_storage field, so it
should be as future proof as reasonably possible. Even IPv8 should fit
the 128 bytes there ;)

If not, we can always add an MPTCP_FULLER_INFO sock opt ;)

I think the current schema should work. I'll add a note in the uapi
file.

Thanks!

/P