mptcp.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp
@ 2022-09-08 13:38 Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 01/10] mptcp: add TCP_FASTOPEN_CONNECT socket option Benjamin Hesmans
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

Add support for TCP_FASTOPEN_CONNECT for MPTCP

This series only consider data in the SYN and not the TFO cookies.
Dmytro SHYTYI has pending patch for the TFO cookies and the TCP_FASTOPEN
socket option.

Benjamin Hesmans (10):
  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
  mptcp: delay mptcp data_ack calculation
  mptcp: allow delayed data_ack calculation
  mptcp: TFO receive: bypass mapping check
  mptcp: allow data that can not be DATA_ACKed
  mptcp: force data_ready
  mptcp: test TFO

 include/net/tcp.h                        |  5 ++-
 net/ipv4/tcp.c                           |  5 +--
 net/ipv4/tcp_fastopen.c                  |  4 ++
 net/ipv4/tcp_minisocks.c                 | 29 ++++++++++++-
 net/mptcp/options.c                      |  1 +
 net/mptcp/protocol.c                     | 32 +++++++++++++-
 net/mptcp/protocol.h                     |  3 +-
 net/mptcp/sockopt.c                      | 18 +++++++-
 net/mptcp/subflow.c                      | 15 ++++++-
 tools/testing/selftests/net/mptcp/tfo.sh | 55 ++++++++++++++++++++++++
 10 files changed, 158 insertions(+), 9 deletions(-)
 create mode 100755 tools/testing/selftests/net/mptcp/tfo.sh

--
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 01/10] mptcp: add TCP_FASTOPEN_CONNECT socket option
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 02/10] tcp: export tcp_sendmsg_fastopen Benjamin Hesmans
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

Set the option for the first subflow only. For the other subflows TFO
can't be used because a mapping would be needed to cover the data in the
SYN.

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---

Notes:
    This patch only consider TCP_FASTOPEN_CONNECT, for TCP_FASTOPEN, Dmytro SHYTYI
    has proposed a different set of patches.

 net/mptcp/sockopt.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 423d3826ca1e..03bd57aef987 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_CONNECT:
 			return true;
 		}
 
@@ -567,7 +568,7 @@ static bool mptcp_supported_sockopt(int level, int optname)
 		/* TCP_REPAIR, TCP_REPAIR_QUEUE, TCP_QUEUE_SEQ, TCP_REPAIR_OPTIONS,
 		 * TCP_REPAIR_WINDOW are not supported, better avoid this mess
 		 */
-		/* TCP_FASTOPEN_KEY, TCP_FASTOPEN TCP_FASTOPEN_CONNECT, TCP_FASTOPEN_NO_COOKIE,
+		/* TCP_FASTOPEN_KEY, TCP_FASTOPEN, TCP_FASTOPEN_NO_COOKIE,
 		 * are not supported fastopen is currently unsupported
 		 */
 	}
@@ -768,6 +769,19 @@ 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)
+{
+	struct socket *sock;
+
+	/* Limit to first subflow, maybe msk->first could be used here? */
+	sock = __mptcp_nmpc_socket(msk);
+	if (!sock)
+		return -EINVAL;
+
+	return tcp_setsockopt(sock->sk, SOL_TCP, TCP_FASTOPEN_CONNECT, optval, optlen);
+}
+
 static int mptcp_setsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 				    sockptr_t optval, unsigned int optlen)
 {
@@ -796,6 +810,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_CONNECT:
+		return mptcp_setsockopt_sol_tcp_fastopen_connect(msk, optval, optlen);
 	}
 
 	return -EOPNOTSUPP;
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 02/10] tcp: export tcp_sendmsg_fastopen
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 01/10] mptcp: add TCP_FASTOPEN_CONNECT socket option Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 03/10] mptcp: handle defer connect in mptcp_sendmsg Benjamin Hesmans
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

It will be used to support TCP FastOpen with MPTCP in a following commit.

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 include/net/tcp.h | 2 ++
 net/ipv4/tcp.c    | 5 ++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index d10962b9f0d0..9a073b58dc4b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -327,6 +327,8 @@ void tcp_remove_empty_skb(struct sock *sk);
 int tcp_v4_tw_remember_stamp(struct inet_timewait_sock *tw);
 int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
 int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size);
+int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg, int *copied,
+			 size_t size, struct ubuf_info *uarg);
 int tcp_sendpage(struct sock *sk, struct page *page, int offset, size_t size,
 		 int flags);
 int tcp_sendpage_locked(struct sock *sk, struct page *page, int offset,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 44bc885bb2db..999db6b28008 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1162,9 +1162,8 @@ 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);
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 03/10] mptcp: handle defer connect in mptcp_sendmsg
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 01/10] mptcp: add TCP_FASTOPEN_CONNECT socket option Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 02/10] tcp: export tcp_sendmsg_fastopen Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 04/10] mptcp: poll allow write call before actual connect Benjamin Hesmans
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

When TCP_FASTOPEN_CONNECT has been set on the socket before a connect,
the defer flag is set and must be handled when sendmsg is called.

This is similar to what is done in tcp_sendmsg_locked()

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---

Notes:
    The error case is probably not correctly handled. To be checked.

 net/mptcp/protocol.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 55442df8fb97..ad81824bdbfe 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1668,6 +1668,7 @@ 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;
@@ -1681,6 +1682,24 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 	lock_sock(sk);
 
+	ssock = __mptcp_nmpc_socket(msk);
+	if (ssock && inet_sk(ssock->sk)->defer_connect) {
+		int copied_syn = 0;
+
+		lock_sock(ssock->sk);
+
+		ret = tcp_sendmsg_fastopen(ssock->sk, msg, &copied_syn, len, NULL);
+		copied += copied_syn;
+		if (ret == -EINPROGRESS && copied_syn > 0) {
+			release_sock(ssock->sk);
+			goto out;
+		} else if (ret) {
+			/* The error case is probably not correctly handled. To be checked. */
+			release_sock(ssock->sk);
+			goto out;
+		}
+	}
+
 	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
 
 	if ((1 << sk->sk_state) & ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) {
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 04/10] mptcp: poll allow write call before actual connect
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
                   ` (2 preceding siblings ...)
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 03/10] mptcp: handle defer connect in mptcp_sendmsg Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 05/10] mptcp: delay mptcp data_ack calculation Benjamin Hesmans
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

If fastopen is used, poll must allow a first write that will trigger
the SYN+data

Similar to what is done in tcp_poll().

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 net/mptcp/protocol.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index ad81824bdbfe..c249ce632c5f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3676,10 +3676,12 @@ static __poll_t mptcp_poll(struct file *file, struct socket *sock,
 {
 	struct sock *sk = sock->sk;
 	struct mptcp_sock *msk;
+	struct socket *ssock;
 	__poll_t mask = 0;
 	int state;
 
 	msk = mptcp_sk(sk);
+	ssock = __mptcp_nmpc_socket(msk);
 	sock_poll_wait(file, sock, wait);
 
 	state = inet_sk_state_load(sk);
@@ -3694,6 +3696,9 @@ static __poll_t mptcp_poll(struct file *file, struct socket *sock,
 	if (state != TCP_SYN_SENT && state != TCP_SYN_RECV) {
 		mask |= mptcp_check_readable(msk);
 		mask |= mptcp_check_writeable(msk);
+	} else if (ssock && state == TCP_SYN_SENT && inet_sk(ssock->sk)->defer_connect) {
+		/* cf tcp_poll() note about TFO */
+		mask |= EPOLLOUT | EPOLLWRNORM;
 	}
 	if (sk->sk_shutdown == SHUTDOWN_MASK || state == TCP_CLOSE)
 		mask |= EPOLLHUP;
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 05/10] mptcp: delay mptcp data_ack calculation
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
                   ` (3 preceding siblings ...)
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 04/10] mptcp: poll allow write call before actual connect Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 06/10] mptcp: allow delayed " Benjamin Hesmans
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

On the listener side, with TFO (and mptcp v1), the socket is created
earlier when the stack does not have the remote key yet. If the full
socket is created when SYN+data is received, avoid this calculation that
would be incorrect.

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 net/mptcp/options.c  | 1 +
 net/mptcp/protocol.c | 2 +-
 net/mptcp/protocol.h | 3 ++-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 30d289044e71..79f83236aa6f 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -91,6 +91,7 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 			ptr += 8;
 		}
 		if (opsize >= TCPOLEN_MPTCP_MPC_ACK) {
+			mp_opt->mpc_ack = 1;
 			mp_opt->rcvr_key = get_unaligned_be64(ptr);
 			ptr += 8;
 		}
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c249ce632c5f..f5a0dd589ad5 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2975,7 +2975,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
 	msk->setsockopt_seq = mptcp_sk(sk)->setsockopt_seq;
 	mptcp_init_sched(msk, mptcp_sk(sk)->sched);
 
-	if (mp_opt->suboptions & OPTIONS_MPTCP_MPC) {
+	if (mp_opt->suboptions & OPTIONS_MPTCP_MPC && !mp_opt->mpc_ack) {
 		msk->can_ack = true;
 		msk->remote_key = mp_opt->sndr_key;
 		mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 1bc9f6e77ddd..afb2f0ce9311 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -155,7 +155,8 @@ struct mptcp_options_received {
 		echo:1,
 		backup:1,
 		deny_join_id0:1,
-		__unused:2;
+		mpc_ack:1,
+		__unused:1;
 	u8	join_id;
 	u64	thmac;
 	u8	hmac[MPTCPOPT_HMAC_LEN];
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 06/10] mptcp: allow delayed data_ack calculation
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
                   ` (4 preceding siblings ...)
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 05/10] mptcp: delay mptcp data_ack calculation Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 07/10] mptcp: TFO receive: bypass mapping check Benjamin Hesmans
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

/!\ Starting from this patch, this is early prototype.

On the listener side, this function is usually called upon third-ack
when the full socket is created, with TFO, the socket is already
created, but at this point with have the material to calculate the
data_ack

It may not be the right place to do that. Any guidance for this patch
would be appreciated =D

For the include, this is just to make it work, but i didn't want to go
too far, if it's not the correct way to things anyway.

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 net/ipv4/tcp_minisocks.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index cb95d88497ae..b31b6a22d278 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -23,6 +23,10 @@
 #include <net/xfrm.h>
 #include <net/busy_poll.h>
 
+/* temp: new MPTCP specific code should be defined in net/mptcp/fastopen.c
+ * and only one function should be called from there */
+#include "../../net/mptcp/protocol.h"
+
 static bool tcp_in_window(u32 seq, u32 end_seq, u32 s_win, u32 e_win)
 {
 	if (seq == s_win)
@@ -571,6 +575,13 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
 	bool paws_reject = false;
 	bool own_req;
 
+	struct mptcp_options_received mp_opt;
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
+	struct mptcp_sock *msk;
+	u64 ack_seq;
+
+	mp_opt.suboptions = 0;
+
 	tmp_opt.saw_tstamp = 0;
 	if (th->doff > (sizeof(struct tcphdr)>>2)) {
 		tcp_parse_options(sock_net(sk), skb, &tmp_opt, 0, NULL);
@@ -744,8 +755,24 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
 	/* For Fast Open no more processing is needed (sk is the
 	 * child socket).
 	 */
-	if (fastopen)
+	if (fastopen) {
+		if (!subflow)
+			return sk;
+
+		msk = mptcp_sk(subflow->conn);
+		pr_debug("%s (%i): fastopen case, update the isn master based on the key\n", __func__, __LINE__);
+		mptcp_get_options(skb, &mp_opt);
+		if (mp_opt.suboptions & OPTIONS_MPTCP_MPC && mp_opt.mpc_ack) {
+			msk->can_ack = true;
+			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("%s (%i) [%llu]\n", __func__, __LINE__, msk->ack_seq);
+			atomic64_set(&msk->rcv_wnd_sent, ack_seq);
+		}
 		return sk;
+	}
 
 	/* While TCP_DEFER_ACCEPT is active, drop bare ACK. */
 	if (req->num_timeout < inet_csk(sk)->icsk_accept_queue.rskq_defer_accept &&
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 07/10] mptcp: TFO receive: bypass mapping check
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
                   ` (5 preceding siblings ...)
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 06/10] mptcp: allow delayed " Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 08/10] mptcp: allow data that can not be DATA_ACKed Benjamin Hesmans
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

With TFO, there is no mapping, the stack can bypass any check related to
the mapping.

=> I'm not fully convinced this is the correct way to do this. I made
this into a small isolated patch to ease the reviews.
Maybe we could MPTCP SKB extension for this? (I didn't check yet)

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 include/net/tcp.h       | 3 ++-
 net/ipv4/tcp_fastopen.c | 2 ++
 net/mptcp/protocol.c    | 6 ++++++
 net/mptcp/subflow.c     | 5 +++++
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 9a073b58dc4b..0c7996e7ad3f 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -881,7 +881,8 @@ struct tcp_skb_cb {
 	__u8		txstamp_ack:1,	/* Record TX timestamp for ack? */
 			eor:1,		/* Is skb MSG_EOR marked? */
 			has_rxtstamp:1,	/* SKB has a RX timestamp	*/
-			unused:5;
+			is_tfo:1,
+			unused:4;
 	__u32		ack_seq;	/* Sequence number ACK'd	*/
 	union {
 		struct {
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 45cc7f1ca296..1b2d045604e3 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -193,6 +193,8 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
 	TCP_SKB_CB(skb)->seq++;
 	TCP_SKB_CB(skb)->tcp_flags &= ~TCPHDR_SYN;
 
+	TCP_SKB_CB(skb)->is_tfo = 1;
+
 	tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
 	__skb_queue_tail(&sk->sk_receive_queue, skb);
 	tp->syn_data_acked = 1;
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f5a0dd589ad5..765c50bd6bb3 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -362,6 +362,12 @@ static bool __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
 
 	has_rxtstamp = TCP_SKB_CB(skb)->has_rxtstamp;
 
+	if (TCP_SKB_CB(skb)->is_tfo) {
+		mptcp_set_owner_r(skb, sk);
+		__skb_queue_tail(&sk->sk_receive_queue, skb);
+		return true;
+	}
+
 	/* the skb map_seq accounts for the skb offset:
 	 * mptcp_subflow_get_mapped_dsn() is based on the current tp->copied_seq
 	 * value
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index c7d49fb6e7bd..c2d1ba21c8c2 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -984,6 +984,11 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 	if (mptcp_check_fallback(ssk))
 		return MAPPING_DUMMY;
 
+	if (TCP_SKB_CB(skb)->is_tfo) {
+		/* subflow->map_valid = 1; */
+		return MAPPING_OK; /* or DUMMY ? */
+	}
+
 	mpext = mptcp_get_ext(skb);
 	if (!mpext || !mpext->use_map) {
 		if (!subflow->map_valid && !skb->len) {
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 08/10] mptcp: allow data that can not be DATA_ACKed
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
                   ` (6 preceding siblings ...)
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 07/10] mptcp: TFO receive: bypass mapping check Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 09/10] mptcp: force data_ready Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 10/10] mptcp: test TFO Benjamin Hesmans
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

With TFO, this first bytes are out of the regular MTPCP seq numbers,
hence they can't be DATA_ACKed. The data ack validation can then be
skipped in this case.

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 net/mptcp/subflow.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index c2d1ba21c8c2..ca5b9853eca3 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1221,6 +1221,14 @@ static bool subflow_check_data_avail(struct sock *ssk)
 		if (WARN_ON_ONCE(!skb))
 			goto no_data;
 
+		/* with TFO, we can have data, but we don't not yet the data ack
+		 * because its not part of the MTPCP seq number.
+		 */
+		if (unlikely(!READ_ONCE(msk->can_ack)) && TCP_SKB_CB(skb)->is_tfo) {
+			pr_err("%s (%i): can't ack yet, but already got data\n", __func__, __LINE__);
+			goto valid_data;
+		}
+
 		/* if msk lacks the remote key, this subflow must provide an
 		 * MP_CAPABLE-based mapping
 		 */
@@ -1240,7 +1248,7 @@ static bool subflow_check_data_avail(struct sock *ssk)
 			mptcp_subflow_discard_data(ssk, skb, old_ack - ack_seq);
 			continue;
 		}
-
+valid_data:
 		WRITE_ONCE(subflow->data_avail, MPTCP_SUBFLOW_DATA_AVAIL);
 		break;
 	}
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 09/10] mptcp: force data_ready
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
                   ` (7 preceding siblings ...)
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 08/10] mptcp: allow data that can not be DATA_ACKed Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 10/10] mptcp: test TFO Benjamin Hesmans
  9 siblings, 0 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

I'm really not sure about why this is needed, but put that in the
review to be complete.

Without this, the user application can hang forever

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 net/ipv4/tcp_fastopen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 1b2d045604e3..9ef357ae0921 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -197,6 +197,8 @@ void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb)
 
 	tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
 	__skb_queue_tail(&sk->sk_receive_queue, skb);
+	pr_debug("%s (%i): is it needed ?\n", __func__, __LINE__);
+	tcp_data_ready(sk);
 	tp->syn_data_acked = 1;
 
 	/* u64_stats_update_begin(&tp->syncp) not needed here,
-- 
2.25.1


-- 


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



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

* [PATCH RFC mptcp-next 10/10] mptcp: test TFO
  2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
                   ` (8 preceding siblings ...)
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 09/10] mptcp: force data_ready Benjamin Hesmans
@ 2022-09-08 13:38 ` Benjamin Hesmans
  2022-09-08 14:10   ` mptcp: test TFO: Build Failure MPTCP CI
  2022-09-08 15:40   ` mptcp: test TFO: Tests Results MPTCP CI
  9 siblings, 2 replies; 13+ messages in thread
From: Benjamin Hesmans @ 2022-09-08 13:38 UTC (permalink / raw)
  To: mptcp; +Cc: Benjamin Hesmans

Work in progress for a future self test. Just for documentation, not
intended to be included as-is.

Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---
 tools/testing/selftests/net/mptcp/tfo.sh | 55 ++++++++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100755 tools/testing/selftests/net/mptcp/tfo.sh

diff --git a/tools/testing/selftests/net/mptcp/tfo.sh b/tools/testing/selftests/net/mptcp/tfo.sh
new file mode 100755
index 000000000000..43440f80f92f
--- /dev/null
+++ b/tools/testing/selftests/net/mptcp/tfo.sh
@@ -0,0 +1,55 @@
+#!/bin/bash
+
+set -x
+
+DO_CLEANUP="${DO_CLEANUP:-1}"
+DO_MPTCPIZE="${DO_MPTCPIZE:-1}"
+
+cleanup() {
+	if [ "${DO_CLEANUP}" == "0" ]; then
+		return
+	fi
+
+	pkill tcpdump
+	pkill python
+
+	ip netns delete client
+	ip netns delete server
+}
+
+netns() {
+	ns="$1"
+	shift
+	ip netns exec "$ns" "$@"
+}
+
+
+trap cleanup EXIT
+
+ip netns add client
+ip netns add server
+
+netns client sysctl net.ipv4.tcp_fastopen=0x5
+netns server sysctl net.ipv4.tcp_fastopen=0x602
+
+netns client ip link add eth0 type veth peer eth0 netns server
+netns client ip addr add 6.6.6.1/24 dev eth0
+netns server ip addr add 6.6.6.6/24 dev eth0
+netns client ip link set eth0 up
+netns server ip link set eth0 up
+
+netns client tcpdump -i eth0 -w ./client.pcap &
+sleep 1
+
+if [ "${DO_MPTCPIZE}" == "0" ]; then
+	LD_PRELOAD=
+else
+	LD_PRELOAD="$PWD/mptcpize/libmptcpwrap.so"
+fi
+
+LD_PRELOAD=${LD_PRELOAD} netns server python3 -m http.server 666 &
+sleep 3
+
+LD_PRELOAD=${LD_PRELOAD} netns client curl --tcp-fastopen -o out.txt http://6.6.6.6:666/tfo.sh
+sleep 3
+
-- 
2.25.1


-- 


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



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

* Re: mptcp: test TFO: Build Failure
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 10/10] mptcp: test TFO Benjamin Hesmans
@ 2022-09-08 14:10   ` MPTCP CI
  2022-09-08 15:40   ` mptcp: test TFO: Tests Results MPTCP CI
  1 sibling, 0 replies; 13+ messages in thread
From: MPTCP CI @ 2022-09-08 14:10 UTC (permalink / raw)
  To: Benjamin Hesmans; +Cc: mptcp

Hi Benjamin,

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/20220908133829.3410092-11-benjamin.hesmans@tessares.net/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/3015610232

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

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)

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

* Re: mptcp: test TFO: Tests Results
  2022-09-08 13:38 ` [PATCH RFC mptcp-next 10/10] mptcp: test TFO Benjamin Hesmans
  2022-09-08 14:10   ` mptcp: test TFO: Build Failure MPTCP CI
@ 2022-09-08 15:40   ` MPTCP CI
  1 sibling, 0 replies; 13+ messages in thread
From: MPTCP CI @ 2022-09-08 15:40 UTC (permalink / raw)
  To: Benjamin Hesmans; +Cc: mptcp

Hi Benjamin,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 13 failed test(s): mptcp_connect_mmap packetdrill_add_addr packetdrill_dss packetdrill_fastclose packetdrill_mp_capable packetdrill_mp_prio packetdrill_mp_reset packetdrill_sockopts selftest_diag selftest_mptcp_connect selftest_mptcp_join selftest_mptcp_sockopt selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/4705920293797888
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4705920293797888/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 4 failed test(s): selftest_diag selftest_mptcp_connect selftest_mptcp_join selftest_mptcp_sockopt - Critical: Global Timeout ❌:
  - Task: https://cirrus-ci.com/task/5831820200640512
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5831820200640512/summary/summary.txt

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


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)

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

end of thread, other threads:[~2022-09-08 15:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-08 13:38 [PATCH RFC mptcp-next 00/10] Implement TCP_FASTOPEN_CONNECT for mptcp Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 01/10] mptcp: add TCP_FASTOPEN_CONNECT socket option Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 02/10] tcp: export tcp_sendmsg_fastopen Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 03/10] mptcp: handle defer connect in mptcp_sendmsg Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 04/10] mptcp: poll allow write call before actual connect Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 05/10] mptcp: delay mptcp data_ack calculation Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 06/10] mptcp: allow delayed " Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 07/10] mptcp: TFO receive: bypass mapping check Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 08/10] mptcp: allow data that can not be DATA_ACKed Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 09/10] mptcp: force data_ready Benjamin Hesmans
2022-09-08 13:38 ` [PATCH RFC mptcp-next 10/10] mptcp: test TFO Benjamin Hesmans
2022-09-08 14:10   ` mptcp: test TFO: Build Failure MPTCP CI
2022-09-08 15:40   ` mptcp: test TFO: Tests Results MPTCP CI

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).