Re: [MPTCP][PATCH RFC 1/5] mptcp: don't send RST for single subflow

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Fri, 3 Sep 2021, Geliang Tang wrote:

> From: Geliang Tang <geliangtang@xiaomi.com>
>
> When a bad checksum is detected and a single subflow is in use, don't
> send RST + MP_FAIL, send data_ack + MP_FAIL instead.
>
> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
> ---
> net/mptcp/subflow.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 1de7ce883c37..dfcd84abc13e 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1167,14 +1167,14 @@ static bool subflow_check_data_avail(struct sock *ssk)
> 	/* RFC 8684 section 3.7. */
> 	if (subflow->send_mp_fail) {
> 		if (mptcp_has_another_subflow(ssk)) {

Hi Geliang -

Thanks for sharing these RFC patches.


RFC8684 says that this infinite mapping case is when there is one subflow, 
*and* all the data is contiguous. So this condition needs to check for 
contiguous data on the subflow.

We don't currently track whether the data is contiguous on a subflow, so 
that tracking needs to be added. When retransmission happens, we could set 
a "noncontiguous" flag in the msk, and once all retransmissions are 
DATA_ACK'd that flag could be cleared.

> +			ssk->sk_err = EBADMSG;
> +			tcp_set_state(ssk, TCP_CLOSE);
> +			subflow->reset_transient = 0;
> +			subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
> +			tcp_send_active_reset(ssk, GFP_ATOMIC);
> 			while ((skb = skb_peek(&ssk->sk_receive_queue)))
> 				sk_eat_skb(ssk, skb);
> 		}
> -		ssk->sk_err = EBADMSG;
> -		tcp_set_state(ssk, TCP_CLOSE);
> -		subflow->reset_transient = 0;
> -		subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
> -		tcp_send_active_reset(ssk, GFP_ATOMIC);
> 		WRITE_ONCE(subflow->data_avail, 0);
> 		return true;
> 	}
> -- 
> 2.31.1

--
Mat Martineau
Intel