Re: [MPTCP][PATCH v8 mptcp-next 3/4] mptcp: add deny_join_id0 in mptcp_options_received

[Paolo Abeni <pabeni@redhat.com>]

Hello,

On Tue, 2021-05-25 at 17:17 +0800, Geliang Tang wrote:
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 1e921b5103bf..0d30008f0313 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -83,6 +83,9 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>  		if (flags & MPTCP_CAP_CHECKSUM_REQD)
>  			mp_opt->csum_reqd = 1;
>  
> +		if (flags & MPTCP_CAP_DENY_JOIN_ID0)
> +			mp_opt->deny_join_id0 = 1;
> +
>  		mp_opt->mp_capable = 1;
>  		if (opsize >= TCPOLEN_MPTCP_MPC_SYNACK) {
>  			mp_opt->sndr_key = get_unaligned_be64(ptr);
> @@ -360,6 +363,7 @@ void mptcp_get_options(const struct sock *sk,
>  	mp_opt->mp_prio = 0;
>  	mp_opt->reset = 0;
>  	mp_opt->csum_reqd = READ_ONCE(msk->csum_enabled);
> +	mp_opt->deny_join_id0 = 0;
>  
>  	length = (th->doff * 4) - sizeof(struct tcphdr);
>  	ptr = (const unsigned char *)(th + 1);
> @@ -1047,6 +1051,8 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>  	}
>  
>  	mptcp_get_options(sk, skb, &mp_opt);
> +	if (mp_opt.deny_join_id0)
> +		WRITE_ONCE(msk->pm.remote_deny_join_id0, true);

I'm sorry for the long delay. Overall LGTM. The only thing I see worth
mentioning is the above test: is done for each incoming packet, but we
really need to check it for MPC ACK packet, right? if so the test
should be probably moved into the suitable section of
check_fully_established() - e.g. after the 'if (!mp_opt->mp_capable) {'
block.

No need for a resend, a squash-to patch will suffice  - if my above
assumption is correct.

BTW it looks like there are no check for the C bit being unmodified
between MPC syn and MPC ack, and AFAICS the RFC don't mention that
case. I think/guess we can simply fetch the MPC ack value.

Cheers,

Paolo

p.s. this really means acked-by ;)