Re: [MPTCP][PATCH mptcp-next] mptcp: don't fallback when data_fin enabled

[Matthieu Baerts <matthieu.baerts@tessares.net>]

Hi Geliang,

On 07/05/2021 09:58, Geliang Tang wrote:
> This patch fixed this fallback error:

Thank you for the patch!

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index f3e379a0f08d..940b32511443 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -901,7 +901,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
>  	 * MP_JOIN subflows.
>  	 */
>  	if (!mp_opt->mp_capable) {
> -		if (subflow->mp_join)
> +		if (subflow->mp_join || mptcp_data_fin_enabled(msk))>  			goto reset;

I don't think we should send a reset here. For me this behaviour is normal.

When looking at the debug trace:

> <7>[ 1485.133416] MPTCP: msk=00000000875940e7, token=2833190200 side=0
> <7>[ 1485.134972] MPTCP: msk=00000000875940e7 snd_data_fin_enable=0
shutdown=3 state=4 pending=0

It looks like the userspace has closed the connection:

  mptcp_close() → __mptcp_wr_shutdown()

There we do:

  /* will be ignored by fallback sockets */
  WRITE_ONCE(msk->write_seq, msk->write_seq + 1);
  WRITE_ONCE(msk->snd_data_fin_enable, 1);

> <7>[ 1485.134987] MPTCP: msk=00000000875940e7 snd_data_fin_enable=1
pending=0 snd_nxt=13562085501921405457

From:

  __mptcp_check_send_data_fin(sk);

> <7>[ 1485.135233] MPTCP: msk=00000000875940e7 state=4

From:

  mptcp_close()

> <7>[ 1485.135472] MPTCP: check_fully_established:fallback to TCP
(msk=00000000875940e7)

It means we certainly got a FIN back:

  mptcp_incoming_options() → check_fully_established()

> <7>[ 1485.339305] MPTCP: msk=00000000875940e7 snd_data_fin_enable=1
pending=0 snd_nxt=13562085501921405458 write_seq=13562085501921405458

From:

  __mptcp_check_send_data_fin()

But here we will not send the DATA_FIN because we did a fallback to TCP.
But we change the state to properly close the socket.

> <7>[ 1485.339329] MPTCP: msk=00000000875940e7

By chance, do you have a packet trace to confirm that?

It's unclear to me if we send a DATA_FIN in this case. I guess we should
not if we are not in fully_established and I think that's what we do but
I'm unsure now.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net