Re: [PATCH mptcp-next 2/2] mptcp: add sk_is_msk helper

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 10/12/2025 09:12, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch introduces a sk_is_msk() helper modeled after sk_is_tcp() to
> determine whether the socket is an MPTCP one. Unlike sk_is_mptcp(), which
> accepts a subflow socket as its parameter, this new helper specifically
> accepts an MPTCP socket parameter.
> 
> Note: This helper will be useful in the subsequent "MPTCP KTLS support"
> series.

It might be easier to introduce it there then, see below.

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/mptcp.h  | 12 ++++++++++++
>  net/mptcp/protocol.h |  2 +-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 4cf59e83c1c5..82660374859a 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -150,6 +150,13 @@ static inline bool rsk_drop_req(const struct request_sock *req)
>  	return tcp_rsk(req)->is_mptcp && tcp_rsk(req)->drop_req;
>  }
>  
> +static inline bool sk_is_msk(const struct sock *sk)
> +{
> +	return sk_is_inet(sk) &&
> +	       sk->sk_type == SOCK_STREAM &&
> +	       sk->sk_protocol == IPPROTO_MPTCP;
> +}
> +
>  void mptcp_space(const struct sock *ssk, int *space, int *full_space);
>  bool mptcp_syn_options(struct sock *sk, const struct sk_buff *skb,
>  		       unsigned int *size, struct mptcp_out_options *opts);
> @@ -258,6 +265,11 @@ static inline bool rsk_drop_req(const struct request_sock *req)
>  	return false;
>  }
>  
> +static inline bool sk_is_msk(const struct sock *sk)

I do wonder if we shouldn't take this opportunity to rename sk_is_mptcp
to something less confusing: sk has to be a TCP sk and we are looking at
subflows, not MPTCP socket... tp_is_subflow or tp_is_mptcp or
tp_is_mptcp_subflow or sk_is_subflow or ...

But to be discussed first: sk_is_mptcp is mainly used in TCP code and
that might cause issues for the backports.

I don't know what people think about that. We already discussed that before:

  https://github.com/multipath-tcp/mptcp_net-next/issues/453

Same for rsk_is_mptcp.

> +{
> +	return false;
> +}
> +
>  static inline bool mptcp_syn_options(struct sock *sk, const struct sk_buff *skb,
>  				     unsigned int *size,
>  				     struct mptcp_out_options *opts)
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index f418a0a0fa51..8f1fd74ecaa3 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -404,7 +404,7 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
>  })
>  #define mptcp_sk(ptr) ({						\
>  	typeof(ptr) _ptr = (ptr);					\
> -	WARN_ON(_ptr->sk_protocol != IPPROTO_MPTCP);			\
> +	WARN_ON(!sk_is_msk(_ptr));					\

Same here, I don't think we should/need to do that.

>  	container_of_const(_ptr, struct mptcp_sock, sk.icsk_inet.sk);	\
>  })
>  

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.