All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-next v2 0/5] mptcp: add SOL_MPTCP getsockopt support
@ 2021-08-16 16:26 Florian Westphal
  2021-08-16 16:26 ` [PATCH mptcp-next v2 1/5] mptcp: add new mptcp_fill_diag helper Florian Westphal
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Florian Westphal @ 2021-08-16 16:26 UTC (permalink / raw)
  To: mptcp; +Cc: Florian Westphal

Changes in v2:
- move #define to start of file (Mat)
- refer to '"multipath-tcp.org"', not mptcp.org (Mat)
- use __kernel_sa_family_t in uapi header (test robot)
- add test cases with invalid inputs (Paolo)
- avoid duplicate mptcp_sockopt.h in Makefile (Paolo)
- remove _u_optlen (Paolo)

original cover letter:

This adds the MPTCP_INFO, MPTCP_TCPINFO and MPTCP_SUBFLOW_ADDRS
mptcp getsockopt optnames.

MPTCP_INFO exposes the mptcp_info struct as an alternative to the
existing netlink diag interface.

MPTCP_TCPINFO exposes the tcp_info struct.
Unlike SOL_TCP/TCP_INFO, this returns one struct for each active
subflow.

MPTCP_SUBFLOW_ADDRS allows userspace to discover the ip addresses/ports
used by the local and remote endpoints, one for each active tcp subflow.

MPTCP_TCPINFO and MPTCP_SUBFLOW_ADDRS share the same meta-header that
needs to be pre-filled by userspace with the size of the data structures
it expects.  This is done to allow extension of the involved structs
later on, without breaking backwards compatibility.

The meta-structure can also be used to discover the required space
to obtain all information, as kernel will fill in the number of
active subflows even if there is not enough room for the requested info
itself.

More information is available in the individual patches.
Last patch adds test cases for the three optnames.

Florian Westphal (5):
  mptcp: add new mptcp_fill_diag helper
  mptcp: add MPTCP_INFO getsockopt
  mptcp: add MPTCP_TCPINFO getsockopt support
  mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support
  selftests: mptcp: add mptcp getsockopt test cases

 include/linux/socket.h                        |   1 +
 include/net/mptcp.h                           |   4 +
 include/uapi/linux/mptcp.h                    |  29 +
 net/mptcp/mptcp_diag.c                        |  26 +-
 net/mptcp/sockopt.c                           | 276 ++++++++
 tools/testing/selftests/net/mptcp/Makefile    |   2 +-
 .../selftests/net/mptcp/mptcp_sockopt.c       | 638 ++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      |  31 +-
 8 files changed, 979 insertions(+), 28 deletions(-)
 create mode 100644 tools/testing/selftests/net/mptcp/mptcp_sockopt.c

-- 
2.31.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH mptcp-next v2 1/5] mptcp: add new mptcp_fill_diag helper
  2021-08-16 16:26 [PATCH mptcp-next v2 0/5] mptcp: add SOL_MPTCP getsockopt support Florian Westphal
@ 2021-08-16 16:26 ` Florian Westphal
  2021-08-16 16:26 ` [PATCH mptcp-next v2 2/5] mptcp: add MPTCP_INFO getsockopt Florian Westphal
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2021-08-16 16:26 UTC (permalink / raw)
  To: mptcp; +Cc: Florian Westphal

Will be re-used from getsockopt path.
Since diag can be a module, we can't export the helper from diag, it
needs to be moved to core.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/mptcp.h    |  4 ++++
 net/mptcp/mptcp_diag.c | 26 +-------------------------
 net/mptcp/sockopt.c    | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 6026bbefbffd..f83fa48408b3 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -12,6 +12,8 @@
 #include <linux/tcp.h>
 #include <linux/types.h>
 
+struct mptcp_info;
+struct mptcp_sock;
 struct seq_file;
 
 /* MPTCP sk_buff extension data */
@@ -121,6 +123,8 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb);
 void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 			 struct mptcp_out_options *opts);
 
+void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info);
+
 /* move the skb extension owership, with the assumption that 'to' is
  * newly allocated
  */
diff --git a/net/mptcp/mptcp_diag.c b/net/mptcp/mptcp_diag.c
index f48eb6315bbb..fdf0c1f15a65 100644
--- a/net/mptcp/mptcp_diag.c
+++ b/net/mptcp/mptcp_diag.c
@@ -113,37 +113,13 @@ static void mptcp_diag_get_info(struct sock *sk, struct inet_diag_msg *r,
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct mptcp_info *info = _info;
-	u32 flags = 0;
-	bool slow;
-	u8 val;
 
 	r->idiag_rqueue = sk_rmem_alloc_get(sk);
 	r->idiag_wqueue = sk_wmem_alloc_get(sk);
 	if (!info)
 		return;
 
-	slow = lock_sock_fast(sk);
-	info->mptcpi_subflows = READ_ONCE(msk->pm.subflows);
-	info->mptcpi_add_addr_signal = READ_ONCE(msk->pm.add_addr_signaled);
-	info->mptcpi_add_addr_accepted = READ_ONCE(msk->pm.add_addr_accepted);
-	info->mptcpi_local_addr_used = READ_ONCE(msk->pm.local_addr_used);
-	info->mptcpi_subflows_max = mptcp_pm_get_subflows_max(msk);
-	val = mptcp_pm_get_add_addr_signal_max(msk);
-	info->mptcpi_add_addr_signal_max = val;
-	val = mptcp_pm_get_add_addr_accept_max(msk);
-	info->mptcpi_add_addr_accepted_max = val;
-	info->mptcpi_local_addr_max = mptcp_pm_get_local_addr_max(msk);
-	if (test_bit(MPTCP_FALLBACK_DONE, &msk->flags))
-		flags |= MPTCP_INFO_FLAG_FALLBACK;
-	if (READ_ONCE(msk->can_ack))
-		flags |= MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED;
-	info->mptcpi_flags = flags;
-	info->mptcpi_token = READ_ONCE(msk->token);
-	info->mptcpi_write_seq = READ_ONCE(msk->write_seq);
-	info->mptcpi_snd_una = READ_ONCE(msk->snd_una);
-	info->mptcpi_rcv_nxt = READ_ONCE(msk->ack_seq);
-	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
-	unlock_sock_fast(sk, slow);
+	mptcp_diag_fill_info(msk, info);
 }
 
 static const struct inet_diag_handler mptcp_diag_handler = {
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8c03afac5ca0..54f0d521a399 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -670,6 +670,38 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
 	return ret;
 }
 
+void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
+{
+	struct sock *sk = &msk->sk.icsk_inet.sk;
+	bool slow = lock_sock_fast(sk);
+	u32 flags = 0;
+	u8 val;
+
+	info->mptcpi_subflows = READ_ONCE(msk->pm.subflows);
+	info->mptcpi_add_addr_signal = READ_ONCE(msk->pm.add_addr_signaled);
+	info->mptcpi_add_addr_accepted = READ_ONCE(msk->pm.add_addr_accepted);
+	info->mptcpi_local_addr_used = READ_ONCE(msk->pm.local_addr_used);
+	info->mptcpi_subflows_max = mptcp_pm_get_subflows_max(msk);
+	val = mptcp_pm_get_add_addr_signal_max(msk);
+	info->mptcpi_add_addr_signal_max = val;
+	val = mptcp_pm_get_add_addr_accept_max(msk);
+	info->mptcpi_add_addr_accepted_max = val;
+	info->mptcpi_local_addr_max = mptcp_pm_get_local_addr_max(msk);
+	if (test_bit(MPTCP_FALLBACK_DONE, &msk->flags))
+		flags |= MPTCP_INFO_FLAG_FALLBACK;
+	if (READ_ONCE(msk->can_ack))
+		flags |= MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED;
+	info->mptcpi_flags = flags;
+	info->mptcpi_token = READ_ONCE(msk->token);
+	info->mptcpi_write_seq = READ_ONCE(msk->write_seq);
+	info->mptcpi_snd_una = READ_ONCE(msk->snd_una);
+	info->mptcpi_rcv_nxt = READ_ONCE(msk->ack_seq);
+	info->mptcpi_csum_enabled = READ_ONCE(msk->csum_enabled);
+
+	unlock_sock_fast(sk, slow);
+}
+EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
+
 static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 				    char __user *optval, int __user *optlen)
 {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH mptcp-next v2 2/5] mptcp: add MPTCP_INFO getsockopt
  2021-08-16 16:26 [PATCH mptcp-next v2 0/5] mptcp: add SOL_MPTCP getsockopt support Florian Westphal
  2021-08-16 16:26 ` [PATCH mptcp-next v2 1/5] mptcp: add new mptcp_fill_diag helper Florian Westphal
@ 2021-08-16 16:26 ` Florian Westphal
  2021-08-16 16:26 ` [PATCH mptcp-next v2 3/5] mptcp: add MPTCP_TCPINFO getsockopt support Florian Westphal
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2021-08-16 16:26 UTC (permalink / raw)
  To: mptcp; +Cc: Florian Westphal

Its not compatible with multipath-tcp.org kernel one.

1. The out-of-tree implementation defines a different 'struct mptcp_info',
   with embedded __user addresses for additional data such as
   endpoint addresses.

2. Mat Martineau points out that embedded __user addresses doesn't work
with BPF_CGROUP_RUN_PROG_GETSOCKOPT() which assumes that copying in
optsize bytes from optval provides all data that got copied to userspace.

This provides mptcp_info data for the given mptcp socket.

Userspace sets optlen to the size of the structure it expects.
The kernel updates it to contain the number of bytes that it copied.

This allows to append more information to the structure later.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/socket.h     |  1 +
 include/uapi/linux/mptcp.h |  4 ++++
 net/mptcp/sockopt.c        | 40 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index fd9ce51582d8..a2326a963232 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -364,6 +364,7 @@ struct ucred {
 #define SOL_KCM		281
 #define SOL_TLS		282
 #define SOL_XDP		283
+#define SOL_MPTCP	284
 
 /* IPX options */
 #define IPX_TYPE	1
diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index f66038b9551f..c450feba6f9c 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -4,6 +4,7 @@
 
 #include <linux/const.h>
 #include <linux/types.h>
+#include <linux/in.h>
 
 #define MPTCP_SUBFLOW_FLAG_MCAP_REM		_BITUL(0)
 #define MPTCP_SUBFLOW_FLAG_MCAP_LOC		_BITUL(1)
@@ -193,4 +194,7 @@ enum mptcp_event_attr {
 #define MPTCP_RST_EBADPERF	5
 #define MPTCP_RST_EMIDDLEBOX	6
 
+/* MPTCP socket options */
+#define MPTCP_INFO 1
+
 #endif /* _UAPI_MPTCP_H */
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 54f0d521a399..f7683c22911f 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -673,10 +673,14 @@ static int mptcp_getsockopt_first_sf_only(struct mptcp_sock *msk, int level, int
 void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 {
 	struct sock *sk = &msk->sk.icsk_inet.sk;
-	bool slow = lock_sock_fast(sk);
 	u32 flags = 0;
+	bool slow;
 	u8 val;
 
+	memset(info, 0, sizeof(*info));
+
+	slow = lock_sock_fast(sk);
+
 	info->mptcpi_subflows = READ_ONCE(msk->pm.subflows);
 	info->mptcpi_add_addr_signal = READ_ONCE(msk->pm.add_addr_signaled);
 	info->mptcpi_add_addr_accepted = READ_ONCE(msk->pm.add_addr_accepted);
@@ -702,6 +706,27 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 }
 EXPORT_SYMBOL_GPL(mptcp_diag_fill_info);
 
+static int mptcp_getsockopt_info(struct mptcp_sock *msk, char __user *optval, int __user *optlen)
+{
+	struct mptcp_info m_info;
+	int len;
+
+	if (get_user(len, optlen))
+		return -EFAULT;
+
+	len = min_t(unsigned int, len, sizeof(struct mptcp_info));
+
+	mptcp_diag_fill_info(msk, &m_info);
+
+	if (put_user(len, optlen))
+		return -EFAULT;
+
+	if (copy_to_user(optval, &m_info, len))
+		return -EFAULT;
+
+	return 0;
+}
+
 static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 				    char __user *optval, int __user *optlen)
 {
@@ -716,6 +741,17 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	return -EOPNOTSUPP;
 }
 
+static int mptcp_getsockopt_sol_mptcp(struct mptcp_sock *msk, int optname,
+				      char __user *optval, int __user *optlen)
+{
+	switch (optname) {
+	case MPTCP_INFO:
+		return mptcp_getsockopt_info(msk, optval, optlen);
+	}
+
+	return -EOPNOTSUPP;
+}
+
 int mptcp_getsockopt(struct sock *sk, int level, int optname,
 		     char __user *optval, int __user *option)
 {
@@ -738,6 +774,8 @@ int mptcp_getsockopt(struct sock *sk, int level, int optname,
 
 	if (level == SOL_TCP)
 		return mptcp_getsockopt_sol_tcp(msk, optname, optval, option);
+	if (level == SOL_MPTCP)
+		return mptcp_getsockopt_sol_mptcp(msk, optname, optval, option);
 	return -EOPNOTSUPP;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH mptcp-next v2 3/5] mptcp: add MPTCP_TCPINFO getsockopt support
  2021-08-16 16:26 [PATCH mptcp-next v2 0/5] mptcp: add SOL_MPTCP getsockopt support Florian Westphal
  2021-08-16 16:26 ` [PATCH mptcp-next v2 1/5] mptcp: add new mptcp_fill_diag helper Florian Westphal
  2021-08-16 16:26 ` [PATCH mptcp-next v2 2/5] mptcp: add MPTCP_INFO getsockopt Florian Westphal
@ 2021-08-16 16:26 ` Florian Westphal
  2021-08-18  0:54   ` Mat Martineau
  2021-08-16 16:26 ` [PATCH mptcp-next v2 4/5] mptcp: add MPTCP_SUBFLOW_ADDRS " Florian Westphal
  2021-08-16 16:26 ` [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases Florian Westphal
  4 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2021-08-16 16:26 UTC (permalink / raw)
  To: mptcp; +Cc: Florian Westphal

Allow users to retrieve TCP_INFO data of all subflows.

Users need to pre-initialize a meta header that has to be
prepended to the data buffer that will be filled with the tcp info data.

The meta header looks like this:

struct mptcp_subflow_data {
 __u32 size_subflow_data;/* size of this structure in userspace */
 __u32 num_subflows;	/* must be 0, set by kernel */
 __u32 size_kernel;	/* must be 0, set by kernel */
 __u32 size_user;	/* size of one element in data[] */
} __attribute__((aligned(8)));

size_subflow_data has to be set to 'sizeof(struct mptcp_subflow_data)'.
This allows to extend mptcp_subflow_data structure later on without
breaking backwards compatibility.

If the structure is extended later on, kernel knows where the
userspace-provided meta header ends, even if userspace uses an older
(smaller) version of the structure.

num_subflows must be set to 0. If the getsockopt request succeeds (return
value is 0), it will be updated to contain the number of active subflows
for the given logical connection.

size_kernel must be set to 0. If the getsockopt request is successful,
it will contain the size of the 'struct tcp_info' as known by the kernel.
This is informational only.

size_user must be set to 'sizeof(struct tcp_info)'.

This allows the kernel to only fill in the space reserved/expected by
userspace.

Example:

struct my_tcp_info {
  struct mptcp_subflow_data d;
  struct tcp_info ti[2];
};
struct my_tcp_info ti;
socklen_t olen;

memset(&ti, 0, sizeof(ti));

ti.d.size_subflow_data = sizeof(struct mptcp_subflow_data);
ti.d.size_user = sizeof(struct tcp_info);
olen = sizeof(ti);

ret = getsockopt(fd, SOL_MPTCP, MPTCP_TCPINFO, &ti, &olen);
if (ret < 0)
	die_perror("getsockopt MPTCP_TCPINFO");

mptcp_subflow_data.num_subflows is populated with the number of
subflows that exist on the kernel side for the logical mptcp connection.

This allows userspace to re-try with a larger tcp_info array if the number
of subflows was larger than the available space in the ti[] array.

olen has to be set to the number of bytes that userspace has allocated to
receive the kernel data.  It will be updated to contain the real number
bytes that have been copied to by the kernel.

In the above example, if the number if subflows was 1, olen is equal to
'sizeof(struct mptcp_subflow_data) + sizeof(struct tcp_info).
For 2 or more subflows olen is equal to 'sizeof(struct my_tcp_info)'.

If there was more data that could not be copied due to lack of space
in the option buffer, userspace can detect this by checking
mptcp_subflow_data->num_subflows.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/uapi/linux/mptcp.h |  10 +++-
 net/mptcp/sockopt.c        | 115 +++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index c450feba6f9c..7c368fccd83e 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -194,7 +194,15 @@ enum mptcp_event_attr {
 #define MPTCP_RST_EBADPERF	5
 #define MPTCP_RST_EMIDDLEBOX	6
 
+struct mptcp_subflow_data {
+	__u32		size_subflow_data;		/* size of this structure in userspace */
+	__u32		num_subflows;			/* must be 0, set by kernel */
+	__u32		size_kernel;			/* must be 0, set by kernel */
+	__u32		size_user;			/* size of one element in data[] */
+} __attribute__((aligned(8)));
+
 /* MPTCP socket options */
-#define MPTCP_INFO 1
+#define MPTCP_INFO		1
+#define MPTCP_TCPINFO		2
 
 #endif /* _UAPI_MPTCP_H */
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index f7683c22911f..77838933760c 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -14,6 +14,8 @@
 #include <net/mptcp.h>
 #include "protocol.h"
 
+#define MIN_INFO_OPTLEN_SIZE	16
+
 static struct sock *__mptcp_tcp_fallback(struct mptcp_sock *msk)
 {
 	sock_owned_by_me((const struct sock *)msk);
@@ -727,6 +729,117 @@ 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)
+{
+	u32 copylen = min_t(u32, sfd->size_subflow_data, sizeof(*sfd));
+
+	if (copied)
+		copied += sfd->size_subflow_data;
+	else
+		copied = copylen;
+
+	if (put_user(copied, optlen))
+		return -EFAULT;
+
+	if (copy_to_user(optval, sfd, copylen))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
+				  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(MIN_INFO_OPTLEN_SIZE != sizeof(*sfd));
+
+	if (len < MIN_INFO_OPTLEN_SIZE)
+		return -EINVAL;
+
+	memset(sfd, 0, sizeof(*sfd));
+
+	copylen = min_t(unsigned int, len, sizeof(*sfd));
+	if (copy_from_user(sfd, optval, copylen))
+		return -EFAULT;
+
+	/* size_subflow_data is u32, but len is signed */
+	if (sfd->size_subflow_data > INT_MAX ||
+	    sfd->size_user > INT_MAX)
+		return -EINVAL;
+
+	if (sfd->size_subflow_data < MIN_INFO_OPTLEN_SIZE ||
+	    sfd->size_subflow_data > len)
+		return -EINVAL;
+
+	if (sfd->num_subflows || sfd->size_kernel)
+		return -EINVAL;
+
+	return len - sfd->size_subflow_data;
+}
+
+static int mptcp_getsockopt_tcpinfo(struct mptcp_sock *msk, char __user *optval,
+				    int __user *optlen)
+{
+	struct mptcp_subflow_context *subflow;
+	struct sock *sk = &msk->sk.icsk_inet.sk;
+	unsigned int sfcount = 0, copied = 0;
+	struct mptcp_subflow_data sfd;
+	char __user *infoptr;
+	int len;
+
+	len = mptcp_get_subflow_data(&sfd, optval, optlen);
+	if (len < 0)
+		return len;
+
+	sfd.size_kernel = sizeof(struct tcp_info);
+	sfd.size_user = min_t(unsigned int, sfd.size_user,
+			      sizeof(struct tcp_info));
+
+	infoptr = optval + sfd.size_subflow_data;
+
+	lock_sock(sk);
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+		++sfcount;
+
+		if (len && len >= sfd.size_user) {
+			struct tcp_info info;
+
+			tcp_get_info(ssk, &info);
+
+			if (copy_to_user(infoptr, &info, 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_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 				    char __user *optval, int __user *optlen)
 {
@@ -747,6 +860,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_TCPINFO:
+		return mptcp_getsockopt_tcpinfo(msk, optval, optlen);
 	}
 
 	return -EOPNOTSUPP;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH mptcp-next v2 4/5] mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support
  2021-08-16 16:26 [PATCH mptcp-next v2 0/5] mptcp: add SOL_MPTCP getsockopt support Florian Westphal
                   ` (2 preceding siblings ...)
  2021-08-16 16:26 ` [PATCH mptcp-next v2 3/5] mptcp: add MPTCP_TCPINFO getsockopt support Florian Westphal
@ 2021-08-16 16:26 ` Florian Westphal
  2021-08-18  0:31   ` Mat Martineau
  2021-08-16 16:26 ` [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases Florian Westphal
  4 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2021-08-16 16:26 UTC (permalink / raw)
  To: mptcp; +Cc: Florian Westphal

This retrieves the address pairs of all subflows currently
active for a given mptcp connection.

It re-uses the same meta-header as for MPTCP_TCPINFO.

A new structure is provided to hold the subflow
address data:

struct mptcp_subflow_addrs {
	union {
		sa_family_t sa_family;
		struct sockaddr sa_local;
		struct sockaddr_in sin_local;
		struct sockaddr_in6 sin6_local;
		struct sockaddr_storage ss_local;
	};
	union {
		struct sockaddr sa_remote;
		struct sockaddr_in sin_remote;
		struct sockaddr_in6 sin6_remote;
		struct sockaddr_storage ss_remote;
	};
};

Usage of the new getsockopt is very similar to
MPTCP_TCPINFO one.

Userspace allocates a
'struct mptcp_subflow_data', followed by one or
more 'struct mptcp_subflow_addrs', then inits the
mptcp_subflow_data structure as follows:

struct mptcp_subflow_addrs *sf_addr;
struct mptcp_subflow_data *addr;
socklen_t olen = sizeof(*addr) + (8 * sizeof(*sf_addr));

addr = malloc(olen);
addr->size_subflow_data = sizeof(*addr);
addr->num_subflows = 0;
addr->size_kernel = 0;
addr->size_user = sizeof(struct mptcp_subflow_addrs);

sf_addr = (struct mptcp_subflow_addrs *)(addr + 1);

and then retrieves the endpoint addresses via:
ret = getsockopt(fd, SOL_MPTCP, MPTCP_SUBFLOW_ADDRS,
		 addr, &olen);

If the call succeeds, kernel will have added up to 8
endpoint addresses after the 'mptcp_subflow_data' header.

Userspace needs to re-check 'olen' value to detect how
many bytes have been filled in by the kernel.

Userspace can check addr->num_subflows to discover when
there were more subflows that available data space.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/uapi/linux/mptcp.h | 17 +++++++
 net/mptcp/sockopt.c        | 91 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 7c368fccd83e..c368c11a0ed9 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -201,8 +201,25 @@ struct mptcp_subflow_data {
 	__u32		size_user;			/* size of one element in data[] */
 } __attribute__((aligned(8)));
 
+struct mptcp_subflow_addrs {
+	union {
+		__kernel_sa_family_t sa_family;
+		struct sockaddr sa_local;
+		struct sockaddr_in sin_local;
+		struct sockaddr_in6 sin6_local;
+		struct sockaddr_storage ss_local;
+	};
+	union {
+		struct sockaddr sa_remote;
+		struct sockaddr_in sin_remote;
+		struct sockaddr_in6 sin6_remote;
+		struct sockaddr_storage ss_remote;
+	};
+};
+
 /* MPTCP socket options */
 #define MPTCP_INFO		1
 #define MPTCP_TCPINFO		2
+#define MPTCP_SUBFLOW_ADDRS	3
 
 #endif /* _UAPI_MPTCP_H */
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 77838933760c..51c488108b80 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -840,6 +840,95 @@ static int mptcp_getsockopt_tcpinfo(struct mptcp_sock *msk, char __user *optval,
 	return 0;
 }
 
+static void mptcp_get_sub_addrs(const struct sock *sk, struct mptcp_subflow_addrs *a)
+{
+	struct inet_sock *inet = inet_sk(sk);
+
+	memset(a, 0, sizeof(*a));
+
+	if (sk->sk_family == AF_INET) {
+		a->sin_local.sin_family = AF_INET;
+		a->sin_local.sin_port = inet->inet_sport;
+		a->sin_local.sin_addr.s_addr = inet->inet_rcv_saddr;
+
+		if (!a->sin_local.sin_addr.s_addr)
+			a->sin_local.sin_addr.s_addr = inet->inet_saddr;
+
+		a->sin_remote.sin_family = AF_INET;
+		a->sin_remote.sin_port = inet->inet_dport;
+		a->sin_remote.sin_addr.s_addr = inet->inet_daddr;
+#if IS_ENABLED(CONFIG_IPV6)
+	} else if (sk->sk_family == AF_INET6) {
+		const struct ipv6_pinfo *np = inet6_sk(sk);
+
+		a->sin6_local.sin6_family = AF_INET6;
+		a->sin6_local.sin6_port = inet->inet_sport;
+
+		if (ipv6_addr_any(&sk->sk_v6_rcv_saddr))
+			a->sin6_local.sin6_addr = np->saddr;
+		else
+			a->sin6_local.sin6_addr = sk->sk_v6_rcv_saddr;
+
+		a->sin6_remote.sin6_family = AF_INET6;
+		a->sin6_remote.sin6_port = inet->inet_dport;
+		a->sin6_remote.sin6_addr = sk->sk_v6_daddr;
+#endif
+	}
+}
+
+static int mptcp_getsockopt_subflow_addrs(struct mptcp_sock *msk, char __user *optval,
+					  int __user *optlen)
+{
+	struct mptcp_subflow_context *subflow;
+	struct sock *sk = &msk->sk.icsk_inet.sk;
+	unsigned int sfcount = 0, copied = 0;
+	struct mptcp_subflow_data sfd;
+	char __user *addrptr;
+	int len;
+
+	len = mptcp_get_subflow_data(&sfd, optval, optlen);
+	if (len < 0)
+		return len;
+
+	sfd.size_kernel = sizeof(struct mptcp_subflow_addrs);
+	sfd.size_user = min_t(unsigned int, sfd.size_user,
+			      sizeof(struct mptcp_subflow_addrs));
+
+	addrptr = optval + sfd.size_subflow_data;
+
+	lock_sock(sk);
+
+	mptcp_for_each_subflow(msk, subflow) {
+		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+
+		++sfcount;
+
+		if (len && len >= sfd.size_user) {
+			struct mptcp_subflow_addrs a;
+
+			mptcp_get_sub_addrs(ssk, &a);
+
+			if (copy_to_user(addrptr, &a, sfd.size_user)) {
+				release_sock(sk);
+				return -EFAULT;
+			}
+
+			addrptr += 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_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 				    char __user *optval, int __user *optlen)
 {
@@ -862,6 +951,8 @@ static int mptcp_getsockopt_sol_mptcp(struct mptcp_sock *msk, int optname,
 		return mptcp_getsockopt_info(msk, optval, optlen);
 	case MPTCP_TCPINFO:
 		return mptcp_getsockopt_tcpinfo(msk, optval, optlen);
+	case MPTCP_SUBFLOW_ADDRS:
+		return mptcp_getsockopt_subflow_addrs(msk, optval, optlen);
 	}
 
 	return -EOPNOTSUPP;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases
  2021-08-16 16:26 [PATCH mptcp-next v2 0/5] mptcp: add SOL_MPTCP getsockopt support Florian Westphal
                   ` (3 preceding siblings ...)
  2021-08-16 16:26 ` [PATCH mptcp-next v2 4/5] mptcp: add MPTCP_SUBFLOW_ADDRS " Florian Westphal
@ 2021-08-16 16:26 ` Florian Westphal
  2021-08-17  2:17     ` kernel test robot
  2021-08-18  0:51   ` Mat Martineau
  4 siblings, 2 replies; 11+ messages in thread
From: Florian Westphal @ 2021-08-16 16:26 UTC (permalink / raw)
  To: mptcp; +Cc: Florian Westphal

Add a test program that retrieves the three info types:
1. mptcp meta information
2. tcp info for subflow
3. subflow endpoint addresses

For all three rudimentary checks are added.

1. Meta information checks that the logical mptcp
   sequence numbers advance as expected, based on the bytes read
   (init seq + bytes_received/sent) and the connection state
   (after close, we should exect 1 extra byte due to FIN).

2. TCP info checks the number of bytes sent/received vs.
   sums of read/write syscall return values.

3. Subflow endpoint addresses are checked vs. getsockname/getpeername
   result.

Tests for forward compatibility (0-initialisation of output-only
fields in mptcp_subflow_data structure) are added as well.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/testing/selftests/net/mptcp/Makefile    |   2 +-
 .../selftests/net/mptcp/mptcp_sockopt.c       | 638 ++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      |  31 +-
 3 files changed, 668 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/net/mptcp/mptcp_sockopt.c

diff --git a/tools/testing/selftests/net/mptcp/Makefile b/tools/testing/selftests/net/mptcp/Makefile
index f1464f09b080..bbf4e448bad9 100644
--- a/tools/testing/selftests/net/mptcp/Makefile
+++ b/tools/testing/selftests/net/mptcp/Makefile
@@ -8,7 +8,7 @@ CFLAGS =  -Wall -Wl,--no-as-needed -O2 -g  -I$(top_srcdir)/usr/include
 TEST_PROGS := mptcp_connect.sh pm_netlink.sh mptcp_join.sh diag.sh \
 	      simult_flows.sh mptcp_sockopt.sh
 
-TEST_GEN_FILES = mptcp_connect pm_nl_ctl
+TEST_GEN_FILES = mptcp_connect pm_nl_ctl mptcp_sockopt
 
 TEST_FILES := settings
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
new file mode 100644
index 000000000000..036347d809bc
--- /dev/null
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
@@ -0,0 +1,638 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+
+#include <assert.h>
+#include <errno.h>
+#include <limits.h>
+#include <string.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <strings.h>
+#include <unistd.h>
+
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <netdb.h>
+#include <netinet/in.h>
+
+#include <linux/tcp.h>
+
+static int pf = AF_INET;
+
+#ifndef IPPROTO_MPTCP
+#define IPPROTO_MPTCP 262
+#endif
+#ifndef SOL_MPTCP
+#define SOL_MPTCP 284
+#endif
+
+#ifndef TCP_TCPINFO
+struct mptcp_subflow_data {
+	__u32		size_subflow_data;		/* size of this structure in userspace */
+	__u32		num_subflows;			/* must be 0, set by kernel */
+	__u32		size_kernel;			/* must be 0, set by kernel */
+	__u32		size_user;			/* size of one element in data[] */
+} __attribute__((aligned(8)));
+
+struct mptcp_subflow_addrs {
+	union {
+		sa_family_t sa_family;
+		struct sockaddr sa_local;
+		struct sockaddr_in sin_local;
+		struct sockaddr_in6 sin6_local;
+		struct sockaddr_storage ss_local;
+	};
+	union {
+		struct sockaddr sa_remote;
+		struct sockaddr_in sin_remote;
+		struct sockaddr_in6 sin6_remote;
+		struct sockaddr_storage ss_remote;
+	};
+};
+
+#define MPTCP_INFO		1
+#define MPTCP_TCPINFO		2
+#define MPTCP_SUBFLOW_ADDRS	3
+#endif
+
+struct mptcp_info {
+	__u8	mptcpi_subflows;
+	__u8	mptcpi_add_addr_signal;
+	__u8	mptcpi_add_addr_accepted;
+	__u8	mptcpi_subflows_max;
+	__u8	mptcpi_add_addr_signal_max;
+	__u8	mptcpi_add_addr_accepted_max;
+	__u32	mptcpi_flags;
+	__u32	mptcpi_token;
+	__u64	mptcpi_write_seq;
+	__u64	mptcpi_snd_una;
+	__u64	mptcpi_rcv_nxt;
+	__u8	mptcpi_local_addr_used;
+	__u8	mptcpi_local_addr_max;
+	__u8	mptcpi_csum_enabled;
+};
+
+enum sockopt_check_flags {
+	EOF_RECEIVED = 1 << 0,
+};
+
+struct so_state {
+	struct mptcp_info mi;
+};
+
+static void die_perror(const char *msg)
+{
+	perror(msg);
+	exit(1);
+}
+
+static void die_usage(int r)
+{
+	fprintf(stderr, "Usage: mptcp_sockopt [-6]\n");
+	exit(r);
+}
+
+static void xerror(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	fputc('\n', stderr);
+	exit(1);
+}
+
+static const char *getxinfo_strerr(int err)
+{
+	if (err == EAI_SYSTEM)
+		return strerror(errno);
+
+	return gai_strerror(err);
+}
+
+static void xgetaddrinfo(const char *node, const char *service,
+			 const struct addrinfo *hints,
+			 struct addrinfo **res)
+{
+	int err = getaddrinfo(node, service, hints, res);
+
+	if (err) {
+		const char *errstr = getxinfo_strerr(err);
+
+		fprintf(stderr, "Fatal: getaddrinfo(%s:%s): %s\n",
+			node ? node : "", service ? service : "", errstr);
+		exit(1);
+	}
+}
+
+static int sock_listen_mptcp(const char * const listenaddr,
+			     const char * const port)
+{
+	int sock;
+	struct addrinfo hints = {
+		.ai_protocol = IPPROTO_TCP,
+		.ai_socktype = SOCK_STREAM,
+		.ai_flags = AI_PASSIVE | AI_NUMERICHOST
+	};
+
+	hints.ai_family = pf;
+
+	struct addrinfo *a, *addr;
+	int one = 1;
+
+	xgetaddrinfo(listenaddr, port, &hints, &addr);
+	hints.ai_family = pf;
+
+	for (a = addr; a; a = a->ai_next) {
+		sock = socket(a->ai_family, a->ai_socktype, IPPROTO_MPTCP);
+		if (sock < 0)
+			continue;
+
+		if (-1 == setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &one,
+				     sizeof(one)))
+			perror("setsockopt");
+
+		if (bind(sock, a->ai_addr, a->ai_addrlen) == 0)
+			break; /* success */
+
+		perror("bind");
+		close(sock);
+		sock = -1;
+	}
+
+	freeaddrinfo(addr);
+
+	if (sock < 0)
+		xerror("could not create listen socket");
+
+	if (listen(sock, 20))
+		die_perror("listen");
+
+	return sock;
+}
+
+static int sock_connect_mptcp(const char * const remoteaddr,
+			      const char * const port, int proto)
+{
+	struct addrinfo hints = {
+		.ai_protocol = IPPROTO_TCP,
+		.ai_socktype = SOCK_STREAM,
+	};
+	struct addrinfo *a, *addr;
+	int sock = -1;
+
+	hints.ai_family = pf;
+
+	xgetaddrinfo(remoteaddr, port, &hints, &addr);
+	for (a = addr; a; a = a->ai_next) {
+		sock = socket(a->ai_family, a->ai_socktype, proto);
+		if (sock < 0)
+			continue;
+
+		if (connect(sock, a->ai_addr, a->ai_addrlen) == 0)
+			break; /* success */
+
+		die_perror("connect");
+	}
+
+	if (sock < 0)
+		xerror("could not create connect socket");
+
+	freeaddrinfo(addr);
+	return sock;
+}
+
+static void parse_opts(int argc, char **argv)
+{
+	int c;
+
+	while ((c = getopt(argc, argv, "h6")) != -1) {
+		switch (c) {
+		case 'h':
+			die_usage(0);
+			break;
+		case '6':
+			pf = AF_INET6;
+			break;
+		default:
+			die_usage(1);
+			break;
+		}
+	}
+}
+
+static void do_getsockopt_bogus_sf_data(int fd, int optname)
+{
+	struct mptcp_subflow_data good_data;
+	struct bogus_data {
+		struct mptcp_subflow_data d;
+		char buf[2];
+	} bd;
+	socklen_t olen, _olen;
+	int ret;
+
+	memset(&bd, 0, sizeof(bd));
+	memset(&good_data, 0, sizeof(good_data));
+
+	olen = sizeof(good_data);
+	good_data.size_subflow_data = olen;
+
+	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
+	assert(ret < 0); /* 0 size_subflow_data */
+	assert(olen == sizeof(good_data));
+
+	bd.d = good_data;
+
+	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
+	assert(ret == 0);
+	assert(olen == sizeof(good_data));
+	assert(bd.d.num_subflows == 1);
+	assert(bd.d.size_kernel > 0);
+	assert(bd.d.size_user == 0);
+
+	bd.d = good_data;
+	_olen = rand() % olen;
+	olen = _olen;
+	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
+	assert(ret < 0);	/* bogus olen */
+	assert(olen == _olen);	/* must be unchanged */
+
+	bd.d = good_data;
+	olen = sizeof(good_data);
+	bd.d.size_kernel = 1;
+	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
+	assert(ret < 0); /* size_kernel not 0 */
+
+	bd.d = good_data;
+	olen = sizeof(good_data);
+	bd.d.num_subflows = 1;
+	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
+	assert(ret < 0); /* num_subflows not 0 */
+
+	/* forward compat check: larger struct mptcp_subflow_data on 'old' kernel */
+	bd.d = good_data;
+	olen = sizeof(bd);
+	bd.d.size_subflow_data = sizeof(bd);
+
+	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
+	assert(ret == 0);
+
+	assert(olen == sizeof(good_data));	/* olen must be truncated to real data size filled by kernel */
+	assert(bd.d.size_subflow_data == sizeof(bd));
+
+	bd.d = good_data;
+	bd.d.size_subflow_data += 1;
+	bd.d.size_user = 1;
+	olen = bd.d.size_subflow_data + 1;
+	_olen = olen;
+
+	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &_olen);
+	assert(ret == 0);
+	assert(olen == _olen);	/* no truncation, kernel should have filled 1 byte of optname payload in buf[1] */
+	assert(bd.d.size_subflow_data == sizeof(good_data) + 1);
+	assert(bd.buf[0] == 0);
+}
+
+static void do_getsockopt_mptcp_info(struct so_state *s, int fd, size_t r, size_t w, uint32_t flags)
+{
+	struct mptcp_info i;
+	socklen_t olen;
+	int ret;
+
+	olen = sizeof(i);
+	ret = getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &i, &olen);
+
+	if (ret < 0)
+		die_perror("getsockopt MPTCP_INFO");
+
+	assert(olen == sizeof(i));
+
+	if (s->mi.mptcpi_write_seq == 0)
+		s->mi = i;
+
+	assert(s->mi.mptcpi_write_seq + w == i.mptcpi_write_seq);
+
+	if (flags & EOF_RECEIVED)
+		r += 1;
+
+	assert(s->mi.mptcpi_rcv_nxt + r == i.mptcpi_rcv_nxt);
+}
+
+static void do_getsockopt_tcp_info(int fd, size_t r, size_t w)
+{
+	struct my_tcp_info {
+		struct mptcp_subflow_data d;
+		struct tcp_info ti[2];
+	} ti;
+	socklen_t olen;
+	int ret;
+
+	memset(&ti, 0, sizeof(ti));
+
+	ti.d.size_subflow_data = sizeof(struct mptcp_subflow_data);
+	ti.d.size_user = sizeof(struct tcp_info);
+	olen = sizeof(ti);
+
+	ret = getsockopt(fd, SOL_MPTCP, MPTCP_TCPINFO, &ti, &olen);
+	if (ret < 0)
+		die_perror("getsockopt MPTCP_TCPINFO");
+
+	assert(olen <= sizeof(ti));
+	assert(ti.d.size_user == ti.d.size_kernel);
+	assert(ti.d.size_user == sizeof(struct tcp_info));
+	assert(ti.d.num_subflows == 1);
+
+	assert(olen > (socklen_t)sizeof(struct mptcp_subflow_data));
+	olen -= sizeof(struct mptcp_subflow_data);
+	assert(olen == sizeof(struct tcp_info));
+
+	assert(ti.ti[0].tcpi_bytes_sent == w);
+	assert(ti.ti[0].tcpi_bytes_received == r);
+
+	do_getsockopt_bogus_sf_data(fd, MPTCP_TCPINFO);
+}
+
+static void do_getsockopt_subflow_addrs(int fd)
+{
+	struct sockaddr_storage remote, local;
+	socklen_t olen, rlen, llen;
+	int ret;
+	struct my_addrs {
+		struct mptcp_subflow_data d;
+		struct mptcp_subflow_addrs addr[2];
+	} addrs;
+
+	memset(&addrs, 0, sizeof(addrs));
+	memset(&local, 0, sizeof(local));
+	memset(&remote, 0, sizeof(remote));
+
+	addrs.d.size_subflow_data = sizeof(struct mptcp_subflow_data);
+	addrs.d.size_user = sizeof(struct mptcp_subflow_addrs);
+	olen = sizeof(addrs);
+
+	ret = getsockopt(fd, SOL_MPTCP, MPTCP_SUBFLOW_ADDRS, &addrs, &olen);
+	if (ret < 0)
+		die_perror("getsockopt MPTCP_SUBFLOW_ADDRS");
+
+	assert(olen <= sizeof(addrs));
+	assert(addrs.d.size_user == addrs.d.size_kernel);
+	assert(addrs.d.size_user == sizeof(struct mptcp_subflow_addrs));
+	assert(addrs.d.num_subflows == 1);
+
+	assert(olen > (socklen_t)sizeof(struct mptcp_subflow_data));
+	olen -= sizeof(struct mptcp_subflow_data);
+	assert(olen == sizeof(struct mptcp_subflow_addrs));
+
+	llen = sizeof(local);
+	ret = getsockname(fd, (struct sockaddr *)&local, &llen);
+	if (ret < 0)
+		die_perror("getsockname");
+	rlen = sizeof(remote);
+	ret = getpeername(fd, (struct sockaddr *)&remote, &rlen);
+	if (ret < 0)
+		die_perror("getpeername");
+
+	assert(rlen > 0);
+	assert(rlen == llen);
+
+	assert(remote.ss_family == local.ss_family);
+
+	assert(memcmp(&local, &addrs.addr[0].ss_local, sizeof(local)) == 0);
+	assert(memcmp(&remote, &addrs.addr[0].ss_remote, sizeof(remote)) == 0);
+
+	memset(&addrs, 0, sizeof(addrs));
+
+	addrs.d.size_subflow_data = sizeof(struct mptcp_subflow_data);
+	addrs.d.size_user = sizeof(sa_family_t);
+	olen = sizeof(addrs.d) + sizeof(sa_family_t);
+
+	ret = getsockopt(fd, SOL_MPTCP, MPTCP_SUBFLOW_ADDRS, &addrs, &olen);
+	assert(ret == 0);
+	assert(olen == sizeof(addrs.d) + sizeof(sa_family_t));
+
+	assert(addrs.addr[0].sa_family == pf);
+	assert(addrs.addr[0].sa_family == local.ss_family);
+
+	assert(memcmp(&local, &addrs.addr[0].ss_local, sizeof(local)) != 0);
+	assert(memcmp(&remote, &addrs.addr[0].ss_remote, sizeof(remote)) != 0);
+
+	do_getsockopt_bogus_sf_data(fd, MPTCP_SUBFLOW_ADDRS);
+}
+
+static void do_getsockopts(struct so_state *s, int fd, size_t r, size_t w, uint32_t flags)
+{
+	do_getsockopt_mptcp_info(s, fd, r, w, flags);
+
+	do_getsockopt_tcp_info(fd, r, w);
+
+	do_getsockopt_subflow_addrs(fd);
+}
+
+static void connect_one_server(int fd)
+{
+	char buf[4096], buf2[4096];
+	struct so_state s;
+	size_t len, i;
+	ssize_t ret;
+
+	memset(&s, 0, sizeof(s));
+
+	len = rand() % (sizeof(buf) - 1);
+
+	if (len < 128)
+		len = 128;
+
+	for (i = 0; i < len ; i++) {
+		buf[i] = rand() % 26;
+		buf[i] += 'A';
+	}
+
+	buf[i] = '\n';
+
+	do_getsockopts(&s, fd, 0, 0, 0);
+
+	ret = write(fd, buf, len);
+	if (ret < 0)
+		die_perror("write");
+
+	if (ret != (ssize_t)len)
+		xerror("short write");
+
+	sleep(1);
+	do_getsockopts(&s, fd, 0, ret, 0);
+
+	ret = read(fd, buf2, sizeof(buf2));
+	if (ret < 0)
+		die_perror("read");
+
+	if ((size_t)ret != len)
+		xerror("incomplete read");
+
+	if (memcmp(buf, buf2, len))
+		xerror("data corruption");
+
+	do_getsockopts(&s, fd, ret, ret, 0);
+	close(fd);
+}
+
+static void process_one_client(int fd)
+{
+	ssize_t ret, ret2, ret3;
+	struct so_state s;
+	char buf[4096];
+
+	memset(&s, 0, sizeof(s));
+
+	do_getsockopts(&s, fd, 0, 0, 0);
+
+	ret = read(fd, buf, sizeof(buf));
+	if (ret < 0)
+		die_perror("read");
+
+	sleep(1);
+	do_getsockopts(&s, fd, ret, 0, 0);
+
+	ret2 = write(fd, buf, ret);
+	if (ret2 < 0)
+		die_perror("write");
+
+	/* wait for hangup */
+	ret3 = read(fd, buf, 1);
+	if (ret3 != 0)
+		xerror("expected EOF, got %lu", ret3);
+
+	do_getsockopts(&s, fd, ret, ret2, EOF_RECEIVED);
+	close(ret2);
+}
+
+static int xaccept(int s)
+{
+	int fd = accept(s, NULL, 0);
+
+	if (fd < 0)
+		die_perror("accept");
+
+	return fd;
+}
+
+static int server(int readyfd)
+{
+	int fd, r;
+
+	switch (pf) {
+	case AF_INET:
+		fd = sock_listen_mptcp("127.0.0.1", "15432");
+		break;
+	case AF_INET6:
+		fd = sock_listen_mptcp("::1", "15432");
+		break;
+	default:
+		xerror("Unknown pf %d\n", pf);
+	}
+
+	r = close(readyfd);
+	if (r < 0)
+		die_perror("close pipe");
+
+	alarm(15);
+	r = xaccept(fd);
+
+	process_one_client(r);
+
+	return 0;
+}
+
+static int client(void)
+{
+	int fd;
+
+	alarm(15);
+
+	switch (pf) {
+	case AF_INET:
+		fd = sock_connect_mptcp("127.0.0.1", "15432", IPPROTO_MPTCP);
+		break;
+	case AF_INET6:
+		fd = sock_connect_mptcp("::1", "15432", IPPROTO_MPTCP);
+		break;
+	default:
+		xerror("Unknown pf %d\n", pf);
+	}
+
+	connect_one_server(fd);
+
+	return 0;
+}
+
+static pid_t xfork(void)
+{
+	pid_t p = fork();
+
+	if (p < 0)
+		die_perror("fork");
+
+	return p;
+}
+
+static int rcheck(int wstatus, const char *what)
+{
+	if (WIFEXITED(wstatus)) {
+		if (WEXITSTATUS(wstatus) == 0)
+			return 0;
+		fprintf(stderr, "%s exited, status=%d\n", what, WEXITSTATUS(wstatus));
+		return WEXITSTATUS(wstatus);
+	} else if (WIFSIGNALED(wstatus)) {
+		xerror("%s killed by signal %d\n", what, WTERMSIG(wstatus));
+	} else if (WIFSTOPPED(wstatus)) {
+		xerror("%s stopped by signal %d\n", what, WSTOPSIG(wstatus));
+	}
+
+	return 111;
+}
+
+int main(int argc, char *argv[])
+{
+	int e1, e2, wstatus;
+	pid_t s, c, ret;
+	int pipefds[2];
+
+	parse_opts(argc, argv);
+
+	e1 = pipe(pipefds);
+	if (e1 < 0)
+		die_perror("pipe");
+
+	s = xfork();
+	if (s == 0)
+		return server(pipefds[1]);
+
+	close(pipefds[1]);
+
+	/* wait until server bound a socket */
+	e1 = read(pipefds[0], &e1, 4);
+	if (e1 < 0)
+		die_perror("read from pipe");
+
+	close(pipefds[0]);
+	c = xfork();
+	if (c == 0)
+		return client();
+
+	ret = waitpid(s, &wstatus, 0);
+	if (ret == -1)
+		die_perror("waitpid");
+	e1 = rcheck(wstatus, "server");
+	ret = waitpid(c, &wstatus, 0);
+	if (ret == -1)
+		die_perror("waitpid");
+	e2 = rcheck(wstatus, "client");
+
+	return e1 ? e1 : e2;
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 1579e471a5e7..41de643788b8 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -239,12 +239,35 @@ make_file()
 	echo "Created $name (size $size KB) containing data sent by $who"
 }
 
+do_mptcp_sockopt_tests()
+{
+	local lret=0
+
+	./mptcp_sockopt
+	lret=$?
+
+	if [ $lret -ne 0 ]; then
+		echo "FAIL: SOL_MPTCP getsockopt" 1>&2
+		ret=$lret
+		return
+	fi
+
+	./mptcp_sockopt -6
+	lret=$?
+
+	if [ $lret -ne 0 ]; then
+		echo "FAIL: SOL_MPTCP getsockopt (ipv6)" 1>&2
+		ret=$lret
+		return
+	fi
+}
+
 run_tests()
 {
 	listener_ns="$1"
 	connector_ns="$2"
 	connect_addr="$3"
-	lret=0
+	local lret=0
 
 	do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP ${connect_addr}
 
@@ -268,9 +291,13 @@ trap cleanup EXIT
 run_tests $ns1 $ns2 10.0.1.1
 run_tests $ns1 $ns2 dead:beef:1::1
 
-
 if [ $ret -eq 0 ];then
 	echo "PASS: all packets had packet mark set"
 fi
 
+do_mptcp_sockopt_tests
+if [ $ret -eq 0 ];then
+	echo "PASS: SOL_MPTCP getsockopt has expected information"
+fi
+
 exit $ret
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases
  2021-08-16 16:26 ` [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases Florian Westphal
@ 2021-08-17  2:17     ` kernel test robot
  2021-08-18  0:51   ` Mat Martineau
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-08-17  2:17 UTC (permalink / raw)
  To: Florian Westphal, mptcp; +Cc: clang-built-linux, kbuild-all, Florian Westphal

[-- Attachment #1: Type: text/plain, Size: 2568 bytes --]

Hi Florian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kselftest/next]
[also build test WARNING on mptcp/export linus/master v5.14-rc6 next-20210816]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Florian-Westphal/mptcp-add-SOL_MPTCP-getsockopt-support/20210817-003412
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
config: i386-randconfig-a012-20210816 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 44d0a99a12ec7ead4d2f5ef649ba05b40f6d463d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7eb1b6e289708ffa4da5aa98540a83a8b22bc452
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Florian-Westphal/mptcp-add-SOL_MPTCP-getsockopt-support/20210817-003412
        git checkout 7eb1b6e289708ffa4da5aa98540a83a8b22bc452
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> mptcp_sockopt.c:536:2: warning: variable 'fd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
           default:
           ^~~~~~~
   mptcp_sockopt.c:545:14: note: uninitialized use occurs here
           r = xaccept(fd);
                       ^~
   mptcp_sockopt.c:527:8: note: initialize the variable 'fd' to silence this warning
           int fd, r;
                 ^
                  = 0
   mptcp_sockopt.c:565:2: warning: variable 'fd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
           default:
           ^~~~~~~
   mptcp_sockopt.c:569:21: note: uninitialized use occurs here
           connect_one_server(fd);
                              ^~
   mptcp_sockopt.c:554:8: note: initialize the variable 'fd' to silence this warning
           int fd;
                 ^
                  = 0
   2 warnings generated.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38223 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases
@ 2021-08-17  2:17     ` kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-08-17  2:17 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]

Hi Florian,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kselftest/next]
[also build test WARNING on mptcp/export linus/master v5.14-rc6 next-20210816]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Florian-Westphal/mptcp-add-SOL_MPTCP-getsockopt-support/20210817-003412
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
config: i386-randconfig-a012-20210816 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 44d0a99a12ec7ead4d2f5ef649ba05b40f6d463d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7eb1b6e289708ffa4da5aa98540a83a8b22bc452
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Florian-Westphal/mptcp-add-SOL_MPTCP-getsockopt-support/20210817-003412
        git checkout 7eb1b6e289708ffa4da5aa98540a83a8b22bc452
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> mptcp_sockopt.c:536:2: warning: variable 'fd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
           default:
           ^~~~~~~
   mptcp_sockopt.c:545:14: note: uninitialized use occurs here
           r = xaccept(fd);
                       ^~
   mptcp_sockopt.c:527:8: note: initialize the variable 'fd' to silence this warning
           int fd, r;
                 ^
                  = 0
   mptcp_sockopt.c:565:2: warning: variable 'fd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
           default:
           ^~~~~~~
   mptcp_sockopt.c:569:21: note: uninitialized use occurs here
           connect_one_server(fd);
                              ^~
   mptcp_sockopt.c:554:8: note: initialize the variable 'fd' to silence this warning
           int fd;
                 ^
                  = 0
   2 warnings generated.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38223 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH mptcp-next v2 4/5] mptcp: add MPTCP_SUBFLOW_ADDRS getsockopt support
  2021-08-16 16:26 ` [PATCH mptcp-next v2 4/5] mptcp: add MPTCP_SUBFLOW_ADDRS " Florian Westphal
@ 2021-08-18  0:31   ` Mat Martineau
  0 siblings, 0 replies; 11+ messages in thread
From: Mat Martineau @ 2021-08-18  0:31 UTC (permalink / raw)
  To: Florian Westphal; +Cc: mptcp

On Mon, 16 Aug 2021, Florian Westphal wrote:

> This retrieves the address pairs of all subflows currently
> active for a given mptcp connection.
>
> It re-uses the same meta-header as for MPTCP_TCPINFO.
>
> A new structure is provided to hold the subflow
> address data:
>
> struct mptcp_subflow_addrs {
> 	union {
> 		sa_family_t sa_family;

Looks like the update to __kernel_sa_family_t was in the code but not the 
comment.

> 		struct sockaddr sa_local;
> 		struct sockaddr_in sin_local;
> 		struct sockaddr_in6 sin6_local;
> 		struct sockaddr_storage ss_local;
> 	};
> 	union {
> 		struct sockaddr sa_remote;
> 		struct sockaddr_in sin_remote;
> 		struct sockaddr_in6 sin6_remote;
> 		struct sockaddr_storage ss_remote;
> 	};
> };
>
> Usage of the new getsockopt is very similar to
> MPTCP_TCPINFO one.
>
> Userspace allocates a
> 'struct mptcp_subflow_data', followed by one or
> more 'struct mptcp_subflow_addrs', then inits the
> mptcp_subflow_data structure as follows:
>
> struct mptcp_subflow_addrs *sf_addr;
> struct mptcp_subflow_data *addr;
> socklen_t olen = sizeof(*addr) + (8 * sizeof(*sf_addr));
>
> addr = malloc(olen);
> addr->size_subflow_data = sizeof(*addr);
> addr->num_subflows = 0;
> addr->size_kernel = 0;
> addr->size_user = sizeof(struct mptcp_subflow_addrs);
>
> sf_addr = (struct mptcp_subflow_addrs *)(addr + 1);
>
> and then retrieves the endpoint addresses via:
> ret = getsockopt(fd, SOL_MPTCP, MPTCP_SUBFLOW_ADDRS,
> 		 addr, &olen);
>
> If the call succeeds, kernel will have added up to 8
> endpoint addresses after the 'mptcp_subflow_data' header.
>
> Userspace needs to re-check 'olen' value to detect how
> many bytes have been filled in by the kernel.
>
> Userspace can check addr->num_subflows to discover when
> there were more subflows that available data space.

                            ^^^^ than?

>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> include/uapi/linux/mptcp.h | 17 +++++++
> net/mptcp/sockopt.c        | 91 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 108 insertions(+)
>
> diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
> index 7c368fccd83e..c368c11a0ed9 100644
> --- a/include/uapi/linux/mptcp.h
> +++ b/include/uapi/linux/mptcp.h
> @@ -201,8 +201,25 @@ struct mptcp_subflow_data {
> 	__u32		size_user;			/* size of one element in data[] */
> } __attribute__((aligned(8)));
>
> +struct mptcp_subflow_addrs {
> +	union {
> +		__kernel_sa_family_t sa_family;
> +		struct sockaddr sa_local;
> +		struct sockaddr_in sin_local;
> +		struct sockaddr_in6 sin6_local;
> +		struct sockaddr_storage ss_local;
> +	};
> +	union {
> +		struct sockaddr sa_remote;
> +		struct sockaddr_in sin_remote;
> +		struct sockaddr_in6 sin6_remote;
> +		struct sockaddr_storage ss_remote;
> +	};
> +};
> +
> /* MPTCP socket options */
> #define MPTCP_INFO		1
> #define MPTCP_TCPINFO		2
> +#define MPTCP_SUBFLOW_ADDRS	3
>
> #endif /* _UAPI_MPTCP_H */
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 77838933760c..51c488108b80 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -840,6 +840,95 @@ static int mptcp_getsockopt_tcpinfo(struct mptcp_sock *msk, char __user *optval,
> 	return 0;
> }
>
> +static void mptcp_get_sub_addrs(const struct sock *sk, struct mptcp_subflow_addrs *a)
> +{
> +	struct inet_sock *inet = inet_sk(sk);
> +
> +	memset(a, 0, sizeof(*a));
> +
> +	if (sk->sk_family == AF_INET) {
> +		a->sin_local.sin_family = AF_INET;
> +		a->sin_local.sin_port = inet->inet_sport;
> +		a->sin_local.sin_addr.s_addr = inet->inet_rcv_saddr;
> +
> +		if (!a->sin_local.sin_addr.s_addr)
> +			a->sin_local.sin_addr.s_addr = inet->inet_saddr;
> +
> +		a->sin_remote.sin_family = AF_INET;
> +		a->sin_remote.sin_port = inet->inet_dport;
> +		a->sin_remote.sin_addr.s_addr = inet->inet_daddr;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	} else if (sk->sk_family == AF_INET6) {
> +		const struct ipv6_pinfo *np = inet6_sk(sk);
> +
> +		a->sin6_local.sin6_family = AF_INET6;
> +		a->sin6_local.sin6_port = inet->inet_sport;
> +
> +		if (ipv6_addr_any(&sk->sk_v6_rcv_saddr))
> +			a->sin6_local.sin6_addr = np->saddr;
> +		else
> +			a->sin6_local.sin6_addr = sk->sk_v6_rcv_saddr;
> +
> +		a->sin6_remote.sin6_family = AF_INET6;
> +		a->sin6_remote.sin6_port = inet->inet_dport;
> +		a->sin6_remote.sin6_addr = sk->sk_v6_daddr;
> +#endif
> +	}
> +}
> +
> +static int mptcp_getsockopt_subflow_addrs(struct mptcp_sock *msk, char __user *optval,
> +					  int __user *optlen)
> +{
> +	struct mptcp_subflow_context *subflow;
> +	struct sock *sk = &msk->sk.icsk_inet.sk;

Reverse-xmas...

Other than these minor things, the kernel code looks good in patches 1-4.


-Mat



> +	unsigned int sfcount = 0, copied = 0;
> +	struct mptcp_subflow_data sfd;
> +	char __user *addrptr;
> +	int len;
> +
> +	len = mptcp_get_subflow_data(&sfd, optval, optlen);
> +	if (len < 0)
> +		return len;
> +
> +	sfd.size_kernel = sizeof(struct mptcp_subflow_addrs);
> +	sfd.size_user = min_t(unsigned int, sfd.size_user,
> +			      sizeof(struct mptcp_subflow_addrs));
> +
> +	addrptr = optval + sfd.size_subflow_data;
> +
> +	lock_sock(sk);
> +
> +	mptcp_for_each_subflow(msk, subflow) {
> +		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> +
> +		++sfcount;
> +
> +		if (len && len >= sfd.size_user) {
> +			struct mptcp_subflow_addrs a;
> +
> +			mptcp_get_sub_addrs(ssk, &a);
> +
> +			if (copy_to_user(addrptr, &a, sfd.size_user)) {
> +				release_sock(sk);
> +				return -EFAULT;
> +			}
> +
> +			addrptr += 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_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
> 				    char __user *optval, int __user *optlen)
> {
> @@ -862,6 +951,8 @@ static int mptcp_getsockopt_sol_mptcp(struct mptcp_sock *msk, int optname,
> 		return mptcp_getsockopt_info(msk, optval, optlen);
> 	case MPTCP_TCPINFO:
> 		return mptcp_getsockopt_tcpinfo(msk, optval, optlen);
> +	case MPTCP_SUBFLOW_ADDRS:
> +		return mptcp_getsockopt_subflow_addrs(msk, optval, optlen);
> 	}
>
> 	return -EOPNOTSUPP;
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases
  2021-08-16 16:26 ` [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases Florian Westphal
  2021-08-17  2:17     ` kernel test robot
@ 2021-08-18  0:51   ` Mat Martineau
  1 sibling, 0 replies; 11+ messages in thread
From: Mat Martineau @ 2021-08-18  0:51 UTC (permalink / raw)
  To: Florian Westphal; +Cc: mptcp

On Mon, 16 Aug 2021, Florian Westphal wrote:

> Add a test program that retrieves the three info types:
> 1. mptcp meta information
> 2. tcp info for subflow
> 3. subflow endpoint addresses
>
> For all three rudimentary checks are added.
>
> 1. Meta information checks that the logical mptcp
>   sequence numbers advance as expected, based on the bytes read
>   (init seq + bytes_received/sent) and the connection state
>   (after close, we should exect 1 extra byte due to FIN).
>
> 2. TCP info checks the number of bytes sent/received vs.
>   sums of read/write syscall return values.
>
> 3. Subflow endpoint addresses are checked vs. getsockname/getpeername
>   result.
>
> Tests for forward compatibility (0-initialisation of output-only
> fields in mptcp_subflow_data structure) are added as well.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> tools/testing/selftests/net/mptcp/Makefile    |   2 +-
> .../selftests/net/mptcp/mptcp_sockopt.c       | 638 ++++++++++++++++++
> .../selftests/net/mptcp/mptcp_sockopt.sh      |  31 +-
> 3 files changed, 668 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/net/mptcp/mptcp_sockopt.c
>
> diff --git a/tools/testing/selftests/net/mptcp/Makefile b/tools/testing/selftests/net/mptcp/Makefile
> index f1464f09b080..bbf4e448bad9 100644
> --- a/tools/testing/selftests/net/mptcp/Makefile
> +++ b/tools/testing/selftests/net/mptcp/Makefile
> @@ -8,7 +8,7 @@ CFLAGS =  -Wall -Wl,--no-as-needed -O2 -g  -I$(top_srcdir)/usr/include
> TEST_PROGS := mptcp_connect.sh pm_netlink.sh mptcp_join.sh diag.sh \
> 	      simult_flows.sh mptcp_sockopt.sh
>
> -TEST_GEN_FILES = mptcp_connect pm_nl_ctl
> +TEST_GEN_FILES = mptcp_connect pm_nl_ctl mptcp_sockopt
>
> TEST_FILES := settings
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.c b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> new file mode 100644
> index 000000000000..036347d809bc
> --- /dev/null
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.c
> @@ -0,0 +1,638 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define _GNU_SOURCE
> +
> +#include <assert.h>
> +#include <errno.h>
> +#include <limits.h>
> +#include <string.h>
> +#include <stdarg.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <strings.h>
> +#include <unistd.h>
> +
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +
> +#include <netdb.h>
> +#include <netinet/in.h>
> +
> +#include <linux/tcp.h>
> +
> +static int pf = AF_INET;
> +
> +#ifndef IPPROTO_MPTCP
> +#define IPPROTO_MPTCP 262
> +#endif
> +#ifndef SOL_MPTCP
> +#define SOL_MPTCP 284
> +#endif
> +
> +#ifndef TCP_TCPINFO
> +struct mptcp_subflow_data {
> +	__u32		size_subflow_data;		/* size of this structure in userspace */
> +	__u32		num_subflows;			/* must be 0, set by kernel */
> +	__u32		size_kernel;			/* must be 0, set by kernel */
> +	__u32		size_user;			/* size of one element in data[] */
> +} __attribute__((aligned(8)));
> +
> +struct mptcp_subflow_addrs {
> +	union {
> +		sa_family_t sa_family;

Need to sync up with the change to __kernel_sa_family_t

> +		struct sockaddr sa_local;
> +		struct sockaddr_in sin_local;
> +		struct sockaddr_in6 sin6_local;
> +		struct sockaddr_storage ss_local;
> +	};
> +	union {
> +		struct sockaddr sa_remote;
> +		struct sockaddr_in sin_remote;
> +		struct sockaddr_in6 sin6_remote;
> +		struct sockaddr_storage ss_remote;
> +	};
> +};
> +
> +#define MPTCP_INFO		1
> +#define MPTCP_TCPINFO		2
> +#define MPTCP_SUBFLOW_ADDRS	3
> +#endif
> +
> +struct mptcp_info {
> +	__u8	mptcpi_subflows;
> +	__u8	mptcpi_add_addr_signal;
> +	__u8	mptcpi_add_addr_accepted;
> +	__u8	mptcpi_subflows_max;
> +	__u8	mptcpi_add_addr_signal_max;
> +	__u8	mptcpi_add_addr_accepted_max;
> +	__u32	mptcpi_flags;
> +	__u32	mptcpi_token;
> +	__u64	mptcpi_write_seq;
> +	__u64	mptcpi_snd_una;
> +	__u64	mptcpi_rcv_nxt;
> +	__u8	mptcpi_local_addr_used;
> +	__u8	mptcpi_local_addr_max;
> +	__u8	mptcpi_csum_enabled;
> +};
> +
> +enum sockopt_check_flags {
> +	EOF_RECEIVED = 1 << 0,
> +};
> +
> +struct so_state {
> +	struct mptcp_info mi;
> +};
> +
> +static void die_perror(const char *msg)
> +{
> +	perror(msg);
> +	exit(1);
> +}
> +
> +static void die_usage(int r)
> +{
> +	fprintf(stderr, "Usage: mptcp_sockopt [-6]\n");
> +	exit(r);
> +}
> +
> +static void xerror(const char *fmt, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, fmt);
> +	vfprintf(stderr, fmt, ap);
> +	va_end(ap);
> +	fputc('\n', stderr);
> +	exit(1);
> +}
> +
> +static const char *getxinfo_strerr(int err)
> +{
> +	if (err == EAI_SYSTEM)
> +		return strerror(errno);
> +
> +	return gai_strerror(err);
> +}
> +
> +static void xgetaddrinfo(const char *node, const char *service,
> +			 const struct addrinfo *hints,
> +			 struct addrinfo **res)
> +{
> +	int err = getaddrinfo(node, service, hints, res);
> +
> +	if (err) {
> +		const char *errstr = getxinfo_strerr(err);
> +
> +		fprintf(stderr, "Fatal: getaddrinfo(%s:%s): %s\n",
> +			node ? node : "", service ? service : "", errstr);
> +		exit(1);
> +	}
> +}
> +
> +static int sock_listen_mptcp(const char * const listenaddr,
> +			     const char * const port)
> +{
> +	int sock;
> +	struct addrinfo hints = {
> +		.ai_protocol = IPPROTO_TCP,
> +		.ai_socktype = SOCK_STREAM,
> +		.ai_flags = AI_PASSIVE | AI_NUMERICHOST
> +	};
> +
> +	hints.ai_family = pf;
> +
> +	struct addrinfo *a, *addr;
> +	int one = 1;
> +
> +	xgetaddrinfo(listenaddr, port, &hints, &addr);
> +	hints.ai_family = pf;
> +
> +	for (a = addr; a; a = a->ai_next) {
> +		sock = socket(a->ai_family, a->ai_socktype, IPPROTO_MPTCP);
> +		if (sock < 0)
> +			continue;
> +
> +		if (-1 == setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &one,
> +				     sizeof(one)))
> +			perror("setsockopt");
> +
> +		if (bind(sock, a->ai_addr, a->ai_addrlen) == 0)
> +			break; /* success */
> +
> +		perror("bind");
> +		close(sock);
> +		sock = -1;
> +	}
> +
> +	freeaddrinfo(addr);
> +
> +	if (sock < 0)
> +		xerror("could not create listen socket");
> +
> +	if (listen(sock, 20))
> +		die_perror("listen");
> +
> +	return sock;
> +}
> +
> +static int sock_connect_mptcp(const char * const remoteaddr,
> +			      const char * const port, int proto)
> +{
> +	struct addrinfo hints = {
> +		.ai_protocol = IPPROTO_TCP,
> +		.ai_socktype = SOCK_STREAM,
> +	};
> +	struct addrinfo *a, *addr;
> +	int sock = -1;
> +
> +	hints.ai_family = pf;
> +
> +	xgetaddrinfo(remoteaddr, port, &hints, &addr);
> +	for (a = addr; a; a = a->ai_next) {
> +		sock = socket(a->ai_family, a->ai_socktype, proto);
> +		if (sock < 0)
> +			continue;
> +
> +		if (connect(sock, a->ai_addr, a->ai_addrlen) == 0)
> +			break; /* success */
> +
> +		die_perror("connect");
> +	}
> +
> +	if (sock < 0)
> +		xerror("could not create connect socket");
> +
> +	freeaddrinfo(addr);
> +	return sock;
> +}
> +
> +static void parse_opts(int argc, char **argv)
> +{
> +	int c;
> +
> +	while ((c = getopt(argc, argv, "h6")) != -1) {
> +		switch (c) {
> +		case 'h':
> +			die_usage(0);
> +			break;
> +		case '6':
> +			pf = AF_INET6;
> +			break;
> +		default:
> +			die_usage(1);
> +			break;
> +		}
> +	}
> +}
> +
> +static void do_getsockopt_bogus_sf_data(int fd, int optname)
> +{
> +	struct mptcp_subflow_data good_data;
> +	struct bogus_data {
> +		struct mptcp_subflow_data d;
> +		char buf[2];
> +	} bd;
> +	socklen_t olen, _olen;
> +	int ret;
> +
> +	memset(&bd, 0, sizeof(bd));
> +	memset(&good_data, 0, sizeof(good_data));
> +
> +	olen = sizeof(good_data);
> +	good_data.size_subflow_data = olen;
> +
> +	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
> +	assert(ret < 0); /* 0 size_subflow_data */
> +	assert(olen == sizeof(good_data));
> +
> +	bd.d = good_data;
> +
> +	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
> +	assert(ret == 0);
> +	assert(olen == sizeof(good_data));
> +	assert(bd.d.num_subflows == 1);
> +	assert(bd.d.size_kernel > 0);
> +	assert(bd.d.size_user == 0);
> +
> +	bd.d = good_data;
> +	_olen = rand() % olen;
> +	olen = _olen;
> +	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
> +	assert(ret < 0);	/* bogus olen */
> +	assert(olen == _olen);	/* must be unchanged */
> +
> +	bd.d = good_data;
> +	olen = sizeof(good_data);
> +	bd.d.size_kernel = 1;
> +	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
> +	assert(ret < 0); /* size_kernel not 0 */
> +
> +	bd.d = good_data;
> +	olen = sizeof(good_data);
> +	bd.d.num_subflows = 1;
> +	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
> +	assert(ret < 0); /* num_subflows not 0 */
> +
> +	/* forward compat check: larger struct mptcp_subflow_data on 'old' kernel */
> +	bd.d = good_data;
> +	olen = sizeof(bd);
> +	bd.d.size_subflow_data = sizeof(bd);
> +
> +	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &olen);
> +	assert(ret == 0);
> +
> +	assert(olen == sizeof(good_data));	/* olen must be truncated to real data size filled by kernel */
> +	assert(bd.d.size_subflow_data == sizeof(bd));
> +
> +	bd.d = good_data;
> +	bd.d.size_subflow_data += 1;
> +	bd.d.size_user = 1;
> +	olen = bd.d.size_subflow_data + 1;
> +	_olen = olen;
> +
> +	ret = getsockopt(fd, SOL_MPTCP, optname, &bd, &_olen);
> +	assert(ret == 0);
> +	assert(olen == _olen);	/* no truncation, kernel should have filled 1 byte of optname payload in buf[1] */
> +	assert(bd.d.size_subflow_data == sizeof(good_data) + 1);
> +	assert(bd.buf[0] == 0);

Thanks for checking all these variations, here and in similar functions 
below!

> +}
> +
> +static void do_getsockopt_mptcp_info(struct so_state *s, int fd, size_t r, size_t w, uint32_t flags)
> +{
> +	struct mptcp_info i;
> +	socklen_t olen;
> +	int ret;
> +
> +	olen = sizeof(i);
> +	ret = getsockopt(fd, SOL_MPTCP, MPTCP_INFO, &i, &olen);
> +
> +	if (ret < 0)
> +		die_perror("getsockopt MPTCP_INFO");
> +
> +	assert(olen == sizeof(i));
> +
> +	if (s->mi.mptcpi_write_seq == 0)
> +		s->mi = i;
> +
> +	assert(s->mi.mptcpi_write_seq + w == i.mptcpi_write_seq);
> +
> +	if (flags & EOF_RECEIVED)
> +		r += 1;
> +
> +	assert(s->mi.mptcpi_rcv_nxt + r == i.mptcpi_rcv_nxt);
> +}
> +
> +static void do_getsockopt_tcp_info(int fd, size_t r, size_t w)
> +{
> +	struct my_tcp_info {
> +		struct mptcp_subflow_data d;
> +		struct tcp_info ti[2];
> +	} ti;
> +	socklen_t olen;
> +	int ret;
> +
> +	memset(&ti, 0, sizeof(ti));
> +
> +	ti.d.size_subflow_data = sizeof(struct mptcp_subflow_data);
> +	ti.d.size_user = sizeof(struct tcp_info);
> +	olen = sizeof(ti);
> +
> +	ret = getsockopt(fd, SOL_MPTCP, MPTCP_TCPINFO, &ti, &olen);
> +	if (ret < 0)
> +		die_perror("getsockopt MPTCP_TCPINFO");
> +
> +	assert(olen <= sizeof(ti));
> +	assert(ti.d.size_user == ti.d.size_kernel);
> +	assert(ti.d.size_user == sizeof(struct tcp_info));
> +	assert(ti.d.num_subflows == 1);
> +
> +	assert(olen > (socklen_t)sizeof(struct mptcp_subflow_data));
> +	olen -= sizeof(struct mptcp_subflow_data);
> +	assert(olen == sizeof(struct tcp_info));
> +
> +	assert(ti.ti[0].tcpi_bytes_sent == w);
> +	assert(ti.ti[0].tcpi_bytes_received == r);
> +
> +	do_getsockopt_bogus_sf_data(fd, MPTCP_TCPINFO);
> +}
> +
> +static void do_getsockopt_subflow_addrs(int fd)
> +{
> +	struct sockaddr_storage remote, local;
> +	socklen_t olen, rlen, llen;
> +	int ret;
> +	struct my_addrs {
> +		struct mptcp_subflow_data d;
> +		struct mptcp_subflow_addrs addr[2];
> +	} addrs;
> +
> +	memset(&addrs, 0, sizeof(addrs));
> +	memset(&local, 0, sizeof(local));
> +	memset(&remote, 0, sizeof(remote));
> +
> +	addrs.d.size_subflow_data = sizeof(struct mptcp_subflow_data);
> +	addrs.d.size_user = sizeof(struct mptcp_subflow_addrs);
> +	olen = sizeof(addrs);
> +
> +	ret = getsockopt(fd, SOL_MPTCP, MPTCP_SUBFLOW_ADDRS, &addrs, &olen);
> +	if (ret < 0)
> +		die_perror("getsockopt MPTCP_SUBFLOW_ADDRS");
> +
> +	assert(olen <= sizeof(addrs));
> +	assert(addrs.d.size_user == addrs.d.size_kernel);
> +	assert(addrs.d.size_user == sizeof(struct mptcp_subflow_addrs));
> +	assert(addrs.d.num_subflows == 1);
> +
> +	assert(olen > (socklen_t)sizeof(struct mptcp_subflow_data));
> +	olen -= sizeof(struct mptcp_subflow_data);
> +	assert(olen == sizeof(struct mptcp_subflow_addrs));
> +
> +	llen = sizeof(local);
> +	ret = getsockname(fd, (struct sockaddr *)&local, &llen);
> +	if (ret < 0)
> +		die_perror("getsockname");
> +	rlen = sizeof(remote);
> +	ret = getpeername(fd, (struct sockaddr *)&remote, &rlen);
> +	if (ret < 0)
> +		die_perror("getpeername");
> +
> +	assert(rlen > 0);
> +	assert(rlen == llen);
> +
> +	assert(remote.ss_family == local.ss_family);
> +
> +	assert(memcmp(&local, &addrs.addr[0].ss_local, sizeof(local)) == 0);
> +	assert(memcmp(&remote, &addrs.addr[0].ss_remote, sizeof(remote)) == 0);
> +
> +	memset(&addrs, 0, sizeof(addrs));
> +
> +	addrs.d.size_subflow_data = sizeof(struct mptcp_subflow_data);
> +	addrs.d.size_user = sizeof(sa_family_t);
> +	olen = sizeof(addrs.d) + sizeof(sa_family_t);
> +
> +	ret = getsockopt(fd, SOL_MPTCP, MPTCP_SUBFLOW_ADDRS, &addrs, &olen);
> +	assert(ret == 0);
> +	assert(olen == sizeof(addrs.d) + sizeof(sa_family_t));
> +
> +	assert(addrs.addr[0].sa_family == pf);
> +	assert(addrs.addr[0].sa_family == local.ss_family);
> +
> +	assert(memcmp(&local, &addrs.addr[0].ss_local, sizeof(local)) != 0);
> +	assert(memcmp(&remote, &addrs.addr[0].ss_remote, sizeof(remote)) != 0);
> +
> +	do_getsockopt_bogus_sf_data(fd, MPTCP_SUBFLOW_ADDRS);
> +}
> +
> +static void do_getsockopts(struct so_state *s, int fd, size_t r, size_t w, uint32_t flags)
> +{
> +	do_getsockopt_mptcp_info(s, fd, r, w, flags);
> +
> +	do_getsockopt_tcp_info(fd, r, w);
> +
> +	do_getsockopt_subflow_addrs(fd);
> +}
> +
> +static void connect_one_server(int fd)
> +{
> +	char buf[4096], buf2[4096];
> +	struct so_state s;
> +	size_t len, i;
> +	ssize_t ret;
> +
> +	memset(&s, 0, sizeof(s));
> +
> +	len = rand() % (sizeof(buf) - 1);
> +
> +	if (len < 128)
> +		len = 128;
> +
> +	for (i = 0; i < len ; i++) {
> +		buf[i] = rand() % 26;
> +		buf[i] += 'A';
> +	}
> +
> +	buf[i] = '\n';
> +
> +	do_getsockopts(&s, fd, 0, 0, 0);
> +
> +	ret = write(fd, buf, len);
> +	if (ret < 0)
> +		die_perror("write");
> +
> +	if (ret != (ssize_t)len)
> +		xerror("short write");
> +
> +	sleep(1);
> +	do_getsockopts(&s, fd, 0, ret, 0);
> +
> +	ret = read(fd, buf2, sizeof(buf2));
> +	if (ret < 0)
> +		die_perror("read");
> +
> +	if ((size_t)ret != len)
> +		xerror("incomplete read");
> +
> +	if (memcmp(buf, buf2, len))
> +		xerror("data corruption");
> +
> +	do_getsockopts(&s, fd, ret, ret, 0);
> +	close(fd);
> +}
> +
> +static void process_one_client(int fd)
> +{
> +	ssize_t ret, ret2, ret3;
> +	struct so_state s;
> +	char buf[4096];
> +
> +	memset(&s, 0, sizeof(s));
> +
> +	do_getsockopts(&s, fd, 0, 0, 0);
> +
> +	ret = read(fd, buf, sizeof(buf));
> +	if (ret < 0)
> +		die_perror("read");
> +
> +	sleep(1);
> +	do_getsockopts(&s, fd, ret, 0, 0);
> +
> +	ret2 = write(fd, buf, ret);
> +	if (ret2 < 0)
> +		die_perror("write");
> +
> +	/* wait for hangup */
> +	ret3 = read(fd, buf, 1);
> +	if (ret3 != 0)
> +		xerror("expected EOF, got %lu", ret3);
> +
> +	do_getsockopts(&s, fd, ret, ret2, EOF_RECEIVED);
> +	close(ret2);
> +}
> +
> +static int xaccept(int s)
> +{
> +	int fd = accept(s, NULL, 0);
> +
> +	if (fd < 0)
> +		die_perror("accept");
> +
> +	return fd;
> +}
> +
> +static int server(int readyfd)
> +{
> +	int fd, r;
> +
> +	switch (pf) {
> +	case AF_INET:
> +		fd = sock_listen_mptcp("127.0.0.1", "15432");
> +		break;
> +	case AF_INET6:
> +		fd = sock_listen_mptcp("::1", "15432");
> +		break;
> +	default:
> +		xerror("Unknown pf %d\n", pf);
> +	}
> +
> +	r = close(readyfd);
> +	if (r < 0)
> +		die_perror("close pipe");
> +
> +	alarm(15);
> +	r = xaccept(fd);

The kbuild error (fd undefined in 'default' path through the switch above) 
is bogus because xerror() will exit. Same in the client function below. 
But given that this is test code, might be good to initialize fd just to 
avoid the noise from automated checkers?

Looks good otherwise.


-Mat


> +
> +	process_one_client(r);
> +
> +	return 0;
> +}
> +
> +static int client(void)
> +{
> +	int fd;
> +
> +	alarm(15);
> +
> +	switch (pf) {
> +	case AF_INET:
> +		fd = sock_connect_mptcp("127.0.0.1", "15432", IPPROTO_MPTCP);
> +		break;
> +	case AF_INET6:
> +		fd = sock_connect_mptcp("::1", "15432", IPPROTO_MPTCP);
> +		break;
> +	default:
> +		xerror("Unknown pf %d\n", pf);
> +	}
> +
> +	connect_one_server(fd);
> +
> +	return 0;
> +}
> +
> +static pid_t xfork(void)
> +{
> +	pid_t p = fork();
> +
> +	if (p < 0)
> +		die_perror("fork");
> +
> +	return p;
> +}
> +
> +static int rcheck(int wstatus, const char *what)
> +{
> +	if (WIFEXITED(wstatus)) {
> +		if (WEXITSTATUS(wstatus) == 0)
> +			return 0;
> +		fprintf(stderr, "%s exited, status=%d\n", what, WEXITSTATUS(wstatus));
> +		return WEXITSTATUS(wstatus);
> +	} else if (WIFSIGNALED(wstatus)) {
> +		xerror("%s killed by signal %d\n", what, WTERMSIG(wstatus));
> +	} else if (WIFSTOPPED(wstatus)) {
> +		xerror("%s stopped by signal %d\n", what, WSTOPSIG(wstatus));
> +	}
> +
> +	return 111;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	int e1, e2, wstatus;
> +	pid_t s, c, ret;
> +	int pipefds[2];
> +
> +	parse_opts(argc, argv);
> +
> +	e1 = pipe(pipefds);
> +	if (e1 < 0)
> +		die_perror("pipe");
> +
> +	s = xfork();
> +	if (s == 0)
> +		return server(pipefds[1]);
> +
> +	close(pipefds[1]);
> +
> +	/* wait until server bound a socket */
> +	e1 = read(pipefds[0], &e1, 4);
> +	if (e1 < 0)
> +		die_perror("read from pipe");
> +
> +	close(pipefds[0]);
> +	c = xfork();
> +	if (c == 0)
> +		return client();
> +
> +	ret = waitpid(s, &wstatus, 0);
> +	if (ret == -1)
> +		die_perror("waitpid");
> +	e1 = rcheck(wstatus, "server");
> +	ret = waitpid(c, &wstatus, 0);
> +	if (ret == -1)
> +		die_perror("waitpid");
> +	e2 = rcheck(wstatus, "client");
> +
> +	return e1 ? e1 : e2;
> +}
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index 1579e471a5e7..41de643788b8 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -239,12 +239,35 @@ make_file()
> 	echo "Created $name (size $size KB) containing data sent by $who"
> }
>
> +do_mptcp_sockopt_tests()
> +{
> +	local lret=0
> +
> +	./mptcp_sockopt
> +	lret=$?
> +
> +	if [ $lret -ne 0 ]; then
> +		echo "FAIL: SOL_MPTCP getsockopt" 1>&2
> +		ret=$lret
> +		return
> +	fi
> +
> +	./mptcp_sockopt -6
> +	lret=$?
> +
> +	if [ $lret -ne 0 ]; then
> +		echo "FAIL: SOL_MPTCP getsockopt (ipv6)" 1>&2
> +		ret=$lret
> +		return
> +	fi
> +}
> +
> run_tests()
> {
> 	listener_ns="$1"
> 	connector_ns="$2"
> 	connect_addr="$3"
> -	lret=0
> +	local lret=0
>
> 	do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP ${connect_addr}
>
> @@ -268,9 +291,13 @@ trap cleanup EXIT
> run_tests $ns1 $ns2 10.0.1.1
> run_tests $ns1 $ns2 dead:beef:1::1
>
> -
> if [ $ret -eq 0 ];then
> 	echo "PASS: all packets had packet mark set"
> fi
>
> +do_mptcp_sockopt_tests
> +if [ $ret -eq 0 ];then
> +	echo "PASS: SOL_MPTCP getsockopt has expected information"
> +fi
> +
> exit $ret
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH mptcp-next v2 3/5] mptcp: add MPTCP_TCPINFO getsockopt support
  2021-08-16 16:26 ` [PATCH mptcp-next v2 3/5] mptcp: add MPTCP_TCPINFO getsockopt support Florian Westphal
@ 2021-08-18  0:54   ` Mat Martineau
  0 siblings, 0 replies; 11+ messages in thread
From: Mat Martineau @ 2021-08-18  0:54 UTC (permalink / raw)
  To: Florian Westphal; +Cc: mptcp


On Mon, 16 Aug 2021, Florian Westphal wrote:

> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index f7683c22911f..77838933760c 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c

...

> @@ -727,6 +729,117 @@ 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)
> +{
> +	u32 copylen = min_t(u32, sfd->size_subflow_data, sizeof(*sfd));
> +
> +	if (copied)
> +		copied += sfd->size_subflow_data;
> +	else
> +		copied = copylen;
> +
> +	if (put_user(copied, optlen))
> +		return -EFAULT;
> +
> +	if (copy_to_user(optval, sfd, copylen))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int mptcp_get_subflow_data(struct mptcp_subflow_data *sfd,
> +				  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(MIN_INFO_OPTLEN_SIZE != sizeof(*sfd));

checkpatch complains that the constant should be on the right.


--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-18  0:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-16 16:26 [PATCH mptcp-next v2 0/5] mptcp: add SOL_MPTCP getsockopt support Florian Westphal
2021-08-16 16:26 ` [PATCH mptcp-next v2 1/5] mptcp: add new mptcp_fill_diag helper Florian Westphal
2021-08-16 16:26 ` [PATCH mptcp-next v2 2/5] mptcp: add MPTCP_INFO getsockopt Florian Westphal
2021-08-16 16:26 ` [PATCH mptcp-next v2 3/5] mptcp: add MPTCP_TCPINFO getsockopt support Florian Westphal
2021-08-18  0:54   ` Mat Martineau
2021-08-16 16:26 ` [PATCH mptcp-next v2 4/5] mptcp: add MPTCP_SUBFLOW_ADDRS " Florian Westphal
2021-08-18  0:31   ` Mat Martineau
2021-08-16 16:26 ` [PATCH mptcp-next v2 5/5] selftests: mptcp: add mptcp getsockopt test cases Florian Westphal
2021-08-17  2:17   ` kernel test robot
2021-08-17  2:17     ` kernel test robot
2021-08-18  0:51   ` Mat Martineau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.