[RFC PATCH mptcp-next v9 0/6] mptcp: Fast Open Mechanism

[RFC PATCH mptcp-next v9 0/6] mptcp: Fast Open Mechanism

[Dmytro Shytyi]

[PATCH v9] includes "client-server" partial support for:
1. MPTCP cookie request from client (seems OK).
2. MPTCP cookie offering from server (seems OK).
3. MPTCP SYN+DATA+COOKIE from client (seems OK).
4. subsequent write + read on the opened socket (seems OK).
===Changes between v8 and v9
- Code is refactored (Max. reuse of existing linux kernel code).
- impact on fastopen.c is reduced to minima due to added function
subflow_v4_send_synack().
===Future work
-Adress the appearance of "MPTCP FIN" as duplicated acks.
-Integrate presented in the last patch selftests. 

Dmytro Shytyi (6):
  mptcp: add mptcp_stream_connect to protocol.h
  mptcp: add mptcp_setsockopt_fastopen
  mptcp: reuse tcp_sendmsg_fastopen()
  mptcp: fix retrans., add mptfo vars to msk
  mptcp: add subflow_v(4,6)_send_synack()
  selftests: mptfo initiator/listener

 include/net/tcp.h                             |   3 +
 net/ipv4/tcp.c                                |  24 ++++-
 net/ipv4/tcp_fastopen.c                       |  19 ++--
 net/mptcp/Makefile                            |   2 +-
 net/mptcp/fastopen.c                          |  46 ++++++++
 net/mptcp/options.c                           |   5 +
 net/mptcp/protocol.c                          |  35 +++++-
 net/mptcp/protocol.h                          |  10 ++
 net/mptcp/sockopt.c                           |   3 +
 net/mptcp/subflow.c                           |  70 ++++++++++++
 tools/testing/selftests/net/mptcp/mptfo.sh    |  13 +++
 .../selftests/net/mptcp/mptfo_initiator.c     |  43 ++++++++
 .../selftests/net/mptcp/mptfo_listener.c      | 100 ++++++++++++++++++
 13 files changed, 356 insertions(+), 17 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.25.1

[RFC PATCH mptcp-next v9 1/6] mptcp: add mptcp_stream_connect to protocol.h

[Dmytro Shytyi]

In the following patches we will call mptcp_stream_connect() from
function tcp_sendmsg_fastopen() in file "net/ipv4/tcp.c", thus make
such symbol visible.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 net/mptcp/protocol.c | 4 ++--
 net/mptcp/protocol.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c0bf48e3149b..470045793181 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3484,8 +3484,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;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 93c535440a5c..632161b13950 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -837,6 +837,7 @@ void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
 void mptcp_event_addr_announced(const struct sock *ssk, const struct mptcp_addr_info *info);
 void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
 bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
+int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_len, int flags);
 
 static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
 {
-- 
2.25.1

[RFC PATCH mptcp-next v9 2/6] mptcp: add mptcp_setsockopt_fastopen

[Dmytro Shytyi]

Add set MPTFO socket option for MPTCP. This option is needed for
the listen socket to accept MPTFO.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 net/mptcp/Makefile   |  2 +-
 net/mptcp/fastopen.c | 32 ++++++++++++++++++++++++++++++++
 net/mptcp/protocol.h |  5 +++++
 net/mptcp/sockopt.c  |  3 +++
 4 files changed, 41 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..9ef49a2d2ea2
--- /dev/null
+++ b/net/mptcp/fastopen.c
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0
+ * MPTCP Fast Open Mechanism. Copyright (c) 2021-2022, Dmytro SHYTYI
+ */
+
+#include "protocol.h"
+
+int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
+				      unsigned int optlen)
+{
+	struct sock *sk = (struct sock *)msk;
+	struct net *net = sock_net(sk);
+	int val;
+	int ret;
+
+	ret = 0;
+
+	if (copy_from_sockptr(&val, optval, sizeof(val)))
+		return -EFAULT;
+
+	lock_sock(sk);
+
+	if (val >= 0 && ((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
+		tcp_fastopen_init_key_once(net);
+		fastopen_queue_tune(sk, val);
+	} else {
+		ret = -EINVAL;
+	}
+
+	release_sock(sk);
+
+	return ret;
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 632161b13950..57596cdfb1f9 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -839,6 +839,11 @@ void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
 bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_len, int flags);
 
+// Fast Open Mechanism functions begin
+int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
+				      unsigned int optlen);
+// Fast Open Mechanism functions end
+
 static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
 {
 	return READ_ONCE(msk->pm.addr_signal) &
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 423d3826ca1e..f62f0d63b8e6 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:
 			return true;
 		}
 
@@ -796,6 +797,8 @@ 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:
+		return mptcp_setsockopt_sol_tcp_fastopen(msk, optval, optlen);
 	}
 
 	return -EOPNOTSUPP;
-- 
2.25.1

[RFC PATCH mptcp-next v9 3/6] mptcp: reuse tcp_sendmsg_fastopen()

[Dmytro Shytyi]

In the following patches we will reuse modified tcp_sendmsg_fastopen().
We call it from mptcp_sendmsg().

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 include/net/tcp.h    |  3 +++
 net/ipv4/tcp.c       | 24 +++++++++++++++++++-----
 net/mptcp/protocol.c | 29 +++++++++++++++++++++++++++--
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 735e957f7f4b..a7d49e42470a 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1754,6 +1754,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 9251c99d3cfd..41fa9e840e0e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -280,6 +280,9 @@
 #include <asm/ioctls.h>
 #include <net/busy_poll.h>
 
+#include <net/mptcp.h>
+#include "../mptcp/protocol.h"
+
 /* Track pending CMSGs. */
 enum {
 	TCP_CMSG_INQ = 1,
@@ -1162,9 +1165,9 @@ void tcp_free_fastopen_req(struct tcp_sock *tp)
 	}
 }
 
-static 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);
@@ -1197,8 +1200,19 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
 		}
 	}
 	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 mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
+
+		release_sock(sk);
+		release_sock(subflow->conn);
+		err = mptcp_stream_connect(sk->sk_socket, uaddr,
+					   msg->msg_namelen, msg->msg_flags);
+		lock_sock(subflow->conn);
+		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 470045793181..d5c502d141b4 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1668,14 +1668,39 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	struct page_frag *pfrag;
+	struct socket *ssock;
 	size_t copied = 0;
 	int ret = 0;
 	long timeo;
 
+	lock_sock(sk);
+
+	ssock = __mptcp_nmpc_socket(msk);
+
+	if (ssock && inet_sk(ssock->sk)->defer_connect) {
+		release_sock(sk);
+		goto fastopen;
+	} else {
+		release_sock(sk);
+	}
 	/* we don't support FASTOPEN yet */
-	if (msg->msg_flags & MSG_FASTOPEN)
-		return -EOPNOTSUPP;
+	if (msg->msg_flags & MSG_FASTOPEN) {
+fastopen:
+		lock_sock(sk);
+
+		ssock = __mptcp_nmpc_socket(msk);
+
+		lock_sock(ssock->sk);
 
+		if (ssock) {
+			int copied_syn_fastopen = 0;
+
+			ret = tcp_sendmsg_fastopen(ssock->sk, msg, &copied_syn_fastopen, len, NULL);
+			copied += copied_syn_fastopen;
+		}
+		release_sock(ssock->sk);
+		release_sock(sk);
+	}
 	/* silently ignore everything else */
 	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;
 
-- 
2.25.1

[RFC PATCH mptcp-next v9 4/6] mptcp: fix retrans., add mptfo vars to msk

[Dmytro Shytyi]

We use mptcp_gen_msk_ackseq_fasopen()
when we know this is the first chunk of data after MPTFO.
Without it I observe infinite retransmissions.

Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 net/mptcp/fastopen.c | 14 ++++++++++++++
 net/mptcp/options.c  |  5 +++++
 net/mptcp/protocol.h |  3 +++
 3 files changed, 22 insertions(+)

diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index 9ef49a2d2ea2..40c3fb8c75e3 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -30,3 +30,17 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
 
 	return ret;
 }
+
+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 57596cdfb1f9..b9e251848099 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,
@@ -842,6 +843,8 @@ int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_l
 // Fast Open Mechanism functions begin
 int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
 				      unsigned int optlen);
+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)
-- 
2.25.1

Re: [RFC PATCH mptcp-next v9 4/6] mptcp: fix retrans., add mptfo vars to msk

[Paolo Abeni]

On Wed, 2022-09-21 at 14:55 +0200, Dmytro Shytyi wrote:
> We use mptcp_gen_msk_ackseq_fasopen()
> when we know this is the first chunk of data after MPTFO.
> Without it I observe infinite retransmissions.

The commit message should probably re-phrase to something more
descriptive. 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.

Thanks,

Paolo

Re: [RFC PATCH mptcp-next v9 4/6] mptcp: fix retrans., add mptfo vars to msk

[Dmytro Shytyi]

On 9/21/2022 8:02 PM, Paolo Abeni 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.

Thanks.


FIxed locally, will be present in next v10 patch.


Best,

Dmytro

[RFC PATCH mptcp-next v9 5/6] 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/protocol.c    |  2 +-
 net/mptcp/protocol.h    |  1 +
 net/mptcp/subflow.c     | 70 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 85 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/protocol.c b/net/mptcp/protocol.c
index d5c502d141b4..6a593be6076b 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -200,7 +200,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 b9e251848099..58a04144fff0 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -845,6 +845,7 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
 				      unsigned int optlen);
 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);
 // 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..7deb80c2af69 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -307,6 +307,74 @@ 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 tcp_request_sock *tcp_r_sock = tcp_rsk(req);
+	struct sock *socket = mptcp_subflow_ctx(sk)->conn;
+	struct inet_request_sock *ireq = inet_rsk(req);
+	struct mptcp_sock *msk = mptcp_sk(socket);
+	struct sock *var_sk = subflow->tcp_sock;
+	struct tcp_sock *tp = tcp_sk(sk);
+	struct sk_buff *skb;
+
+	//<evenutally clear tstamp_ok, as needed depending on cookie size>
+	//We add ts here as in the "if" below it has no effect.
+	if (foc->len > -1) {
+		ireq->tstamp_ok = 0;
+	}
+	if (synack_type == TCP_SYNACK_FASTOPEN) {
+		// <mark subflow/msk as "mptfo">
+		msk->is_mptfo = 1;
+
+		skb = skb_peek(&var_sk->sk_receive_queue);
+
+		//<dequeue the skb from sk receive queue>
+		__skb_unlink(skb, &var_sk->sk_receive_queue);
+		skb_ext_reset(skb);
+		skb_orphan(skb);
+
+		//<set the skb mapping>
+		//Solves: WARNING: at 704 _mptcp_move_skbs_from_subflow+0x5d0/0x651
+		tp->copied_seq += tp->rcv_nxt - tcp_r_sock->rcv_isn - 1;
+
+		subflow->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
+
+		//Solves: BAD mapping: ssn=0 map_seq=1 map_data_len=3
+		subflow->ssn_offset = tp->copied_seq - 1;
+
+		//<acquire the msk data lock>
+		lock_sock((struct sock *)msk);
+
+		//<move skb into msk receive queue>
+		mptcp_set_owner_r(skb, (struct sock *)msk);
+		__skb_queue_tail(&msk->receive_queue, skb);
+		atomic64_set(&msk->rcv_wnd_sent, mptcp_subflow_get_mapped_dsn(subflow));
+
+		//<call msk data_ready>
+		((struct sock *)msk)->sk_data_ready((struct sock *)msk);
+
+		//<release the msk data lock>
+		release_sock((struct sock *)msk);
+	}
+	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)
+{
+	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 +1988,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 +2002,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.25.1

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

[Paolo Abeni]

On Wed, 2022-09-21 at 14:55 +0200, 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/protocol.c    |  2 +-
>  net/mptcp/protocol.h    |  1 +
>  net/mptcp/subflow.c     | 70 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 85 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;
> +		}
>  	}
> -

Why the above chunk is needed?

>  	if (tcp_fastopen_no_cookie(sk, dst, TFO_SERVER_COOKIE_NOT_REQD))
>  		goto fastopen;
>  
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index d5c502d141b4..6a593be6076b 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -200,7 +200,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 b9e251848099..58a04144fff0 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -845,6 +845,7 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>  				      unsigned int optlen);
>  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);
>  // 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..7deb80c2af69 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -307,6 +307,74 @@ 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,

If you use 'ssk' instead of 'sk'

> +				  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 tcp_request_sock *tcp_r_sock = tcp_rsk(req);
> +	struct sock *socket = mptcp_subflow_ctx(sk)->conn;

than you can use 'sk' here and respect mptcp convection for variable
names.

> +	struct inet_request_sock *ireq = inet_rsk(req);
> +	struct mptcp_sock *msk = mptcp_sk(socket);
> +	struct sock *var_sk = subflow->tcp_sock;

This should be actually equal to ssk

> +	struct tcp_sock *tp = tcp_sk(sk);
> +	struct sk_buff *skb;

All the above variable should be moved under the 'if (synack_type ==
TCP_SYNACK_FASTOPEN) {' branch, as there are used only there.

Additionally if you move all the code in the 'if (synack_type ==
TCP_SYNACK_FASTOPEN)' branch in a separate helper you can later reuse
it in subflow_v6_send_synack()

> +
> +	//<evenutally clear tstamp_ok, as needed depending on cookie size>
> +	//We add ts here as in the "if" below it has no effect.

Please, don't use '//' for comments

> +	if (foc->len > -1) {
> +		ireq->tstamp_ok = 0;
> +	}

No need to add the '{' for a single line 'then' statement; addtionally
I guess you can move the above check under the below if, and you should
check for 'foc != NULL'

> +	if (synack_type == TCP_SYNACK_FASTOPEN) {
> +		// <mark subflow/msk as "mptfo">
> +		msk->is_mptfo = 1;
> +
> +		skb = skb_peek(&var_sk->sk_receive_queue);
> +
> +		//<dequeue the skb from sk receive queue>
> +		__skb_unlink(skb, &var_sk->sk_receive_queue);
> +		skb_ext_reset(skb);
> +		skb_orphan(skb);
> +
> +		//<set the skb mapping>
> +		//Solves: WARNING: at 704 _mptcp_move_skbs_from_subflow+0x5d0/0x651

Please, drop the WARNING reference.

> +		tp->copied_seq += tp->rcv_nxt - tcp_r_sock->rcv_isn - 1;
> +
> +		subflow->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
> +
> +		//Solves: BAD mapping: ssn=0 map_seq=1 map_data_len=3

Same here.

> +		subflow->ssn_offset = tp->copied_seq - 1;
> +
> +		//<acquire the msk data lock>
> +		lock_sock((struct sock *)msk);

the mptcp data lock is acquired with:

		 mptcp_data_lock((struct sock *)msk)

which become
		mptcp_data_lock(sk);

if you apply the mptcp variable name conventions.

I think you additionally need to init the mptcp CB for skb.

I'm not sure what will happen to later skb coming via
__mptcp_move_skb(). You must ensure that they will not be coalesced.
Probably correctly initializing the mptcp CB should ensure the above.

> +
> +		//<move skb into msk receive queue>
> +		mptcp_set_owner_r(skb, (struct sock *)msk);
> +		__skb_queue_tail(&msk->receive_queue, skb);
> +		atomic64_set(&msk->rcv_wnd_sent, mptcp_subflow_get_mapped_dsn(subflow));
> +
> +		//<call msk data_ready>
> +		((struct sock *)msk)->sk_data_ready((struct sock *)msk);

or just:
		sk->sk_data_ready(sk); 

(when respecting the mptcp variables name conventions)

> +
> +		//<release the msk data lock>
> +		release_sock((struct sock *)msk);
> +	}
> +	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)
> +{

This is not enough, you need to do the same things as under the 'if
(synack_type == TCP_SYNACK_FASTOPEN) {' conditional in
subflow_v4_send_synack()

> +	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 +1988,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 +2002,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;

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

[Dmytro Shytyi]

Hello Paolo,

Thank you for comments.

My comments are in-line + the new patch in the bottom of this message 
with corrections.

On 9/21/2022 8:00 PM, Paolo Abeni wrote:
> On Wed, 2022-09-21 at 14:55 +0200, 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/protocol.c    |  2 +-
>>   net/mptcp/protocol.h    |  1 +
>>   net/mptcp/subflow.c     | 70 +++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 85 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;
>> +		}
>>   	}
>> -
> Why the above chunk is needed?

I didn't check why yet, but without this I don't have Cookie reply from 
listener even if I activate fastopen in sysctl.

I realise this is not good to do. Currently it is more like bypass. I am 
going to look at if later.


>>   	if (tcp_fastopen_no_cookie(sk, dst, TFO_SERVER_COOKIE_NOT_REQD))
>>   		goto fastopen;
>>   
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index d5c502d141b4..6a593be6076b 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -200,7 +200,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 b9e251848099..58a04144fff0 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -845,6 +845,7 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>>   				      unsigned int optlen);
>>   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);
>>   // 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..7deb80c2af69 100644
>> --- a/net/mptcp/subflow.c
>> +++ b/net/mptcp/subflow.c
>> @@ -307,6 +307,74 @@ 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,
> If you use 'ssk' instead of 'sk'

I dont think it is a good idea to change sk to ssk as argument and use 
it because of "const" compiler should throw an error if I will try to 
dequeue, etc from const struct sock *sk ant it is right. So I retreived 
ssk from subflow without const to do what is needed.

But still I modify the naming scheme as you suggested.

>> +				  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 tcp_request_sock *tcp_r_sock = tcp_rsk(req);
>> +	struct sock *socket = mptcp_subflow_ctx(sk)->conn;
> than you can use 'sk' here and respect mptcp convection for variable
> names.
Yes, Fixed.
>> +	struct inet_request_sock *ireq = inet_rsk(req);
>> +	struct mptcp_sock *msk = mptcp_sk(socket);
>> +	struct sock *var_sk = subflow->tcp_sock;
> This should be actually equal to ssk
var_sk changed to ssk
>> +	struct tcp_sock *tp = tcp_sk(sk);
>> +	struct sk_buff *skb;
> All the above variable should be moved under the 'if (synack_type ==
> TCP_SYNACK_FASTOPEN) {' branch, as there are used only there.
>
> Additionally if you move all the code in the 'if (synack_type ==
> TCP_SYNACK_FASTOPEN)' branch in a separate helper you can later reuse
> it in subflow_v6_send_synack()
>
>> +
>> +	//<evenutally clear tstamp_ok, as needed depending on cookie size>
>> +	//We add ts here as in the "if" below it has no effect.
> Please, don't use '//' for comments

Oh sorry, Yes in C we don't use these "//", I replaced it with "/* */".


>> +	if (foc->len > -1) {
>> +		ireq->tstamp_ok = 0;
>> +	}
> No need to add the '{' for a single line 'then' statement; addtionally
> I guess you can move the above check under the below if, and you should
> check for 'foc != NULL'


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, but it should.

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 keep "foc" check outside "if" statement for the moment.



>
>> +	if (synack_type == TCP_SYNACK_FASTOPEN) {
>> +		// <mark subflow/msk as "mptfo">
>> +		msk->is_mptfo = 1;
>> +
>> +		skb = skb_peek(&var_sk->sk_receive_queue);
>> +
>> +		//<dequeue the skb from sk receive queue>
>> +		__skb_unlink(skb, &var_sk->sk_receive_queue);
>> +		skb_ext_reset(skb);
>> +		skb_orphan(skb);
>> +
>> +		//<set the skb mapping>
>> +		//Solves: WARNING: at 704 _mptcp_move_skbs_from_subflow+0x5d0/0x651
> Please, drop the WARNING reference.
Fixed
>> +		tp->copied_seq += tp->rcv_nxt - tcp_r_sock->rcv_isn - 1;
>> +
>> +		subflow->map_seq = mptcp_subflow_get_mapped_dsn(subflow);
>> +
>> +		//Solves: BAD mapping: ssn=0 map_seq=1 map_data_len=3
> Same here.
FIxed
>> +		subflow->ssn_offset = tp->copied_seq - 1;
>> +
>> +		//<acquire the msk data lock>
>> +		lock_sock((struct sock *)msk);
> the mptcp data lock is acquired with:
>
> 		 mptcp_data_lock((struct sock *)msk)
>
> which become
> 		mptcp_data_lock(sk);
>
> if you apply the mptcp variable name conventions.
Fixed
> I think you additionally need to init the mptcp CB for skb.
>
> I'm not sure what will happen to later skb coming via
> __mptcp_move_skb(). You must ensure that they will not be coalesced.
> Probably correctly initializing the mptcp CB should ensure the above.

Could you please evolve the comment? Set the MPTCP_SKB_CB: offset , 
map_seq, end_seq?


>> +
>> +		//<move skb into msk receive queue>
>> +		mptcp_set_owner_r(skb, (struct sock *)msk);
>> +		__skb_queue_tail(&msk->receive_queue, skb);
>> +		atomic64_set(&msk->rcv_wnd_sent, mptcp_subflow_get_mapped_dsn(subflow));
>> +
>> +		//<call msk data_ready>
>> +		((struct sock *)msk)->sk_data_ready((struct sock *)msk);
> or just:
> 		sk->sk_data_ready(sk);
>
> (when respecting the mptcp variables name conventions)
Fixed.
>> +
>> +		//<release the msk data lock>
>> +		release_sock((struct sock *)msk);
>> +	}
>> +	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)
>> +{
> This is not enough, you need to do the same things as under the 'if
> (synack_type == TCP_SYNACK_FASTOPEN) {' conditional in
> subflow_v4_send_synack()

Yes, it was a placeholder. I was wondering if the content is  the same 
as in v4 probably I could add a helper and reuse in v4/6. And hopefully 
you suggest to do so in this message :)


>> +	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 +1988,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 +2002,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;


Here is a commit with modifictions:

 From 13de5d435a4323519235c811719bd9d5fd1802de Mon Sep 17 00:00:00 2001
From: Dmytro Shytyi <dmytro@shytyi.net>
Date: Tue, 20 Sep 2022 19:53:56 +0200
Subject: [RFC PATCH mptcp-next v9 5/6] mptcp: add 
subflow_v(4,6)_send_synack()

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/protocol.c    |  2 +-
  net/mptcp/protocol.h    |  1 +
  net/mptcp/subflow.c     | 85 +++++++++++++++++++++++++++++++++++++++++
  4 files changed, 100 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/protocol.c b/net/mptcp/protocol.c
index d5c502d141b4..6a593be6076b 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -200,7 +200,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 b9e251848099..58a04144fff0 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -845,6 +845,7 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct 
mptcp_sock *msk, sockptr_t optval,
                        unsigned int optlen);
  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);
  // 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..cca888bae261 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -307,6 +307,89 @@ static struct dst_entry *subflow_v4_route_req(const 
struct sock *sk,
      return NULL;
  }

+static void subflow_fastopen_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);
+}
+
+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 != NULL)
+        ireq->tstamp_ok = 0;
+
+    if (synack_type == TCP_SYNACK_FASTOPEN) {
+        subflow_fastopen_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 != NULL)
+        ireq->tstamp_ok = 0;
+
+    if (synack_type == TCP_SYNACK_FASTOPEN) {
+        subflow_fastopen_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 +2003,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 +2017,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.25.1

[RFC PATCH mptcp-next v9 6/6] 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.25.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/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: 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): selftest_mptfo selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/5220647671431168
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5220647671431168/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 1 failed test(s): selftest_mptfo 🔴:
  - Task: https://cirrus-ci.com/task/6723649817280512
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6723649817280512/summary/summary.txt

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


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: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Dmytro Shytyi]

Hello Benjamin, All,

I excuse for the later if I made any mistake.

My thought is comming from experience with the patch.


Will huge respect, I think this patch _*MUST NOT*_ be accepted because 
of multiple reasons:

1. it violates the RFC 8684 [1] section B1:

"When a TFO initiator first connects to a listener, it cannot 
immediately include data in the SYN for security reasons[RFC7413 
<https://www.rfc-editor.org/rfc/rfc8684.html#RFC7413>]. Instead, it 
requests a cookie that will be used in subsequent connections."


Also I created environment[3] using commit[2], I tested v0, v2 and I do 
not see the mptcp capable option in SYN.

2. Abscense of MP_CAPABLE in SYN  violates the RFC 8684 [1] section B3.


3. Patch uses an original Idea of another autor from mailing list (Reuse 
the TCP FASTOPEN option in MPTCP).


[1] RFC 8684: TCP Extensions for Multipath Operation with Multiple 
Addresses (rfc-editor.org) 
<https://www.rfc-editor.org/rfc/rfc8684.html#name-tfo-cookie-request-with-mpt>

[2][PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp 
(kernel.org) 
<https://lore.kernel.org/mptcp/20220908133829.3410092-1-benjamin.hesmans@tessares.net/T/#m021bdd02260ccf1a556f7bbe5657acf9bf0cdb24>

[3]  preparation of environment provided by Benjamin's patch:

+ DO_CLEANUP=1
+ DO_MPTCPIZE=1
+ trap cleanup EXIT
+ ip netns add client
+ ip netns add server
+ netns client sysctl net.ipv4.tcp_fastopen=0x5
+ ns=client
+ shift
+ ip netns exec client sysctl net.ipv4.tcp_fastopen=0x5
net.ipv4.tcp_fastopen = 0x5
+ netns server sysctl net.ipv4.tcp_fastopen=0x602
+ ns=server
+ shift
+ ip netns exec server sysctl net.ipv4.tcp_fastopen=0x602
net.ipv4.tcp_fastopen = 0x602
+ netns client ip link add eth0 type veth peer eth0 netns server
+ ns=client
+ shift
+ ip netns exec client ip link add eth0 type veth peer eth0 netns server
+ netns client ip addr add 6.6.6.1/24 dev eth0
+ ns=client
+ shift
+ ip netns exec client ip addr add 6.6.6.1/24 dev eth0
+ netns server ip addr add 6.6.6.6/24 dev eth0
+ ns=server
+ shift
+ ip netns exec server ip addr add 6.6.6.6/24 dev eth0
+ netns client ip link set eth0 up
+ ns=client
+ shift
+ ip netns exec client ip link set eth0 up
+ netns server ip link set eth0 up
+ ns=server
+ shift
+ ip netns exec server ip link set eth0 up
[   36.774284] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
+ sleep 1
+ netns client tcpdump -i eth0 -w ./client.pcap
+ ns=client
+ shift
+ ip netns exec client tcpdump -i eth0 -w ./client.pcap
[   36.796976] device eth0 entered promiscuous mode
tcpdump: listening on eth0, link-type EN10MB (Ethernet), snapshot length 
262144 bytes
+ '[' 1 == 0 ']'
+ LD_PRELOAD=/home/awesome/workspace/userspace/mptcpize/libmptcpwrap.so
+ sleep 3
+ LD_PRELOAD=/home/awesome/workspace/userspace/mptcpize/libmptcpwrap.so
+ netns server python3 -m http.server 666
+ ns=server
+ shift
+ ip netns exec server python3 -m http.server 666
[   37.801129] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
Serving HTTP on :: port 666 (http://[::]:666/) ...
+ LD_PRELOAD=/home/awesome/workspace/userspace/mptcpize/libmptcpwrap.so
+ netns client curl --tcp-fastopen -o out.txt http://6.6.6.6:666/tfo.sh
+ ns=client
+ shift
+ ip netns exec client curl --tcp-fastopen -o out.txt 
http://6.6.6.6:666/tfo.sh
   % Total    % Received % Xferd  Average Speed   Time    Time Time  Current
                                  Dload  Upload   Total   Spent Left  Speed
   0     0    0     0    0     0      0      0 --:--:-- --:--:-- 
--:--:--     0::ffff:6.6.6.1 - - [22/Sep/2022 17:54:31]d
::ffff:6.6.6.1 - - [22/Sep/2022 17:54:31] "GET /tfo.sh HTTP/1.1" 404 -
100   469  100   469    0     0  53289      0 --:--:-- --:--:-- --:--:-- 
58625
+ sleep 3
+ cleanup
+ '[' 1 == 0 ']'
+ pkill tcpdump
[   43.839628] device eth0 left promiscuous mode
24 packets captured
25 packets received by filter
0 packets dropped by kernel
+ pkill python
+ ip netns delete client
Terminated
+ ip netns delete server


> The series only consider the sender side.
>
> Compared to the previous RFC patches, these ones focus on
> the sender side only. It corresponds to the 4 first patches from the RFC
> series.
>
> The sending part is less complex and even if it looks like we are
> converging for the receive part, there are still discussions on-going
> there.
>
> Again, thank you Dmytro for the previous work done. As already discussed
> on the ML and meeting, this approach was slightly different from what
> Dmytro originally proposed. Here tcp_sendmsg_fastopen() is exported and
> re-used and TCP_FASTOPEN_CONNECT is supported.
>
> MSG_FASTOPEN will be handled by Dmytro's patches.
>
> Individual changelogs have been added per patch.
>
> v2:
> - Drop support for MSG_FASTOPEN because we were not sure that it was the
>    correct way to do it.
> - latest patch of the series: apply comment from Paolo concerning
>    mptcp_poll()
>
> Benjamin Hesmans (4):
>    mptcp: add TCP_FASTOPEN_CONNECT socket option
>    tcp: export tcp_sendmsg_fastopen
>    mptcp: handle defer connect in mptcp_sendmsg
>    mptcp: poll allow write call before actual connect
>
>   include/net/tcp.h    |  2 ++
>   net/ipv4/tcp.c       |  5 ++---
>   net/mptcp/protocol.c | 26 ++++++++++++++++++++++++++
>   net/mptcp/sockopt.c  | 19 ++++++++++++++++++-
>   4 files changed, 48 insertions(+), 4 deletions(-)
>
> -- 
> 2.25.1
>

Re: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Matthieu Baerts]

Hi Dmytro,

On 23/09/2022 01:23, Dmytro Shytyi wrote:
> Hello Benjamin, All,
> 
> I excuse for the later if I made any mistake.
> 
> My thought is comming from experience with the patch.
> 
> 
> Will huge respect, I think this patch _*MUST NOT*_ be accepted because
> of multiple reasons:
> 
> 1. it violates the RFC 8684 [1] section B1:
> 
> "When a TFO initiator first connects to a listener, it cannot
> immediately include data in the SYN for security reasons[RFC7413
> <https://www.rfc-editor.org/rfc/rfc8684.html#RFC7413>]. Instead, it
> requests a cookie that will be used in subsequent connections."
> 
> 
> Also I created environment[3] using commit[2], I tested v0, v2 and I do
> not see the mptcp capable option in SYN.

From what I see in your setup, you set net.ipv4.tcp_fastopen:
- The sender has the 0x4 flag to send data in the opening SYN regardless
of cookie availability and without a cookie option.
- The receiver has the 0x200 flag to accept data-in-SYN w/o any cookie
option present

See the link below for more details about the bitmap:

https://www.kernel.org/doc/html/latest/networking/ip-sysctl.html

If you change these values, I guess you will see the TFO cookie option.

Please note that the proper way to validate exchanged packet is to use
Packetdrill. The following PR proves the implementation works with TFO
cookies:

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

> 2. Abscense of MP_CAPABLE in SYN  violates the RFC 8684 [1] section B3.

The Packetdrill tests also shows everything is OK with MPTCP.
I suppose there is an issue with your test environment and some paths
(e.g. libmptcpwrap.so) are probably wrong, justifying why you get a 404
error when doing the curl:

  "GET /tfo.sh HTTP/1.1" 404

Please do the validation without mptcpize, e.g. with packetdrill:

https://github.com/multipath-tcp/packetdrill/blob/f3672b80a687e0e2a59926992f28c165783ecf8b/gtests/net/mptcp/fastopen/client-TCP_FASTOPEN_CONNECT.pkt

> 3. Patch uses an original Idea of another autor from mailing list (Reuse
> the TCP FASTOPEN option in MPTCP).

A v3 mentioning you is going to be sent as discussed at the last meeting.

We should indeed mention the authors of the original idea to have TFO in
MPTCP:

https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/

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

Re: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Dmytro Shytyi]

Hello,

I am sharing experience from using the patch with environment that is 
provided to test the patch with selftest ( As i mentioned before).
I take me some time to launch the patch. Hope this will help.

On 9/23/2022 12:58 PM, Matthieu Baerts wrote:
> Hi Dmytro,
>
> On 23/09/2022 01:23, Dmytro Shytyi wrote:
>> Hello Benjamin, All,
>>
>> I excuse for the later if I made any mistake.
>>
>> My thought is comming from experience with the patch.
>>
>>
>> Will huge respect, I think this patch _*MUST NOT*_ be accepted because
>> of multiple reasons:
>>
>> 1. it violates the RFC 8684 [1] section B1:
>>
>> "When a TFO initiator first connects to a listener, it cannot
>> immediately include data in the SYN for security reasons[RFC7413
>> <https://www.rfc-editor.org/rfc/rfc8684.html#RFC7413>]. Instead, it
>> requests a cookie that will be used in subsequent connections."
>>
>>
>> Also I created environment[3] using commit[2], I tested v0, v2 and I do
>> not see the mptcp capable option in SYN.
>  From what I see in your setup, you set net.ipv4.tcp_fastopen:
> - The sender has the 0x4 flag to send data in the opening SYN regardless
> of cookie availability and without a cookie option.
> - The receiver has the 0x200 flag to accept data-in-SYN w/o any cookie
> option present
>
> See the link below for more details about the bitmap:
>
> https://www.kernel.org/doc/html/latest/networking/ip-sysctl.html
>
> If you change these values, I guess you will see the TFO cookie option.
>
> Please note that the proper way to validate exchanged packet is to use
> Packetdrill. The following PR proves the implementation works with TFO
> cookies:
>
> https://github.com/multipath-tcp/packetdrill/pull/87

Could you please verify this with wireshark before the acceptence?

In evironment provided by the benjamins patch I cannot see the cookie, 
nither if I create sender in C  (that is not using mptcpize as in the 
exemple from packet drill:

mptcp: new fastopen-invalid-buf-ptr test · 
multipath-tcp/packetdrill@af86f4d (github.com) 
<https://github.com/multipath-tcp/packetdrill/commit/af86f4d67c2595b6edd667a89ba9a838308142e2>

or this

Support TCP Fast Open · Issue #49 · rust-lang/socket2 (github.com) 
<https://github.com/rust-lang/socket2/issues/49>

The cookie is not seen neither with curl:

  neither with *.c client, configured

neither with:

-netns client sysctl net.ipv4.tcp_fastopen=1

-netns server sysctl net.ipv4.tcp_fastopen=3

neighter with default Benjamin's setup.


I attach an exemple of c.* file in case if I made any mistake.

"

sock_fd = socket(AF_INET, SOCK_STREAM, IPPROTO_MPTCP);

I attach the code from *.C file:

setsockopt(sock_fd, SOL_TCP, TCP_FASTOPEN_CONNECT, &enable, sizeof(enable));

connect(sock_fd, (struct sockaddr *) &daddr, sizeof(daddr));

ret = sendto(sock_fd, sendline, strlen(sendline), 0,(struct sockaddr 
*)&daddr, sizeof(daddr));

"

>> 2. Abscense of MP_CAPABLE in SYN  violates the RFC 8684 [1] section B3.
> The Packetdrill tests also shows everything is OK with MPTCP.
> I suppose there is an issue with your test environment and some paths
> (e.g. libmptcpwrap.so) are probably wrong, justifying why you get a 404
> error when doing the curl:
>
>    "GET /tfo.sh HTTP/1.1" 404
>
> Please do the validation without mptcpize, e.g. with packetdrill:
>
> https://github.com/multipath-tcp/packetdrill/blob/f3672b80a687e0e2a59926992f28c165783ecf8b/gtests/net/mptcp/fastopen/client-TCP_FASTOPEN_CONNECT.pkt
>
>> 3. Patch uses an original Idea of another autor from mailing list (Reuse
>> the TCP FASTOPEN option in MPTCP).
> A v3 mentioning you is going to be sent as discussed at the last meeting.
>
> We should indeed mention the authors of the original idea to have TFO in
> MPTCP:
>
> https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/
>
> Cheers,
> Matt

It seems this draft didn't get the consensus and was not accepted by 
IETF as RFC. How the abstract RFC related to the original idea to reuse 
TFO option from regular TCP in linux kernel implementation?

Re: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Matthieu Baerts]

On 23/09/2022 15:41, Dmytro Shytyi wrote:
> Hello,
> 
> I am sharing experience from using the patch with environment that is
> provided to test the patch with selftest ( As i mentioned before).
> I take me some time to launch the patch. Hope this will help.

Probably best to check with packetdrill as usually recommended to
validate such modifications. The manual steps can lead to a wrong
environment.

> 
> On 9/23/2022 12:58 PM, Matthieu Baerts wrote:
>> Hi Dmytro,
>>
>> On 23/09/2022 01:23, Dmytro Shytyi wrote:
>>> Hello Benjamin, All,
>>>
>>> I excuse for the later if I made any mistake.
>>>
>>> My thought is comming from experience with the patch.
>>>
>>>
>>> Will huge respect, I think this patch _*MUST NOT*_ be accepted because
>>> of multiple reasons:
>>>
>>> 1. it violates the RFC 8684 [1] section B1:
>>>
>>> "When a TFO initiator first connects to a listener, it cannot
>>> immediately include data in the SYN for security reasons[RFC7413
>>> <https://www.rfc-editor.org/rfc/rfc8684.html#RFC7413>]. Instead, it
>>> requests a cookie that will be used in subsequent connections."
>>>
>>>
>>> Also I created environment[3] using commit[2], I tested v0, v2 and I do
>>> not see the mptcp capable option in SYN.
>>  From what I see in your setup, you set net.ipv4.tcp_fastopen:
>> - The sender has the 0x4 flag to send data in the opening SYN regardless
>> of cookie availability and without a cookie option.
>> - The receiver has the 0x200 flag to accept data-in-SYN w/o any cookie
>> option present
>>
>> See the link below for more details about the bitmap:
>>
>> https://www.kernel.org/doc/html/latest/networking/ip-sysctl.html
>>
>> If you change these values, I guess you will see the TFO cookie option.
>>
>> Please note that the proper way to validate exchanged packet is to use
>> Packetdrill. The following PR proves the implementation works with TFO
>> cookies:
>>
>> https://github.com/multipath-tcp/packetdrill/pull/87
> 
> Could you please verify this with wireshark before the acceptence?

Here is the output from Packetdrill:

> root@(none):/opt/packetdrill/gtests/net/mptcp# ../packetdrill/packetdrill -v ./fastopen/client-TCP_FASTOPEN_CONNECT.pkt
> socket syscall: 1663941238.170913
> fcntl syscall: 1663941238.171243
> setsockopt syscall: 1663941238.171551
> connect syscall: 1663941238.171964
> outbound sniffed packet:  0.001058 S 1708222581:1708222581(0) win 65535 <mss 1460,sackOK,TS val 1122428945 ecr 0,nop,wscale 8,FO,nop,nop,mp_capable v1 flags: |H| >
> inbound injected packet:  0.012799 S. 0:0(0) ack 1708222582 win 65535 <mss 1460,sackOK,TS val 700 ecr 1122428945,nop,wscale 8,FO abcd1234,nop,nop,mp_capable v1 flags: |H| sender_key: 2>
> outbound sniffed packet:  0.015375 . 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428959 ecr 700,mp_capable v1 flags: |H| sender_key: 12211970130642946725 receiver_key: 2>
> close syscall: 1663941238.199617
> outbound sniffed packet:  0.028711 . 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428959 ecr 700,dss dack4 3007449509 dsn8 17003222625898214342 ssn 0 dll 1 no_checksum flags: MmAF,nop,nop>
> inbound injected packet:  0.030712 . 1:1(0) ack 1708222582 win 450 <nop,nop,TS val 700 ecr 1122428959,dss dack4 764163015 dsn4 3007449509 ssn 0 dll 1 no_checksum flags: MAF,nop,nop>
> outbound sniffed packet:  0.032187 . 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428976 ecr 700,dss dack4 3007449510 flags: A>
> outbound sniffed packet:  0.033824 R. 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428978 ecr 700,mp_fastclose receiver key: 2,mp_reset 0>
> socket syscall: 1663941238.306640
> fcntl syscall: 1663941238.307030
> setsockopt syscall: 1663941238.307543
> connect syscall: 1663941238.308340
> write syscall: 1663941238.309088
> outbound sniffed packet:  0.138181 S 3843296367:3843296867(500) win 65535 <mss 1460,sackOK,TS val 1122429082 ecr 0,nop,wscale 8,FO abcd1234,nop,nop,mp_capable v1 flags: |H| >
> inbound injected packet:  0.151901 S. 0:0(0) ack 3843296868 win 65535 <mss 1460,sackOK,TS val 700 ecr 1122429082,nop,wscale 8,mp_capable v1 flags: |H| sender_key: 2>


And the output from tcpdump when doing the same test:

> root@(none):/opt/packetdrill/gtests/net/mptcp# ../packetdrill/packetdrill ./fastopen/client-TCP_FASTOPEN_CONNECT.pkt
> 13:53:44.586369 tun0  Out IP6 fe80::2ecf:46fa:841:cae > ff02::2: ICMP6, router solicitation, length 8
> 13:53:44.860976 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [S], seq 2694673015, win 65535, options [mss 1460,sackOK,TS val 2036993134 ecr 0,nop,wscale 8,tfo  cookiereq,nop,nop,mptcp capable v1], length 0
> 13:53:44.871047 tun0  In  IP 192.0.2.1.8080 > 192.168.49.174.60676: Flags [S.], seq 0, ack 2694673016, win 65535, options [mss 1460,sackOK,TS val 700 ecr 2036993134,nop,wscale 8,tfo  cookie abcd1234,nop,nop,mptcp capable v1 {0x200000000000000}], length 0
> 13:53:44.871063 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993144 ecr 700,mptcp capable v1 {0x3827b4480bfa870f,0x200000000000000}], length 0
> 13:53:44.881125 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993144 ecr 700,mptcp dss fin ack 3007449509 seq 5431916111306864352 subseq 0 len 1,nop,nop], length 0
> 13:53:44.881210 tun0  In  IP 192.0.2.1.8080 > 192.168.49.174.60676: Flags [.], ack 1, win 450, options [nop,nop,TS val 700 ecr 2036993144,mptcp dss fin ack 2016065249 seq 3007449509 subseq 0 len 1,nop,nop], length 0
> 13:53:44.881226 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993154 ecr 700,mptcp dss ack 3007449510], length 0
> 13:53:44.881260 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [R.], seq 1, ack 1, win 256, options [nop,nop,TS val 2036993154 ecr 700,mptcp fast-close key 0x200000000000000,mptcp unknown], length 0
> 13:53:44.982454 ?     Out IP 192.168.49.174.60680 > 192.0.2.1.8080: Flags [S], seq 1531938899:1531939399, win 65535, options [mss 1460,sackOK,TS val 2036993255 ecr 0,nop,wscale 8,tfo  cookie abcd1234,nop,nop,mptcp capable v1], length 500: HTTP
> 13:53:44.992484 ?     In  IP 192.0.2.1.8080 > 192.168.49.174.60680: Flags [S.], seq 0, ack 1531939400, win 65535, options [mss 1460,sackOK,TS val 700 ecr 2036993255,nop,wscale 8,mptcp capable v1 {0x200000000000000}], length 0
> 13:53:44.992494 ?     Out IP 192.168.49.174.60680 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993265 ecr 700,mptcp capable v1 {0x75382dfb6dafc693,0x200000000000000}], length 0
> 13:53:44.992539 ?     Out IP 192.168.49.174.60680 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993265 ecr 700,mptcp dss fin ack 3007449509 seq 6967145468519815204 subseq 0 len 1,nop,nop], length 0

We can see:

- in the first SYN:
  - "tfo cookiereq"
  - "mptcp capable v1"
  - "length 0"

- in the second SYN:
  - "tfo  cookie abcd1234"
  - "mptcp capable v1"
  - "length 500"

Everything seems then OK.

>>> 3. Patch uses an original Idea of another autor from mailing list (Reuse
>>> the TCP FASTOPEN option in MPTCP).
>> A v3 mentioning you is going to be sent as discussed at the last meeting.
>>
>> We should indeed mention the authors of the original idea to have TFO in
>> MPTCP:
>>
>> https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/
>>
>> Cheers,
>> Matt
> 
> It seems this draft didn't get the consensus and was not accepted by
> IETF as RFC.


This link is just to point to the original idea of having TFO supported
with MPTCP. This document was used as a base for the evolution of the
MPTCPv0 (RFC6824) and it was the main reason why we have an MPTCPv1
(RFC8684) where the MP_CAPABLE is different from the v0 to allow TFO
cookie options in the initial SYN. So yes, it was somehow accepted by
the IETF but as part of RFC8684.

> How the abstract RFC related to the original idea to reuse
> TFO option from regular TCP in linux kernel implementation?
Sorry, what do you mean here?
Implementing TFO in MPTCP Linux Upstream is part of the roadmap from the
beginning, see ticket 59 on Github.

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

Re: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Dmytro Shytyi]

On 9/23/2022 4:08 PM, Matthieu Baerts wrote:
> On 23/09/2022 15:41, Dmytro Shytyi wrote:
>> Hello,
>>
>> I am sharing experience from using the patch with environment that is
>> provided to test the patch with selftest ( As i mentioned before).
>> I take me some time to launch the patch. Hope this will help.
> Probably best to check with packetdrill as usually recommended to
> validate such modifications. The manual steps can lead to a wrong
> environment.
>
>> On 9/23/2022 12:58 PM, Matthieu Baerts wrote:
>>> Hi Dmytro,
>>>
>>> On 23/09/2022 01:23, Dmytro Shytyi wrote:
>>>> Hello Benjamin, All,
>>>>
>>>> I excuse for the later if I made any mistake.
>>>>
>>>> My thought is comming from experience with the patch.
>>>>
>>>>
>>>> Will huge respect, I think this patch _*MUST NOT*_ be accepted because
>>>> of multiple reasons:
>>>>
>>>> 1. it violates the RFC 8684 [1] section B1:
>>>>
>>>> "When a TFO initiator first connects to a listener, it cannot
>>>> immediately include data in the SYN for security reasons[RFC7413
>>>> <https://www.rfc-editor.org/rfc/rfc8684.html#RFC7413>]. Instead, it
>>>> requests a cookie that will be used in subsequent connections."
>>>>
>>>>
>>>> Also I created environment[3] using commit[2], I tested v0, v2 and I do
>>>> not see the mptcp capable option in SYN.
>>>   From what I see in your setup, you set net.ipv4.tcp_fastopen:
>>> - The sender has the 0x4 flag to send data in the opening SYN regardless
>>> of cookie availability and without a cookie option.
>>> - The receiver has the 0x200 flag to accept data-in-SYN w/o any cookie
>>> option present
>>>
>>> See the link below for more details about the bitmap:
>>>
>>> https://www.kernel.org/doc/html/latest/networking/ip-sysctl.html
>>>
>>> If you change these values, I guess you will see the TFO cookie option.
>>>
>>> Please note that the proper way to validate exchanged packet is to use
>>> Packetdrill. The following PR proves the implementation works with TFO
>>> cookies:
>>>
>>> https://github.com/multipath-tcp/packetdrill/pull/87
>> Could you please verify this with wireshark before the acceptence?
> Here is the output from Packetdrill:
>
>> root@(none):/opt/packetdrill/gtests/net/mptcp# ../packetdrill/packetdrill -v ./fastopen/client-TCP_FASTOPEN_CONNECT.pkt
>> socket syscall: 1663941238.170913
>> fcntl syscall: 1663941238.171243
>> setsockopt syscall: 1663941238.171551
>> connect syscall: 1663941238.171964
>> outbound sniffed packet:  0.001058 S 1708222581:1708222581(0) win 65535 <mss 1460,sackOK,TS val 1122428945 ecr 0,nop,wscale 8,FO,nop,nop,mp_capable v1 flags: |H| >
>> inbound injected packet:  0.012799 S. 0:0(0) ack 1708222582 win 65535 <mss 1460,sackOK,TS val 700 ecr 1122428945,nop,wscale 8,FO abcd1234,nop,nop,mp_capable v1 flags: |H| sender_key: 2>
>> outbound sniffed packet:  0.015375 . 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428959 ecr 700,mp_capable v1 flags: |H| sender_key: 12211970130642946725 receiver_key: 2>
>> close syscall: 1663941238.199617
>> outbound sniffed packet:  0.028711 . 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428959 ecr 700,dss dack4 3007449509 dsn8 17003222625898214342 ssn 0 dll 1 no_checksum flags: MmAF,nop,nop>
>> inbound injected packet:  0.030712 . 1:1(0) ack 1708222582 win 450 <nop,nop,TS val 700 ecr 1122428959,dss dack4 764163015 dsn4 3007449509 ssn 0 dll 1 no_checksum flags: MAF,nop,nop>
>> outbound sniffed packet:  0.032187 . 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428976 ecr 700,dss dack4 3007449510 flags: A>
>> outbound sniffed packet:  0.033824 R. 1708222582:1708222582(0) ack 1 win 256 <nop,nop,TS val 1122428978 ecr 700,mp_fastclose receiver key: 2,mp_reset 0>
>> socket syscall: 1663941238.306640
>> fcntl syscall: 1663941238.307030
>> setsockopt syscall: 1663941238.307543
>> connect syscall: 1663941238.308340
>> write syscall: 1663941238.309088
>> outbound sniffed packet:  0.138181 S 3843296367:3843296867(500) win 65535 <mss 1460,sackOK,TS val 1122429082 ecr 0,nop,wscale 8,FO abcd1234,nop,nop,mp_capable v1 flags: |H| >
>> inbound injected packet:  0.151901 S. 0:0(0) ack 3843296868 win 65535 <mss 1460,sackOK,TS val 700 ecr 1122429082,nop,wscale 8,mp_capable v1 flags: |H| sender_key: 2>
>
> And the output from tcpdump when doing the same test:
>
>> root@(none):/opt/packetdrill/gtests/net/mptcp# ../packetdrill/packetdrill ./fastopen/client-TCP_FASTOPEN_CONNECT.pkt
>> 13:53:44.586369 tun0  Out IP6 fe80::2ecf:46fa:841:cae > ff02::2: ICMP6, router solicitation, length 8
>> 13:53:44.860976 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [S], seq 2694673015, win 65535, options [mss 1460,sackOK,TS val 2036993134 ecr 0,nop,wscale 8,tfo  cookiereq,nop,nop,mptcp capable v1], length 0
>> 13:53:44.871047 tun0  In  IP 192.0.2.1.8080 > 192.168.49.174.60676: Flags [S.], seq 0, ack 2694673016, win 65535, options [mss 1460,sackOK,TS val 700 ecr 2036993134,nop,wscale 8,tfo  cookie abcd1234,nop,nop,mptcp capable v1 {0x200000000000000}], length 0
>> 13:53:44.871063 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993144 ecr 700,mptcp capable v1 {0x3827b4480bfa870f,0x200000000000000}], length 0
>> 13:53:44.881125 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993144 ecr 700,mptcp dss fin ack 3007449509 seq 5431916111306864352 subseq 0 len 1,nop,nop], length 0
>> 13:53:44.881210 tun0  In  IP 192.0.2.1.8080 > 192.168.49.174.60676: Flags [.], ack 1, win 450, options [nop,nop,TS val 700 ecr 2036993144,mptcp dss fin ack 2016065249 seq 3007449509 subseq 0 len 1,nop,nop], length 0
>> 13:53:44.881226 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993154 ecr 700,mptcp dss ack 3007449510], length 0
>> 13:53:44.881260 tun0  Out IP 192.168.49.174.60676 > 192.0.2.1.8080: Flags [R.], seq 1, ack 1, win 256, options [nop,nop,TS val 2036993154 ecr 700,mptcp fast-close key 0x200000000000000,mptcp unknown], length 0
>> 13:53:44.982454 ?     Out IP 192.168.49.174.60680 > 192.0.2.1.8080: Flags [S], seq 1531938899:1531939399, win 65535, options [mss 1460,sackOK,TS val 2036993255 ecr 0,nop,wscale 8,tfo  cookie abcd1234,nop,nop,mptcp capable v1], length 500: HTTP
>> 13:53:44.992484 ?     In  IP 192.0.2.1.8080 > 192.168.49.174.60680: Flags [S.], seq 0, ack 1531939400, win 65535, options [mss 1460,sackOK,TS val 700 ecr 2036993255,nop,wscale 8,mptcp capable v1 {0x200000000000000}], length 0
>> 13:53:44.992494 ?     Out IP 192.168.49.174.60680 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993265 ecr 700,mptcp capable v1 {0x75382dfb6dafc693,0x200000000000000}], length 0
>> 13:53:44.992539 ?     Out IP 192.168.49.174.60680 > 192.0.2.1.8080: Flags [.], ack 1, win 256, options [nop,nop,TS val 2036993265 ecr 700,mptcp dss fin ack 3007449509 seq 6967145468519815204 subseq 0 len 1,nop,nop], length 0
> We can see:
>
> - in the first SYN:
>    - "tfo cookiereq"
>    - "mptcp capable v1"
>    - "length 0"
>
> - in the second SYN:
>    - "tfo  cookie abcd1234"
>    - "mptcp capable v1"
>    - "length 500"
>
> Everything seems then OK.

Yes. Indeed. In this case my environment is wrongly configured. I 
appoligise for creating the disturbance about functionality of the patch 
( as mentioned in the beggining of the original message)

>>>> 3. Patch uses an original Idea of another autor from mailing list (Reuse
>>>> the TCP FASTOPEN option in MPTCP).
>>> A v3 mentioning you is going to be sent as discussed at the last meeting.
>>>
>>> We should indeed mention the authors of the original idea to have TFO in
>>> MPTCP:
>>>
>>> https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/
>>>
>>> Cheers,
>>> Matt
>> It seems this draft didn't get the consensus and was not accepted by
>> IETF as RFC.
>
> This link is just to point to the original idea of having TFO supported
> with MPTCP. This document was used as a base for the evolution of the
> MPTCPv0 (RFC6824) and it was the main reason why we have an MPTCPv1
> (RFC8684) where the MP_CAPABLE is different from the v0 to allow TFO
> cookie options in the initial SYN. So yes, it was somehow accepted by
> the IETF but as part of RFC8684.
>
>> How the abstract RFC related to the original idea to reuse
>> TFO option from regular TCP in linux kernel implementation?
> Sorry, what do you mean here?
> Implementing TFO in MPTCP Linux Upstream is part of the roadmap from the
> beginning, see ticket 59 on Github.

I think there could be some confision:

1. One thing to implement TFO in MPTCP (draft)

2. Second thing is roadmap to implement in in upstream Linux.

What I'm talking about is: "reuse regular TFO option from regular TCP in 
MPTCP".


Best,

Dmytro.


> Cheers,
> Matt

[PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Benjamin Hesmans]

The series only consider the sender side.

Compared to the previous RFC patches, these ones focus on
the sender side only. It corresponds to the 4 first patches from the RFC
series.

The sending part is less complex and even if it looks like we are
converging for the receive part, there are still discussions on-going
there.

Again, thank you Dmytro for the previous work done. As already discussed
on the ML and meeting, this approach was slightly different from what
Dmytro originally proposed. Here tcp_sendmsg_fastopen() is exported and
re-used and TCP_FASTOPEN_CONNECT is supported.

MSG_FASTOPEN will be handled by Dmytro's patches.

Individual changelogs have been added per patch.

v2:
- Drop support for MSG_FASTOPEN because we were not sure that it was the
  correct way to do it.
- latest patch of the series: apply comment from Paolo concerning
  mptcp_poll()

Benjamin Hesmans (4):
  mptcp: add TCP_FASTOPEN_CONNECT socket option
  tcp: export tcp_sendmsg_fastopen
  mptcp: handle defer connect in mptcp_sendmsg
  mptcp: poll allow write call before actual connect

 include/net/tcp.h    |  2 ++
 net/ipv4/tcp.c       |  5 ++---
 net/mptcp/protocol.c | 26 ++++++++++++++++++++++++++
 net/mptcp/sockopt.c  | 19 ++++++++++++++++++-
 4 files changed, 48 insertions(+), 4 deletions(-)

-- 
2.25.1


-- 


Disclaimer: https://www.tessares.net/mail-disclaimer/ 
<https://www.tessares.net/mail-disclaimer/>

Re: [PATCH mptcp-next v2 0/4] mptcp: add support for TFO, sender side only

[Matthieu Baerts]

Hello,

On 22/09/2022 15:56, Benjamin Hesmans wrote:
> The series only consider the sender side.
> 
> The sending part is less complex and even if it looks like we are
> converging for the receive part, there are still discussions on-going
> there.

FYI, packetdrill tests validating this series are available on GitHub:

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

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