1. Patchew
  2. MPTCP
  3. [RFC PATCH mptcp-next v8 0/7] mptcp: Fast Open Mechanism
  4. View patch

[RFC PATCH mptcp-next v8 4/7] mptfo variables for msk, options. Fix loop retrans

Dmytro Shytyi posted 7 patches 1 year, 2 months ago
Maintainers: Eric Dumazet <edumazet@google.com>, "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>, David Ahern <dsahern@kernel.org>, Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, Shuah Khan <shuah@kernel.org>
There is a newer version of this series
[RFC PATCH mptcp-next v8 4/7] mptfo variables for msk, options. Fix loop retrans
Posted by Dmytro Shytyi 1 year, 2 months ago 
Introduce mptfo variables for msk and options.
Also fix the infinite retransmissions in the end of second session.

The variable 2nd ack received in struct mptcp_options_received identifies the
received ack on the listener side during 3way handshake in mptfo context
and miningless alone if used alone. It is further used(checked)
in conjunction in the same "if" statement with variable is_mptfo
from struct mptcp_sock.

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

diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
index 9ef49a2d2ea2..92885e459f93 100644
--- a/net/mptcp/fastopen.c
+++ b/net/mptcp/fastopen.c
@@ -30,3 +30,17 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
 
 	return ret;
 }
+
+void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
+				   struct mptcp_options_received mp_opt)
+{
+	u64 ack_seq;
+
+	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("ack_seq=%llu sndr_key=%llu", msk->ack_seq, mp_opt.sndr_key);
+	atomic64_set(&msk->rcv_wnd_sent, ack_seq);
+}
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 30d289044e71..8852a13cfe62 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->hns_2nd_ack_rcvd = 1;
 			mp_opt->rcvr_key = get_unaligned_be64(ptr);
 			ptr += 8;
 		}
@@ -1124,6 +1125,9 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 		return sk->sk_state != TCP_CLOSE;
 
 	if (unlikely(mp_opt.suboptions != OPTION_MPTCP_DSS)) {
+		if (mp_opt.suboptions & OPTIONS_MPTCP_MPC && mp_opt.hns_2nd_ack_rcvd && msk->is_mptfo)
+			mptcp_gen_msk_ackseq_fastopen(msk, subflow, mp_opt);
+
 		if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
 		    msk->local_key == mp_opt.rcvr_key) {
 			WRITE_ONCE(msk->rcv_fastclose, true);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 57596cdfb1f9..3b9a349a7080 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;
+		__unused:1;
+	u8	hns_2nd_ack_rcvd:1;
 	u8	join_id;
 	u64	thmac;
 	u8	hmac[MPTCPOPT_HMAC_LEN];
@@ -282,6 +283,7 @@ struct mptcp_sock {
 	bool		use_64bit_ack; /* Set when we received a 64-bit DSN */
 	bool		csum_enabled;
 	bool		allow_infinite_fallback;
+	bool		is_mptfo;
 	u8		mpc_endpoint_id;
 	u8		recvmsg_inq:1,
 			cork:1,
@@ -842,6 +844,8 @@ int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_l
 // Fast Open Mechanism functions begin
 int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
 				      unsigned int optlen);
+void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
+				   struct mptcp_options_received mp_opt);
 // Fast Open Mechanism functions end
 
 static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
-- 
2.25.1
Re: [RFC PATCH mptcp-next v8 4/7] mptfo variables for msk, options. Fix loop retrans
Posted by Paolo Abeni 1 year, 2 months ago 
On Tue, 2022-09-20 at 14:52 +0200, Dmytro Shytyi wrote:
> Introduce mptfo variables for msk and options.
> Also fix the infinite retransmissions in the end of second session.

I think the above sentence belongs more the changelog than to the
commit message itself.

> The variable 2nd ack received in struct mptcp_options_received identifies the
> received ack on the listener side during 3way handshake in mptfo context
> and miningless alone if used alone. It is further used(checked)
> in conjunction in the same "if" statement with variable is_mptfo
> from struct mptcp_sock.
> 
> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
> ---
>  net/mptcp/fastopen.c | 14 ++++++++++++++
>  net/mptcp/options.c  |  4 ++++
>  net/mptcp/protocol.h |  6 +++++-
>  3 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
> index 9ef49a2d2ea2..92885e459f93 100644
> --- a/net/mptcp/fastopen.c
> +++ b/net/mptcp/fastopen.c
> @@ -30,3 +30,17 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>  
>  	return ret;
>  }
> +
> +void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
> +				   struct mptcp_options_received mp_opt)
> +{
> +	u64 ack_seq;
> +
> +	msk->can_ack = true;
> +	msk->remote_key = mp_opt.sndr_key;

likely the above two statements need WRITE_ONCE() annotation.

Additionally it's better to add some safeguards here:
- ssk should be in TCP_SYN_RECV state - to be verified, it's guess and
I haven't check it yet.
- msk should be in the same state of the ssk subflow.

> +	mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq);
> +	ack_seq++;
> +	WRITE_ONCE(msk->ack_seq, ack_seq);
> +	pr_debug("ack_seq=%llu sndr_key=%llu", msk->ack_seq, mp_opt.sndr_key);
> +	atomic64_set(&msk->rcv_wnd_sent, ack_seq);
> +}
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 30d289044e71..8852a13cfe62 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->hns_2nd_ack_rcvd = 1;

Not needed, see below...

>  			mp_opt->rcvr_key = get_unaligned_be64(ptr);
>  			ptr += 8;
>  		}
> @@ -1124,6 +1125,9 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>  		return sk->sk_state != TCP_CLOSE;
>  
>  	if (unlikely(mp_opt.suboptions != OPTION_MPTCP_DSS)) {
> +		if (mp_opt.suboptions & OPTIONS_MPTCP_MPC && mp_opt.hns_2nd_ack_rcvd && msk->is_mptfo)

You can simply check for:

		if (mp_opt.suboptions & OPTIONS_MPTCP_MPC_ACK) && msk->is_mptfo)

> +			mptcp_gen_msk_ackseq_fastopen(msk, subflow, mp_opt);
> +
>  		if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
>  		    msk->local_key == mp_opt.rcvr_key) {
>  			WRITE_ONCE(msk->rcv_fastclose, true);
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 57596cdfb1f9..3b9a349a7080 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;
> +		__unused:1;
> +	u8	hns_2nd_ack_rcvd:1;

... again not needed, see above ;)

>  	u8	join_id;
>  	u64	thmac;
>  	u8	hmac[MPTCPOPT_HMAC_LEN];
> @@ -282,6 +283,7 @@ struct mptcp_sock {
>  	bool		use_64bit_ack; /* Set when we received a 64-bit DSN */
>  	bool		csum_enabled;
>  	bool		allow_infinite_fallback;
> +	bool		is_mptfo;
>  	u8		mpc_endpoint_id;
>  	u8		recvmsg_inq:1,
>  			cork:1,
> @@ -842,6 +844,8 @@ int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_l
>  // Fast Open Mechanism functions begin
>  int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>  				      unsigned int optlen);
> +void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
> +				   struct mptcp_options_received mp_opt);
>  // Fast Open Mechanism functions end
>  
>  static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
Re: [RFC PATCH mptcp-next v8 4/7] mptfo variables for msk, options. Fix loop retrans
Posted by Dmytro Shytyi 1 year, 2 months ago 
On 9/20/2022 4:56 PM, Paolo Abeni wrote:
> On Tue, 2022-09-20 at 14:52 +0200, Dmytro Shytyi wrote:
>> Introduce mptfo variables for msk and options.
>> Also fix the infinite retransmissions in the end of second session.
> I think the above sentence belongs more the changelog than to the
> commit message itself.

Fixed in v9.


>> The variable 2nd ack received in struct mptcp_options_received identifies the
>> received ack on the listener side during 3way handshake in mptfo context
>> and miningless alone if used alone. It is further used(checked)
>> in conjunction in the same "if" statement with variable is_mptfo
>> from struct mptcp_sock.
>>
>> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
>> ---
>>   net/mptcp/fastopen.c | 14 ++++++++++++++
>>   net/mptcp/options.c  |  4 ++++
>>   net/mptcp/protocol.h |  6 +++++-
>>   3 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
>> index 9ef49a2d2ea2..92885e459f93 100644
>> --- a/net/mptcp/fastopen.c
>> +++ b/net/mptcp/fastopen.c
>> @@ -30,3 +30,17 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>>   
>>   	return ret;
>>   }
>> +
>> +void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
>> +				   struct mptcp_options_received mp_opt)
>> +{
>> +	u64 ack_seq;
>> +
>> +	msk->can_ack = true;
>> +	msk->remote_key = mp_opt.sndr_key;
> likely the above two statements need WRITE_ONCE() annotation.

I have added in v9 WRITE_ONCE() to the above two statements.


> Additionally it's better to add some safeguards here:
> - ssk should be in TCP_SYN_RECV state - to be verified, it's guess and
> I haven't check it yet.
> - msk should be in the same state of the ssk subflow.
>
>> +	mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq);
>> +	ack_seq++;
>> +	WRITE_ONCE(msk->ack_seq, ack_seq);
>> +	pr_debug("ack_seq=%llu sndr_key=%llu", msk->ack_seq, mp_opt.sndr_key);
>> +	atomic64_set(&msk->rcv_wnd_sent, ack_seq);
>> +}
>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index 30d289044e71..8852a13cfe62 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->hns_2nd_ack_rcvd = 1;
> Not needed, see below...


Removed in v9

>>   			mp_opt->rcvr_key = get_unaligned_be64(ptr);
>>   			ptr += 8;
>>   		}
>> @@ -1124,6 +1125,9 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>>   		return sk->sk_state != TCP_CLOSE;
>>   
>>   	if (unlikely(mp_opt.suboptions != OPTION_MPTCP_DSS)) {
>> +		if (mp_opt.suboptions & OPTIONS_MPTCP_MPC && mp_opt.hns_2nd_ack_rcvd && msk->is_mptfo)
> You can simply check for:
>
> 		if (mp_opt.suboptions & OPTIONS_MPTCP_MPC_ACK) && msk->is_mptfo)


Removed in v9

>> +			mptcp_gen_msk_ackseq_fastopen(msk, subflow, mp_opt);
>> +
>>   		if ((mp_opt.suboptions & OPTION_MPTCP_FASTCLOSE) &&
>>   		    msk->local_key == mp_opt.rcvr_key) {
>>   			WRITE_ONCE(msk->rcv_fastclose, true);
>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>> index 57596cdfb1f9..3b9a349a7080 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;
>> +		__unused:1;
>> +	u8	hns_2nd_ack_rcvd:1;
> ... again not needed, see above ;)
Removed in v9
>>   	u8	join_id;
>>   	u64	thmac;
>>   	u8	hmac[MPTCPOPT_HMAC_LEN];
>> @@ -282,6 +283,7 @@ struct mptcp_sock {
>>   	bool		use_64bit_ack; /* Set when we received a 64-bit DSN */
>>   	bool		csum_enabled;
>>   	bool		allow_infinite_fallback;
>> +	bool		is_mptfo;
>>   	u8		mpc_endpoint_id;
>>   	u8		recvmsg_inq:1,
>>   			cork:1,
>> @@ -842,6 +844,8 @@ int mptcp_stream_connect(struct socket *sock, struct sockaddr *uaddr, int addr_l
>>   // Fast Open Mechanism functions begin
>>   int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>>   				      unsigned int optlen);
>> +void mptcp_gen_msk_ackseq_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
>> +				   struct mptcp_options_received mp_opt);
>>   // Fast Open Mechanism functions end
>>   
>>   static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)

Best,

Dmytro.

Patchew 2.1-devel-0870f84

© 2016 - 2023 Red Hat, Inc.