Re: [RFC PATCH mptcp-next v7 07/11] mptfo variables for msk, options. Fix loop retrans

[Dmytro Shytyi <dmytro@shytyi.net>]

This is refactored in v8.


Best regards,

Dmytro

On 9/19/2022 1:11 PM, Paolo Abeni wrote:
> On Sun, 2022-09-18 at 00:28 +0200, Dmytro Shytyi wrote:
>> Introduce mptfo variables for msk and options.
>> Also fixe the infinite retransmissions in the end of second session.
>> Suggestion @palbeni (SEP 1) during the meting to 'look at ack'
>>
>> Signed-off-by: Dmytro Shytyi <dmytro@shytyi.net>
>> ---
>>   net/mptcp/fastopen.c | 16 ++++++++++++++++
>>   net/mptcp/options.c  |  3 +++
>>   net/mptcp/protocol.h |  6 +++++-
>>   3 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/fastopen.c b/net/mptcp/fastopen.c
>> index 436e773d798a..149a4b1d3dac 100644
>> --- a/net/mptcp/fastopen.c
>> +++ b/net/mptcp/fastopen.c
>> @@ -91,3 +91,19 @@ int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>>   
>>   	return ret;
>>   }
>> +
>> +void mptcp_treat_hshake_ack_fastopen(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
>> +				     struct mptcp_options_received mp_opt)
>> +{
>> +	u64 ack_seq;
>> +
>> +	if (mp_opt.suboptions & OPTIONS_MPTCP_MPC && mp_opt.is_mptfo && msk->is_mptfo) {
>> +		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);
> It's not clear to me why the above is needed. According to the RFC we
> should not do additional MPTCP-level key management for TFO' sake ?!?
>
>> +	}
>> +}
>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index 30d289044e71..185b069e57f4 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->is_mptfo = 1;
> This looks incorrect. Plain MPTCP MP_CAPABLE handshake will be marked
> as TFO regardless of the actual TCP option exchange.
>
> It looks like the only already available hook to catch the 'incoming
> fast open' info would be implementing custom mptcp subflow {v4,v6}
> send_synack and checking the 'foc' pointer.
>
> Otherwise you can add TCPOPT_FASTOPEN parsing capability to
> mptcp_parse_option(), but that would be more duplicate code, I think it
> would be better to avoid that if possible.
>
>>   			mp_opt->rcvr_key = get_unaligned_be64(ptr);
>>   			ptr += 8;
>>   		}
>> @@ -1124,6 +1125,8 @@ 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)) {
>> +		mptcp_treat_hshake_ack_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 8caaeeedb9da..b90279c734ae 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;
>> +		is_mptfo:1,
>> +		__unused: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,
>> @@ -845,6 +847,8 @@ int mptcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
>>   			   size_t *copied);
>>   int mptcp_setsockopt_sol_tcp_fastopen(struct mptcp_sock *msk, sockptr_t optval,
>>   				      unsigned int optlen);
>> +void mptcp_treat_hshake_ack_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)