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

[Paolo Abeni <pabeni@redhat.com>]

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?

Thanks!

Paolo