[RFC PATCH mptcp-next v15 0/5] mptcp: Fast Open Mechanism

[RFC PATCH mptcp-next v15 0/5] mptcp: Fast Open Mechanism

[Dmytro Shytyi]

These patches focus on the Initiator and partially on Listener side.
The next options in userspace are available:
a) sendto(..., ..., ..., MSG_FASTOPEN, ..., ...);
b) setsockopt(..., SOL_TCP, TCP_FASTOPEN, ..., ...);

These patches implement Appendix-B of RFC8684 (MPTFO).

We would like to credit Paulo Abeni, Mat Martineau, Matthieu Baerts and
Benjamin Hesmans for advices and ideas that improved these patches.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
v14 -> v15:
 - Reduce code size to perform MPTFO selftests. Reuse run_tests_lo().
 - In v15 proposed change of selftests introduce a reproducible issue.

Observed behavior: 
# INFO: with MPTFO start
# ns2 MPTCP -> ns1 (10.0.1.1:10054      ) MPTCP (duration    81ms) [ OK ]
# ns2 MPTCP -> ns1 (10.0.1.1:10055      ) MPTCP (duration    85ms) [ FAIL ] file received by server does not match (in, out):
# -rw------- 1 root root 4561948 Nov  6 14:44 /tmp/tmp.9LKqAENkkp
# Trailing bytes are:
# MPTCP_TEST_FILE_END_MARKER
# -rw------- 1 root root 4560528 Nov  6 14:46 /tmp/tmp.sZoeneg8ix
# Trailing bytes are:
# MPTCP_TEST_FILE_END_MARKER

Analysis:
- It seems like at the second round when data supposed to be sent in SYN, output data misses first 1420 bytes, that should be a part of the [Syn] packet. 
- With local tests (without selftest) the server in userspace receives the data in the SYN during the second round.
- To confirm that data is sent in SYN, the tcpdump was launched during tests (context of tests with tcpdump: in func sock_connect_mptcp the var was changed: char wbuf[8192] to char wbuf[511] for observavility purposes.
Output of pcap is presented below.
root@9a011dab3a59:/mnt/mptcp_net-next/tools/testing/selftests/net/mptcp# tcpdump -r 635ee290-Fz3fsx-ns2-ns1-MPTCP-MPTCP-10.0.1.1-10042-listener.pcap | grep -w "[S]"
reading from file 635ee290-Fz3fsx-ns2-ns1-MPTCP-MPTCP-10.0.1.1-10042-listener.pcap, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 65535
20:51:50.770789 sit0  In  IP 10.0.1.2.32944 > 10.0.1.1.10042: Flags [S], seq 980486723, win 64240, options [mss 1460,sackOK,TS val 2478157679 ecr 0,nop,wscale 7,tfo  cookiereq,nop,nop,mptcp capable v1], length 0
20:51:50.770807 sit0  Out IP 10.0.1.1.10042 > 10.0.1.2.32944: Flags [S.], seq 1788401934, ack 980486724, win 65160, options [mss 1460,nop,nop,sackOK,nop,wscale 7,tfo  cookie de367be85d45564d,nop,nop,mptcp capable v1 {0x2b02096a5416249e}], length 0
root@9a011dab3a59:/mnt/mptcp_net-next/tools/testing/selftests/net/mptcp# tcpdump -r 635ee290-Fz3fsx-ns2-ns1-MPTCP-MPTCP-10.0.1.1-10043-listener.pcap | grep -w "[S]"
reading from file 635ee290-Fz3fsx-ns2-ns1-MPTCP-MPTCP-10.0.1.1-10043-listener.pcap, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 65535
20:51:53.766945 sit0  In  IP 10.0.1.2.50620 > 10.0.1.1.10043: Flags [S], seq 831892394:831892905, win 64240, options [mss 1460,sackOK,TS val 2478160675 ecr 0,nop,wscale 7,tfo  cookie de367be85d45564d,nop,nop,mptcp capable v1], length 511
20:51:53.766965 sit0  Out IP 10.0.1.1.10043 > 10.0.1.2.50620: Flags [S.], seq 4054084662, ack 831892906, win 65160, options [mss 1460,sackOK,TS val 1954362321 ecr 2478160675,nop,wscale 7,mptcp capable v1 {0x7bf8471b32013c7d}], length 0
root@9a011dab3a59:/mnt/mptcp_net-next/tools/testing/selftests/net/mptcp# tcpdump -r 635ee290-Fz3fsx-ns2-ns1-MPTCP-MPTCP-10.0.1.1-10044-listener.pcap | grep -w "[S]"
reading from file 635ee290-Fz3fsx-ns2-ns1-MPTCP-MPTCP-10.0.1.1-10044-listener.pcap, link-type LINUX_SLL2 (Linux cooked v2), snapshot length 65535
20:51:57.084750 sit0  In  IP 10.0.1.2.43624 > 10.0.1.1.10044: Flags [S], seq 835270974:835271485, win 64240, options [mss 1460,sackOK,TS val 2478163993 ecr 0,nop,wscale 7,tfo  cookie de367be85d45564d,nop,nop,mptcp capable v1], length 511
20:51:57.084772 sit0  Out IP 10.0.1.1.10044 > 10.0.1.2.43624: Flags [S.], seq 650287874, ack 835271486, win 65160, options [mss 1460,sackOK,TS val 1954365639 ecr 2478163993,nop,wscale 7,mptcp capable v1 {0x7b3cb5cf253aec1b}], length 0

---

Dmytro Shytyi (5):
  mptcp: introduce MSG_FASTOPEN flag
  mptcp: implement delayed seq generation for passive fastopen
  mptcp: add subflow_v(4,6)_send_synack()
  mptcp: add TCP_FASTOPEN sock option
  selftests: mptcp: mptfo Initiator/Listener

 net/mptcp/Makefile                            |   2 +-
 net/mptcp/fastopen.c                          |  66 ++++++++++
 net/mptcp/options.c                           |   5 +
 net/mptcp/protocol.c                          |  20 +--
 net/mptcp/protocol.h                          |  18 +++
 net/mptcp/sockopt.c                           |   5 +-
 net/mptcp/subflow.c                           |  42 ++++++
 .../selftests/net/mptcp/mptcp_connect.c       | 121 +++++++++++++-----
 .../selftests/net/mptcp/mptcp_connect.sh      |  21 +++
 9 files changed, 253 insertions(+), 47 deletions(-)
 create mode 100644 net/mptcp/fastopen.c

-- 
2.34.1

[RFC PATCH mptcp-next v15 1/5] mptcp: introduce MSG_FASTOPEN flag

[Dmytro Shytyi]

In the following patches we will analyse the MSG_FASTOPEN flag
in the mptcp_sendmsg() and invoke the MPTFO.

Signed-of-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 net/mptcp/protocol.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 3196d2a350f9..f4247218fd1b 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1704,17 +1704,14 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int ret = 0;
 	long timeo;
 
-	/* we don't support FASTOPEN yet */
-	if (msg->msg_flags & MSG_FASTOPEN)
-		return -EOPNOTSUPP;
-
 	/* silently ignore everything else */
-	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;
+	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | MSG_FASTOPEN;
 
 	lock_sock(sk);
 
 	ssock = __mptcp_nmpc_socket(msk);
-	if (unlikely(ssock && inet_sk(ssock->sk)->defer_connect)) {
+	if (unlikely(ssock && (inet_sk(ssock->sk)->defer_connect ||
+			       msg->msg_flags & MSG_FASTOPEN))) {
 		int copied_syn = 0;
 
 		ret = mptcp_sendmsg_fastopen(sk, ssock->sk, msg, len, &copied_syn);
-- 
2.34.1

[RFC PATCH mptcp-next v15 2/5] mptcp: implement delayed seq generation for passive fastopen

[Dmytro Shytyi]

With fastopen in place the first subflow socket is created
before the MPC handshake completes, and we need to properly initialize
the sequence numbers at MPC ACK reception.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 net/mptcp/Makefile   |  2 +-
 net/mptcp/fastopen.c | 19 +++++++++++++++++++
 net/mptcp/options.c  |  5 +++++
 net/mptcp/protocol.h |  6 ++++++
 4 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 net/mptcp/fastopen.c

diff --git a/net/mptcp/Makefile b/net/mptcp/Makefile
index 8a7f68efa35f..c42ad8609876 100644
--- a/net/mptcp/Makefile
+++ b/net/mptcp/Makefile
@@ -2,7 +2,7 @@
 obj-$(CONFIG_MPTCP) += mptcp.o
 
 mptcp-y := protocol.o subflow.o options.o token.o crypto.o ctrl.o pm.o diag.o \
-	   mib.o pm_netlink.o sockopt.o pm_userspace.o sched.o
+	   mib.o pm_netlink.o sockopt.o pm_userspace.o sched.o fastopen.o
 
 obj-$(CONFIG_SYN_COOKIES) += syncookies.o
 obj-$(CONFIG_INET_MPTCP_DIAG) += mptcp_diag.o
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
new file mode 100644
index 000000000000..d6fb45e6be4f
--- /dev/null
+++ b/net/mptcp/fastopen.c
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * MPTCP Fast Open Mechanism. Copyright (c) 2021-2022, Dmytro SHYTYI
+ */
+
+#include "protocol.h"
+
+void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
+				   struct mptcp_options_received mp_opt)
+{
+	u64 ack_seq;
+
+	WRITE_ONCE(msk->can_ack, true);
+	WRITE_ONCE(msk->remote_key, mp_opt.sndr_key);
+	mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq);
+	ack_seq++;
+	WRITE_ONCE(msk->ack_seq, ack_seq);
+	pr_debug("ack_seq=%llu sndr_key=%llu", msk->ack_seq, mp_opt.sndr_key);
+	atomic64_set(&msk->rcv_wnd_sent, ack_seq);
+}
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 30d289044e71..0b6c4535750c 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1208,6 +1208,11 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 			mpext->dsn64 = 1;
 			mpext->mpc_map = 1;
 			mpext->data_fin = 0;
+
+			if (msk->is_mptfo) {
+				mptcp_gen_msk_ackseq_fastopen(msk, subflow, mp_opt);
+				mpext->data_seq = READ_ONCE(msk->ack_seq);
+			}
 		} else {
 			mpext->data_seq = mp_opt.data_seq;
 			mpext->subflow_seq = mp_opt.subflow_seq;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 2358a4083eb3..f4b6f817ec46 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -282,6 +282,7 @@ struct mptcp_sock {
 	bool		use_64bit_ack; /* Set when we received a 64-bit DSN */
 	bool		csum_enabled;
 	bool		allow_infinite_fallback;
+	bool		is_mptfo;
 	u8		mpc_endpoint_id;
 	u8		recvmsg_inq:1,
 			cork:1,
@@ -841,6 +842,11 @@ void mptcp_event_addr_announced(const struct sock *ssk, const struct mptcp_addr_
 void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
 bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 
+// Fast Open Mechanism functions begin
+void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
+				   struct mptcp_options_received mp_opt);
+// Fast Open Mechanism functions end
+
 static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
 {
 	return READ_ONCE(msk->pm.addr_signal) &
-- 
2.34.1

[RFC PATCH mptcp-next v15 3/5] mptcp: add subflow_v(4,6)_send_synack()

[Dmytro Shytyi]

In this patch we add skb to the msk, dequeue it from sk, remove TSs and
do skb mapping.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 net/mptcp/fastopen.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.c | 11 +----------
 net/mptcp/protocol.h | 12 +++++++++++
 net/mptcp/subflow.c  | 42 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 10 deletions(-)

diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index d6fb45e6be4f..2a36a83cd846 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -4,6 +4,53 @@
 
 #include "protocol.h"
 
+void subflow_fastopen_send_synack_set_params(struct mptcp_subflow_context *subflow,
+					     struct request_sock *req)
+{
+	struct tcp_request_sock *treq = tcp_rsk(req);
+	struct sock *ssk = subflow->tcp_sock;
+	struct sock *sk = subflow->conn;
+	struct mptcp_sock *msk;
+	struct sk_buff *skb;
+	struct tcp_sock *tp;
+	u32 offset;
+
+	msk = mptcp_sk(sk);
+	tp = tcp_sk(ssk);
+
+	/* mark subflow/msk as "mptfo" */
+	msk->is_mptfo = 1;
+
+	skb = skb_peek(&ssk->sk_receive_queue);
+
+	/* dequeue the skb from sk receive queue */
+	__skb_unlink(skb, &ssk->sk_receive_queue);
+	skb_ext_reset(skb);
+	skb_orphan(skb);
+
+	/* set the skb mapping */
+	tp->copied_seq += tp->rcv_nxt - treq->rcv_isn - 1;
+	subflow->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
+	subflow->ssn_offset = tp->copied_seq - 1;
+
+	/* initialize MPTCP_CB */
+	offset = tp->copied_seq - TCP_SKB_CB(skb)->seq;
+	MPTCP_SKB_CB(skb)->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
+	MPTCP_SKB_CB(skb)->end_seq = MPTCP_SKB_CB(skb)->map_seq +
+				     (skb->len - offset);
+	MPTCP_SKB_CB(skb)->offset = offset;
+	MPTCP_SKB_CB(skb)->has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
+
+	mptcp_data_lock(sk);
+
+	mptcp_set_owner_r(skb, sk);
+	__skb_queue_tail(&msk->receive_queue, skb);
+
+	(sk)->sk_data_ready(sk);
+
+	mptcp_data_unlock(sk);
+}
+
 void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
 				   struct mptcp_options_received mp_opt)
 {
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f4247218fd1b..bf04f4161070 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -36,15 +36,6 @@ struct mptcp6_sock {
 };
 #endif
 
-struct mptcp_skb_cb {
-	u64 map_seq;
-	u64 end_seq;
-	u32 offset;
-	u8  has_rxtstamp:1;
-};
-
-#define MPTCP_SKB_CB(__skb)	((struct mptcp_skb_cb *)&((__skb)->cb[0]))
-
 enum {
 	MPTCP_CMSG_TS = BIT(0),
 	MPTCP_CMSG_INQ = BIT(1),
@@ -200,7 +191,7 @@ static void mptcp_rfree(struct sk_buff *skb)
 	mptcp_rmem_uncharge(sk, len);
 }
 
-static void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk)
+void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk)
 {
 	skb_orphan(skb);
 	skb->sk = sk;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index f4b6f817ec46..b3dfd0dacde5 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -126,6 +126,15 @@
 #define MPTCP_CONNECTED		6
 #define MPTCP_RESET_SCHEDULER	7
 
+struct mptcp_skb_cb {
+	u64 map_seq;
+	u64 end_seq;
+	u32 offset;
+	u8  has_rxtstamp:1;
+};
+
+#define MPTCP_SKB_CB(__skb)	((struct mptcp_skb_cb *)&((__skb)->cb[0]))
+
 static inline bool before64(__u64 seq1, __u64 seq2)
 {
 	return (__s64)(seq1 - seq2) < 0;
@@ -845,6 +854,9 @@ bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 // Fast Open Mechanism functions begin
 void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
 				   struct mptcp_options_received mp_opt);
+void mptcp_set_owner_r(struct sk_buff *skb, struct sock *sk);
+void subflow_fastopen_send_synack_set_params(struct mptcp_subflow_context *subflow,
+					     struct request_sock *req);
 // Fast Open Mechanism functions end
 
 static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 02a54d59697b..5e28b02822e4 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -307,7 +307,47 @@ static struct dst_entry *subflow_v4_route_req(const struct sock *sk,
 	return NULL;
 }
 
+static int subflow_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
+				  struct flowi *fl,
+				  struct request_sock *req,
+				  struct tcp_fastopen_cookie *foc,
+				  enum tcp_synack_type synack_type,
+				  struct sk_buff *syn_skb)
+{
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
+	struct inet_request_sock *ireq = inet_rsk(req);
+
+	/* clear tstamp_ok, as needed depending on cookie */
+	if (foc && foc->len > -1)
+		ireq->tstamp_ok = 0;
+
+	if (synack_type == TCP_SYNACK_FASTOPEN)
+		subflow_fastopen_send_synack_set_params(subflow, req);
+
+	return tcp_request_sock_ipv4_ops.send_synack(sk, dst, fl, req, foc, synack_type, syn_skb);
+}
+
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
+static int subflow_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
+				  struct flowi *fl,
+				  struct request_sock *req,
+				  struct tcp_fastopen_cookie *foc,
+				  enum tcp_synack_type synack_type,
+				  struct sk_buff *syn_skb)
+{
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
+	struct inet_request_sock *ireq = inet_rsk(req);
+
+	/* clear tstamp_ok, as needed depending on cookie */
+	if (foc && foc->len > -1)
+		ireq->tstamp_ok = 0;
+
+	if (synack_type == TCP_SYNACK_FASTOPEN)
+		subflow_fastopen_send_synack_set_params(subflow, req);
+
+	return tcp_request_sock_ipv6_ops.send_synack(sk, dst, fl, req, foc, synack_type, syn_skb);
+}
+
 static struct dst_entry *subflow_v6_route_req(const struct sock *sk,
 					      struct sk_buff *skb,
 					      struct flowi *fl,
@@ -1927,6 +1967,7 @@ void __init mptcp_subflow_init(void)
 
 	subflow_request_sock_ipv4_ops = tcp_request_sock_ipv4_ops;
 	subflow_request_sock_ipv4_ops.route_req = subflow_v4_route_req;
+	subflow_request_sock_ipv4_ops.send_synack = subflow_v4_send_synack;
 
 	subflow_specific = ipv4_specific;
 	subflow_specific.conn_request = subflow_v4_conn_request;
@@ -1940,6 +1981,7 @@ void __init mptcp_subflow_init(void)
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 	subflow_request_sock_ipv6_ops = tcp_request_sock_ipv6_ops;
 	subflow_request_sock_ipv6_ops.route_req = subflow_v6_route_req;
+	subflow_request_sock_ipv6_ops.send_synack = subflow_v6_send_synack;
 
 	subflow_v6_specific = ipv6_specific;
 	subflow_v6_specific.conn_request = subflow_v6_conn_request;
-- 
2.34.1

[RFC PATCH mptcp-next v15 4/5] mptcp: add TCP_FASTOPEN sock option

[Dmytro Shytyi]

We add the TCP_FASTOPEN socket option in this patch that is going to
 be set by the listener side.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 net/mptcp/sockopt.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index f85e9bbfe86f..7f6977295256 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -559,6 +559,7 @@ static bool mptcp_supported_sockopt(int level, int optname)
 		case TCP_NOTSENT_LOWAT:
 		case TCP_TX_DELAY:
 		case TCP_INQ:
+		case TCP_FASTOPEN:
 		case TCP_FASTOPEN_CONNECT:
 		case TCP_FASTOPEN_NO_COOKIE:
 			return true;
@@ -569,7 +570,7 @@ static bool mptcp_supported_sockopt(int level, int optname)
 		/* TCP_REPAIR, TCP_REPAIR_QUEUE, TCP_QUEUE_SEQ, TCP_REPAIR_OPTIONS,
 		 * TCP_REPAIR_WINDOW are not supported, better avoid this mess
 		 */
-		/* TCP_FASTOPEN_KEY, TCP_FASTOPEN are not supported because
+		/* TCP_FASTOPEN_KEY is not supported because
 		 * fastopen for the listener side is currently unsupported
 		 */
 	}
@@ -801,6 +802,7 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 		/* See tcp.c: TCP_DEFER_ACCEPT does not fail */
 		mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname, optval, optlen);
 		return 0;
+	case TCP_FASTOPEN:
 	case TCP_FASTOPEN_CONNECT:
 	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
@@ -1166,6 +1168,7 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_INFO:
 	case TCP_CC_INFO:
 	case TCP_DEFER_ACCEPT:
+	case TCP_FASTOPEN:
 	case TCP_FASTOPEN_CONNECT:
 	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_getsockopt_first_sf_only(msk, SOL_TCP, optname,
-- 
2.34.1

[RFC PATCH mptcp-next v15 5/5] selftests: mptcp: mptfo Initiator/Listener

[Dmytro Shytyi]

This patch adds the selftests support for mptfo in mptcp_connect.c
We introduce mptfo option, that sets "TCP_FASTOPEN" + "MSG_FASTOPEN"
We call sendto() instead of connect().

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 .../selftests/net/mptcp/mptcp_connect.c       | 121 +++++++++++++-----
 .../selftests/net/mptcp/mptcp_connect.sh      |  21 +++
 2 files changed, 113 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index e54653ea2ed4..4bc855159c52 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -26,11 +26,13 @@
 
 #include <netdb.h>
 #include <netinet/in.h>
+#include <netinet/tcp.h>
 
-#include <linux/tcp.h>
 #include <linux/time_types.h>
 #include <linux/sockios.h>
 
+
+
 extern int optind;
 
 #ifndef IPPROTO_MPTCP
@@ -83,6 +85,7 @@ struct cfg_cmsg_types {
 
 struct cfg_sockopt_types {
 	unsigned int transparent:1;
+	unsigned int mptfo:1;
 };
 
 struct tcp_inq_state {
@@ -232,6 +235,14 @@ static void set_transparent(int fd, int pf)
 	}
 }
 
+static void set_mptfo(int fd, int pf)
+{
+	int qlen = 25;
+
+	if (setsockopt(fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) == -1)
+		perror("TCP_FASTOPEN");
+}
+
 static int do_ulp_so(int sock, const char *name)
 {
 	return setsockopt(sock, IPPROTO_TCP, TCP_ULP, name, strlen(name));
@@ -300,6 +311,9 @@ static int sock_listen_mptcp(const char * const listenaddr,
 		if (cfg_sockopt_types.transparent)
 			set_transparent(sock, pf);
 
+		if (cfg_sockopt_types.mptfo)
+			set_mptfo(sock, pf);
+
 		if (bind(sock, a->ai_addr, a->ai_addrlen) == 0)
 			break; /* success */
 
@@ -330,13 +344,18 @@ static int sock_listen_mptcp(const char * const listenaddr,
 
 static int sock_connect_mptcp(const char * const remoteaddr,
 			      const char * const port, int proto,
-			      struct addrinfo **peer)
+			      struct addrinfo **peer,
+			      int infd,
+			      unsigned int *woff)
 {
 	struct addrinfo hints = {
 		.ai_protocol = IPPROTO_TCP,
 		.ai_socktype = SOCK_STREAM,
 	};
 	struct addrinfo *a, *addr;
+	unsigned int wlen = 0;
+	int syn_copied = 0;
+	char wbuf[8192];
 	int sock = -1;
 
 	hints.ai_family = pf;
@@ -354,14 +373,33 @@ static int sock_connect_mptcp(const char * const remoteaddr,
 		if (cfg_mark)
 			set_mark(sock, cfg_mark);
 
-		if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
-			*peer = a;
-			break; /* success */
-		}
+		if (cfg_sockopt_types.mptfo) {
+			if (wlen == 0)
+				wlen = read(infd, wbuf, sizeof(wbuf));
 
-		perror("connect()");
-		close(sock);
-		sock = -1;
+			syn_copied = sendto(sock, wbuf, wlen, MSG_FASTOPEN,
+					    a->ai_addr, a->ai_addrlen);
+			if (syn_copied) {
+				*woff = wlen;
+				*peer = a;
+				break; /* success */
+			}
+		} else {
+			if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
+				*peer = a;
+				break; /* success */
+			}
+		}
+		if (cfg_sockopt_types.mptfo) {
+			perror("sendto()");
+			close(sock);
+			sock = -1;
+		} else {
+
+			perror("connect()");
+			close(sock);
+			sock = -1;
+		}
 	}
 
 	freeaddrinfo(addr);
@@ -571,13 +609,14 @@ static void shut_wr(int fd)
 	shutdown(fd, SHUT_WR);
 }
 
-static int copyfd_io_poll(int infd, int peerfd, int outfd, bool *in_closed_after_out)
+static int copyfd_io_poll(int infd, int peerfd, int outfd,
+			  bool *in_closed_after_out, unsigned int woff)
 {
 	struct pollfd fds = {
 		.fd = peerfd,
 		.events = POLLIN | POLLOUT,
 	};
-	unsigned int woff = 0, wlen = 0, total_wlen = 0, total_rlen = 0;
+	unsigned int wlen = 0, total_wlen = 0, total_rlen = 0;
 	char wbuf[8192];
 
 	set_nonblock(peerfd, true);
@@ -839,7 +878,7 @@ static int copyfd_io_sendfile(int infd, int peerfd, int outfd,
 	return err;
 }
 
-static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd)
+static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, unsigned int woff)
 {
 	bool in_closed_after_out = false;
 	struct timespec start, end;
@@ -851,7 +890,7 @@ static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd)
 
 	switch (cfg_mode) {
 	case CFG_MODE_POLL:
-		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out);
+		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out, woff);
 		break;
 
 	case CFG_MODE_MMAP:
@@ -1033,7 +1072,7 @@ int main_loop_s(int listensock)
 
 		SOCK_TEST_TCPULP(remotesock, 0);
 
-		copyfd_io(fd, remotesock, 1, true);
+		copyfd_io(fd, remotesock, 1, true, 0);
 	} else {
 		perror("accept");
 		return 1;
@@ -1130,6 +1169,11 @@ static void parse_setsock_options(const char *name)
 		return;
 	}
 
+	if (strncmp(name, "MPTFO", len) == 0) {
+		cfg_sockopt_types.mptfo = 1;
+		return;
+	}
+
 	fprintf(stderr, "Unrecognized setsockopt option %s\n", name);
 	exit(1);
 }
@@ -1168,23 +1212,25 @@ int main_loop(void)
 {
 	int fd, ret, fd_in = 0;
 	struct addrinfo *peer;
+	unsigned int woff = 0;
 
-	/* listener is ready. */
-	fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer);
-	if (fd < 0)
-		return 2;
-
+	if (!cfg_sockopt_types.mptfo) {
+		/* listener is ready. */
+		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, 0, 0);
+		if (fd < 0)
+			return 2;
 again:
-	check_getpeername_connect(fd);
+		check_getpeername_connect(fd);
 
-	SOCK_TEST_TCPULP(fd, cfg_sock_proto);
+		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
 
-	if (cfg_rcvbuf)
-		set_rcvbuf(fd, cfg_rcvbuf);
-	if (cfg_sndbuf)
-		set_sndbuf(fd, cfg_sndbuf);
-	if (cfg_cmsg_types.cmsg_enabled)
-		apply_cmsg_types(fd, &cfg_cmsg_types);
+		if (cfg_rcvbuf)
+			set_rcvbuf(fd, cfg_rcvbuf);
+		if (cfg_sndbuf)
+			set_sndbuf(fd, cfg_sndbuf);
+		if (cfg_cmsg_types.cmsg_enabled)
+			apply_cmsg_types(fd, &cfg_cmsg_types);
+	}
 
 	if (cfg_input) {
 		fd_in = open(cfg_input, O_RDONLY);
@@ -1192,8 +1238,25 @@ int main_loop(void)
 			xerror("can't open %s:%d", cfg_input, errno);
 	}
 
-	/* close the client socket open only if we are not going to reconnect */
-	ret = copyfd_io(fd_in, fd, 1, 0);
+	if (cfg_sockopt_types.mptfo) {
+		/* sendto() instead of connect */
+		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &woff);
+		if (fd < 0)
+			return 2;
+
+		check_getpeername_connect(fd);
+
+		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
+
+		if (cfg_rcvbuf)
+			set_rcvbuf(fd, cfg_rcvbuf);
+		if (cfg_sndbuf)
+			set_sndbuf(fd, cfg_sndbuf);
+		if (cfg_cmsg_types.cmsg_enabled)
+			apply_cmsg_types(fd, &cfg_cmsg_types);
+	}
+
+	ret = copyfd_io(fd_in, fd, 1, 0, 0);
 	if (ret)
 		return ret;
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 621af6895f4d..60198b91a530 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -762,6 +762,23 @@ run_tests_peekmode()
 	run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1 "-P ${peekmode}"
 }
 
+run_tests_mptfo()
+{
+	echo "INFO: with MPTFO start"
+	ip netns exec "$ns1" sysctl -q net.ipv4.tcp_fastopen=2
+	ip netns exec "$ns2" sysctl -q net.ipv4.tcp_fastopen=1
+
+	run_tests_lo "$ns1" "$ns2" 10.0.1.1 0 "-o MPTFO"
+	run_tests_lo "$ns1" "$ns2" 10.0.1.1 0 "-o MPTFO"
+
+	run_tests_lo "$ns1" "$ns2" dead:beef:1::1 0 "-o MPTFO"
+	run_tests_lo "$ns1" "$ns2" dead:beef:1::1 0 "-o MPTFO"
+
+	ip netns exec "$ns1" sysctl -q net.ipv4.tcp_fastopen=0
+	ip netns exec "$ns2" sysctl -q net.ipv4.tcp_fastopen=0
+	echo "INFO: with MPTFO end"
+}
+
 run_tests_disconnect()
 {
 	local peekmode="$1"
@@ -901,6 +918,10 @@ run_tests_peekmode "saveWithPeek"
 run_tests_peekmode "saveAfterPeek"
 stop_if_error "Tests with peek mode have failed"
 
+# MPTFO (MultiPath TCP Fatopen tests)
+run_tests_mptfo
+stop_if_error "Tests with MPTFO have failed"
+
 # connect to ns4 ip address, ns2 should intercept/proxy
 run_test_transparent 10.0.3.1 "tproxy ipv4"
 run_test_transparent dead:beef:3::1 "tproxy ipv6"
-- 
2.34.1

Re: selftests: mptcp: mptfo Initiator/Listener: Tests Results

[MPTCP CI]

Hi Dmytro,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 2 failed test(s): mptcp_connect_mmap selftest_mptcp_connect 🔴:
  - Task: https://cirrus-ci.com/task/5790712473059328
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5790712473059328/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 3 failed test(s): mptcp_connect_mmap packetdrill_sockopts selftest_mptcp_connect 🔴:
  - Task: https://cirrus-ci.com/task/5227762519638016
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5227762519638016/summary/summary.txt

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


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

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

For more details:

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


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

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

Re: [RFC PATCH mptcp-next v15 5/5] selftests: mptcp: mptfo Initiator/Listener

[Dmytro Shytyi]

Hello all,

@Matthieu Baerts,

Please find the first packetdrill test in this github repo: 
https://github.com/dmytroshytyi/MPTFO

Precisely,

*.pkt:

https://github.com/dmytroshytyi/MPTFO/blob/main/client-MSG_FASTOPEN.pkt

and result of test execution:

https://github.com/dmytroshytyi/MPTFO/blob/main/packetdrill-sender-test-result.png


Also I created a pull request in multipath-tcp/packetdrill:

https://github.com/multipath-tcp/packetdrill/pull/99


Best regards,

Dmytro SHYTYI.



On 11/6/2022 4:24 PM, Dmytro Shytyi wrote:
> This patch adds the selftests support for mptfo in mptcp_connect.c
> We introduce mptfo option, that sets "TCP_FASTOPEN" + "MSG_FASTOPEN"
> We call sendto() instead of connect().
>
> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
> ---
>   .../selftests/net/mptcp/mptcp_connect.c       | 121 +++++++++++++-----
>   .../selftests/net/mptcp/mptcp_connect.sh      |  21 +++
>   2 files changed, 113 insertions(+), 29 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index e54653ea2ed4..4bc855159c52 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -26,11 +26,13 @@
>   
>   #include <netdb.h>
>   #include <netinet/in.h>
> +#include <netinet/tcp.h>
>   
> -#include <linux/tcp.h>
>   #include <linux/time_types.h>
>   #include <linux/sockios.h>
>   
> +
> +
>   extern int optind;
>   
>   #ifndef IPPROTO_MPTCP
> @@ -83,6 +85,7 @@ struct cfg_cmsg_types {
>   
>   struct cfg_sockopt_types {
>   	unsigned int transparent:1;
> +	unsigned int mptfo:1;
>   };
>   
>   struct tcp_inq_state {
> @@ -232,6 +235,14 @@ static void set_transparent(int fd, int pf)
>   	}
>   }
>   
> +static void set_mptfo(int fd, int pf)
> +{
> +	int qlen = 25;
> +
> +	if (setsockopt(fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) == -1)
> +		perror("TCP_FASTOPEN");
> +}
> +
>   static int do_ulp_so(int sock, const char *name)
>   {
>   	return setsockopt(sock, IPPROTO_TCP, TCP_ULP, name, strlen(name));
> @@ -300,6 +311,9 @@ static int sock_listen_mptcp(const char * const listenaddr,
>   		if (cfg_sockopt_types.transparent)
>   			set_transparent(sock, pf);
>   
> +		if (cfg_sockopt_types.mptfo)
> +			set_mptfo(sock, pf);
> +
>   		if (bind(sock, a->ai_addr, a->ai_addrlen) == 0)
>   			break; /* success */
>   
> @@ -330,13 +344,18 @@ static int sock_listen_mptcp(const char * const listenaddr,
>   
>   static int sock_connect_mptcp(const char * const remoteaddr,
>   			      const char * const port, int proto,
> -			      struct addrinfo **peer)
> +			      struct addrinfo **peer,
> +			      int infd,
> +			      unsigned int *woff)
>   {
>   	struct addrinfo hints = {
>   		.ai_protocol = IPPROTO_TCP,
>   		.ai_socktype = SOCK_STREAM,
>   	};
>   	struct addrinfo *a, *addr;
> +	unsigned int wlen = 0;
> +	int syn_copied = 0;
> +	char wbuf[8192];
>   	int sock = -1;
>   
>   	hints.ai_family = pf;
> @@ -354,14 +373,33 @@ static int sock_connect_mptcp(const char * const remoteaddr,
>   		if (cfg_mark)
>   			set_mark(sock, cfg_mark);
>   
> -		if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
> -			*peer = a;
> -			break; /* success */
> -		}
> +		if (cfg_sockopt_types.mptfo) {
> +			if (wlen == 0)
> +				wlen = read(infd, wbuf, sizeof(wbuf));
>   
> -		perror("connect()");
> -		close(sock);
> -		sock = -1;
> +			syn_copied = sendto(sock, wbuf, wlen, MSG_FASTOPEN,
> +					    a->ai_addr, a->ai_addrlen);
> +			if (syn_copied) {
> +				*woff = wlen;
> +				*peer = a;
> +				break; /* success */
> +			}
> +		} else {
> +			if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
> +				*peer = a;
> +				break; /* success */
> +			}
> +		}
> +		if (cfg_sockopt_types.mptfo) {
> +			perror("sendto()");
> +			close(sock);
> +			sock = -1;
> +		} else {
> +
> +			perror("connect()");
> +			close(sock);
> +			sock = -1;
> +		}
>   	}
>   
>   	freeaddrinfo(addr);
> @@ -571,13 +609,14 @@ static void shut_wr(int fd)
>   	shutdown(fd, SHUT_WR);
>   }
>   
> -static int copyfd_io_poll(int infd, int peerfd, int outfd, bool *in_closed_after_out)
> +static int copyfd_io_poll(int infd, int peerfd, int outfd,
> +			  bool *in_closed_after_out, unsigned int woff)
>   {
>   	struct pollfd fds = {
>   		.fd = peerfd,
>   		.events = POLLIN | POLLOUT,
>   	};
> -	unsigned int woff = 0, wlen = 0, total_wlen = 0, total_rlen = 0;
> +	unsigned int wlen = 0, total_wlen = 0, total_rlen = 0;
>   	char wbuf[8192];
>   
>   	set_nonblock(peerfd, true);
> @@ -839,7 +878,7 @@ static int copyfd_io_sendfile(int infd, int peerfd, int outfd,
>   	return err;
>   }
>   
> -static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd)
> +static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, unsigned int woff)
>   {
>   	bool in_closed_after_out = false;
>   	struct timespec start, end;
> @@ -851,7 +890,7 @@ static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd)
>   
>   	switch (cfg_mode) {
>   	case CFG_MODE_POLL:
> -		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out);
> +		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out, woff);
>   		break;
>   
>   	case CFG_MODE_MMAP:
> @@ -1033,7 +1072,7 @@ int main_loop_s(int listensock)
>   
>   		SOCK_TEST_TCPULP(remotesock, 0);
>   
> -		copyfd_io(fd, remotesock, 1, true);
> +		copyfd_io(fd, remotesock, 1, true, 0);
>   	} else {
>   		perror("accept");
>   		return 1;
> @@ -1130,6 +1169,11 @@ static void parse_setsock_options(const char *name)
>   		return;
>   	}
>   
> +	if (strncmp(name, "MPTFO", len) == 0) {
> +		cfg_sockopt_types.mptfo = 1;
> +		return;
> +	}
> +
>   	fprintf(stderr, "Unrecognized setsockopt option %s\n", name);
>   	exit(1);
>   }
> @@ -1168,23 +1212,25 @@ int main_loop(void)
>   {
>   	int fd, ret, fd_in = 0;
>   	struct addrinfo *peer;
> +	unsigned int woff = 0;
>   
> -	/* listener is ready. */
> -	fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer);
> -	if (fd < 0)
> -		return 2;
> -
> +	if (!cfg_sockopt_types.mptfo) {
> +		/* listener is ready. */
> +		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, 0, 0);
> +		if (fd < 0)
> +			return 2;
>   again:
> -	check_getpeername_connect(fd);
> +		check_getpeername_connect(fd);
>   
> -	SOCK_TEST_TCPULP(fd, cfg_sock_proto);
> +		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
>   
> -	if (cfg_rcvbuf)
> -		set_rcvbuf(fd, cfg_rcvbuf);
> -	if (cfg_sndbuf)
> -		set_sndbuf(fd, cfg_sndbuf);
> -	if (cfg_cmsg_types.cmsg_enabled)
> -		apply_cmsg_types(fd, &cfg_cmsg_types);
> +		if (cfg_rcvbuf)
> +			set_rcvbuf(fd, cfg_rcvbuf);
> +		if (cfg_sndbuf)
> +			set_sndbuf(fd, cfg_sndbuf);
> +		if (cfg_cmsg_types.cmsg_enabled)
> +			apply_cmsg_types(fd, &cfg_cmsg_types);
> +	}
>   
>   	if (cfg_input) {
>   		fd_in = open(cfg_input, O_RDONLY);
> @@ -1192,8 +1238,25 @@ int main_loop(void)
>   			xerror("can't open %s:%d", cfg_input, errno);
>   	}
>   
> -	/* close the client socket open only if we are not going to reconnect */
> -	ret = copyfd_io(fd_in, fd, 1, 0);
> +	if (cfg_sockopt_types.mptfo) {
> +		/* sendto() instead of connect */
> +		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &woff);
> +		if (fd < 0)
> +			return 2;
> +
> +		check_getpeername_connect(fd);
> +
> +		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
> +
> +		if (cfg_rcvbuf)
> +			set_rcvbuf(fd, cfg_rcvbuf);
> +		if (cfg_sndbuf)
> +			set_sndbuf(fd, cfg_sndbuf);
> +		if (cfg_cmsg_types.cmsg_enabled)
> +			apply_cmsg_types(fd, &cfg_cmsg_types);
> +	}
> +
> +	ret = copyfd_io(fd_in, fd, 1, 0, 0);
>   	if (ret)
>   		return ret;
>   
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 621af6895f4d..60198b91a530 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -762,6 +762,23 @@ run_tests_peekmode()
>   	run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1 "-P ${peekmode}"
>   }
>   
> +run_tests_mptfo()
> +{
> +	echo "INFO: with MPTFO start"
> +	ip netns exec "$ns1" sysctl -q net.ipv4.tcp_fastopen=2
> +	ip netns exec "$ns2" sysctl -q net.ipv4.tcp_fastopen=1
> +
> +	run_tests_lo "$ns1" "$ns2" 10.0.1.1 0 "-o MPTFO"
> +	run_tests_lo "$ns1" "$ns2" 10.0.1.1 0 "-o MPTFO"
> +
> +	run_tests_lo "$ns1" "$ns2" dead:beef:1::1 0 "-o MPTFO"
> +	run_tests_lo "$ns1" "$ns2" dead:beef:1::1 0 "-o MPTFO"
> +
> +	ip netns exec "$ns1" sysctl -q net.ipv4.tcp_fastopen=0
> +	ip netns exec "$ns2" sysctl -q net.ipv4.tcp_fastopen=0
> +	echo "INFO: with MPTFO end"
> +}
> +
>   run_tests_disconnect()
>   {
>   	local peekmode="$1"
> @@ -901,6 +918,10 @@ run_tests_peekmode "saveWithPeek"
>   run_tests_peekmode "saveAfterPeek"
>   stop_if_error "Tests with peek mode have failed"
>   
> +# MPTFO (MultiPath TCP Fatopen tests)
> +run_tests_mptfo
> +stop_if_error "Tests with MPTFO have failed"
> +
>   # connect to ns4 ip address, ns2 should intercept/proxy
>   run_test_transparent 10.0.3.1 "tproxy ipv4"
>   run_test_transparent dead:beef:3::1 "tproxy ipv6"

Re: [RFC PATCH mptcp-next v15 5/5] selftests: mptcp: mptfo Initiator/Listener

[Paolo Abeni]

On Sun, 2022-11-06 at 15:24 +0000, Dmytro Shytyi wrote:
> This patch adds the selftests support for mptfo in mptcp_connect.c
> We introduce mptfo option, that sets "TCP_FASTOPEN" + "MSG_FASTOPEN"
> We call sendto() instead of connect().
> 
> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
> ---
>  .../selftests/net/mptcp/mptcp_connect.c       | 121 +++++++++++++-----
>  .../selftests/net/mptcp/mptcp_connect.sh      |  21 +++
>  2 files changed, 113 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index e54653ea2ed4..4bc855159c52 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -26,11 +26,13 @@
>  
>  #include <netdb.h>
>  #include <netinet/in.h>
> +#include <netinet/tcp.h>
>  
> -#include <linux/tcp.h>

Funnily enough, the above change breaks the build on my host. I see the
CI does not complain about it, but I still would avoid it

>  #include <linux/time_types.h>
>  #include <linux/sockios.h>
>  
> +
> +

Unneeded empty lines, do not add them

>  extern int optind;
>  
>  #ifndef IPPROTO_MPTCP
> @@ -83,6 +85,7 @@ struct cfg_cmsg_types {
>  
>  struct cfg_sockopt_types {
>  	unsigned int transparent:1;
> +	unsigned int mptfo:1;
>  };
>  
>  struct tcp_inq_state {
> @@ -232,6 +235,14 @@ static void set_transparent(int fd, int pf)
>  	}
>  }
>  
> +static void set_mptfo(int fd, int pf)
> +{
> +	int qlen = 25;
> +
> +	if (setsockopt(fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen)) == -1)

If you use IPPROTO_TCP here instead of SOL_TCP - yep, it's not what the
man page says - you can avoid the initial problematic chunck

> +		perror("TCP_FASTOPEN");
> +}
> +
>  static int do_ulp_so(int sock, const char *name)
>  {
>  	return setsockopt(sock, IPPROTO_TCP, TCP_ULP, name, strlen(name));
> @@ -300,6 +311,9 @@ static int sock_listen_mptcp(const char * const listenaddr,
>  		if (cfg_sockopt_types.transparent)
>  			set_transparent(sock, pf);
>  
> +		if (cfg_sockopt_types.mptfo)
> +			set_mptfo(sock, pf);
> +
>  		if (bind(sock, a->ai_addr, a->ai_addrlen) == 0)
>  			break; /* success */
>  
> @@ -330,13 +344,18 @@ static int sock_listen_mptcp(const char * const listenaddr,
>  
>  static int sock_connect_mptcp(const char * const remoteaddr,
>  			      const char * const port, int proto,
> -			      struct addrinfo **peer)
> +			      struct addrinfo **peer,
> +			      int infd,
> +			      unsigned int *woff)
>  {
>  	struct addrinfo hints = {
>  		.ai_protocol = IPPROTO_TCP,
>  		.ai_socktype = SOCK_STREAM,
>  	};
>  	struct addrinfo *a, *addr;
> +	unsigned int wlen = 0;
> +	int syn_copied = 0;
> +	char wbuf[8192];
>  	int sock = -1;
>  
>  	hints.ai_family = pf;
> @@ -354,14 +373,33 @@ static int sock_connect_mptcp(const char * const remoteaddr,
>  		if (cfg_mark)
>  			set_mark(sock, cfg_mark);
>  
> -		if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
> -			*peer = a;
> -			break; /* success */
> -		}
> +		if (cfg_sockopt_types.mptfo) {
> +			if (wlen == 0)
> +				wlen = read(infd, wbuf, sizeof(wbuf));
>  
> -		perror("connect()");
> -		close(sock);
> -		sock = -1;
> +			syn_copied = sendto(sock, wbuf, wlen, MSG_FASTOPEN,
> +					    a->ai_addr, a->ai_addrlen);
> +			if (syn_copied) {
> +				*woff = wlen;

This is likely causing the self-test failures. syn_copied can be lower
than wlen - or even negative. You must check for errors, and save the
exact amount of bytes copied. Additionally you need to preserve wbuf
contents, or read but unsent bytes will be lost.

I'll try to send an incremental patch asap.

> +				*peer = a;
> +				break; /* success */
> +			}
> +		} else {
> +			if (connect(sock, a->ai_addr, a->ai_addrlen) == 0) {
> +				*peer = a;
> +				break; /* success */
> +			}
> +		}
> +		if (cfg_sockopt_types.mptfo) {
> +			perror("sendto()");
> +			close(sock);
> +			sock = -1;
> +		} else {
> +
> +			perror("connect()");
> +			close(sock);
> +			sock = -1;
> +		}
>  	}
>  
>  	freeaddrinfo(addr);
> @@ -571,13 +609,14 @@ static void shut_wr(int fd)
>  	shutdown(fd, SHUT_WR);
>  }
>  
> -static int copyfd_io_poll(int infd, int peerfd, int outfd, bool *in_closed_after_out)
> +static int copyfd_io_poll(int infd, int peerfd, int outfd,
> +			  bool *in_closed_after_out, unsigned int woff)
>  {
>  	struct pollfd fds = {
>  		.fd = peerfd,
>  		.events = POLLIN | POLLOUT,
>  	};
> -	unsigned int woff = 0, wlen = 0, total_wlen = 0, total_rlen = 0;
> +	unsigned int wlen = 0, total_wlen = 0, total_rlen = 0;
>  	char wbuf[8192];
>  
>  	set_nonblock(peerfd, true);
> @@ -839,7 +878,7 @@ static int copyfd_io_sendfile(int infd, int peerfd, int outfd,
>  	return err;
>  }
>  
> -static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd)
> +static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd, unsigned int woff)
>  {
>  	bool in_closed_after_out = false;
>  	struct timespec start, end;
> @@ -851,7 +890,7 @@ static int copyfd_io(int infd, int peerfd, int outfd, bool close_peerfd)
>  
>  	switch (cfg_mode) {
>  	case CFG_MODE_POLL:
> -		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out);
> +		ret = copyfd_io_poll(infd, peerfd, outfd, &in_closed_after_out, woff);
>  		break;
>  
>  	case CFG_MODE_MMAP:
> @@ -1033,7 +1072,7 @@ int main_loop_s(int listensock)
>  
>  		SOCK_TEST_TCPULP(remotesock, 0);
>  
> -		copyfd_io(fd, remotesock, 1, true);
> +		copyfd_io(fd, remotesock, 1, true, 0);
>  	} else {
>  		perror("accept");
>  		return 1;
> @@ -1130,6 +1169,11 @@ static void parse_setsock_options(const char *name)
>  		return;
>  	}
>  
> +	if (strncmp(name, "MPTFO", len) == 0) {
> +		cfg_sockopt_types.mptfo = 1;
> +		return;
> +	}
> +
>  	fprintf(stderr, "Unrecognized setsockopt option %s\n", name);
>  	exit(1);
>  }
> @@ -1168,23 +1212,25 @@ int main_loop(void)
>  {
>  	int fd, ret, fd_in = 0;
>  	struct addrinfo *peer;
> +	unsigned int woff = 0;
>  
> -	/* listener is ready. */
> -	fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer);
> -	if (fd < 0)
> -		return 2;
> -
> +	if (!cfg_sockopt_types.mptfo) {
> +		/* listener is ready. */
> +		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, 0, 0);
> +		if (fd < 0)
> +			return 2;
>  again:
> -	check_getpeername_connect(fd);
> +		check_getpeername_connect(fd);
>  
> -	SOCK_TEST_TCPULP(fd, cfg_sock_proto);
> +		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
>  
> -	if (cfg_rcvbuf)
> -		set_rcvbuf(fd, cfg_rcvbuf);
> -	if (cfg_sndbuf)
> -		set_sndbuf(fd, cfg_sndbuf);
> -	if (cfg_cmsg_types.cmsg_enabled)
> -		apply_cmsg_types(fd, &cfg_cmsg_types);
> +		if (cfg_rcvbuf)
> +			set_rcvbuf(fd, cfg_rcvbuf);
> +		if (cfg_sndbuf)
> +			set_sndbuf(fd, cfg_sndbuf);
> +		if (cfg_cmsg_types.cmsg_enabled)
> +			apply_cmsg_types(fd, &cfg_cmsg_types);
> +	}
>  
>  	if (cfg_input) {
>  		fd_in = open(cfg_input, O_RDONLY);
> @@ -1192,8 +1238,25 @@ int main_loop(void)
>  			xerror("can't open %s:%d", cfg_input, errno);
>  	}
>  
> -	/* close the client socket open only if we are not going to reconnect */
> -	ret = copyfd_io(fd_in, fd, 1, 0);
> +	if (cfg_sockopt_types.mptfo) {
> +		/* sendto() instead of connect */
> +		fd = sock_connect_mptcp(cfg_host, cfg_port, cfg_sock_proto, &peer, fd_in, &woff);
> +		if (fd < 0)
> +			return 2;
> +
> +		check_getpeername_connect(fd);
> +
> +		SOCK_TEST_TCPULP(fd, cfg_sock_proto);
> +
> +		if (cfg_rcvbuf)
> +			set_rcvbuf(fd, cfg_rcvbuf);
> +		if (cfg_sndbuf)
> +			set_sndbuf(fd, cfg_sndbuf);
> +		if (cfg_cmsg_types.cmsg_enabled)
> +			apply_cmsg_types(fd, &cfg_cmsg_types);
> +	}
> +
> +	ret = copyfd_io(fd_in, fd, 1, 0, 0);

Here GCC is reporting an uninitialized warning for fd. I think it's
better to avoid special case for TFO above, and let
sock_connect_mptcp() deal with that.


Cheers,

Paolo

Re: [RFC PATCH mptcp-next v15 0/5] mptcp: Fast Open Mechanism

[Matthieu Baerts]

Hi Dmytro, Paolo,

On 06/11/2022 16:24, Dmytro Shytyi wrote:
> These patches focus on the Initiator and partially on Listener side.
> The next options in userspace are available:
> a) sendto(..., ..., ..., MSG_FASTOPEN, ..., ...);
> b) setsockopt(..., SOL_TCP, TCP_FASTOPEN, ..., ...);
> 
> These patches implement Appendix-B of RFC8684 (MPTFO).
> 
> We would like to credit Paulo Abeni, Mat Martineau, Matthieu Baerts and
> Benjamin Hesmans for advices and ideas that improved these patches.
> 
> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>

(...)

> Dmytro Shytyi (5):
>   mptcp: introduce MSG_FASTOPEN flag
>   mptcp: implement delayed seq generation for passive fastopen
>   mptcp: add subflow_v(4,6)_send_synack()
>   mptcp: add TCP_FASTOPEN sock option
>   selftests: mptcp: mptfo Initiator/Listener

Thank you for the patches, reviews and Squash-to patches!

These patches and the ones from Paolo are now in our tree (feat. for
net-next-next) with Paolo's ACK on patch 1/5 and 4/5 and the Co-Dev ones
automatically added on the 3 others.

Please note that I had two minor conflicts when applying Dmytro's
patches before the BPF series already in our tree + some issues reported
by checkpatch I didn't fix (yet).

I also had more complex conflicts in Paolo's first squash-to patch: it
is modifying both patches 2/5 and 3/5, I had to split it. Please check
everything is still OK after that.

New patches for t/upstream:
- c87fe09b3765: mptcp: introduce MSG_FASTOPEN flag
- b08f0e118b1e: mptcp: implement delayed seq generation for passive fastopen
- bbd23a7de970: mptcp: add subflow_v(4,6)_send_synack()
- a2e147cb3f0a: mptcp: add TCP_FASTOPEN sock option
- 132e3b9f088e: selftests: mptcp: mptfo Initiator/Listener
- Results: 5f9e0bf4aacf..120554402db6 (export)

Paolo's patches:

- 512d25e34a03: mptcp: track accurately the incoming MPC suboption type
- Results: 120554402db6..a6011a4a534a (export)

- c0cda5b7f267: "squashed" (with conflicts) in "mptcp: implement delayed
seq generation for passive fastopen" (part 1)
- 7f98dbf4215e: "Signed-off-by" + "Co-developed-by"
- 6d231fd79244: conflict in t/mptcp-add-subflow_v-4-6-_send_synack
- b2bb9c9570a0: "squashed" in "mptcp: add subflow_v(4,6)_send_synack()"
(part 2)
- 7d3cac0330e4: "Signed-off-by" + "Co-developed-by"
- Results: a6011a4a534a..7141982e8319 (export)

- 27f8b8024c74: "squashed" in "selftests: mptcp: mptfo Initiator/Listener"
- ebe619034d7f: "Signed-off-by" + "Co-developed-by"
- Results: 7141982e8319..dfaa717d485a (export)


Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20221110T185015

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