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

Dmytro Shytyi posted 4 patches 3 years, 4 months ago
[RFC PATCH mptcp-next v11 3/4] mptcp: add mptcp_subflow_conn_sock()
Posted by Dmytro Shytyi 3 years, 4 months ago
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
Re: [RFC PATCH mptcp-next v11 3/4] mptcp: add mptcp_subflow_conn_sock()
Posted by Matthieu Baerts 3 years, 4 months ago
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 3/4] mptcp: add mptcp_subflow_conn_sock()
Posted by Dmytro Shytyi 3 years, 4 months ago
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.