Re: [PATCH mptcp-next 1/2] mptcp: add MP_FAIL response support

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

On Thu, 3 Mar 2022, Geliang Tang wrote:

> This patch added a new struct member mp_fail_response_expect in struct
> mptcp_subflow_context to support MP_FAIL response. In the single subflow
> with checksum error and contiguous data special case, a MP_FAIL sent in
> response to another MP_FAIL.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/pm.c       | 11 +++++++++--
> net/mptcp/protocol.h |  1 +
> net/mptcp/subflow.c  |  2 ++
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index d0d31d5c198a..c9080230b800 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -279,8 +279,15 @@ void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
>
> 	pr_debug("fail_seq=%llu", fail_seq);
>
> -	if (!mptcp_has_another_subflow(sk) && READ_ONCE(msk->allow_infinite_fallback))
> -		subflow->send_infinite_map = 1;
> +	if (!mptcp_has_another_subflow(sk) && READ_ONCE(msk->allow_infinite_fallback)) {
> +		if (!subflow->mp_fail_response_expect) {
> +			subflow->send_mp_fail = 1;
> +			MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPFAILTX);
> +			subflow->send_infinite_map = 1;
> +		} else {
> +			subflow->mp_fail_response_expect = 0;

I don't think there's any need to clear this flag. Once the MP_FAIL 
response is received, this side is supposed to fall back and another 
incoming MP_FAIL would be strange to see. But if one somehow propagated 
here, it would be better to "expect" it by having the flag still set so no 
more MP_FAILs are sent in response. I mentioned this before in my RFCv2 
review - if you think this line of code should remain, please explain.

Thanks!

-Mat

> +		}
> +	}
> }
>
> /* path manager helpers */
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index c8bada4537e2..39aa22595add 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -448,6 +448,7 @@ struct mptcp_subflow_context {
> 		backup : 1,
> 		send_mp_prio : 1,
> 		send_mp_fail : 1,
> +		mp_fail_response_expect : 1,
> 		send_fastclose : 1,
> 		send_infinite_map : 1,
> 		rx_eof : 1,
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 30ffb00661bb..63a256a94b65 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1217,6 +1217,8 @@ static bool subflow_check_data_avail(struct sock *ssk)
> 				tcp_send_active_reset(ssk, GFP_ATOMIC);
> 				while ((skb = skb_peek(&ssk->sk_receive_queue)))
> 					sk_eat_skb(ssk, skb);
> +			} else {
> +				subflow->mp_fail_response_expect = 1;
> 			}
> 			WRITE_ONCE(subflow->data_avail, MPTCP_SUBFLOW_NODATA);
> 			return true;
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel