Re: [PATCH mptcp-next v15 3/7] Squash to "mptcp: add mptcpi_subflows_total counter"

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

Thank you for the new version!

On 12/10/2023 10:52, Geliang Tang wrote:
> When the socket shuts down, it's state is TCPF_FIN_WAIT2. Add this into

Good point, but is it not first TCP_FIN_WAIT1? (then FIN_WAIT2, etc.)

> __mptcp_has_initial_subflow.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/protocol.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 6508179e94a6..ccb97b747527 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1081,7 +1081,8 @@ static inline bool __mptcp_has_initial_subflow(const struct mptcp_sock *msk)
>  {
>  	struct sock *ssk = READ_ONCE(msk->first);
>  
> -	return ssk && inet_sk_state_load(ssk) != TCP_CLOSE;
> +	return ssk &&
> +	       !((1 << inet_sk_state_load(ssk)) & (TCPF_FIN_WAIT2 | TCPF_CLOSE));

I see that we decrement "msk->pm.subflows" when we start closing the
subflow. To be coherent with this counter -- and also because there are
more "close/closing" state, should we not have:

   inet_sk_state_load(ssk) != TCP_ESTABLISHED;

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net