Re: [PATCH mptcp-net 3/7] mptcp: tfo: record 'deny join id0' info

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

Thanks for this fix.

On Fri, 2025-08-29 at 22:33 +0200, Matthieu Baerts (NGI0) wrote:
> When TFO is used, the check to see if the 'C' flag (deny join id0)
> was
> set was bypassed.
> 
> This flag can be set when TFO is used, so the check should also be
> done
> when TFO is used.
> 
> Fixes: dfc8d0603033 ("mptcp: implement delayed seq generation for
> passive fastopen")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/options.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index
> d47b8a9bc2df2f14645b1b3d3e10fea1b38567b1..cf531f2d815cdfbc772b837def6
> e7d558e64d558 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -985,14 +985,14 @@ static bool check_fully_established(struct
> mptcp_sock *msk, struct sock *ssk,
>  		return false;
>  	}
>  
> -	if (mp_opt->deny_join_id0)
> -		WRITE_ONCE(msk->pm.remote_deny_join_id0, true);
> -
>  	if (unlikely(!READ_ONCE(msk->pm.server_side)))
>  		/* DO-NOT-MERGE: use WARN i/o pr_warn: only for
> MPTCP export */
>  		WARN_ONCE(1, "bogus mpc option on established client
> sk");
>  
>  set_fully_established:
> +	if (mp_opt->deny_join_id0)
> +		WRITE_ONCE(msk->pm.remote_deny_join_id0, true);
> +

This set_fully_established label is not only jumped to during TFO but
also in several other scenarios. I think maybe instead of relocating
the deny_join_id0 check code after this label, we should add a new
label before the deny_join_id0 check and jump to the new label only in
the case of TFO.

WDYT?

Thanks,
-Geliang

>  	mptcp_data_lock((struct sock *)msk);
>  	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
>  	mptcp_data_unlock((struct sock *)msk);