[RFC PATCH mptcp-next v11 0/4] mptcp: Fast Open: Initiator

[RFC PATCH mptcp-next v11 0/4] mptcp: Fast Open: Initiator

[Dmytro Shytyi]

These patches focus on the Initiator side.
These patches implement: sendto(..., ..., ..., MSG_FASTOPEN, ..., ...);
We would like to credit Paulo Abeni, Mat Martineau, Matthieu Baerts and
Benjamin Hesmans for advices and ideas that improved these patches.
Without this collaboration this state of work would not be presented.

Origins of these patches were in the root of discovery of the
possibility to _reuse the TCP FASTOPEN option in the linux
upstream MPTCP_ (First commit was sent to the mailing list on 22 OCT 
2021: see https://lore.kernel.org/mptcp/
17ca66cd439.10a0a3ce11621928.1543611905599720914@shytyi.net/').

Dmytro Shytyi (4):
  mptcp: add mptcp_setsockopt_fastopen
  mptcp: add mptcp_stream_connect to *.h
  mptcp: add mptcp_subflow_conn_sock()
  mptcp: reuse tcp_sendmsg_fastopen()

 include/net/mptcp.h  |  9 +++++++++
 include/net/tcp.h    |  3 +++
 net/ipv4/tcp.c       | 21 ++++++++++++++++-----
 net/mptcp/Makefile   |  2 +-
 net/mptcp/fastopen.c | 37 +++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.c | 35 +++++++++++++++++++++++++++++++----
 net/mptcp/protocol.h |  5 +++++
 net/mptcp/sockopt.c  |  3 +++
 8 files changed, 105 insertions(+), 10 deletions(-)
 create mode 100644 net/mptcp/fastopen.c

-- 
2.25.1

[RFC PATCH mptcp-next v11 1/4] 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 | 37 +++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.h |  5 +++++
 net/mptcp/sockopt.c  |  3 +++
 4 files changed, 46 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..086cbad49979
--- /dev/null
+++ b/net/mptcp/fastopen.c
@@ -0,0 +1,37 @@
+/* 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);
+	struct socket *ssock;
+	struct sock *ssk;
+	int val;
+	int ret;
+
+	ret = 0;
+
+	if (copy_from_sockptr(&val, optval, sizeof(val)))
+		return -EFAULT;
+
+	ssock = __mptcp_nmpc_socket(msk);
+	ssk = ssock->sk;
+	net = sock_net(ssk);
+	lock_sock(ssk);
+
+	if (val >= 0 && ((1 << ssk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
+		tcp_fastopen_init_key_once(net);
+		fastopen_queue_tune(ssk, val);
+	} else {
+		ret = -EINVAL;
+	}
+
+	release_sock(ssk);
+
+	return ret;
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 93c535440a5c..522647804aed 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -838,6 +838,11 @@ void mptcp_event_addr_announced(const struct sock *ssk, const struct mptcp_addr_
 void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
 bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 
+// Fast Open Mechanism functions begin
+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 v11 2/4] mptcp: add mptcp_stream_connect to *.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>
---
 include/net/mptcp.h  | 8 ++++++++
 net/mptcp/protocol.c | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index c25939b2af68..d9908b839059 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -150,6 +150,7 @@ void mptcp_write_options(struct tcphdr *th, __be32 *ptr, struct tcp_sock *tp,
 			 struct mptcp_out_options *opts);
 
 void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info);
+int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_len, int flags);
 
 /* move the skb extension owership, with the assumption that 'to' is
  * newly allocated
@@ -286,6 +287,13 @@ static inline bool mptcp_skb_can_collapse(const struct sk_buff *to,
 
 static inline void mptcp_space(const struct sock *ssk, int *s, int *fs) { }
 static inline void mptcp_seq_show(struct seq_file *seq) { }
+static inline int mptcp_stream_connect(struct socket *sock,
+				       struct sockaddr *uaddr,
+				       int addr_len,
+				       int flags)
+{
+
+}
 
 static inline int mptcp_subflow_init_cookie_req(struct request_sock *req,
 						const struct sock *sk_listener,
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7d4e197ec567..c39ed726c1c0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3543,8 +3543,8 @@ static void mptcp_subflow_early_fallback(struct mptcp_sock *msk,
 	__mptcp_do_fallback(msk);
 }
 
-static int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr,
-				int addr_len, int flags)
+int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr,
+			 int addr_len, int flags)
 {
 	struct mptcp_sock *msk = mptcp_sk(sock->sk);
 	struct mptcp_subflow_context *subflow;
-- 
2.25.1

[RFC PATCH mptcp-next v11 3/4] mptcp: add mptcp_subflow_conn_sock()

[Dmytro Shytyi]

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

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

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index d9908b839059..ccf2b42837a1 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -151,6 +151,7 @@ void mptcp_write_options(struct tcphdr *th, __be32 *ptr, struct tcp_sock *tp,
 
 void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info);
 int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_len, int flags);
+struct sock *mptcp_subflow_conn_sock(struct sock *sk);
 
 /* move the skb extension owership, with the assumption that 'to' is
  * newly allocated
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5702ca9b952d..a22c8044de17 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1197,8 +1197,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 sock *parent = mptcp_subflow_conn_sock(sk);
+
+		release_sock(sk);
+		release_sock(parent);
+		err = mptcp_stream_connect(sk->sk_socket, uaddr,
+					   msg->msg_namelen, msg->msg_flags);
+		lock_sock(parent);
+		lock_sock(sk);
+	}
 	/* fastopen_req could already be freed in __inet_stream_connect
 	 * if the connection times out or gets rst
 	 */
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c39ed726c1c0..fb08e1f458c0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -58,6 +58,12 @@ static void __mptcp_check_send_data_fin(struct sock *sk);
 DEFINE_PER_CPU(struct mptcp_delegated_action, mptcp_delegated_actions);
 static struct net_device mptcp_napi_dev;
 
+struct sock *
+mptcp_subflow_conn_sock(struct sock *sk)
+{
+	return ((mptcp_subflow_ctx(sk))->conn);
+}
+
 /* If msk has an initial subflow socket, and the MP_CAPABLE handshake has not
  * completed yet or has failed, return the subflow socket.
  * Otherwise return NULL.
-- 
2.25.1

[RFC PATCH mptcp-next v11 4/4] 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-of-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
---
 include/net/tcp.h    |  3 +++
 net/ipv4/tcp.c       |  6 +++---
 net/mptcp/protocol.c | 25 +++++++++++++++++++++++--
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 27e8d378c70a..97cb014ddcab 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1755,6 +1755,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 a22c8044de17..daa611671d9a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1162,9 +1162,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);
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index fb08e1f458c0..dd2414fdcf04 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1679,9 +1679,30 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	long timeo;
 
 	/* we don't support FASTOPEN yet */
-	if (msg->msg_flags & MSG_FASTOPEN)
-		return -EOPNOTSUPP;
+	if (msg->msg_flags & MSG_FASTOPEN) {
+		struct socket *ssock;
+		struct sock *ssk;
 
+		lock_sock(sk);
+
+		ssock = __mptcp_nmpc_socket(msk);
+		ssk = ssock->sk;
+
+		lock_sock(ssk);
+
+		if (ssock) {
+			int copied_syn_fastopen = 0;
+
+			ret = tcp_sendmsg_fastopen(ssk, msg, &copied_syn_fastopen, len, NULL);
+			copied += copied_syn_fastopen;
+			if (ret) {
+				release_sock(ssk);
+				goto do_error;
+			}
+		}
+		release_sock(ssock->sk);
+		release_sock(sk);
+	}
 	/* silently ignore everything else */
 	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;
 
-- 
2.25.1

Re: [RFC PATCH mptcp-next v11 4/4] mptcp: reuse tcp_sendmsg_fastopen()

[Paolo Abeni]

Hello,

On Mon, 2022-09-26 at 01:26 +0200, Dmytro Shytyi wrote:
> In the following patches we will reuse modified tcp_sendmsg_fastopen().
> We call it from mptcp_sendmsg().
> 
> Signed-of-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
> ---
>  include/net/tcp.h    |  3 +++
>  net/ipv4/tcp.c       |  6 +++---
>  net/mptcp/protocol.c | 25 +++++++++++++++++++++++--
>  3 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 27e8d378c70a..97cb014ddcab 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1755,6 +1755,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 a22c8044de17..daa611671d9a 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1162,9 +1162,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);
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index fb08e1f458c0..dd2414fdcf04 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1679,9 +1679,30 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>  	long timeo;
>  
>  	/* we don't support FASTOPEN yet */
> -	if (msg->msg_flags & MSG_FASTOPEN)
> -		return -EOPNOTSUPP;
> +	if (msg->msg_flags & MSG_FASTOPEN) {
> +		struct socket *ssock;
> +		struct sock *ssk;
>  
> +		lock_sock(sk);
> +
> +		ssock = __mptcp_nmpc_socket(msk);
> +		ssk = ssock->sk;
> +
> +		lock_sock(ssk);
> +
> +		if (ssock) {
> +			int copied_syn_fastopen = 0;
> +
> +			ret = tcp_sendmsg_fastopen(ssk, msg, &copied_syn_fastopen, len, NULL);
> +			copied += copied_syn_fastopen;
> +			if (ret) {
> +				release_sock(ssk);
> +				goto do_error;
> +			}
> +		}
> +		release_sock(ssock->sk);
> +		release_sock(sk);
> +	}
>  	/* silently ignore everything else */
>  	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;

the above/previous patches duplicate quit a bit of code. Instead the
following should be enough (replacing patches 2, 3, 4 here):
---
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c99eb4ce948e..c50e8f85bb11 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1673,17 +1673,14 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	int ret = 0;
 	long timeo;
 
-	/* we don't support FASTOPEN yet */
-	if (msg->msg_flags & MSG_FASTOPEN)
-		return -EOPNOTSUPP;
-
 	/* silently ignore everything else */
-	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;
+	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | MSG_FASTOPEN;
 
 	lock_sock(sk);
 
 	ssock = __mptcp_nmpc_socket(msk);
-	if (unlikely(ssock && inet_sk(ssock->sk)->defer_connect)) {
+	if (unlikely(ssock && inet_sk(ssock->sk)->defer_connect ||
+		     msg->msg_flags & MSG_FASTOPEN)) {
 		struct sock *ssk = ssock->sk;
 		int copied_syn = 0;
 

Re: [RFC PATCH mptcp-next v11 1/4] mptcp: add mptcp_setsockopt_fastopen

[Paolo Abeni]

On Mon, 2022-09-26 at 01:25 +0200, Dmytro Shytyi wrote:
> 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 | 37 +++++++++++++++++++++++++++++++++++++
>  net/mptcp/protocol.h |  5 +++++
>  net/mptcp/sockopt.c  |  3 +++
>  4 files changed, 46 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..086cbad49979
> --- /dev/null
> +++ b/net/mptcp/fastopen.c
> @@ -0,0 +1,37 @@
> +/* 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);
> +	struct socket *ssock;
> +	struct sock *ssk;
> +	int val;
> +	int ret;
> +
> +	ret = 0;
> +
> +	if (copy_from_sockptr(&val, optval, sizeof(val)))
> +		return -EFAULT;
> +
> +	ssock = __mptcp_nmpc_socket(msk);
> +	ssk = ssock->sk;
> +	net = sock_net(ssk);
> +	lock_sock(ssk);
> +
> +	if (val >= 0 && ((1 << ssk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
> +		tcp_fastopen_init_key_once(net);
> +		fastopen_queue_tune(ssk, val);

Instead you could directly call:

	return tcp_setsockopt(ssock->sk, SOL_TCP, TCP_FASTOPEN, optval, optlen);

Side note, we could add a generic helper to invoke tcp_setsockopt() on
the mpc subflow, on the give tcp socket option, and reuse such helper
for: TCP_FASTOPEN, TCP_FASTOPEN_CONNECT, TCP_DEFER_ACCEPT... but that
can/should be done later.

Cheers,

Paolo

Re: [RFC PATCH mptcp-next v11 0/4] mptcp: Fast Open: Initiator

[Paolo Abeni]

On Mon, 2022-09-26 at 01:25 +0200, Dmytro Shytyi wrote:
> These patches focus on the Initiator side.
> These patches implement: sendto(..., ..., ..., MSG_FASTOPEN, ..., ...);
> We would like to credit Paulo Abeni, Mat Martineau, Matthieu Baerts and
> Benjamin Hesmans for advices and ideas that improved these patches.
> Without this collaboration this state of work would not be presented.
> 
> Origins of these patches were in the root of discovery of the
> possibility to _reuse the TCP FASTOPEN option in the linux
> upstream MPTCP_ (First commit was sent to the mailing list on 22 OCT 
> 2021: see https://lore.kernel.org/mptcp/
> 17ca66cd439.10a0a3ce11621928.1543611905599720914@shytyi.net/').
> 
> Dmytro Shytyi (4):
>   mptcp: add mptcp_setsockopt_fastopen
>   mptcp: add mptcp_stream_connect to *.h
>   mptcp: add mptcp_subflow_conn_sock()
>   mptcp: reuse tcp_sendmsg_fastopen()
> 
>  include/net/mptcp.h  |  9 +++++++++
>  include/net/tcp.h    |  3 +++
>  net/ipv4/tcp.c       | 21 ++++++++++++++++-----
>  net/mptcp/Makefile   |  2 +-
>  net/mptcp/fastopen.c | 37 +++++++++++++++++++++++++++++++++++++
>  net/mptcp/protocol.c | 35 +++++++++++++++++++++++++++++++----
>  net/mptcp/protocol.h |  5 +++++
>  net/mptcp/sockopt.c  |  3 +++
>  8 files changed, 105 insertions(+), 10 deletions(-)
>  create mode 100644 net/mptcp/fastopen.c
> 
Plase, on the next iteration include the rx-path-related patches (e.g.
patches 4, 5 and 6 from v9, updated with the relevant feedback).

Thanks,

Paolo

Re: [RFC PATCH mptcp-next v11 1/4] mptcp: add mptcp_setsockopt_fastopen

[Matthieu Baerts]

Hi Dmytro, Paolo,

On 26/09/2022 16:50, Paolo Abeni wrote:
> On Mon, 2022-09-26 at 01:25 +0200, Dmytro Shytyi wrote:
>> 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 | 37 +++++++++++++++++++++++++++++++++++++
>>  net/mptcp/protocol.h |  5 +++++
>>  net/mptcp/sockopt.c  |  3 +++
>>  4 files changed, 46 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..086cbad49979
>> --- /dev/null
>> +++ b/net/mptcp/fastopen.c
>> @@ -0,0 +1,37 @@
>> +/* 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)

I think we should keep everything related to the sockoptions in
net/mptcp/sockopt.c, no?

>> +{
>> +	struct sock *sk = (struct sock *)msk;
>> +	struct net *net = sock_net(sk);
>> +	struct socket *ssock;
>> +	struct sock *ssk;
>> +	int val;
>> +	int ret;
>> +
>> +	ret = 0;
>> +
>> +	if (copy_from_sockptr(&val, optval, sizeof(val)))
>> +		return -EFAULT;
>> +
>> +	ssock = __mptcp_nmpc_socket(msk);

'ssock' could be NULL if the setsockopt() is called once the connection
has been established.

>> +	ssk = ssock->sk;
>> +	net = sock_net(ssk);
>> +	lock_sock(ssk);
>> +
>> +	if (val >= 0 && ((1 << ssk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
>> +		tcp_fastopen_init_key_once(net);
>> +		fastopen_queue_tune(ssk, val);
> 
> Instead you could directly call:
> 
> 	return tcp_setsockopt(ssock->sk, SOL_TCP, TCP_FASTOPEN, optval, optlen);


I agree, similar to TCP_FASTOPEN_CONNECT

> Side note, we could add a generic helper to invoke tcp_setsockopt() on
> the mpc subflow, on the give tcp socket option, and reuse such helper

Indeed. I was already looking at doing that when implementing
TCP_FASTOPEN_NO_COOKIE. I can share a patch for this other sockopt and
then you can re-use the new helper for this TCP_FASTOPEN.

> for: TCP_FASTOPEN, TCP_FASTOPEN_CONNECT, TCP_DEFER_ACCEPT... but that
> can/should be done later.

Note that for TCP_DEFER_ACCEPT, this is a special case because
apparently, the setsockopt doesn't fail even if a connection has already
been accepted.

@Dmytro: Two last things about the patch:
- you also need to support the 'read' side (getsockopt())
- if I'm not mistaken, this TCP_FASTOPEN option is only needed for the
listener side, not for the sender, no?

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

Re: [RFC PATCH mptcp-next v11 3/4] mptcp: add mptcp_subflow_conn_sock()

[Matthieu Baerts]

Hi Dmytro,

On 26/09/2022 01:26, Dmytro Shytyi wrote:
> In the following patches we will call mptcp_subflow_conn_sock() from 
> tcp_sendmsg_fastopen() in file "net/ipv4/tcp.c", thus make such symbol
> visible.

The patch description and title don't seem to be correct. You are doing
more than that here.

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 5702ca9b952d..a22c8044de17 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1197,8 +1197,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);

Do you think it could be possible not to modify tcp_sendmsg_fastopen()
but instead implement '.connect' callback in 'mptcp_prot'?


> +	} else {
> +		struct sock *parent = mptcp_subflow_conn_sock(sk);
> +
> +		release_sock(sk);
> +		release_sock(parent);

It looks strange to release sockets because, I guess, they are going to
be re-lock just after. It certainly means you want to call a (new)
function which is not doing this locking bit, no?

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

Re: [RFC PATCH mptcp-next v11 0/4] mptcp: Fast Open: Initiator

[Dmytro Shytyi]

Hello Paolo,

I added the patches 4,5,6 from v9 and fixes based on the feedback. Will 
be present in the next version.

On 9/26/2022 4:52 PM, Paolo Abeni wrote:
> On Mon, 2022-09-26 at 01:25 +0200, Dmytro Shytyi wrote:
>> These patches focus on the Initiator side.
>> These patches implement: sendto(..., ..., ..., MSG_FASTOPEN, ..., ...);
>> We would like to credit Paulo Abeni, Mat Martineau, Matthieu Baerts and
>> Benjamin Hesmans for advices and ideas that improved these patches.
>> Without this collaboration this state of work would not be presented.
>>
>> Origins of these patches were in the root of discovery of the
>> possibility to _reuse the TCP FASTOPEN option in the linux
>> upstream MPTCP_ (First commit was sent to the mailing list on 22 OCT
>> 2021: see https://lore.kernel.org/mptcp/
>> 17ca66cd439.10a0a3ce11621928.1543611905599720914@shytyi.net/').
>>
>> Dmytro Shytyi (4):
>>    mptcp: add mptcp_setsockopt_fastopen
>>    mptcp: add mptcp_stream_connect to *.h
>>    mptcp: add mptcp_subflow_conn_sock()
>>    mptcp: reuse tcp_sendmsg_fastopen()
>>
>>   include/net/mptcp.h  |  9 +++++++++
>>   include/net/tcp.h    |  3 +++
>>   net/ipv4/tcp.c       | 21 ++++++++++++++++-----
>>   net/mptcp/Makefile   |  2 +-
>>   net/mptcp/fastopen.c | 37 +++++++++++++++++++++++++++++++++++++
>>   net/mptcp/protocol.c | 35 +++++++++++++++++++++++++++++++----
>>   net/mptcp/protocol.h |  5 +++++
>>   net/mptcp/sockopt.c  |  3 +++
>>   8 files changed, 105 insertions(+), 10 deletions(-)
>>   create mode 100644 net/mptcp/fastopen.c
>>
> Plase, on the next iteration include the rx-path-related patches (e.g.
> patches 4, 5 and 6 from v9, updated with the relevant feedback).
>
> Thanks,
>
> Paolo
>
>

Re: [RFC PATCH mptcp-next v11 1/4] mptcp: add mptcp_setsockopt_fastopen

[Dmytro Shytyi]

Hello Paolo, Matthieu,


On 9/26/2022 5:01 PM, Matthieu Baerts wrote:
> Hi Dmytro, Paolo,
>
> On 26/09/2022 16:50, Paolo Abeni wrote:
>> On Mon, 2022-09-26 at 01:25 +0200, Dmytro Shytyi wrote:
>>> 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 | 37 +++++++++++++++++++++++++++++++++++++
>>>   net/mptcp/protocol.h |  5 +++++
>>>   net/mptcp/sockopt.c  |  3 +++
>>>   4 files changed, 46 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..086cbad49979
>>> --- /dev/null
>>> +++ b/net/mptcp/fastopen.c
>>> @@ -0,0 +1,37 @@
>>> +/* 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)
> I think we should keep everything related to the sockoptions in
> net/mptcp/sockopt.c, no?

I have multiple functions that are related to fastopen in fastopen.c. 
Probably it is not a bad idea to to keep all fastopen details in one file?

>>> +{
>>> +	struct sock *sk = (struct sock *)msk;
>>> +	struct net *net = sock_net(sk);
>>> +	struct socket *ssock;
>>> +	struct sock *ssk;
>>> +	int val;
>>> +	int ret;
>>> +
>>> +	ret = 0;
>>> +
>>> +	if (copy_from_sockptr(&val, optval, sizeof(val)))
>>> +		return -EFAULT;
>>> +
>>> +	ssock = __mptcp_nmpc_socket(msk);
> 'ssock' could be NULL if the setsockopt() is called once the connection
> has been established.


Thanks, in this case need to return error.

>>> +	ssk = ssock->sk;
>>> +	net = sock_net(ssk);
>>> +	lock_sock(ssk);
>>> +
>>> +	if (val >= 0 && ((1 << ssk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN))) {
>>> +		tcp_fastopen_init_key_once(net);
>>> +		fastopen_queue_tune(ssk, val);
>> Instead you could directly call:
>>
>> 	return tcp_setsockopt(ssock->sk, SOL_TCP, TCP_FASTOPEN, optval, optlen);
>
> I agree, similar to TCP_FASTOPEN_CONNECT


Indeed this is done here, when pass TCP_FASTOPEN (tcp.c - net/ipv4/tcp.c 
- Linux source code (v5.19.11) - Bootlin 
<https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp.c#L3636>)

So we could use directly  as you both suggested.

>> Side note, we could add a generic helper to invoke tcp_setsockopt() on
>> the mpc subflow, on the give tcp socket option, and reuse such helper
> Indeed. I was already looking at doing that when implementing
> TCP_FASTOPEN_NO_COOKIE. I can share a patch for this other sockopt and
> then you can re-use the new helper for this TCP_FASTOPEN.
>
>> for: TCP_FASTOPEN, TCP_FASTOPEN_CONNECT, TCP_DEFER_ACCEPT... but that
>> can/should be done later.
> Note that for TCP_DEFER_ACCEPT, this is a special case because
> apparently, the setsockopt doesn't fail even if a connection has already
> been accepted.
>
> @Dmytro: Two last things about the patch:
> - you also need to support the 'read' side (getsockopt())
> - if I'm not mistaken, this TCP_FASTOPEN option is only needed for the
> listener side, not for the sender, no?
>
> Cheers,
> Matt

Re: [RFC PATCH mptcp-next v11 4/4] mptcp: reuse tcp_sendmsg_fastopen()

[Dmytro Shytyi]

Hello,

On 9/26/2022 4:46 PM, Paolo Abeni wrote:
> Hello,
>
> On Mon, 2022-09-26 at 01:26 +0200, Dmytro Shytyi wrote:
>> In the following patches we will reuse modified tcp_sendmsg_fastopen().
>> We call it from mptcp_sendmsg().
>>
>> Signed-of-by: Benjamin Hesmans <benjamin.hesmans@tessares.net>
>> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
>> ---
>>   include/net/tcp.h    |  3 +++
>>   net/ipv4/tcp.c       |  6 +++---
>>   net/mptcp/protocol.c | 25 +++++++++++++++++++++++--
>>   3 files changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>> index 27e8d378c70a..97cb014ddcab 100644
>> --- a/include/net/tcp.h
>> +++ b/include/net/tcp.h
>> @@ -1755,6 +1755,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 a22c8044de17..daa611671d9a 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -1162,9 +1162,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);
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index fb08e1f458c0..dd2414fdcf04 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -1679,9 +1679,30 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>>   	long timeo;
>>   
>>   	/* we don't support FASTOPEN yet */
>> -	if (msg->msg_flags & MSG_FASTOPEN)
>> -		return -EOPNOTSUPP;
>> +	if (msg->msg_flags & MSG_FASTOPEN) {
>> +		struct socket *ssock;
>> +		struct sock *ssk;
>>   
>> +		lock_sock(sk);
>> +
>> +		ssock = __mptcp_nmpc_socket(msk);
>> +		ssk = ssock->sk;
>> +
>> +		lock_sock(ssk);
>> +
>> +		if (ssock) {
>> +			int copied_syn_fastopen = 0;
>> +
>> +			ret = tcp_sendmsg_fastopen(ssk, msg, &copied_syn_fastopen, len, NULL);
>> +			copied += copied_syn_fastopen;
>> +			if (ret) {
>> +				release_sock(ssk);
>> +				goto do_error;
>> +			}
>> +		}
>> +		release_sock(ssock->sk);
>> +		release_sock(sk);
>> +	}
>>   	/* silently ignore everything else */
>>   	msg->msg_flags &= MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL;
> the above/previous patches duplicate quit a bit of code. Instead the
> following should be enough (replacing patches 2, 3, 4 here):


Indeed. You are right. We could use this  and seems like it works :)

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

Re: [RFC PATCH mptcp-next v11 3/4] mptcp: add mptcp_subflow_conn_sock()

[Dmytro Shytyi]

Hello Matthieu,


On 9/26/2022 7:11 PM, Matthieu Baerts wrote:
> Hi Dmytro,
>
> On 26/09/2022 01:26, Dmytro Shytyi wrote:
>> In the following patches we will call mptcp_subflow_conn_sock() from
>> tcp_sendmsg_fastopen() in file "net/ipv4/tcp.c", thus make such symbol
>> visible.
> The patch description and title don't seem to be correct. You are doing
> more than that here.

Thanks, I deleted this patch in next version.


>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index 5702ca9b952d..a22c8044de17 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -1197,8 +1197,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);
> Do you think it could be possible not to modify tcp_sendmsg_fastopen()
> but instead implement '.connect' callback in 'mptcp_prot'?
>
>
>> +	} else {
>> +		struct sock *parent = mptcp_subflow_conn_sock(sk);
>> +
>> +		release_sock(sk);
>> +		release_sock(parent);
> It looks strange to release sockets because, I guess, they are going to
> be re-lock just after. It certainly means you want to call a (new)
> function which is not doing this locking bit, no?
No need to address this question, as I deleted this patch in new version.
> Cheers,
> Matt\

Thanks.

Dmytro.

Re: [RFC PATCH mptcp-next v11 1/4] mptcp: add mptcp_setsockopt_fastopen

[Matthieu Baerts]

Hi Dmytro,

On 27/09/2022 06:01, Dmytro Shytyi wrote:
> Hello Paolo, Matthieu,
> 
> 
> On 9/26/2022 5:01 PM, Matthieu Baerts wrote:
>> Hi Dmytro, Paolo,
>>
>> On 26/09/2022 16:50, Paolo Abeni wrote:
>>> On Mon, 2022-09-26 at 01:25 +0200, Dmytro Shytyi wrote:
>>>> 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 | 37 +++++++++++++++++++++++++++++++++++++
>>>>   net/mptcp/protocol.h |  5 +++++
>>>>   net/mptcp/sockopt.c  |  3 +++
>>>>   4 files changed, 46 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..086cbad49979
>>>> --- /dev/null
>>>> +++ b/net/mptcp/fastopen.c
>>>> @@ -0,0 +1,37 @@
>>>> +/* 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)
>> I think we should keep everything related to the sockoptions in
>> net/mptcp/sockopt.c, no?
> 
> I have multiple functions that are related to fastopen in fastopen.c.
> Probably it is not a bad idea to to keep all fastopen details in one file?

We already have a file dedicated to all socket options. I don't think we
should have an exception for TFO, especially if there is nothing
specific to TFO thanks to tcp_setsockopt().

Also, you can use the new mptcp_setsockopt_first_sf_only() helper I sent
a few minutes ago on the ML. For TCP_FASTOPEN sockopt, you now need a
commit similar to "mptcp: add TCP_FASTOPEN_NO_COOKIE support" by just
adding "TCP_FASTOPEN" in three different places in sockopt.c, see:


https://lore.kernel.org/mptcp/20220927150306.1552795-1-matthieu.baerts@tessares.net/T/

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

Re: [RFC PATCH mptcp-next v11 1/4] mptcp: add mptcp_setsockopt_fastopen

[Dmytro Shytyi]

Hello,

On 9/27/2022 5:22 PM, Matthieu Baerts wrote:
> Hi Dmytro,
>
> On 27/09/2022 06:01, Dmytro Shytyi wrote:
>> Hello Paolo, Matthieu,
>>
>>
>> On 9/26/2022 5:01 PM, Matthieu Baerts wrote:
>>> Hi Dmytro, Paolo,
>>>
>>> On 26/09/2022 16:50, Paolo Abeni wrote:
>>>> On Mon, 2022-09-26 at 01:25 +0200, Dmytro Shytyi wrote:
>>>>> 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 | 37 +++++++++++++++++++++++++++++++++++++
>>>>>    net/mptcp/protocol.h |  5 +++++
>>>>>    net/mptcp/sockopt.c  |  3 +++
>>>>>    4 files changed, 46 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..086cbad49979
>>>>> --- /dev/null
>>>>> +++ b/net/mptcp/fastopen.c
>>>>> @@ -0,0 +1,37 @@
>>>>> +/* 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)
>>> I think we should keep everything related to the sockoptions in
>>> net/mptcp/sockopt.c, no?
>> I have multiple functions that are related to fastopen in fastopen.c.
>> Probably it is not a bad idea to to keep all fastopen details in one file?
> We already have a file dedicated to all socket options. I don't think we
> should have an exception for TFO, especially if there is nothing
> specific to TFO thanks to tcp_setsockopt().
Okay.
> Also, you can use the new mptcp_setsockopt_first_sf_only() helper I sent
> a few minutes ago on the ML. For TCP_FASTOPEN sockopt, you now need a
> commit similar to "mptcp: add TCP_FASTOPEN_NO_COOKIE support" by just
> adding "TCP_FASTOPEN" in three different places in sockopt.c, see:
>
>
> https://lore.kernel.org/mptcp/20220927150306.1552795-1-matthieu.baerts@tessares.net/T/

Thanks!


> Cheers,
> Matt