Re: [PATCH mptcp-next RFC 3/3] mptcp: pm: kernel: skip operating on closing connections

[Paolo Abeni <pabeni@redhat.com>]

On 8/14/26 8:31 PM, Matthieu Baerts (NGI0) wrote:
> When iterating over each MPTCP connection after having manipulated MPTCP
> endpoints, there is no need to operate on closing (or not ready)
> connections.
> 
> We can then safely skip those.
> 
> Note that mptcp_nl_add_subflow_or_signal_addr() was already checking the
> fully_established state, but it is better to check the connection state,
> which is what mptcp_is_fully_established() is doing.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_kernel.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3..c185e53fe42b 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -969,7 +969,7 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net,
>  		struct sock *sk = (struct sock *)msk;
>  		struct mptcp_addr_info mpc_addr;
>  
> -		if (!READ_ONCE(msk->fully_established) ||
> +		if (!mptcp_is_fully_established(sk) ||
>  		    mptcp_pm_is_userspace(msk))
>  			goto next;

I'm a little double-minded here.

The add/removal operations are issued by the user space to update
subflows, as a consequence of endpoint addition/removal.

Possibly removing the subflow even for not fully established connections
could be the right thing to do, I'm not 110% sure either way. Possibly
worth to call this out with a comment?

Also the asymmetry with the user-space PM is a bit strange. I can't
recall why it was needed in the first place?!?

/P