Re: [MPTCP][PATCH v6 mptcp-next 2/5] mptcp: MP_FAIL suboption receiving

[Paolo Abeni <pabeni@redhat.com>]

On Wed, 2021-07-28 at 17:35 +0800, Geliang Tang wrote:
> From: Geliang Tang <geliangtang@xiaomi.com>
> 
> This patch added handling for receiving MP_FAIL suboption.
> 
> Add a new members mp_fail and fail_seq in struct mptcp_options_received.
> When MP_FAIL suboption is received, set mp_fail to 1 and save the sequence
> number to fail_seq.
> 
> Then invoke mptcp_pm_mp_fail_received to deal with the MP_FAIL suboption.
> 
> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
> ---
>  net/mptcp/options.c  | 16 ++++++++++++++++
>  net/mptcp/pm.c       |  5 +++++
>  net/mptcp/protocol.h |  3 +++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 2b15063c8009..cd9ec4acf127 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -336,6 +336,16 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>  		mp_opt->reset_reason = *ptr;
>  		break;
>  
> +	case MPTCPOPT_MP_FAIL:
> +		if (opsize != TCPOLEN_MPTCP_FAIL)
> +			break;
> +
> +		ptr += 2;
> +		mp_opt->mp_fail = 1;
> +		mp_opt->fail_seq = get_unaligned_be64(ptr);
> +		pr_debug("MP_FAIL: data_seq=%llu", mp_opt->fail_seq);
> +		break;
> +
>  	default:
>  		break;
>  	}
> @@ -364,6 +374,7 @@ void mptcp_get_options(const struct sock *sk,
>  	mp_opt->reset = 0;
>  	mp_opt->csum_reqd = READ_ONCE(msk->csum_enabled);
>  	mp_opt->deny_join_id0 = 0;
> +	mp_opt->mp_fail = 0;
>  
>  	length = (th->doff * 4) - sizeof(struct tcphdr);
>  	ptr = (const unsigned char *)(th + 1);
> @@ -1147,6 +1158,11 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>  		mp_opt.mp_prio = 0;
>  	}
>  
> +	if (mp_opt.mp_fail) {
> +		mptcp_pm_mp_fail_received(sk, mp_opt.fail_seq);
> +		mp_opt.mp_fail = 0;
> +	}
> +

Side note not specifically related to this patch: usually we get a
single MPTCP subopt per packet: a DSS. So we could optimize this code
path with something alike:

	if (unlikely(any subopt other than dss is present))
		// go checking all of them individually

To do the above we likely need to wrap all the 'mp_capable',
'fastclose', 'rm_addr' flags in a single bitmask. e.v. using a union.

/P