[RFC PATCH mptcp-next v12 0/7] mptcp: Fast Open Mechanism

[RFC PATCH mptcp-next v12 0/7] 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.

Dmytro Shytyi (5):
  mptcp: introduce MSG_FASTOPEN flag.
  mptcp: fix retrans., add mptfo vars to msk
  mptcp: add subflow_v(4,6)_send_synack()
  mptcp: add TCP_FASTOPEN option
  selftests: mptfo initiator/listener

Matthieu Baerts (2):
  mptcp: sockopt: make 'tcp_fastopen_connect' generic
  mptcp: add TCP_FASTOPEN_NO_COOKIE support

 include/net/mptcp.h                           |   9 ++
 include/net/tcp.h                             |   3 +
 net/ipv4/tcp.c                                |  20 +++-
 net/ipv4/tcp_fastopen.c                       |  19 ++--
 net/mptcp/Makefile                            |   2 +-
 net/mptcp/fastopen.c                          |  62 +++++++++++
 net/mptcp/options.c                           |   5 +
 net/mptcp/protocol.c                          |  21 ++--
 net/mptcp/protocol.h                          |   9 ++
 net/mptcp/sockopt.c                           |  21 ++--
 net/mptcp/subflow.c                           |  42 ++++++++
 tools/testing/selftests/net/mptcp/mptfo.sh    |  13 +++
 .../selftests/net/mptcp/mptfo_initiator.c     |  43 ++++++++
 .../selftests/net/mptcp/mptfo_listener.c      | 100 ++++++++++++++++++
 14 files changed, 342 insertions(+), 27 deletions(-)
 create mode 100644 net/mptcp/fastopen.c
 create mode 100644 tools/testing/selftests/net/mptcp/mptfo.sh
 create mode 100644 tools/testing/selftests/net/mptcp/mptfo_initiator.c
 create mode 100644 tools/testing/selftests/net/mptcp/mptfo_listener.c

-- 
2.34.1

[RFC PATCH mptcp-next v12 1/7] 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>
---
 include/net/mptcp.h  |  9 +++++++++
 include/net/tcp.h    |  3 +++
 net/ipv4/tcp.c       | 20 ++++++++++++++++----
 net/mptcp/protocol.c | 19 +++++++++++--------
 4 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index c25939b2af68..ccf2b42837a1 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -150,6 +150,8 @@ void mptcp_write_options(struct tcphdr *th, __be32 *ptr, struct tcp_sock *tp,
 			 struct mptcp_out_options *opts);
 
 void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info);
+int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_len, int flags);
+struct sock *mptcp_subflow_conn_sock(struct sock *sk);
 
 /* move the skb extension owership, with the assumption that 'to' is
  * newly allocated
@@ -286,6 +288,13 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
 
 static inline void mptcp_space(const struct sock *ssk, int *s, int *fs) { }
 static inline void mptcp_seq_show(struct seq_file *seq) { }
+static inline int mptcp_stream_connect(struct socket *sock,
+				       struct sockaddr *uaddr,
+				       int addr_len,
+				       int flags)
+{
+
+}
 
 static inline int mptcp_subflow_init_cookie_req(struct request_sock *req,
 						const struct sock *sk_listener,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4f71cc15ff8e..e53d26d74dec 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1757,6 +1757,9 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
 			      struct request_sock *req,
 			      struct tcp_fastopen_cookie *foc,
 			      const struct dst_entry *dst);
+int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
+			 int *copied, size_t size,
+			 struct ubuf_info *uarg);
 void tcp_fastopen_init_key_once(struct net *net);
 bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
 			     struct tcp_fastopen_cookie *cookie);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5237a3f08c94..daa611671d9a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1162,8 +1162,9 @@ void tcp_free_fastopen_req(struct tcp_sock *tp)
 	}
 }
 
-int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
-			 size_t size, struct ubuf_info *uarg)
+int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
+			 int *copied, size_t size,
+			 struct ubuf_info *uarg)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct inet_sock *inet = inet_sk(sk);
@@ -1196,8 +1197,19 @@ int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
 		}
 	}
 	flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
-	err = __inet_stream_connect(sk->sk_socket, uaddr,
-				    msg->msg_namelen, flags, 1);
+	if (!sk_is_mptcp(sk)) {
+		err = __inet_stream_connect(sk->sk_socket, uaddr,
+					    msg->msg_namelen, flags, 1);
+	} else {
+		struct sock *parent = mptcp_subflow_conn_sock(sk);
+
+		release_sock(sk);
+		release_sock(parent);
+		err = mptcp_stream_connect(sk->sk_socket, uaddr,
+					   msg->msg_namelen, msg->msg_flags);
+		lock_sock(parent);
+		lock_sock(sk);
+	}
 	/* fastopen_req could already be freed in __inet_stream_connect
 	 * if the connection times out or gets rst
 	 */
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 8feb684408f7..0db50712bad7 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -58,6 +58,12 @@ static void __mptcp_check_send_data_fin(struct sock *sk);
 DEFINE_PER_CPU(struct mptcp_delegated_action, mptcp_delegated_actions);
 static struct net_device mptcp_napi_dev;
 
+struct sock *
+mptcp_subflow_conn_sock(struct sock *sk)
+{
+	return ((mptcp_subflow_ctx(sk))->conn);
+}
+
 /* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not
  * completed yet or has failed, return the subflow socket.
  * Otherwise return NULL.
@@ -1673,17 +1679,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))) {
 		struct sock *ssk = ssock->sk;
 		int copied_syn = 0;
 
@@ -3568,8 +3571,8 @@ static void mptcp_subflow_early_fallback(struct mptcp_sock *msk,
 	__mptcp_do_fallback(msk);
 }
 
-static int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr,
-				int addr_len, int flags)
+int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr,
+			 int addr_len, int flags)
 {
 	struct mptcp_sock *msk = mptcp_sk(sock->sk);
 	struct mptcp_subflow_context *subflow;
-- 
2.34.1

[RFC PATCH mptcp-next v12 2/7] mptcp: fix retrans., add mptfo vars to msk

[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 93c535440a5c..a9708a2eb2bc 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,
@@ -838,6 +839,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 v12 3/7] 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/ipv4/tcp_fastopen.c | 19 ++++++++++++------
 net/mptcp/fastopen.c    | 43 +++++++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.c    |  2 +-
 net/mptcp/protocol.h    |  3 +++
 net/mptcp/subflow.c     | 42 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 45cc7f1ca296..d6b1380525ea 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -356,13 +356,20 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
 	if (foc->len == 0) /* Client requests a cookie */
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENCOOKIEREQD);
 
-	if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
-	      (syn_data || foc->len >= 0) &&
-	      tcp_fastopen_queue_check(sk))) {
-		foc->len = -1;
-		return NULL;
+	if (sk_is_mptcp(sk)) {
+		if (((syn_data || foc->len >= 0) &&
+		     tcp_fastopen_queue_check(sk))) {
+			foc->len = -1;
+			return NULL;
+		}
+	} else {
+		if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
+		      (syn_data || foc->len >= 0) &&
+		      tcp_fastopen_queue_check(sk))) {
+			foc->len = -1;
+			return NULL;
+		}
 	}
-
 	if (tcp_fastopen_no_cookie(sk, dst, TFO_SERVER_COOKIE_NOT_REQD))
 		goto fastopen;
 
diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index d6fb45e6be4f..1d824a4d9606 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -17,3 +17,46 @@ void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_
 	pr_debug("ack_seq=%llu sndr_key=%llu", msk->ack_seq, mp_opt.sndr_key);
 	atomic64_set(&msk->rcv_wnd_sent, ack_seq);
 }
+
+void subflow_fastopen_send_synack_set_params(struct mptcp_subflow_context *subflow,
+					     struct request_sock *req)
+{
+	struct tcp_request_sock *tcp_r_sock = 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;
+
+	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 - tcp_r_sock->rcv_isn - 1;
+	subflow->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
+	subflow->ssn_offset = tp->copied_seq - 1;
+
+	/* <acquire the msk data lock> */
+	mptcp_data_lock(sk);
+
+	/* <move skb into msk receive queue> */
+	mptcp_set_owner_r(skb, sk);
+	__skb_queue_tail(&msk->receive_queue, skb);
+	atomic64_set(&msk->rcv_wnd_sent, mptcp_subflow_get_mapped_dsn(subflow));
+
+	/* <call msk data_ready> */
+	(sk)->sk_data_ready(sk);
+
+	/* <release the msk data lock> */
+	mptcp_data_unlock(sk);
+}
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 0db50712bad7..7e63b414011c 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -206,7 +206,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 a9708a2eb2bc..9c46e802a73a 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -842,6 +842,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 07dd23d0fe04..c48143bff114 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -307,6 +307,46 @@ 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);
+
+	/* <evenutally clear tstamp_ok, as needed depending on cookie size> */
+	if (foc)
+		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);
+}
+
+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);
+
+	/* <evenutally clear tstamp_ok, as needed depending on cookie size> */
+	if (foc)
+		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);
+}
+
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 static struct dst_entry *subflow_v6_route_req(const struct sock *sk,
 					      struct sk_buff *skb,
@@ -1920,6 +1960,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;
@@ -1933,6 +1974,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 v12 4/7] mptcp: sockopt: make 'tcp_fastopen_connect' generic

[Dmytro Shytyi]

From: Matthieu Baerts <matthieu.baerts@tessares.net>

This is patch provided by Matthieu. As it seems it not yet merged I
temprorarly integrated in into my series for compilation.
'https://lore.kernel.org/mptcp/
20220927150306.1552795-1-matthieu.baerts@tessares.net/T/'
When it will be merged I will delete it from this set of patches.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 net/mptcp/sockopt.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index c7cb68c725b2..8d3b09d75c3a 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -769,17 +769,17 @@ static int mptcp_setsockopt_sol_tcp_defer(struct mptcp_sock *msk, sockptr_t optv
 	return tcp_setsockopt(listener->sk, SOL_TCP, TCP_DEFER_ACCEPT, optval, optlen);
 }
 
-static int mptcp_setsockopt_sol_tcp_fastopen_connect(struct mptcp_sock *msk, sockptr_t optval,
-						     unsigned int optlen)
+static int mptcp_setsockopt_first_sf_only(struct mptcp_sock *msk, int level, int optname,
+					  sockptr_t optval, unsigned int optlen)
 {
 	struct socket *sock;
 
-	/* Limit to first subflow */
+	/* Limit to first subflow, before the connection establishment */
 	sock = __mptcp_nmpc_socket(msk);
 	if (!sock)
 		return -EINVAL;
 
-	return tcp_setsockopt(sock->sk, SOL_TCP, TCP_FASTOPEN_CONNECT, optval, optlen);
+	return tcp_setsockopt(sock->sk, level, optname, optval, optlen);
 }
 
 static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
@@ -811,7 +811,8 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_DEFER_ACCEPT:
 		return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen);
 	case TCP_FASTOPEN_CONNECT:
-		return mptcp_setsockopt_sol_tcp_fastopen_connect(msk, optval, optlen);
+		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
+						      optval, optlen);
 	}
 
 	return -EOPNOTSUPP;
-- 
2.34.1

[RFC PATCH mptcp-next v12 5/7] mptcp: add TCP_FASTOPEN_NO_COOKIE support

[Dmytro Shytyi]

From: Matthieu Baerts <matthieu.baerts@tessares.net>

This is patch provided by Matthieu. As it seems it not yet merged I
temprorarly integrated in into my series for compilation.
'https://lore.kernel.org/mptcp/
20220927150306.1552795-1-matthieu.baerts@tessares.net/T/'
When it will be merged I will delete it from this set of patches.

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 net/mptcp/sockopt.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8d3b09d75c3a..1857281a0dd5 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -560,6 +560,7 @@ static bool mptcp_supported_sockopt(int level, int optname)
 		case TCP_TX_DELAY:
 		case TCP_INQ:
 		case TCP_FASTOPEN_CONNECT:
+		case TCP_FASTOPEN_NO_COOKIE:
 			return true;
 		}
 
@@ -568,8 +569,8 @@ 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, TCP_FASTOPEN_NO_COOKIE,
-		 * are not supported fastopen is currently unsupported
+		/* TCP_FASTOPEN_KEY, TCP_FASTOPEN are not supported because
+		 * fastopen for the listener side is currently unsupported
 		 */
 	}
 	return false;
@@ -811,6 +812,7 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_DEFER_ACCEPT:
 		return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen);
 	case TCP_FASTOPEN_CONNECT:
+	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
 						      optval, optlen);
 	}
@@ -1175,6 +1177,7 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_CC_INFO:
 	case TCP_DEFER_ACCEPT:
 	case TCP_FASTOPEN_CONNECT:
+	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_getsockopt_first_sf_only(msk, SOL_TCP, optname,
 						      optval, optlen);
 	case TCP_INQ:
-- 
2.34.1

[RFC PATCH mptcp-next v12 6/7] mptcp: add TCP_FASTOPEN 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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 1857281a0dd5..ab3ba8558efd 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;
@@ -811,6 +812,7 @@ static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 		return mptcp_setsockopt_sol_tcp_nodelay(msk, optval, optlen);
 	case TCP_DEFER_ACCEPT:
 		return mptcp_setsockopt_sol_tcp_defer(msk, optval, optlen);
+	case TCP_FASTOPEN:
 	case TCP_FASTOPEN_CONNECT:
 	case TCP_FASTOPEN_NO_COOKIE:
 		return mptcp_setsockopt_first_sf_only(msk, SOL_TCP, optname,
@@ -1176,6 +1178,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 v12 7/7] selftests: mptfo initiator/listener

[Dmytro Shytyi]

MPTFO tests: these are examples of initiator (sendto) and listener,
probably are going to be integrated to the mptcp_connect.* selftests

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 tools/testing/selftests/net/mptcp/mptfo.sh    |  13 +++
 .../selftests/net/mptcp/mptfo_initiator.c     |  43 ++++++++
 .../selftests/net/mptcp/mptfo_listener.c      | 100 ++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 tools/testing/selftests/net/mptcp/mptfo.sh
 create mode 100644 tools/testing/selftests/net/mptcp/mptfo_initiator.c
 create mode 100644 tools/testing/selftests/net/mptcp/mptfo_listener.c

diff --git a/tools/testing/selftests/net/mptcp/mptfo.sh b/tools/testing/selftests/net/mptcp/mptfo.sh
new file mode 100644
index 000000000000..9ed0cb281094
--- /dev/null
+++ b/tools/testing/selftests/net/mptcp/mptfo.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+#This is an example of environmen that was used to generate wireshark
+sudo ip netns add server
+sudo ip netns add client
+sudo ip link add veth0 type veth peer name veth1
+sudo ip link set veth1 netns server
+sudo ip link set veth0 netns client
+sudo ip netns exec client ip a a 10.10.0.1/24 dev veth0
+sudo ip netns exec server ip a a 10.10.0.2/24 dev veth1
+sudo ip netns exec client ip link set dev  veth0 up
+sudo ip netns exec server ip link set dev  veth1 up
+sudo ip netns exec server bash -c "echo 2 > /proc/sys/net/ipv4/tcp_fastopen"
+sudo ip netns exec client bash -c "echo 1 > /proc/sys/net/ipv4/tcp_fastopen"
diff --git a/tools/testing/selftests/net/mptcp/mptfo_initiator.c b/tools/testing/selftests/net/mptcp/mptfo_initiator.c
new file mode 100644
index 000000000000..05e6a3d62bb8
--- /dev/null
+++ b/tools/testing/selftests/net/mptcp/mptfo_initiator.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/socket.h>
+#include <unistd.h>
+#include <netinet/tcp.h>
+#include <string.h>
+#include <signal.h>
+
+#define SERVER_PORT 7003
+
+int main(int argc, char *argv[])
+{
+	unsigned char valsyn[3] = "abc";
+	struct sockaddr_in daddr;
+	char *valend = "fff";
+	char *val1 = "zz1";
+	char *val2 = "zz2";
+	char *val3 = "zz3";
+	int sock_fd = -1;
+	int ret;
+
+	memset(&daddr, 0, sizeof(daddr));
+	inet_pton(AF_INET, "10.10.0.2", &daddr.sin_addr);
+	daddr.sin_family = AF_INET;
+	daddr.sin_port = htons(SERVER_PORT);
+
+	sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP);
+
+	ret = sendto(sock_fd, valsyn, 3, MSG_FASTOPEN, (struct sockaddr *) &daddr, sizeof(daddr));
+	ret = write(sock_fd, val1, 3);
+	ret = write(sock_fd, val2, 3);
+	ret = write(sock_fd, val2, 3);
+	ret = write(sock_fd, val2, 3);
+	ret = write(sock_fd, val3, 3);
+	ret = write(sock_fd, valend, 3);
+
+	close(sock_fd);
+	return EXIT_SUCCESS;
+}
diff --git a/tools/testing/selftests/net/mptcp/mptfo_listener.c b/tools/testing/selftests/net/mptcp/mptfo_listener.c
new file mode 100644
index 000000000000..6890349b14bb
--- /dev/null
+++ b/tools/testing/selftests/net/mptcp/mptfo_listener.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <arpa/inet.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <netinet/in.h>
+#include <linux/in.h>
+#include <netinet/tcp.h>
+
+#define CLIENT_QUEUE_LEN 10
+#define SERVER_PORT 7003
+
+int main(void)
+{
+	int listen_sock_fd = -1, client_sock_fd = -1;
+	char str_addr[INET6_ADDRSTRLEN];
+	struct sockaddr_in server_addr;
+	int ret, flag;
+	int qlen = 5;
+	char ch;
+
+	server_addr.sin_family = AF_INET;
+	inet_pton(AF_INET, "10.10.0.2", &server_addr.sin_addr);
+	server_addr.sin_port = htons(SERVER_PORT);
+
+	/* Create socket for listening (client requests) */
+	listen_sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP);
+	if (listen_sock_fd == -1) {
+		perror("socket()server");
+		return EXIT_FAILURE;
+	}
+
+	/* Set socket to reuse address */
+	flag = 1;
+	ret = setsockopt(listen_sock_fd, SOL_SOCKET, SO_REUSEADDR, &flag, sizeof(flag));
+	if (ret == -1) {
+		perror("setsockopt()");
+		return EXIT_FAILURE;
+	}
+
+	ret = setsockopt(listen_sock_fd, SOL_TCP, TCP_FASTOPEN, &qlen, sizeof(qlen));
+	if (ret == -1) {
+		perror("setsockopt()TCP_FASTOPEN");
+		return EXIT_FAILURE;
+	}
+
+	/* Bind address and socket together */
+	ret = bind(listen_sock_fd, (struct sockaddr *)&server_addr, sizeof(server_addr));
+	if (ret == -1) {
+		perror("bind()");
+		close(listen_sock_fd);
+		return EXIT_FAILURE;
+	}
+
+	/* Create listening queue (client requests) */
+	ret = listen(listen_sock_fd, CLIENT_QUEUE_LEN);
+	if (ret == -1) {
+		perror("listen()");
+		close(listen_sock_fd);
+		return EXIT_FAILURE;
+	}
+	perror("Server listening");
+	while (1) {
+		/* Do TCP handshake with client */
+		client_sock_fd = accept(listen_sock_fd,
+				NULL,
+				0);
+		if (client_sock_fd == -1) {
+			perror("accept()");
+			close(listen_sock_fd);
+			return EXIT_FAILURE;
+		} else {
+			perror("ACCEPT_SUCCESS");
+		}
+
+		char rb[1024];
+
+		while (1) {
+			ret = read(client_sock_fd, rb, 3);
+
+			if (ret == -1) {
+				perror("SERVVERread()");
+				close(client_sock_fd);
+				break;
+			} else {
+				fprintf(stderr, "received %c%c%c from client", rb[0], rb[1], rb[2]);
+			}
+			if (rb[0] == 'f' && rb[1] == 'f' && rb[2] == 'f') {
+				close(client_sock_fd);
+				break;
+			}
+
+		}
+	}
+
+	return EXIT_SUCCESS;
+}
-- 
2.34.1

Re: selftests: mptfo initiator/listener: Build Failure

[MPTCP CI]

Hi Dmytro,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20220927225341.14165-8-dmytro@shytyi.net/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/3139594634

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4285eae5e20b

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

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

Re: selftests: 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: 8 failed test(s): packetdrill_fastopen packetdrill_mp_join packetdrill_mp_prio packetdrill_sockopts packetdrill_syscalls selftest_mptcp_join selftest_mptfo selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/4810967652499456
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4810967652499456/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 8 failed test(s): packetdrill_fastopen packetdrill_mp_join packetdrill_mp_prio packetdrill_sockopts packetdrill_syscalls selftest_mptcp_join selftest_mptfo selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/5936867559342080
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5936867559342080/summary/summary.txt

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


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 v12 1/7] mptcp: introduce MSG_FASTOPEN flag.

[Paolo Abeni]

On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
> 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>
> ---
>  include/net/mptcp.h  |  9 +++++++++
>  include/net/tcp.h    |  3 +++
>  net/ipv4/tcp.c       | 20 ++++++++++++++++----
>  net/mptcp/protocol.c | 19 +++++++++++--------
>  4 files changed, 39 insertions(+), 12 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index c25939b2af68..ccf2b42837a1 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -150,6 +150,8 @@ void mptcp_write_options(struct tcphdr *th, __be32 *ptr, struct tcp_sock *tp,
>  			 struct mptcp_out_options *opts);
>  
>  void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info);
> +int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_len, int flags);
> +struct sock *mptcp_subflow_conn_sock(struct sock *sk);
>  
>  /* move the skb extension owership, with the assumption that 'to' is
>   * newly allocated
> @@ -286,6 +288,13 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
>  
>  static inline void mptcp_space(const struct sock *ssk, int *s, int *fs) { }
>  static inline void mptcp_seq_show(struct seq_file *seq) { }
> +static inline int mptcp_stream_connect(struct socket *sock,
> +				       struct sockaddr *uaddr,
> +				       int addr_len,
> +				       int flags)
> +{
> +
> +}
>  
>  static inline int mptcp_subflow_init_cookie_req(struct request_sock *req,
>  						const struct sock *sk_listener,
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 4f71cc15ff8e..e53d26d74dec 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1757,6 +1757,9 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
>  			      struct request_sock *req,
>  			      struct tcp_fastopen_cookie *foc,
>  			      const struct dst_entry *dst);
> +int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
> +			 int *copied, size_t size,
> +			 struct ubuf_info *uarg);
>  void tcp_fastopen_init_key_once(struct net *net);
>  bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
>  			     struct tcp_fastopen_cookie *cookie);
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 5237a3f08c94..daa611671d9a 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1162,8 +1162,9 @@ void tcp_free_fastopen_req(struct tcp_sock *tp)
>  	}
>  }
>  
> -int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
> -			 size_t size, struct ubuf_info *uarg)
> +int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
> +			 int *copied, size_t size,
> +			 struct ubuf_info *uarg)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
>  	struct inet_sock *inet = inet_sk(sk);
> @@ -1196,8 +1197,19 @@ int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
>  		}
>  	}
>  	flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
> -	err = __inet_stream_connect(sk->sk_socket, uaddr,
> -				    msg->msg_namelen, flags, 1);
> +	if (!sk_is_mptcp(sk)) {
> +		err = __inet_stream_connect(sk->sk_socket, uaddr,
> +					    msg->msg_namelen, flags, 1);
> +	} else {
> +		struct sock *parent = mptcp_subflow_conn_sock(sk);
> +
> +		release_sock(sk);
> +		release_sock(parent);
> +		err = mptcp_stream_connect(sk->sk_socket, uaddr,
> +					   msg->msg_namelen, msg->msg_flags);
> +		lock_sock(parent);
> +		lock_sock(sk);
> +	}
>  	/* fastopen_req could already be freed in __inet_stream_connect
>  	 * if the connection times out or gets rst
>  	 */

I'm sorry it looks like I was not clear enough in my previous replies. 

I really think we should avoid this chunk. I thought it only served for
updating the msk socket status, but now I see it is also needed to
properly allocate the token and update the MIBs, right? Does it serve
any other roles?

Anyway I still think you can avoid the above chunck, factoring out the
relevant slice of mptcp_stream_connect() in an helper:

static void __mptcp_pre_connect(struct mptcp_sock *msk, struct sock *ssk)
{
	struct mptcp_subflow_context *subflow;

	mptcp_token_destroy(msk);
        subflow = mptcp_subflow_ctx(ssk);
#ifdef CONFIG_TCP_MD5SIG
        /* no MPTCP if MD5SIG is enabled on this socket or we may run out of
         * TCP option space.
         */
        if (rcu_access_pointer(tcp_sk(ssk)->md5sig_info))
                mptcp_subflow_early_fallback(msk, subflow);
#endif
        if (subflow->request_mptcp && mptcp_token_new_connect(ssk)) {
                MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_TOKENFALLBACKINIT);
                mptcp_subflow_early_fallback(msk, subflow);
        }
        if (likely(!__mptcp_check_fallback(msk)))
                MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MPCAPABLEACTIVE);
}

and call the above in mptcp_sendmsg():

	if (unlikely(ssock && (inet_sk(ssock->sk)->defer_connect ||
			       msg->msg_flags & MSG_FASTOPEN))) {
		struct sock *ssk = ssock->sk;
                int copied_syn = 0;

                lock_sock(ssk);
		if (msg->msg_flags & MSG_FASTOPEN && sk->sk_state == TCP_CLOSE)
			__mptcp_pre_connect(msk, ssk);
		
Likely this patch should be split in 2 separate ones: the new patch
will just create the helper and use it in mptcp_stream_connect.	


Cheers,

Paolo

Re: [RFC PATCH mptcp-next v12 2/7] mptcp: fix retrans., add mptfo vars to msk

[Paolo Abeni]

On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
> 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>

Nit picking: I think this patch subj should be changed to something
alike:

mptcp: implement delayed seq generation for passive fastopen

The main point is that reading the current subject, this looks like a
bugfix, while instead is adding (a piece of) a new feature.

Thanks,

Paolo

Re: [RFC PATCH mptcp-next v12 3/7] mptcp: add subflow_v(4,6)_send_synack()

[Paolo Abeni]

On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
> 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/ipv4/tcp_fastopen.c | 19 ++++++++++++------
>  net/mptcp/fastopen.c    | 43 +++++++++++++++++++++++++++++++++++++++++
>  net/mptcp/protocol.c    |  2 +-
>  net/mptcp/protocol.h    |  3 +++
>  net/mptcp/subflow.c     | 42 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 102 insertions(+), 7 deletions(-)
> 
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index 45cc7f1ca296..d6b1380525ea 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -356,13 +356,20 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
>  	if (foc->len == 0) /* Client requests a cookie */
>  		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENCOOKIEREQD);
>  
> -	if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
> -	      (syn_data || foc->len >= 0) &&
> -	      tcp_fastopen_queue_check(sk))) {
> -		foc->len = -1;
> -		return NULL;
> +	if (sk_is_mptcp(sk)) {
> +		if (((syn_data || foc->len >= 0) &&
> +		     tcp_fastopen_queue_check(sk))) {
> +			foc->len = -1;
> +			return NULL;
> +		}
> +	} else {
> +		if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
> +		      (syn_data || foc->len >= 0) &&
> +		      tcp_fastopen_queue_check(sk))) {
> +			foc->len = -1;
> +			return NULL;
> +		}
>  	}

This should really not be needed; with a proper setup, sock_net(sk)-
>ipv4.sysctl_tcp_fastopen/tcp_fastopen should already be
TFO_SERVER_ENABLE at this point. You can double check that with the
'perf' tool adding a probe on the relevant LoC dumping the sysctl value
(or with a possibly easier way but required a rebuild, with a temporary
printk)

> -
>  	if (tcp_fastopen_no_cookie(sk, dst, TFO_SERVER_COOKIE_NOT_REQD))
>  		goto fastopen;
>  
> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
> index d6fb45e6be4f..1d824a4d9606 100644
> --- a/net/mptcp/fastopen.c
> +++ b/net/mptcp/fastopen.c
> @@ -17,3 +17,46 @@ void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_
>  	pr_debug("ack_seq=%llu sndr_key=%llu", msk->ack_seq, mp_opt.sndr_key);
>  	atomic64_set(&msk->rcv_wnd_sent, ack_seq);
>  }
> +
> +void subflow_fastopen_send_synack_set_params(struct mptcp_subflow_context *subflow,
> +					     struct request_sock *req)
> +{
> +	struct tcp_request_sock *tcp_r_sock = tcp_rsk(req);

The conventional name for a tcp_request_sock variable is 'treq'

> +	struct sock *ssk = subflow->tcp_sock;
> +	struct sock *sk = subflow->conn;
> +	struct mptcp_sock *msk;
> +	struct sk_buff *skb;
> +	struct tcp_sock *tp;
> +
> +	msk = mptcp_sk(sk);
> +	tp = tcp_sk(ssk);
> +
> +	/* <mark subflow/msk as "mptfo"> */

Please remove the '<' '>' from the comments, here and below.

> +	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 - tcp_r_sock->rcv_isn - 1;
> +	subflow->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
> +	subflow->ssn_offset = tp->copied_seq - 1;
> +
> +	/* <acquire the msk data lock> */

the above comment and the next 3 are really not needed.

> +	mptcp_data_lock(sk);
> +
> +	/* <move skb into msk receive queue> */
> +	mptcp_set_owner_r(skb, sk);
> +	__skb_queue_tail(&msk->receive_queue, skb);
> +	atomic64_set(&msk->rcv_wnd_sent, mptcp_subflow_get_mapped_dsn(subflow));

I think the above statement is not needed here? we already have it in
mptcp_gen_msk_ackseq_fastopen() in the previous patch?

Instead I think we need to initialize the MPTCP_CB for skb, to avoid
random coalescing from later skbs.

> +
> +	/* <call msk data_ready> */
> +	(sk)->sk_data_ready(sk);
> +
> +	/* <release the msk data lock> */
> +	mptcp_data_unlock(sk);
> +}
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 0db50712bad7..7e63b414011c 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -206,7 +206,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 a9708a2eb2bc..9c46e802a73a 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -842,6 +842,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 07dd23d0fe04..c48143bff114 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -307,6 +307,46 @@ 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);
> +
> +	/* <evenutally clear tstamp_ok, as needed depending on cookie size> */
> +	if (foc)
> +		ireq->tstamp_ok = 0;

I guess you really need to check the cookie size, too? If the cookie
size is small enough, stripping the timestamp should not be needed.
Also you can move the above 2 lines in
subflow_fastopen_send_synack_set_params()

Cheers,

Paolo

Re: [RFC PATCH mptcp-next v12 4/7] mptcp: sockopt: make 'tcp_fastopen_connect' generic

[Paolo Abeni]

On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
> From: Matthieu Baerts <matthieu.baerts@tessares.net>
> 
> This is patch provided by Matthieu. As it seems it not yet merged I
> temprorarly integrated in into my series for compilation.
> 'https://lore.kernel.org/mptcp/
> 20220927150306.1552795-1-matthieu.baerts@tessares.net/T/'
> When it will be merged I will delete it from this set of patches.
> 
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>

You don't need to include this patch into this series, just rebase the
series on top of Mat patches.


Cheers,

Paolo

Re: [RFC PATCH mptcp-next v12 6/7] mptcp: add TCP_FASTOPEN option

[Paolo Abeni]

On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
> 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 | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index 1857281a0dd5..ab3ba8558efd 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;

You should additionally update the comment below that describe
TCP_FASTOPEN as not supported.

/P

Re: [RFC PATCH mptcp-next v12 7/7] selftests: mptfo initiator/listener

[Paolo Abeni]

On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
> MPTFO tests: these are examples of initiator (sendto) and listener,
> probably are going to be integrated to the mptcp_connect.* selftests
> 
> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>

This must be integrated into the current self-tests suite:

- update the Makefile to build the helper program
  (or consider extending the exiting mptcp_connect program instead,
adding TCP_FASTOPEN support to the '-o' command line option)
- the script must be
  * listed in the Makefile TEST_PROGS
  * fully run the tests, executing the client and server program, 
    collecting the result and showing them in a clear way
  (or consider adding a new section to the mptcp_join.sh script
instead, see commit 5ac1d2d6345190907e260daedd980ab3be512cf0 for a
relevant example. You would need to use mptcp_connect to do this with a
reasonable amount of changes)

Additionally we need testcases for both sendmsg(MSG_FASTOPEN) and for
setsockopt(TCP_FASTOPEN), possibly in the positive (fastopen is
succesful) case and the negative one (switch to plain
connect/msg_fastopen fails).

/P

Re: [RFC PATCH mptcp-next v12 6/7] mptcp: add TCP_FASTOPEN option

[Dmytro Shytyi]

Hello,

Updated in v13.

Dmytro

On 9/28/2022 11:28 AM, Paolo Abeni wrote:
> On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
>> 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 | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
>> index 1857281a0dd5..ab3ba8558efd 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;
> You should additionally update the comment below that describe
> TCP_FASTOPEN as not supported.
>
> /P
>
>

Re: [RFC PATCH mptcp-next v12 4/7] mptcp: sockopt: make 'tcp_fastopen_connect' generic

[Dmytro Shytyi]

Hello,

Rebased on top of Mat patches.

On 9/28/2022 11:26 AM, Paolo Abeni wrote:
> On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
>> From: Matthieu Baerts <matthieu.baerts@tessares.net>
>>
>> This is patch provided by Matthieu. As it seems it not yet merged I
>> temprorarly integrated in into my series for compilation.
>> 'https://lore.kernel.org/mptcp/
>> 20220927150306.1552795-1-matthieu.baerts@tessares.net/T/'
>> When it will be merged I will delete it from this set of patches.
>>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> You don't need to include this patch into this series, just rebase the
> series on top of Mat patches.
>
>
> Cheers,
>
> Paolo
>
>

Re: [RFC PATCH mptcp-next v12 3/7] mptcp: add subflow_v(4,6)_send_synack()

[Dmytro Shytyi]

Hello,

On 9/28/2022 11:23 AM, Paolo Abeni wrote:
> On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
>> 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/ipv4/tcp_fastopen.c | 19 ++++++++++++------
>>   net/mptcp/fastopen.c    | 43 +++++++++++++++++++++++++++++++++++++++++
>>   net/mptcp/protocol.c    |  2 +-
>>   net/mptcp/protocol.h    |  3 +++
>>   net/mptcp/subflow.c     | 42 ++++++++++++++++++++++++++++++++++++++++
>>   5 files changed, 102 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
>> index 45cc7f1ca296..d6b1380525ea 100644
>> --- a/net/ipv4/tcp_fastopen.c
>> +++ b/net/ipv4/tcp_fastopen.c
>> @@ -356,13 +356,20 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
>>   	if (foc->len == 0) /* Client requests a cookie */
>>   		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENCOOKIEREQD);
>>   
>> -	if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
>> -	      (syn_data || foc->len >= 0) &&
>> -	      tcp_fastopen_queue_check(sk))) {
>> -		foc->len = -1;
>> -		return NULL;
>> +	if (sk_is_mptcp(sk)) {
>> +		if (((syn_data || foc->len >= 0) &&
>> +		     tcp_fastopen_queue_check(sk))) {
>> +			foc->len = -1;
>> +			return NULL;
>> +		}
>> +	} else {
>> +		if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
>> +		      (syn_data || foc->len >= 0) &&
>> +		      tcp_fastopen_queue_check(sk))) {
>> +			foc->len = -1;
>> +			return NULL;
>> +		}
>>   	}
> This should really not be needed; with a proper setup, sock_net(sk)-
>> ipv4.sysctl_tcp_fastopen/tcp_fastopen should already be
> TFO_SERVER_ENABLE at this point. You can double check that with the
> 'perf' tool adding a probe on the relevant LoC dumping the sysctl value
> (or with a possibly easier way but required a rebuild, with a temporary
> printk)


In v13 it is ok. No need to add this.

>> -
>>   	if (tcp_fastopen_no_cookie(sk, dst, TFO_SERVER_COOKIE_NOT_REQD))
>>   		goto fastopen;
>>   
>> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
>> index d6fb45e6be4f..1d824a4d9606 100644
>> --- a/net/mptcp/fastopen.c
>> +++ b/net/mptcp/fastopen.c
>> @@ -17,3 +17,46 @@ void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_
>>   	pr_debug("ack_seq=%llu sndr_key=%llu", msk->ack_seq, mp_opt.sndr_key);
>>   	atomic64_set(&msk->rcv_wnd_sent, ack_seq);
>>   }
>> +
>> +void subflow_fastopen_send_synack_set_params(struct mptcp_subflow_context *subflow,
>> +					     struct request_sock *req)
>> +{
>> +	struct tcp_request_sock *tcp_r_sock = tcp_rsk(req);
> The conventional name for a tcp_request_sock variable is 'treq'
Fixed in v13.
>> +	struct sock *ssk = subflow->tcp_sock;
>> +	struct sock *sk = subflow->conn;
>> +	struct mptcp_sock *msk;
>> +	struct sk_buff *skb;
>> +	struct tcp_sock *tp;
>> +
>> +	msk = mptcp_sk(sk);
>> +	tp = tcp_sk(ssk);
>> +
>> +	/* <mark subflow/msk as "mptfo"> */
> Please remove the '<' '>' from the comments, here and below.
Fixed in v13
>> +	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 - tcp_r_sock->rcv_isn - 1;
>> +	subflow->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
>> +	subflow->ssn_offset = tp->copied_seq - 1;
>> +
>> +	/* <acquire the msk data lock> */
> the above comment and the next 3 are really not needed.
Fixed in v13.
>> +	mptcp_data_lock(sk);
>> +
>> +	/* <move skb into msk receive queue> */
>> +	mptcp_set_owner_r(skb, sk);
>> +	__skb_queue_tail(&msk->receive_queue, skb);
>> +	atomic64_set(&msk->rcv_wnd_sent, mptcp_subflow_get_mapped_dsn(subflow));
> I think the above statement is not needed here? we already have it in
> mptcp_gen_msk_ackseq_fastopen() in the previous patch?
fixed in v13.
> Instead I think we need to initialize the MPTCP_CB for skb, to avoid
> random coalescing from later skbs.
Added in v13. Open to any comments on this subject.
>> +
>> +	/* <call msk data_ready> */
>> +	(sk)->sk_data_ready(sk);
>> +
>> +	/* <release the msk data lock> */
>> +	mptcp_data_unlock(sk);
>> +}
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index 0db50712bad7..7e63b414011c 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -206,7 +206,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 a9708a2eb2bc..9c46e802a73a 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -842,6 +842,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 07dd23d0fe04..c48143bff114 100644
>> --- a/net/mptcp/subflow.c
>> +++ b/net/mptcp/subflow.c
>> @@ -307,6 +307,46 @@ 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);
>> +
>> +	/* <evenutally clear tstamp_ok, as needed depending on cookie size> */
>> +	if (foc)
>> +		ireq->tstamp_ok = 0;
> I guess you really need to check the cookie size, too? If the cookie
> size is small enough, stripping the timestamp should not be needed.
> Also you can move the above 2 lines in
> subflow_fastopen_send_synack_set_params()


It seems I should not move them into set_params()... As foc check is 
performed outside

If I move the foc check into "if", no metter how I modify it : "foc != 
NULL" or " for != -1", it doesn't affect the TS.

I suppose I observe this behaviour because of "tcp_conn_request()" 
starting from line 7013: tcp_input.c - net/ipv4/tcp_input.c - Linux 
source code (v5.19.10) - Bootlin 
<https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp_input.c#L7013>


I propably should keep "foc" check outside "if" statement.

> Cheers,
>
> Paolo
>
>

Re: [RFC PATCH mptcp-next v12 2/7] mptcp: fix retrans., add mptfo vars to msk

[Dmytro Shytyi]

Hello,

On 9/28/2022 11:05 AM, Paolo Abeni wrote:
> On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
>> 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>
> Nit picking: I think this patch subj should be changed to something
> alike:
>
> mptcp: implement delayed seq generation for passive fastopen
>
> The main point is that reading the current subject, this looks like a
> bugfix, while instead is adding (a piece of) a new feature.
>
> Thanks,
>
> Paolo
>
Thank you for suggestion. Fixed in v13

Re: [RFC PATCH mptcp-next v12 7/7] selftests: mptfo initiator/listener

[Dmytro Shytyi]

Hello,

Thank you for this comment. I keep it in my notebook for the moment. I 
will come back to tests, when I will see the proper result of Initiator 
and Listener, if you don't mind?

Best,

Dmytro

On 9/28/2022 11:45 AM, Paolo Abeni wrote:
> On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
>> MPTFO tests: these are examples of initiator (sendto) and listener,
>> probably are going to be integrated to the mptcp_connect.* selftests
>>
>> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
> This must be integrated into the current self-tests suite:
>
> - update the Makefile to build the helper program
>    (or consider extending the exiting mptcp_connect program instead,
> adding TCP_FASTOPEN support to the '-o' command line option)
> - the script must be
>    * listed in the Makefile TEST_PROGS
>    * fully run the tests, executing the client and server program,
>      collecting the result and showing them in a clear way
>    (or consider adding a new section to the mptcp_join.sh script
> instead, see commit 5ac1d2d6345190907e260daedd980ab3be512cf0 for a
> relevant example. You would need to use mptcp_connect to do this with a
> reasonable amount of changes)
>
> Additionally we need testcases for both sendmsg(MSG_FASTOPEN) and for
> setsockopt(TCP_FASTOPEN), possibly in the positive (fastopen is
> succesful) case and the negative one (switch to plain
> connect/msg_fastopen fails).
>
> /P
>

Re: [RFC PATCH mptcp-next v12 1/7] mptcp: introduce MSG_FASTOPEN flag.

[Dmytro Shytyi]

Hello,

I'm getting the next stack trace [1] when following this approach, yet 
it needs uarg to be filled, not NULL.

After Matthieu suggestion, I tried to implement the .connect in mptcp, 
but I'm getting errors like "ENOBUFF" or "EINPROGRESS".

Finally I decided to continue with "mptcp_stream_connect()" function in v13.


[1] Code starting with the faulting instruction
===========================================
[   27.736069] RSP: 0018:ffffc90000adfce0 EFLAGS: 00010246
[   27.737115] RAX: 0000000000000000 RBX: ffff888003894440 RCX: 
0000000000000000
[   27.738560] RDX: 0000000000000010 RSI: ffffc90000adfe80 RDI: 
ffff888027ce8000
[   27.739883] RBP: 0000000000000000 R08: 0000000000000001 R09: 
ffff888015710cf0
[   27.741286] R10: 0000000000000001 R11: ffff888003ca78c8 R12: 
00000000ffffff96
[   27.742718] R13: ffffc90000adfdb0 R14: ffffc90000adfe80 R15: 
ffff888027ce8000
[   27.744135] FS:  00007f7cc983a740(0000) GS:ffff88803da00000(0000) 
knlGS:0000000000000000
[   27.745612] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   27.746788] CR2: ffffffffffffffd6 CR3: 000000001d180006 CR4: 
0000000000370ef0
[   27.748125] Call Trace:
[   27.748655]  <TASK>
[   27.749086] __inet_stream_connect (net/ipv4/af_inet.c:663)
[   27.750093] ? sk_reset_timer (net/core/sock.c:3341)
[   27.750803] ? tcp_connect (net/ipv4/tcp_output.c:3882)
[   27.751590] ? kmem_cache_alloc_trace (mm/slub.c:3286)
[   27.752492] tcp_sendmsg_fastopen (net/ipv4/tcp.c:1198)
[   27.753347] mptcp_sendmsg (net/mptcp/protocol.c:1710)
[   27.754047] sock_sendmsg_nosec (net/socket.c:714)
[   27.754831] __sys_sendto (net/socket.c:2117)
[   27.755539] ? handle_mm_fault (mm/memory.c:5151)
[   27.756311] ? do_user_addr_fault (arch/x86/mm/fault.c:1426)
[   27.757175] __x64_sys_sendto (net/socket.c:2129 net/socket.c:2125 
net/socket.c:2125)
[   27.758029] do_syscall_64 (arch/x86/entry/common.c:50 
arch/x86/entry/common.c:80)
[   27.758739] entry_SYSCALL_64_after_hwframe 
(arch/x86/entry/entry_64.S:120)
[   27.759719] RIP: 0033:0x7f7cc99484e6
[ 27.760458] Code: 69 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 
1f 00 41 89 ca 64 8b 04 25 18 00 00 00 85 c0c

On 9/28/2022 11:01 AM, Paolo Abeni wrote:
> On Tue, 2022-09-27 at 22:53 +0000, Dmytro Shytyi wrote:
>> 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>
>> ---
>>   include/net/mptcp.h  |  9 +++++++++
>>   include/net/tcp.h    |  3 +++
>>   net/ipv4/tcp.c       | 20 ++++++++++++++++----
>>   net/mptcp/protocol.c | 19 +++++++++++--------
>>   4 files changed, 39 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
>> index c25939b2af68..ccf2b42837a1 100644
>> --- a/include/net/mptcp.h
>> +++ b/include/net/mptcp.h
>> @@ -150,6 +150,8 @@ void mptcp_write_options(struct tcphdr *th, __be32 *ptr, struct tcp_sock *tp,
>>   			 struct mptcp_out_options *opts);
>>   
>>   void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info);
>> +int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_len, int flags);
>> +struct sock *mptcp_subflow_conn_sock(struct sock *sk);
>>   
>>   /* move the skb extension owership, with the assumption that 'to' is
>>    * newly allocated
>> @@ -286,6 +288,13 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
>>   
>>   static inline void mptcp_space(const struct sock *ssk, int *s, int *fs) { }
>>   static inline void mptcp_seq_show(struct seq_file *seq) { }
>> +static inline int mptcp_stream_connect(struct socket *sock,
>> +				       struct sockaddr *uaddr,
>> +				       int addr_len,
>> +				       int flags)
>> +{
>> +
>> +}
>>   
>>   static inline int mptcp_subflow_init_cookie_req(struct request_sock *req,
>>   						const struct sock *sk_listener,
>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>> index 4f71cc15ff8e..e53d26d74dec 100644
>> --- a/include/net/tcp.h
>> +++ b/include/net/tcp.h
>> @@ -1757,6 +1757,9 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
>>   			      struct request_sock *req,
>>   			      struct tcp_fastopen_cookie *foc,
>>   			      const struct dst_entry *dst);
>> +int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
>> +			 int *copied, size_t size,
>> +			 struct ubuf_info *uarg);
>>   void tcp_fastopen_init_key_once(struct net *net);
>>   bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
>>   			     struct tcp_fastopen_cookie *cookie);
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index 5237a3f08c94..daa611671d9a 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -1162,8 +1162,9 @@ void tcp_free_fastopen_req(struct tcp_sock *tp)
>>   	}
>>   }
>>   
>> -int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
>> -			 size_t size, struct ubuf_info *uarg)
>> +int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
>> +			 int *copied, size_t size,
>> +			 struct ubuf_info *uarg)
>>   {
>>   	struct tcp_sock *tp = tcp_sk(sk);
>>   	struct inet_sock *inet = inet_sk(sk);
>> @@ -1196,8 +1197,19 @@ int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
>>   		}
>>   	}
>>   	flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
>> -	err = __inet_stream_connect(sk->sk_socket, uaddr,
>> -				    msg->msg_namelen, flags, 1);
>> +	if (!sk_is_mptcp(sk)) {
>> +		err = __inet_stream_connect(sk->sk_socket, uaddr,
>> +					    msg->msg_namelen, flags, 1);
>> +	} else {
>> +		struct sock *parent = mptcp_subflow_conn_sock(sk);
>> +
>> +		release_sock(sk);
>> +		release_sock(parent);
>> +		err = mptcp_stream_connect(sk->sk_socket, uaddr,
>> +					   msg->msg_namelen, msg->msg_flags);
>> +		lock_sock(parent);
>> +		lock_sock(sk);
>> +	}
>>   	/* fastopen_req could already be freed in __inet_stream_connect
>>   	 * if the connection times out or gets rst
>>   	 */
> I'm sorry it looks like I was not clear enough in my previous replies.
>
> I really think we should avoid this chunk. I thought it only served for
> updating the msk socket status, but now I see it is also needed to
> properly allocate the token and update the MIBs, right? Does it serve
> any other roles?
>
> Anyway I still think you can avoid the above chunck, factoring out the
> relevant slice of mptcp_stream_connect() in an helper:
>
> static void __mptcp_pre_connect(struct mptcp_sock *msk, struct sock *ssk)
> {
> 	struct mptcp_subflow_context *subflow;
>
> 	mptcp_token_destroy(msk);
>          subflow = mptcp_subflow_ctx(ssk);
> #ifdef CONFIG_TCP_MD5SIG
>          /* no MPTCP if MD5SIG is enabled on this socket or we may run out of
>           * TCP option space.
>           */
>          if (rcu_access_pointer(tcp_sk(ssk)->md5sig_info))
>                  mptcp_subflow_early_fallback(msk, subflow);
> #endif
>          if (subflow->request_mptcp && mptcp_token_new_connect(ssk)) {
>                  MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_TOKENFALLBACKINIT);
>                  mptcp_subflow_early_fallback(msk, subflow);
>          }
>          if (likely(!__mptcp_check_fallback(msk)))
>                  MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MPCAPABLEACTIVE);
> }
>
> and call the above in mptcp_sendmsg():
>
> 	if (unlikely(ssock && (inet_sk(ssock->sk)->defer_connect ||
> 			       msg->msg_flags & MSG_FASTOPEN))) {
> 		struct sock *ssk = ssock->sk;
>                  int copied_syn = 0;
>
>                  lock_sock(ssk);
> 		if (msg->msg_flags & MSG_FASTOPEN && sk->sk_state == TCP_CLOSE)
> 			__mptcp_pre_connect(msk, ssk);
> 		
> Likely this patch should be split in 2 separate ones: the new patch
> will just create the helper and use it in mptcp_stream_connect.	
>
>
> Cheers,
>
> Paolo
>
>

Re: [RFC PATCH mptcp-next v12 1/7] mptcp: introduce MSG_FASTOPEN flag.

[Paolo Abeni]

Hello,

On Sat, 2022-10-01 at 05:08 +0200, Dmytro Shytyi wrote:
> I'm getting the next stack trace [1] when following this approach, yet 
> it needs uarg to be filled, not NULL.
> 
> After Matthieu suggestion, I tried to implement the .connect in mptcp, 
> but I'm getting errors like "ENOBUFF" or "EINPROGRESS".
> 
> Finally I decided to continue with "mptcp_stream_connect()" function in v13.

The reported error is not clear at all to me. It's better to clarify it
before moving to the next version, or steps could (likelly, will) be in
the wrong direction.

> [1] Code starting with the faulting instruction
> ===========================================
> [   27.736069] RSP: 0018:ffffc90000adfce0 EFLAGS: 00010246
> [   27.737115] RAX: 0000000000000000 RBX: ffff888003894440 RCX: 
> 0000000000000000
> [   27.738560] RDX: 0000000000000010 RSI: ffffc90000adfe80 RDI: 
> ffff888027ce8000
> [   27.739883] RBP: 0000000000000000 R08: 0000000000000001 R09: 
> ffff888015710cf0
> [   27.741286] R10: 0000000000000001 R11: ffff888003ca78c8 R12: 
> 00000000ffffff96
> [   27.742718] R13: ffffc90000adfdb0 R14: ffffc90000adfe80 R15: 
> ffff888027ce8000
> [   27.744135] FS:  00007f7cc983a740(0000) GS:ffff88803da00000(0000) 
> knlGS:0000000000000000
> [   27.745612] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   27.746788] CR2: ffffffffffffffd6 CR3: 000000001d180006 CR4: 
> 0000000000370ef0
> [   27.748125] Call Trace:
> [   27.748655]  <TASK>
> [   27.749086] __inet_stream_connect (net/ipv4/af_inet.c:663)
> [   27.750093] ? sk_reset_timer (net/core/sock.c:3341)
> [   27.750803] ? tcp_connect (net/ipv4/tcp_output.c:3882)
> [   27.751590] ? kmem_cache_alloc_trace (mm/slub.c:3286)
> [   27.752492] tcp_sendmsg_fastopen (net/ipv4/tcp.c:1198)
> [   27.753347] mptcp_sendmsg (net/mptcp/protocol.c:1710)
> [   27.754047] sock_sendmsg_nosec (net/socket.c:714)
> [   27.754831] __sys_sendto (net/socket.c:2117)
> [   27.755539] ? handle_mm_fault (mm/memory.c:5151)
> [   27.756311] ? do_user_addr_fault (arch/x86/mm/fault.c:1426)
> [   27.757175] __x64_sys_sendto (net/socket.c:2129 net/socket.c:2125 
> net/socket.c:2125)
> [   27.758029] do_syscall_64 (arch/x86/entry/common.c:50 
> arch/x86/entry/common.c:80)
> [   27.758739] entry_SYSCALL_64_after_hwframe 
> (arch/x86/entry/entry_64.S:120)
> [   27.759719] RIP: 0033:0x7f7cc99484e6
> [ 27.760458] Code: 69 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 
> 1f 00 41 89 ca 64 8b 04 25 18 00 00 00 85 c0c

This backtrace is incomplete/lacks the oopsing address, so is unclear
what really went wrong. Please report the full backtrace.

Important: please include the code you used for this test. No need to
post the full old patches here, you can e.g. share an URL to your git
tree/branch.

The main point is that this backtrace does not look compatible with the
code suggested previously.


Thanks,

Paolo

Re: [RFC PATCH mptcp-next v12 7/7] selftests: mptfo initiator/listener

[Paolo Abeni]

On Sat, 2022-10-01 at 05:03 +0200, Dmytro Shytyi wrote:
> Thank you for this comment. I keep it in my notebook for the moment. I 
> will come back to tests, when I will see the proper result of Initiator 
> and Listener, if you don't mind?

Better self-tests will allow you to more easily validates your patches,
and a complete self-test is required for merging. I suggest to not
postpone this.

Thanks,

Paolo

p.s. to validate this code, thanks to Mat, there are a few packetdrill
available:

https://github.com/multipath-tcp/packetdrill/tree/mptcp-net-next/gtests/net/mptcp/fastopen

you just need to remote the '.TODO' suffix.

Re: [RFC PATCH mptcp-next v12 7/7] selftests: mptfo initiator/listener

[Dmytro Shytyi]

Thank your for response.

I started looking at self tests.


P.S. Packet drill was also was suggested by Matthieu.

On 10/3/2022 10:31 AM, Paolo Abeni wrote:
> On Sat, 2022-10-01 at 05:03 +0200, Dmytro Shytyi wrote:
>> Thank you for this comment. I keep it in my notebook for the moment. I
>> will come back to tests, when I will see the proper result of Initiator
>> and Listener, if you don't mind?
> Better self-tests will allow you to more easily validates your patches,
> and a complete self-test is required for merging. I suggest to not
> postpone this.
>
> Thanks,
>
> Paolo
>
> p.s. to validate this code, thanks to Mat, there are a few packetdrill
> available:
>
> https://github.com/multipath-tcp/packetdrill/tree/mptcp-net-next/gtests/net/mptcp/fastopen
>
> you just need to remote the '.TODO' suffix.
>
>

Re: [RFC PATCH mptcp-next v12 1/7] mptcp: introduce MSG_FASTOPEN flag.

[Paolo Abeni]

On Mon, 2022-10-03 at 10:02 +0200, Paolo Abeni wrote:
> Hello,
> 
> On Sat, 2022-10-01 at 05:08 +0200, Dmytro Shytyi wrote:
> > I'm getting the next stack trace [1] when following this approach, yet 
> > it needs uarg to be filled, not NULL.
> > 
> > After Matthieu suggestion, I tried to implement the .connect in mptcp, 
> > but I'm getting errors like "ENOBUFF" or "EINPROGRESS".
> > 
> > Finally I decided to continue with "mptcp_stream_connect()" function in v13.
> 
> The reported error is not clear at all to me. It's better to clarify it
> before moving to the next version, or steps could (likelly, will) be in
> the wrong direction.
> 
> > [1] Code starting with the faulting instruction
> > ===========================================
> > [   27.736069] RSP: 0018:ffffc90000adfce0 EFLAGS: 00010246
> > [   27.737115] RAX: 0000000000000000 RBX: ffff888003894440 RCX: 
> > 0000000000000000
> > [   27.738560] RDX: 0000000000000010 RSI: ffffc90000adfe80 RDI: 
> > ffff888027ce8000
> > [   27.739883] RBP: 0000000000000000 R08: 0000000000000001 R09: 
> > ffff888015710cf0
> > [   27.741286] R10: 0000000000000001 R11: ffff888003ca78c8 R12: 
> > 00000000ffffff96
> > [   27.742718] R13: ffffc90000adfdb0 R14: ffffc90000adfe80 R15: 
> > ffff888027ce8000
> > [   27.744135] FS:  00007f7cc983a740(0000) GS:ffff88803da00000(0000) 
> > knlGS:0000000000000000
> > [   27.745612] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   27.746788] CR2: ffffffffffffffd6 CR3: 000000001d180006 CR4: 
> > 0000000000370ef0
> > [   27.748125] Call Trace:
> > [   27.748655]  <TASK>
> > [   27.749086] __inet_stream_connect (net/ipv4/af_inet.c:663)
> > [   27.750093] ? sk_reset_timer (net/core/sock.c:3341)
> > [   27.750803] ? tcp_connect (net/ipv4/tcp_output.c:3882)
> > [   27.751590] ? kmem_cache_alloc_trace (mm/slub.c:3286)
> > [   27.752492] tcp_sendmsg_fastopen (net/ipv4/tcp.c:1198)
> > [   27.753347] mptcp_sendmsg (net/mptcp/protocol.c:1710)
> > [   27.754047] sock_sendmsg_nosec (net/socket.c:714)
> > [   27.754831] __sys_sendto (net/socket.c:2117)
> > [   27.755539] ? handle_mm_fault (mm/memory.c:5151)
> > [   27.756311] ? do_user_addr_fault (arch/x86/mm/fault.c:1426)
> > [   27.757175] __x64_sys_sendto (net/socket.c:2129 net/socket.c:2125 
> > net/socket.c:2125)
> > [   27.758029] do_syscall_64 (arch/x86/entry/common.c:50 
> > arch/x86/entry/common.c:80)
> > [   27.758739] entry_SYSCALL_64_after_hwframe 
> > (arch/x86/entry/entry_64.S:120)
> > [   27.759719] RIP: 0033:0x7f7cc99484e6
> > [ 27.760458] Code: 69 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 
> > 1f 00 41 89 ca 64 8b 04 25 18 00 00 00 85 c0c
> 
> This backtrace is incomplete/lacks the oopsing address, so is unclear
> what really went wrong. Please report the full backtrace.
> 
> Important: please include the code you used for this test. No need to
> post the full old patches here, you can e.g. share an URL to your git
> tree/branch.
> 
> The main point is that this backtrace does not look compatible with the
> code suggested previously.

I discussed this with Mat(ttbe) on IRC. It looks like the main problem
is that mptcp don't implement the struct proto/sk->sk_prot->connect,
and the defer_connect is not triggering such path as subflow the socket
is not in SS_UNCONNECTED state on defer connect.

The correct solution would be re-factor the mptcp connect code/hooking
to re-use the inet_stream_connect() infrastructure. I'll try to cook
the relevant patch - not strictily fastclose related - asap, so that
you can rebase this series on top of them. 

Cheers,

Paolo

Re: selftests: mptfo initiator/listener: Build Failure

[MPTCP CI]

Hi Dmytro,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20221001031502.29152-8-dmytro@shytyi.net/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/3197013753

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/7c22bca6e0ab

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

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

Re: selftests: mptfo initiator/listener: Build Failure

[MPTCP CI]

Hi Dmytro,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20220921125558.19483-7-dmytro@shytyi.net/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/3098139757

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/5eb61bb88e80

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

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

Re: selftests: mptfo initiator/listener: Build Failure

[MPTCP CI]

Hi Dmytro,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20220920125243.2880-8-dmytro@shytyi.net/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/3090294936

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/413c19c617ef

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

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

Re: selftests: mptfo initiator/listener: Build Failure

[MPTCP CI]

Hi Dmytro,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20220915235604.26018-10-dmytro@shytyi.net/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/3064785105

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a04a47ac6e30

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

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