Re: [PATCH mptcp-net] mptcp: avoid printing warning once on client side

[Mat Martineau <martineau@kernel.org>]

On Thu, 15 Feb 2024, Matthieu Baerts (NGI0) wrote:

> After the 'Fixes' commit mentioned below, the client side might print
> the following warning once when a subflow is fully established at the
> reception of any valid additional ack:
>
>  MPTCP: bogus mpc option on established client sk
>
> That's a normal situation, and no warning should be printed for that. We
> can then skip the check when the label is used.
>
> Fixes: e4a0fa47e816 ("mptcp: corner case locking for rx path fields initialization")
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Looks good to me:

Reviewed-by: Mat Martineau <martineau@kernel.org>

> ---
> Notes:
> - Should we convert this pr_warn_once() to a WARN_ONCE()? Or just in
>   our tree? Or just in DEBUG mode?

I think it makes sense to keep this patch minimal for -net and stable 
(just moving the label).

Also given the consequences of panic_on_warn, would be better to make any 
changes to WARN_ONCE() in mptcp-next/net-next. I don't see extra 
complexity to modify this warning in our tree or debug mode as being worth 
it, do you think it would be valuable?

- Mat

> ---
> net/mptcp/options.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 23e317ffc901..27ca42c77b02 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -981,10 +981,10 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
> 	if (mp_opt->deny_join_id0)
> 		WRITE_ONCE(msk->pm.remote_deny_join_id0, true);
>
> -set_fully_established:
> 	if (unlikely(!READ_ONCE(msk->pm.server_side)))
> 		pr_warn_once("bogus mpc option on established client sk");
>
> +set_fully_established:
> 	mptcp_data_lock((struct sock *)msk);
> 	__mptcp_subflow_fully_established(msk, subflow, mp_opt);
> 	mptcp_data_unlock((struct sock *)msk);
>
> ---
> base-commit: 52e05de42e1094a943053af93ea08c51a44091f7
> change-id: 20240215-mptcp-fix-bogus-pr-warn-497a3537984b
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>