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

From: Christoph Paasch <cpaasch@apple.com>
To: Paolo Abeni <pabeni@redhat.com>
Cc: Dmytro Shytyi <dmytro@shytyi.net>, mptcp@lists.linux.dev
Subject: Re: [RFC PATCH mptcp-next v7 07/11] mptfo variables for msk, options. Fix loop retrans
Date: Tue, 20 Sep 2022 09:32:56 -0700	[thread overview]
Message-ID: <98630039-4188-4413-9A7D-D2A3A15A224C@apple.com> (raw)
In-Reply-To: <d2dbb643e29250730a2610986cde88c188986c3b.camel@redhat.com>

Hello Paolo,

> On Sep 19, 2022, at 4:26 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> 
> On Mon, 2022-09-19 at 13:11 +0200, 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 ?!?
> 
> Addendum: instead here you likely want to setup-up a special DSS
> mapping for the TFO data. Such mapping will additionally require some
> special handling in  __mptcp_move_skb(), as 'msk->ack_seq' must not be
> incremented by the in-sequence TFO data.
> 
> @Christoph: AFAICS, even with TFO, the initiator nor the listener can't
> send any other data after the syn until the MPC handshake is completed.
> Is that correct?

The initiator can only send data in the SYN. No subsequent packets after that unless the SYN/ACK has been received.

The listener can send data after the SYN/ACK. It does not yet at this stage know about the DATA_ACK it should use, but that is ok.

Hope I could help.

Cheers,
Christoph