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

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

[Benjamin Hesmans]

The series only considers 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.

We would like to credit Sébastien Barré, Gregory Detal, Olivier
Bonaventure and Christoph Paasch for the original idea of supporting TFO
in MPTCP, see https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/

It would be very nice to have these patches accepted in the future
kernel 6.1 which will be the next LTS picked by many vendors: the
modifications are quite small, well isolated and re-using what is done
in TCP for years.

v3:
- Add Dmytro SoB as kindly asked at the last meeting, the code is still
  the same.

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 (3):
  mptcp: add TCP_FASTOPEN_CONNECT socket option
  tcp: export tcp_sendmsg_fastopen
  mptcp: poll allow write call before actual connect

Dmytro Shytyi (1):
  mptcp: handle defer connect in mptcp_sendmsg

 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/>

[PATCH mptcp-next v3 1/4] mptcp: add TCP_FASTOPEN_CONNECT socket option

[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:
    v1:
    - also support getsockopt() for TCP_FASTOPEN_CONNECT
    - remove comment about msk->first. __mptcp_nmpc_socket() should be good here

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

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 423d3826ca1e..c7cb68c725b2 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 */
+	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;
@@ -1157,6 +1173,7 @@ static int mptcp_getsockopt_sol_tcp(struct mptcp_sock *msk, int optname,
 	case TCP_INFO:
 	case TCP_CC_INFO:
 	case TCP_DEFER_ACCEPT:
+	case TCP_FASTOPEN_CONNECT:
 		return mptcp_getsockopt_first_sf_only(msk, SOL_TCP, optname,
 						      optval, optlen);
 	case TCP_INQ:
-- 
2.25.1


-- 


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

[PATCH mptcp-next v3 2/4] tcp: export tcp_sendmsg_fastopen

[Benjamin Hesmans]

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

Co-developed-by: Dmytro Shytyi <dmytro@shytyi.net>
Signed-of-by: Dmytro Shytyi <dmytro@shytyi.net>
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 27e8d378c70a..4f71cc15ff8e 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 5702ca9b952d..5237a3f08c94 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/>

[PATCH mptcp-next v3 3/4] mptcp: handle defer connect in mptcp_sendmsg

[Benjamin Hesmans]

From: Dmytro Shytyi <dmytro@shytyi.net>

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-of-by: Dmytro Shytyi <dmytro@shytyi.net>
Co-developed-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
Signed-off-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
---

Notes:
    v1:
    - error case use latest patch from Paolo (propagate fastclose error)
    - I believe the use __mptcp_nmpc_socket(msk); is correct here (instead of
      msk->first but would be nice is someone can confirm
    - add unlikely for the TFO check
    - propagate the ssk state to the msk (Paolo)
    
    v2:
    - Question from v1's notes answered by Paolo.

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

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7d4e197ec567..f5f20910cd83 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,27 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 	lock_sock(sk);
 
+	ssock = __mptcp_nmpc_socket(msk);
+	if (unlikely(ssock && inet_sk(ssock->sk)->defer_connect)) {
+		struct sock *ssk = ssock->sk;
+		int copied_syn = 0;
+
+		lock_sock(ssk);
+
+		ret = tcp_sendmsg_fastopen(ssk, msg, &copied_syn, len, NULL);
+		copied += copied_syn;
+		if (ret == -EINPROGRESS && copied_syn > 0) {
+			/* reflect the new state on the MPTCP socket */
+			inet_sk_state_store(sk, inet_sk_state_load(ssk));
+			release_sock(ssk);
+			goto out;
+		} else if (ret) {
+			release_sock(ssk);
+			goto do_error;
+		}
+		release_sock(ssk);
+	}
+
 	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/>

[PATCH mptcp-next v3 4/4] mptcp: poll allow write call before actual connect

[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>
---

Notes:
    v2:
    - Using __mptcp_nmpc_socket() without lock on msk socket was unsafe.  Instead
      copy the defer_connect from mptcp_stream_connect() and check this flags
      instead. (Paolo's review)

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

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f5f20910cd83..0eeb8115c9d0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3616,6 +3616,7 @@ static int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr,
 
 do_connect:
 	err = ssock->ops->connect(ssock, uaddr, addr_len, flags);
+	inet_sk(sock->sk)->defer_connect = inet_sk(ssock->sk)->defer_connect;
 	sock->state = ssock->state;
 
 	/* on successful connect, the msk state will be moved to established by
@@ -3754,6 +3755,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 (state == TCP_SYN_SENT && inet_sk(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/>

Re: mptcp: poll allow write call before actual connect: Tests Results

[MPTCP CI]

Hi Benjamin,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6212834240495616
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6212834240495616/summary/summary.txt

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

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


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 v3 0/4] mptcp: add support for TCP_FASTOPEN_CONNECT, sender side only

[Paolo Abeni]

On Fri, 2022-09-23 at 14:19 +0200, Benjamin Hesmans wrote:
> The series only considers 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.
> 
> We would like to credit Sébastien Barré, Gregory Detal, Olivier
> Bonaventure and Christoph Paasch for the original idea of supporting TFO
> in MPTCP, see https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/
> 
> It would be very nice to have these patches accepted in the future
> kernel 6.1 which will be the next LTS picked by many vendors: the
> modifications are quite small, well isolated and re-using what is done
> in TCP for years.
> 
> v3:
> - Add Dmytro SoB as kindly asked at the last meeting, the code is still
>   the same.
> 
> 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 (3):
>   mptcp: add TCP_FASTOPEN_CONNECT socket option
>   tcp: export tcp_sendmsg_fastopen
>   mptcp: poll allow write call before actual connect
> 
> Dmytro Shytyi (1):
>   mptcp: handle defer connect in mptcp_sendmsg
> 
>  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(-)

As noted in last public mtg code-wise LGTM, thanks!

Acked-by: Paolo Abeni <pabeni@redhat.com>

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

[Matthieu Baerts]

Hi Benjamin, Paolo,

On 23/09/2022 14:19, Benjamin Hesmans wrote:
> The series only considers 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.
> 
> We would like to credit Sébastien Barré, Gregory Detal, Olivier
> Bonaventure and Christoph Paasch for the original idea of supporting TFO
> in MPTCP, see https://datatracker.ietf.org/doc/draft-barre-mptcp-tfo/
> 
> It would be very nice to have these patches accepted in the future
> kernel 6.1 which will be the next LTS picked by many vendors: the
> modifications are quite small, well isolated and re-using what is done
> in TCP for years.

Thank you for the v3 and the review!

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Now in our tree (feat. for net-next) without a small typo
(s/Signed-of-by/Signed-off-by/) before Paolo's patches to propagate the
FastClose errors (an extra patch has been added to modify Paolo's series
to take into account the new code from TFO):

New patches for t/upstream:
- 663904b20bf3: mptcp: add TCP_FASTOPEN_CONNECT socket option
- 9b4ba0258cd4: tcp: export tcp_sendmsg_fastopen
- 7ee00b1a8316: mptcp: handle defer connect in mptcp_sendmsg
- 91b572e341d3: mptcp: poll allow write call before actual connect

- e203d313c6a9: Squash to "mptcp: propagate fastclose error": use
'do_error' defined in this patch from Paolo

- Results: 617e33bdd07f..7830189b084e (export)


Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220923T174545

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