Some user-space applications want to monitor the subflows utilization.
Dumping the per subflow tcp_info is not enough, as the PM could close
and re-create the subflows under-the-hood, fooling the accounting.
Even checking the src/dst addresses used by each subflow could not
be enough, because new subflows could re-use the same address/port of
the just closed one.
This patch introduces a new socket option, allow dumping all the relevant
information all-at-once (everything, everywhere...), in a consistent manner.
To reuse the existing helper to manipulate the new struct, keep the binary
layout of the initial few fields the same as mptcp_subflow_data.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v3 -> v4:
- full_info struct re-design (Florian)
v2 -> v3:
- added missing changelog (oops)
---
include/uapi/linux/mptcp.h | 25 ++++++
net/mptcp/sockopt.c | 165 +++++++++++++++++++++++++++++++++----
2 files changed, 174 insertions(+), 16 deletions(-)
diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 32af2d278cb4..37c46cf05795 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,33 @@ struct mptcp_subflow_addrs {
};
};
+struct mptcp_subflow_info {
+ __u32 id;
+ struct mptcp_subflow_addrs addrs;
+};
+
+struct mptcp_subflow_full_info {
+ __u32 size_subflow_full_info; /* size of this structure in userspace */
+ __u32 num_subflows_kern; /* must be 0, set by kernel (real subflow count) */
+ __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_user; /* max subflows that userspace is interested in;
+ * the buffers at subflow_info_addr/tcp_info_addr
+ * are respectively at least:
+ * num_subflows_user * size_sfinfo_user
+ * num_subflows_user * size_tcpinfo_user
+ * bytes wide
+ */
+ __aligned_u64 subflow_info_addr;
+ __aligned_u64 tcp_info_addr;
+} __attribute__((aligned(8)));
+
/* 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..59a174ee1d54 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -14,7 +14,8 @@
#include <net/mptcp.h>
#include "protocol.h"
-#define MIN_INFO_OPTLEN_SIZE 16
+#define MIN_INFO_OPTLEN_SIZE 16
+#define MIN_FULL_INFO_OPTLEN_SIZE 48
static struct sock *__mptcp_tcp_fallback(struct mptcp_sock *msk)
{
@@ -943,12 +944,13 @@ static int mptcp_getsockopt_info(struct mptcp_sock *msk, char __user *optval, in
return 0;
}
-static int mptcp_put_subflow_data(struct mptcp_subflow_data *sfd,
- char __user *optval,
- u32 copied,
- int __user *optlen)
+static int __mptcp_put_subflow_data(struct mptcp_subflow_data *sfd,
+ int size_subflow_data_kern,
+ char __user *optval,
+ u32 copied,
+ int __user *optlen)
{
- u32 copylen = min_t(u32, sfd->size_subflow_data, sizeof(*sfd));
+ u32 copylen = min_t(u32, sfd->size_subflow_data, size_subflow_data_kern);
if (copied)
copied += sfd->size_subflow_data;
@@ -964,25 +966,30 @@ static int mptcp_put_subflow_data(struct mptcp_subflow_data *sfd,
return 0;
}
-static int mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
- char __user *optval, int __user *optlen)
+static int mptcp_put_subflow_data(struct mptcp_subflow_data *sfd,
+ char __user *optval,
+ u32 copied,
+ int __user *optlen)
+{
+ return __mptcp_put_subflow_data(sfd, sizeof(*sfd), optval, copied, optlen);
+}
+
+static int __mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
+ int min_info_optlen_size,
+ char __user *optval,
+ int __user *optlen)
{
int len, copylen;
if (get_user(len, optlen))
return -EFAULT;
- /* if mptcp_subflow_data size is changed, need to adjust
- * this function to deal with programs using old version.
- */
- BUILD_BUG_ON(sizeof(*sfd) != MIN_INFO_OPTLEN_SIZE);
-
- if (len < MIN_INFO_OPTLEN_SIZE)
+ if (len < min_info_optlen_size)
return -EINVAL;
memset(sfd, 0, sizeof(*sfd));
- copylen = min_t(unsigned int, len, sizeof(*sfd));
+ copylen = min_t(unsigned int, len, min_info_optlen_size);
if (copy_from_user(sfd, optval, copylen))
return -EFAULT;
@@ -991,7 +998,7 @@ static int mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
sfd->size_user > INT_MAX)
return -EINVAL;
- if (sfd->size_subflow_data < MIN_INFO_OPTLEN_SIZE ||
+ if (sfd->size_subflow_data < min_info_optlen_size ||
sfd->size_subflow_data > len)
return -EINVAL;
@@ -1001,6 +1008,19 @@ static int mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
return len - sfd->size_subflow_data;
}
+static int mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
+ char __user *optval,
+ int __user *optlen)
+{
+ /* if mptcp_subflow_data size is changed, need to adjust
+ * this function to deal with programs using old version.
+ */
+ BUILD_BUG_ON(sizeof(*sfd) != MIN_INFO_OPTLEN_SIZE);
+
+ return __mptcp_get_subflow_data(sfd, MIN_INFO_OPTLEN_SIZE,
+ optval, optlen);
+}
+
static int mptcp_getsockopt_tcpinfo(struct mptcp_sock *msk, char __user *optval,
int __user *optlen)
{
@@ -1146,6 +1166,117 @@ static int mptcp_getsockopt_subflow_addrs(struct mptcp_sock *msk, char __user *o
return 0;
}
+static int mptcp_get_full_subflow_info(struct mptcp_subflow_full_info *sffi,
+ char __user *optval,
+ int __user *optlen)
+{
+ struct mptcp_subflow_data *sfd = (struct mptcp_subflow_data *)sffi;
+ int len;
+
+ BUILD_BUG_ON(sizeof(*sffi) != MIN_FULL_INFO_OPTLEN_SIZE);
+
+ len = __mptcp_get_subflow_data(sfd, MIN_FULL_INFO_OPTLEN_SIZE,
+ optval, optlen);
+ if (len < 0)
+ return len;
+
+ if (sffi->size_tcpinfo_kernel)
+ return -EINVAL;
+
+ if (sffi->size_sfinfo_user > INT_MAX)
+ return -EINVAL;
+
+ return len;
+}
+
+static int mptcp_put_subflow_full_info(struct mptcp_subflow_full_info *sffi,
+ char __user *optval,
+ u32 copied,
+ int __user *optlen)
+{
+ struct mptcp_subflow_data *sfd = (struct mptcp_subflow_data *)sffi;
+
+ return __mptcp_put_subflow_data(sfd, sizeof(*sffi), optval, copied, optlen);
+}
+
+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;
+ struct mptcp_subflow_full_info sffi;
+ void __user *tcpinfoptr, *sfinfoptr;
+ int len;
+
+ len = mptcp_get_full_subflow_info(&sffi, optval, optlen);
+ if (len < 0)
+ return len;
+
+ /* don't bother filling the mptcp info if there is not enough
+ * user-space-provided storage
+ */
+ if (len > 0) {
+ struct mptcp_info mptcp_info;
+ char __user *infoptr;
+ int mptcp_info_len;
+
+ infoptr = optval + sffi.size_subflow_full_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;
+
+ copied += mptcp_info_len;
+ }
+
+ sffi.size_tcpinfo_kernel = sizeof(struct tcp_info);
+ sffi.size_tcpinfo_user = min_t(unsigned int, sffi.size_tcpinfo_user,
+ sizeof(struct tcp_info));
+ sfinfoptr = (void __force __user *)sffi.subflow_info_addr;
+ sffi.size_sfinfo_kernel = sizeof(struct mptcp_subflow_info);
+ sffi.size_sfinfo_user = min_t(unsigned int, sffi.size_sfinfo_user,
+ sizeof(struct mptcp_subflow_info));
+ tcpinfoptr = (void __force __user *)sffi.tcp_info_addr;
+
+ lock_sock(sk);
+ mptcp_for_each_subflow(msk, subflow) {
+ struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+ if (sfcount++ < sffi.num_subflows_user) {
+ struct mptcp_subflow_info sfinfo;
+ struct tcp_info tcp_info;
+
+ memset(&sfinfo, 0, sizeof(sfinfo));
+ sfinfo.id = subflow->subflow_id;
+ mptcp_get_sub_addrs(ssk, &sfinfo.addrs);
+ if (copy_to_user(sfinfoptr, &sfinfo, sffi.size_sfinfo_user))
+ goto fail_release;
+
+ tcp_get_info(ssk, &tcp_info);
+ if (copy_to_user(tcpinfoptr, &tcp_info, sffi.size_tcpinfo_user))
+ goto fail_release;
+
+ tcpinfoptr += sffi.size_tcpinfo_user;
+ sfinfoptr += sffi.size_sfinfo_user;
+ }
+ }
+ release_sock(sk);
+
+ sffi.num_subflows_kern = sfcount;
+ if (mptcp_put_subflow_full_info(&sffi, optval, copied, optlen))
+ return -EFAULT;
+
+ return 0;
+
+fail_release:
+ release_sock(sk);
+ return -EFAULT;
+}
+
static int mptcp_put_int_option(struct mptcp_sock *msk, char __user *optval,
int __user *optlen, int val)
{
@@ -1219,6 +1350,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
Hi Paolo, On 24/05/2023 15:50, Paolo Abeni wrote: > Some user-space applications want to monitor the subflows utilization. > > Dumping the per subflow tcp_info is not enough, as the PM could close > and re-create the subflows under-the-hood, fooling the accounting. > Even checking the src/dst addresses used by each subflow could not > be enough, because new subflows could re-use the same address/port of > the just closed one. > > This patch introduces a new socket option, allow dumping all the relevant > information all-at-once (everything, everywhere...), in a consistent manner. > > To reuse the existing helper to manipulate the new struct, keep the binary > layout of the initial few fields the same as mptcp_subflow_data. (...) > + lock_sock(sk); > + mptcp_for_each_subflow(msk, subflow) { > + struct sock *ssk = mptcp_subflow_tcp_sock(subflow); > + > + if (sfcount++ < sffi.num_subflows_user) { > + struct mptcp_subflow_info sfinfo; > + struct tcp_info tcp_info; > + > + memset(&sfinfo, 0, sizeof(sfinfo)); > + sfinfo.id = subflow->subflow_id; > + mptcp_get_sub_addrs(ssk, &sfinfo.addrs); Would it be worth it to first check size_sfinfo_user > sizeof(sfinfo.id) before retreiving the addresses? > + if (copy_to_user(sfinfoptr, &sfinfo, sffi.size_sfinfo_user)) > + goto fail_release;> + > + tcp_get_info(ssk, &tcp_info); Same here: just checking if sffi.size_tcpinfo_user > 0, just in case the userspace doesn't one and/or the other. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
Hi Paolo, On 24/05/2023 15:50, Paolo Abeni wrote: > Some user-space applications want to monitor the subflows utilization. > > Dumping the per subflow tcp_info is not enough, as the PM could close > and re-create the subflows under-the-hood, fooling the accounting. > Even checking the src/dst addresses used by each subflow could not > be enough, because new subflows could re-use the same address/port of > the just closed one. > > This patch introduces a new socket option, allow dumping all the relevant > information all-at-once (everything, everywhere...), in a consistent manner. > > To reuse the existing helper to manipulate the new struct, keep the binary > layout of the initial few fields the same as mptcp_subflow_data. > > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > v3 -> v4: > - full_info struct re-design (Florian) > > v2 -> v3: > - added missing changelog (oops) > --- > include/uapi/linux/mptcp.h | 25 ++++++ > net/mptcp/sockopt.c | 165 +++++++++++++++++++++++++++++++++---- > 2 files changed, 174 insertions(+), 16 deletions(-) > > diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h > index 32af2d278cb4..37c46cf05795 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 */ (we don't directly use tcp_info here below, no?) > > #define MPTCP_SUBFLOW_FLAG_MCAP_REM _BITUL(0) > #define MPTCP_SUBFLOW_FLAG_MCAP_LOC _BITUL(1) > @@ -244,9 +245,33 @@ struct mptcp_subflow_addrs { > }; > }; > > +struct mptcp_subflow_info { > + __u32 id; > + struct mptcp_subflow_addrs addrs; > +}; > + > +struct mptcp_subflow_full_info { > + __u32 size_subflow_full_info; /* size of this structure in userspace */ > + __u32 num_subflows_kern; /* must be 0, set by kernel (real subflow count) */ (maybe better to call it num_subflows if it is the "real" number of subflows?) > + __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_user; /* max subflows that userspace is interested in; > + * the buffers at subflow_info_addr/tcp_info_addr > + * are respectively at least: > + * num_subflows_user * size_sfinfo_user > + * num_subflows_user * size_tcpinfo_user > + * bytes wide The "longish" comment seems to suggest the name is not appropriated, maybe: size_arrays(_user) or similar? > + */ > + __aligned_u64 subflow_info_addr; > + __aligned_u64 tcp_info_addr; (the "_addr" suffix is confusing, I thought it was linked to subflow addresses :) why not "_ptr" instead? or put "addr_" as a prefix?) > +} __attribute__((aligned(8))); Out of curiosity, why not including "struct mptcp_info" in "struct mptcp_subflow_full_info" (and strip "_subflow": "mptcp_full_info")? It looks like it is a bit hidden that this getsockopt() also writes mptcp_info in the buffer if there is space and mptcp_info size could be explicitly given making clean it can be set to 0 if we don't want it (instead of not knowing we can give a bigger size to have it included). Plus the userspace has to create a custom structure to get it: struct my_mptcp_full_info { struct mptcp_subflow_full_info i; struct mptcp_info mi; }; (but maybe that is common and fine?) And while at it, why not doing like subflow_info_addr and tcp_info_addr and use a pointer to a structure? I don't know if it would be better, it is just to know why, just because it is not an array I suppose :) Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
© 2016 - 2024 Red Hat, Inc.