[PATCH mptcp-next 0/2] add sk_is_msk helper

[PATCH mptcp-next 0/2] add sk_is_msk helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This set introduces a sk_is_msk() helper.

Geliang Tang (2):
  mptcp: use sk_is_tcp helper
  mptcp: add sk_is_msk helper

 include/net/mptcp.h  | 12 ++++++++++++
 net/mptcp/bpf.c      |  4 ++--
 net/mptcp/protocol.h |  4 ++--
 3 files changed, 16 insertions(+), 4 deletions(-)

-- 
2.51.0

[PATCH mptcp-next 1/2] mptcp: use sk_is_tcp helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch enhances the validation by using sk_is_tcp to determine whether
the socket is a TCP socket. This approach is more rigorous than simply
checking if sk_protocol equals IPPROTO_TCP, as the helper also verifies
sk_family and sk_type.

Note: The modifications in the bpf_sk_stream_memory_free section should be
squashed into the commit "bpf: Export mptcp packet scheduler helpers".

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/bpf.c      | 4 ++--
 net/mptcp/protocol.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
index b261551f5d1b..372a21b96495 100644
--- a/net/mptcp/bpf.c
+++ b/net/mptcp/bpf.c
@@ -193,7 +193,7 @@ static struct bpf_struct_ops bpf_mptcp_sched_ops = {
 
 struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
 {
-	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
+	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))
 		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
 
 	return NULL;
@@ -294,7 +294,7 @@ __bpf_kfunc static bool bpf_sk_stream_memory_free(const struct sock *sk__ign)
 	const struct sock *sk = sk__ign;
 
 	if (sk && sk_fullsock(sk) &&
-	    sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
+	    sk_is_tcp(sk) && sk_is_mptcp(sk))
 		return sk_stream_memory_free(sk);
 
 	return NULL;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index cd5266099993..f418a0a0fa51 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -399,7 +399,7 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
 #undef tcp_sk
 #define tcp_sk(ptr) ({								\
 	typeof(ptr) _ptr = (ptr);						\
-	WARN_ON(_ptr->sk_protocol != IPPROTO_TCP);				\
+	WARN_ON(!sk_is_tcp(_ptr));						\
 	container_of_const(_ptr, struct tcp_sock, inet_conn.icsk_inet.sk);	\
 })
 #define mptcp_sk(ptr) ({						\
-- 
2.51.0

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

[Geliang Tang]

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.

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)
+{
+	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));					\
 	container_of_const(_ptr, struct mptcp_sock, sk.icsk_inet.sk);	\
 })
 
-- 
2.51.0

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

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Critical: 11 Call Trace(s) ❌
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Critical: 11 Call Trace(s) ❌
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Critical: 15 Call Trace(s) - Critical: Global Timeout ❌
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/20092325434

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/fee9e0c74459
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1031929


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next 1/2] mptcp: use sk_is_tcp helper

[Matthieu Baerts]

Hi Geliang,

Thank you for the patches.

On 10/12/2025 09:12, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch enhances the validation by using sk_is_tcp to determine whether
> the socket is a TCP socket. This approach is more rigorous than simply
> checking if sk_protocol equals IPPROTO_TCP, as the helper also verifies
> sk_family and sk_type.
> 
> Note: The modifications in the bpf_sk_stream_memory_free section should be
> squashed into the commit "bpf: Export mptcp packet scheduler helpers".
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/bpf.c      | 4 ++--
>  net/mptcp/protocol.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index b261551f5d1b..372a21b96495 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -193,7 +193,7 @@ static struct bpf_struct_ops bpf_mptcp_sched_ops = {
>  
>  struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
>  {
> -	if (sk && sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> +	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) && sk_is_mptcp(sk))

I know that BPF has a few extra "hidden" checks: are you adding this
because you saw issues where the protocol could be set to TCP, but the
family and type are not the expected ones?

If not, it might be harder to justify these patches upstream.

>  		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
>  
>  	return NULL;
> @@ -294,7 +294,7 @@ __bpf_kfunc static bool bpf_sk_stream_memory_free(const struct sock *sk__ign)
>  	const struct sock *sk = sk__ign;
>  
>  	if (sk && sk_fullsock(sk) &&
> -	    sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> +	    sk_is_tcp(sk) && sk_is_mptcp(sk))
>  		return sk_stream_memory_free(sk);
>  
>  	return NULL;
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index cd5266099993..f418a0a0fa51 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -399,7 +399,7 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
>  #undef tcp_sk
>  #define tcp_sk(ptr) ({								\
>  	typeof(ptr) _ptr = (ptr);						\
> -	WARN_ON(_ptr->sk_protocol != IPPROTO_TCP);				\
> +	WARN_ON(!sk_is_tcp(_ptr));						\

I don't think we should change that here. That will potentially cause
issues with kunit, but also the check here is mainly to catch that
'tcp_sk()' is not called with an MPTCP socket. So only the protocol
check is needed.

>  	container_of_const(_ptr, struct tcp_sock, inet_conn.icsk_inet.sk);	\
>  })
>  #define mptcp_sk(ptr) ({						\

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

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

[Matthieu Baerts]

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.

Re: [PATCH mptcp-next 1/2] mptcp: use sk_is_tcp helper

[Geliang Tang]

Hi Matt,

On Wed, 2025-12-10 at 10:54 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for the patches.
> 
> On 10/12/2025 09:12, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > This patch enhances the validation by using sk_is_tcp to determine
> > whether
> > the socket is a TCP socket. This approach is more rigorous than
> > simply
> > checking if sk_protocol equals IPPROTO_TCP, as the helper also
> > verifies
> > sk_family and sk_type.
> > 
> > Note: The modifications in the bpf_sk_stream_memory_free section
> > should be
> > squashed into the commit "bpf: Export mptcp packet scheduler
> > helpers".
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/bpf.c      | 4 ++--
> >  net/mptcp/protocol.h | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> > index b261551f5d1b..372a21b96495 100644
> > --- a/net/mptcp/bpf.c
> > +++ b/net/mptcp/bpf.c
> > @@ -193,7 +193,7 @@ static struct bpf_struct_ops
> > bpf_mptcp_sched_ops = {
> >  
> >  struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
> >  {
> > -	if (sk && sk_fullsock(sk) && sk->sk_protocol ==
> > IPPROTO_TCP && sk_is_mptcp(sk))
> > +	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) &&
> > sk_is_mptcp(sk))
> 
> I know that BPF has a few extra "hidden" checks: are you adding this
> because you saw issues where the protocol could be set to TCP, but
> the
> family and type are not the expected ones?

No, let's drop this patch then. 

How do you think about patch 2? Is sk_is_msk() a good name? I will
directly include patch 2 in the KTLS series.

Thanks,
-Geliang

> 
> If not, it might be harder to justify these patches upstream.
> 
> >  		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
> >  
> >  	return NULL;
> > @@ -294,7 +294,7 @@ __bpf_kfunc static bool
> > bpf_sk_stream_memory_free(const struct sock *sk__ign)
> >  	const struct sock *sk = sk__ign;
> >  
> >  	if (sk && sk_fullsock(sk) &&
> > -	    sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> > +	    sk_is_tcp(sk) && sk_is_mptcp(sk))
> >  		return sk_stream_memory_free(sk);
> >  
> >  	return NULL;
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index cd5266099993..f418a0a0fa51 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -399,7 +399,7 @@ static inline void msk_owned_by_me(const struct
> > mptcp_sock *msk)
> >  #undef tcp_sk
> >  #define tcp_sk(ptr)
> > ({								\
> >  	typeof(ptr) _ptr =
> > (ptr);						\
> > -	WARN_ON(_ptr->sk_protocol !=
> > IPPROTO_TCP);				\
> > +	WARN_ON(!sk_is_tcp(_ptr));				
> > 		\
> 
> I don't think we should change that here. That will potentially cause
> issues with kunit, but also the check here is mainly to catch that
> 'tcp_sk()' is not called with an MPTCP socket. So only the protocol
> check is needed.
> 
> >  	container_of_const(_ptr, struct tcp_sock,
> > inet_conn.icsk_inet.sk);	\
> >  })
> >  #define mptcp_sk(ptr)
> > ({						\
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next 1/2] mptcp: use sk_is_tcp helper

[Matthieu Baerts]

On 10/12/2025 11:04, Geliang Tang wrote:
> Hi Matt,
> 
> On Wed, 2025-12-10 at 10:54 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> Thank you for the patches.
>>
>> On 10/12/2025 09:12, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> This patch enhances the validation by using sk_is_tcp to determine
>>> whether
>>> the socket is a TCP socket. This approach is more rigorous than
>>> simply
>>> checking if sk_protocol equals IPPROTO_TCP, as the helper also
>>> verifies
>>> sk_family and sk_type.
>>>
>>> Note: The modifications in the bpf_sk_stream_memory_free section
>>> should be
>>> squashed into the commit "bpf: Export mptcp packet scheduler
>>> helpers".
>>>
>>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>>> ---
>>>  net/mptcp/bpf.c      | 4 ++--
>>>  net/mptcp/protocol.h | 2 +-
>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
>>> index b261551f5d1b..372a21b96495 100644
>>> --- a/net/mptcp/bpf.c
>>> +++ b/net/mptcp/bpf.c
>>> @@ -193,7 +193,7 @@ static struct bpf_struct_ops
>>> bpf_mptcp_sched_ops = {
>>>  
>>>  struct mptcp_sock *bpf_mptcp_sock_from_subflow(struct sock *sk)
>>>  {
>>> -	if (sk && sk_fullsock(sk) && sk->sk_protocol ==
>>> IPPROTO_TCP && sk_is_mptcp(sk))
>>> +	if (sk && sk_fullsock(sk) && sk_is_tcp(sk) &&
>>> sk_is_mptcp(sk))
>>
>> I know that BPF has a few extra "hidden" checks: are you adding this
>> because you saw issues where the protocol could be set to TCP, but
>> the
>> family and type are not the expected ones?
> 
> No, let's drop this patch then. 

OK!

> How do you think about patch 2? Is sk_is_msk() a good name? I will
> directly include patch 2 in the KTLS series.

I think sk_is_mptcp() would have been a better name, but it is already
used. It might be good to rename sk_is_mptcp() to tp_is_mptcp(), but I
don't know if it is wise to re-use sk_is_mptcp() for another purpose for
the backports... So sk_is_msk() looks good at the end I think.

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