[PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address

[PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address

[Geliang Tang]

v16:
 - address Matt's comments in v15.

v15:
 - update __mptcp_has_initial_subflow
 - address Matt's comments in v14.

v14:
 - reuse MPTCP_CF_FASTCLOSE flag.
 - use tcp_shutdown instead of mptcp_subflow_shutdown.

v13:
 - invoke mptcp_subflow_shutdown() in patch 1 as Paolo suggested.

v12:
 - address Matt's comments in v11.

v11:
 - avoid sending RSTs.
 - rename 'id 0 subflow' to 'inital subflow'.

Geliang Tang (7):
  mptcp: add __mptcp_subflow_disconnect helper
  Squash to "mptcp: add mptcpi_subflows_total counter"
  Squash to "selftests: mptcp: add chk_subflows_total helper"
  selftests: mptcp: userspace pm remove initial subflow
  mptcp: userspace pm send RM_ADDR for ID 0
  mptcp: userspace pm rename remove_err to out
  selftests: mptcp: userspace pm send RM_ADDR for ID 0

Matthieu Baerts (1):
  selftests: mptcp: join: no RST when rm subflow/addr

 net/mptcp/pm_userspace.c                      | 45 +++++++++++-
 net/mptcp/protocol.c                          | 28 ++++++--
 net/mptcp/protocol.h                          |  2 +-
 .../testing/selftests/net/mptcp/mptcp_join.sh | 68 ++++++++++++++++++-
 4 files changed, 131 insertions(+), 12 deletions(-)

-- 
2.35.3

[PATCH mptcp-next v16 1/8] mptcp: add __mptcp_subflow_disconnect helper

[Geliang Tang]

When closing the msk->first socket in __mptcp_close_ssk(), if there's
another subflow available, it's better to avoid resetting it, just shut
down it.

This patch adds a new helper __mptcp_subflow_disconnect(), and reuse
flag MPTCP_CF_FASTCLOSE in this case. When MPTCP_CF_FASTCLOSE isn't set,
we invoke tcp_shutdown() instead of tcp_disconnect().

Co-developed-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/protocol.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 30e0c29ae0a4..1a54d55f8bb2 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2366,6 +2366,26 @@ bool __mptcp_retransmit_pending_data(struct sock *sk)
 #define MPTCP_CF_PUSH		BIT(1)
 #define MPTCP_CF_FASTCLOSE	BIT(2)
 
+/* be sure to send a reset only if the caller asked for it, also
+ * clean completely the subflow status when the subflow reaches
+ * TCP_CLOSE state
+ */
+static void __mptcp_subflow_disconnect(struct sock *ssk,
+				       struct mptcp_subflow_context *subflow,
+				       unsigned int flags)
+{
+	if (((1 << ssk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)) ||
+	    (flags & MPTCP_CF_FASTCLOSE)) {
+		/* The MPTCP code never wait on the subflow sockets, TCP-level
+		 * disconnect should never fail
+		 */
+		WARN_ON_ONCE(tcp_disconnect(ssk, 0));
+		mptcp_subflow_ctx_reset(subflow);
+	} else {
+		tcp_shutdown(ssk, SEND_SHUTDOWN);
+	}
+}
+
 /* subflow sockets can be either outgoing (connect) or incoming
  * (accept).
  *
@@ -2403,7 +2423,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 	lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
 
 	if ((flags & MPTCP_CF_FASTCLOSE) && !__mptcp_check_fallback(msk)) {
-		/* be sure to force the tcp_disconnect() path,
+		/* be sure to force the tcp_close path
 		 * to generate the egress reset
 		 */
 		ssk->sk_lingertime = 0;
@@ -2413,11 +2433,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 
 	need_push = (flags & MPTCP_CF_PUSH) && __mptcp_retransmit_pending_data(sk);
 	if (!dispose_it) {
-		/* The MPTCP code never wait on the subflow sockets, TCP-level
-		 * disconnect should never fail
-		 */
-		WARN_ON_ONCE(tcp_disconnect(ssk, 0));
-		mptcp_subflow_ctx_reset(subflow);
+		__mptcp_subflow_disconnect(ssk, subflow, flags);
 		release_sock(ssk);
 
 		goto out;
-- 
2.35.3

[PATCH mptcp-next v16 2/8] selftests: mptcp: join: no RST when rm subflow/addr

[Geliang Tang]

From: Matthieu Baerts <matttbe@kernel.org>

Recently, we noticed that some RST were wrongly generated when removing
the initial subflow.

This patch makes sure RST are not sent when removing any subflows or any
addresses.

Signed-off-by: Matthieu Baerts <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 01480663c102..07b2c4bfa183 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2343,6 +2343,7 @@ remove_tests()
 		chk_join_nr 1 1 1
 		chk_rm_tx_nr 1
 		chk_rm_nr 1 1
+		chk_rst_nr 0 0
 	fi
 
 	# multiple subflows, remove
@@ -2355,6 +2356,7 @@ remove_tests()
 			run_tests $ns1 $ns2 10.0.1.1
 		chk_join_nr 2 2 2
 		chk_rm_nr 2 2
+		chk_rst_nr 0 0
 	fi
 
 	# single address, remove
@@ -2367,6 +2369,7 @@ remove_tests()
 		chk_join_nr 1 1 1
 		chk_add_nr 1 1
 		chk_rm_nr 1 1 invert
+		chk_rst_nr 0 0
 	fi
 
 	# subflow and signal, remove
@@ -2380,6 +2383,7 @@ remove_tests()
 		chk_join_nr 2 2 2
 		chk_add_nr 1 1
 		chk_rm_nr 1 1
+		chk_rst_nr 0 0
 	fi
 
 	# subflows and signal, remove
@@ -2394,6 +2398,7 @@ remove_tests()
 		chk_join_nr 3 3 3
 		chk_add_nr 1 1
 		chk_rm_nr 2 2
+		chk_rst_nr 0 0
 	fi
 
 	# addresses remove
@@ -2408,6 +2413,7 @@ remove_tests()
 		chk_join_nr 3 3 3
 		chk_add_nr 3 3
 		chk_rm_nr 3 3 invert
+		chk_rst_nr 0 0
 	fi
 
 	# invalid addresses remove
@@ -2422,6 +2428,7 @@ remove_tests()
 		chk_join_nr 1 1 1
 		chk_add_nr 3 3
 		chk_rm_nr 3 1 invert
+		chk_rst_nr 0 0
 	fi
 
 	# subflows and signal, flush
@@ -2436,6 +2443,7 @@ remove_tests()
 		chk_join_nr 3 3 3
 		chk_add_nr 1 1
 		chk_rm_nr 1 3 invert simult
+		chk_rst_nr 0 0
 	fi
 
 	# subflows flush
@@ -2455,6 +2463,7 @@ remove_tests()
 		else
 			chk_rm_nr 3 3
 		fi
+		chk_rst_nr 0 0
 	fi
 
 	# addresses flush
@@ -2469,6 +2478,7 @@ remove_tests()
 		chk_join_nr 3 3 3
 		chk_add_nr 3 3
 		chk_rm_nr 3 3 invert simult
+		chk_rst_nr 0 0
 	fi
 
 	# invalid addresses flush
@@ -2483,6 +2493,7 @@ remove_tests()
 		chk_join_nr 1 1 1
 		chk_add_nr 3 3
 		chk_rm_nr 3 1 invert
+		chk_rst_nr 0 0
 	fi
 
 	# remove id 0 subflow
@@ -2494,6 +2505,7 @@ remove_tests()
 			run_tests $ns1 $ns2 10.0.1.1
 		chk_join_nr 1 1 1
 		chk_rm_nr 1 1
+		chk_rst_nr 0 0
 	fi
 
 	# remove id 0 address
@@ -2506,6 +2518,7 @@ remove_tests()
 		chk_join_nr 1 1 1
 		chk_add_nr 1 1
 		chk_rm_nr 1 1 invert
+		chk_rst_nr 0 0 invert
 	fi
 }
 
-- 
2.35.3

[PATCH mptcp-next v16 3/8] Squash to "mptcp: add mptcpi_subflows_total counter"

[Geliang Tang]

Update __mptcp_has_initial_subflow().

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/protocol.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 6508179e94a6..1fb4ac3727c4 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1081,7 +1081,7 @@ 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 && inet_sk_state_load(ssk) == TCP_ESTABLISHED;
 }
 
 static inline void mptcp_do_fallback(struct sock *ssk)
-- 
2.35.3

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

[Paolo Abeni]

Hi,

On Fri, 2023-10-13 at 13:46 +0800, Geliang Tang wrote:
> Update __mptcp_has_initial_subflow().
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/protocol.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 6508179e94a6..1fb4ac3727c4 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1081,7 +1081,7 @@ 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 && inet_sk_state_load(ssk) == TCP_ESTABLISHED;

I think the above is not correct, __mptcp_has_initial_subflow() will
return false before connect completes and/or for listener sockets.

You can list explicitly add the valid states with something alike:

	(1 << inet_sk_state_load(ssk)) & (TCPF_ESTABLISHED |
TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_LISTEN | TCPF_CLOSE_WAIT)

I'm unsure if we should include CLOSE_WAIT here: the remote has shut
down, but this end can still send data...

Side important note: you are too fast :) There are a lot of in-flight
patches, and it's difficult to follow each series consistently. I
suggest to focus on a small subset - possibly on a single series at the
time.

e.g. The first 2 patches in this series are IMHO ready to be merged
[*]. If Mat could apply them, you could follow-up with the remaining
bits of this series.

Cheers,

Paolo

[*] modulo some expansion to the changelog of patch 1, but that could
happen even after merging IMHO.

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

[Matthieu Baerts]

Hi Paolo,

On 13/10/2023 12:32, Paolo Abeni wrote:
> Hi,
> 
> On Fri, 2023-10-13 at 13:46 +0800, Geliang Tang wrote:
>> Update __mptcp_has_initial_subflow().
>>
>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>> ---
>>  net/mptcp/protocol.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>> index 6508179e94a6..1fb4ac3727c4 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -1081,7 +1081,7 @@ 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 && inet_sk_state_load(ssk) == TCP_ESTABLISHED;
> 
> I think the above is not correct, __mptcp_has_initial_subflow() will
> return false before connect completes and/or for listener sockets.

Please note that __mptcp_has_initial_subflow() is there just to count
the number of subflows. It is being used with 'msk->pm.subflows' and it
is supposed to have the same "behaviour". Then I don't think we should
increment the subflow counter for listener sockets, no?

> You can list explicitly add the valid states with something alike:
> 
> 	(1 << inet_sk_state_load(ssk)) & (TCPF_ESTABLISHED |
> TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_LISTEN | TCPF_CLOSE_WAIT)
> 
> I'm unsure if we should include CLOSE_WAIT here: the remote has shut
> down, but this end can still send data...

Maybe better, no? As long as the behaviour is similar to the one with
'msk->pm.subflows'.

> Side important note: you are too fast :) There are a lot of in-flight
> patches, and it's difficult to follow each series consistently. I
> suggest to focus on a small subset - possibly on a single series at the
> time.
> 
> e.g. The first 2 patches in this series are IMHO ready to be merged
> [*]. If Mat could apply them, you could follow-up with the remaining
> bits of this series.

Sure, I can do that.

Regarding patch 1/8, do you think we should send that to "-net"? The
patch looks OK to me but on the other hand, it is not a big issue to
reset the initial subflow (but not ideal) if we fear regressions due to
this patch. WDYT?

> [*] modulo some expansion to the changelog of patch 1, but that could
> happen even after merging IMHO.

Indeed. But we might forget :)
So if you have any suggestions, do not hesitate to share them :-)

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

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

[Paolo Abeni]

On Fri, 2023-10-13 at 12:46 +0200, Matthieu Baerts wrote:
> On 13/10/2023 12:32, Paolo Abeni wrote:
> > Hi,
> > 
> > On Fri, 2023-10-13 at 13:46 +0800, Geliang Tang wrote:
> > > Update __mptcp_has_initial_subflow().
> > > 
> > > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > > ---
> > >  net/mptcp/protocol.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > > index 6508179e94a6..1fb4ac3727c4 100644
> > > --- a/net/mptcp/protocol.h
> > > +++ b/net/mptcp/protocol.h
> > > @@ -1081,7 +1081,7 @@ 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 && inet_sk_state_load(ssk) == TCP_ESTABLISHED;
> > 
> > I think the above is not correct, __mptcp_has_initial_subflow() will
> > return false before connect completes and/or for listener sockets.
> 
> Please note that __mptcp_has_initial_subflow() is there just to count
> the number of subflows. It is being used with 'msk->pm.subflows' and it
> is supposed to have the same "behaviour". Then I don't think we should
> increment the subflow counter for listener sockets, no?

If the goal is giving an accurate count of the total number of
subflows, I think we should: the msk listener has 1 subflow: the tcp
listener.

> > You can list explicitly add the valid states with something alike:
> > 
> > 	(1 << inet_sk_state_load(ssk)) & (TCPF_ESTABLISHED |
> > TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_LISTEN | TCPF_CLOSE_WAIT)
> > 
> > I'm unsure if we should include CLOSE_WAIT here: the remote has shut
> > down, but this end can still send data...
> 
> Maybe better, no? As long as the behaviour is similar to the one with
> 'msk->pm.subflows'.

If we keep the way we account for MPJ subflows as a reference
CLOSE_WAIT status must be excluded. I agree/now see it's the better
option.

> > Side important note: you are too fast :) There are a lot of in-flight
> > patches, and it's difficult to follow each series consistently. I
> > suggest to focus on a small subset - possibly on a single series at the
> > time.
> > 
> > e.g. The first 2 patches in this series are IMHO ready to be merged
> > [*]. If Mat could apply them, you could follow-up with the remaining
> > bits of this series.
> 
> Sure, I can do that.
> 
> Regarding patch 1/8, do you think we should send that to "-net"? The
> patch looks OK to me but on the other hand, it is not a big issue to
> reset the initial subflow (but not ideal) if we fear regressions due to
> this patch. WDYT?

I think both patch 1 & 2 should go via -net, but I'm not 110% sure
there will be not regressions free. Perhaps we can let stage a bit in
our tree?

> > [*] modulo some expansion to the changelog of patch 1, but that could
> > happen even after merging IMHO.
> 
> Indeed. But we might forget :)
> So if you have any suggestions, do not hesitate to share them :-)

I would re-phrase the commit message roughly as follow:

"""
When closing the first subflow, the MPTCP protocol unconditionally
calls tcp_disconnect(), which in turn generates a reset if the subflow
is established. 

That is unexpected and different from what MPTCP does with MPJ
subflows, where resets are generated only on FASTCLOSE and other edge
scenarios.

We can't reuse for the first subflow the same code in place for MPJ
subflows, as MPTCP clean them up completely via a tcp_close() call,
while must keep the first subflow socket alive for later re-usage, due
to implementation constraints.

This patch adds a new helper __mptcp_subflow_disconnect() that
encapsulates, a logic similar to tcp_close, issuing a reset only when
the MPTCP_CF_FASTCLOSE flag is set, and performing a clean shutdown
otherwise.
"""

Cheers,

Paolo

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

[Matthieu Baerts]

Hi Paolo,

Thank you for your replies!

On 13/10/2023 17:35, Paolo Abeni wrote:
> On Fri, 2023-10-13 at 12:46 +0200, Matthieu Baerts wrote:
>> On 13/10/2023 12:32, Paolo Abeni wrote:
>>> Hi,
>>>
>>> On Fri, 2023-10-13 at 13:46 +0800, Geliang Tang wrote:
>>>> Update __mptcp_has_initial_subflow().
>>>>
>>>> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
>>>> ---
>>>>  net/mptcp/protocol.h | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>>>> index 6508179e94a6..1fb4ac3727c4 100644
>>>> --- a/net/mptcp/protocol.h
>>>> +++ b/net/mptcp/protocol.h
>>>> @@ -1081,7 +1081,7 @@ 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 && inet_sk_state_load(ssk) == TCP_ESTABLISHED;
>>>
>>> I think the above is not correct, __mptcp_has_initial_subflow() will
>>> return false before connect completes and/or for listener sockets.
>>
>> Please note that __mptcp_has_initial_subflow() is there just to count
>> the number of subflows. It is being used with 'msk->pm.subflows' and it
>> is supposed to have the same "behaviour". Then I don't think we should
>> increment the subflow counter for listener sockets, no?
> 
> If the goal is giving an accurate count of the total number of
> subflows, I think we should: the msk listener has 1 subflow: the tcp
> listener.

Good point! I was thinking about subflows that were visible on the wire,
but yes, there is a subflow there.

If we look at 'ss' output, we will also see one TCP subflow (with ulp
mptcp) for this listener socket, so displaying 1 for the total number of
subflows would make more sense.

So yes, we should add TCPF_LISTEN.

>>> You can list explicitly add the valid states with something alike:
>>>
>>> 	(1 << inet_sk_state_load(ssk)) & (TCPF_ESTABLISHED |
>>> TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_LISTEN | TCPF_CLOSE_WAIT)
>>>
>>> I'm unsure if we should include CLOSE_WAIT here: the remote has shut
>>> down, but this end can still send data...
>>
>> Maybe better, no? As long as the behaviour is similar to the one with
>> 'msk->pm.subflows'.
> 
> If we keep the way we account for MPJ subflows as a reference
> CLOSE_WAIT status must be excluded. I agree/now see it's the better
> option.

OK, so no CLOSE_WAIT, right?

>>> Side important note: you are too fast :) There are a lot of in-flight
>>> patches, and it's difficult to follow each series consistently. I
>>> suggest to focus on a small subset - possibly on a single series at the
>>> time.
>>>
>>> e.g. The first 2 patches in this series are IMHO ready to be merged
>>> [*]. If Mat could apply them, you could follow-up with the remaining
>>> bits of this series.
>>
>> Sure, I can do that.
>>
>> Regarding patch 1/8, do you think we should send that to "-net"? The
>> patch looks OK to me but on the other hand, it is not a big issue to
>> reset the initial subflow (but not ideal) if we fear regressions due to
>> this patch. WDYT?
> 
> I think both patch 1 & 2 should go via -net, but I'm not 110% sure
> there will be not regressions free. Perhaps we can let stage a bit in
> our tree?

Sure, good idea!

I can apply these patches now.

>>> [*] modulo some expansion to the changelog of patch 1, but that could
>>> happen even after merging IMHO.
>>
>> Indeed. But we might forget :)
>> So if you have any suggestions, do not hesitate to share them :-)
> 
> I would re-phrase the commit message roughly as follow:
> 
> """
> When closing the first subflow, the MPTCP protocol unconditionally
> calls tcp_disconnect(), which in turn generates a reset if the subflow
> is established. 
> 
> That is unexpected and different from what MPTCP does with MPJ
> subflows, where resets are generated only on FASTCLOSE and other edge
> scenarios.
> 
> We can't reuse for the first subflow the same code in place for MPJ
> subflows, as MPTCP clean them up completely via a tcp_close() call,
> while must keep the first subflow socket alive for later re-usage, due
> to implementation constraints.
> 
> This patch adds a new helper __mptcp_subflow_disconnect() that
> encapsulates, a logic similar to tcp_close, issuing a reset only when
> the MPTCP_CF_FASTCLOSE flag is set, and performing a clean shutdown
> otherwise.
> """

I suggest to also modify the title, something like:

    mptcp: avoid sending RST when closing the initial subflow

And this Fixes tag:

    Fixes: c2b2ae3925b6 ("mptcp: handle correctly disconnect() failures")

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

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

[Paolo Abeni]

On Mon, 2023-10-16 at 13:53 +0200, Matthieu Baerts wrote:
> On 13/10/2023 17:35, Paolo Abeni wrote:
> > On Fri, 2023-10-13 at 12:46 +0200, Matthieu Baerts wrote:
> > > On 13/10/2023 12:32, Paolo Abeni wrote:
> > > 
> > > > You can list explicitly add the valid states with something alike:
> > > > 
> > > > 	(1 << inet_sk_state_load(ssk)) & (TCPF_ESTABLISHED |
> > > > TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_LISTEN | TCPF_CLOSE_WAIT)
> > > > 
> > > > I'm unsure if we should include CLOSE_WAIT here: the remote has shut
> > > > down, but this end can still send data...
> > > 
> > > Maybe better, no? As long as the behaviour is similar to the one with
> > > 'msk->pm.subflows'.
> > 
> > If we keep the way we account for MPJ subflows as a reference
> > CLOSE_WAIT status must be excluded. I agree/now see it's the better
> > option.
> 
> OK, so no CLOSE_WAIT, right?

Exactly, no CLOSE_WAIT

> I suggest to also modify the title, something like:
> 
>     mptcp: avoid sending RST when closing the initial subflow
> 
> And this Fixes tag:
> 
>     Fixes: c2b2ae3925b6 ("mptcp: handle correctly disconnect() failures")

Fine by me.

Thanks!

Paolo

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

[Matthieu Baerts]

Hi Paolo, Geliang,

On 16/10/2023 15:50, Paolo Abeni wrote:
> On Mon, 2023-10-16 at 13:53 +0200, Matthieu Baerts wrote:
>> On 13/10/2023 17:35, Paolo Abeni wrote:
>>> On Fri, 2023-10-13 at 12:46 +0200, Matthieu Baerts wrote:
>>>> On 13/10/2023 12:32, Paolo Abeni wrote:
>>>>
>>>>> You can list explicitly add the valid states with something alike:
>>>>>
>>>>> 	(1 << inet_sk_state_load(ssk)) & (TCPF_ESTABLISHED |
>>>>> TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_LISTEN | TCPF_CLOSE_WAIT)
>>>>>
>>>>> I'm unsure if we should include CLOSE_WAIT here: the remote has shut
>>>>> down, but this end can still send data...
>>>>
>>>> Maybe better, no? As long as the behaviour is similar to the one with
>>>> 'msk->pm.subflows'.
>>>
>>> If we keep the way we account for MPJ subflows as a reference
>>> CLOSE_WAIT status must be excluded. I agree/now see it's the better
>>> option.
>>
>> OK, so no CLOSE_WAIT, right?
> 
> Exactly, no CLOSE_WAIT

@Geliang: do you mind including what Paolo suggested -- without
CLOSE_WAIT -- in the next version?

>> I suggest to also modify the title, something like:
>>
>>     mptcp: avoid sending RST when closing the initial subflow
>>
>> And this Fixes tag:
>>
>>     Fixes: c2b2ae3925b6 ("mptcp: handle correctly disconnect() failures")
> 
> Fine by me.

Great, just applied the patches in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- 6c7131fb5cc7: mptcp: avoid sending RST when closing the initial subflow
- 1f39dcb567ca: selftests: mptcp: join: no RST when rm subflow/addr
- Results: 83a52a5f95c9..2fa503488095 (export-net)
- Results: c0f977cdc194..92a5724f0df6 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20231016T201914
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20231016T201914

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

[PATCH mptcp-next v16 4/8] Squash to "selftests: mptcp: add chk_subflows_total helper"

[Geliang Tang]

Please update the commit log too:

'''
            ss -ti | grep -c tcp-ulp-mptcp.
'''

->

'''
            ss -ti state established | grep -c tcp-ulp-mptcp.
'''

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 07b2c4bfa183..74f40cdf97c3 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1905,8 +1905,8 @@ chk_subflows_total()
 	print_check "$info $1:$2"
 
 	# if not, count the TCP connections that are in fact MPTCP subflows
-	cnt1=$(ss -N $ns1 -ti | grep -c tcp-ulp-mptcp)
-	cnt2=$(ss -N $ns2 -ti | grep -c tcp-ulp-mptcp)
+	cnt1=$(ss -N $ns1 -ti state established | grep -c tcp-ulp-mptcp)
+	cnt2=$(ss -N $ns2 -ti state established | grep -c tcp-ulp-mptcp)
 
 	if [ "$1" != "$cnt1" ] || [ "$2" != "$cnt2" ]; then
 		fail_test "got subflows $cnt1:$cnt2 expected $1:$2"
-- 
2.35.3

[PATCH mptcp-next v16 5/8] selftests: mptcp: userspace pm remove initial subflow

[Geliang Tang]

This patch adds a selftest for userpsace PM to remove the initial
subflow. Use userspace_pm_add_sf() to add a subflow, and pass initial
ip address to userspace_pm_rm_sf() to remove the initial subflow.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 74f40cdf97c3..dcb8464a5f65 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3507,6 +3507,30 @@ userspace_tests()
 		kill_events_pids
 		wait $tests_pid
 	fi
+
+	# userspace pm remove initial subflow
+	if reset_with_events "userspace pm remove initial subflow" &&
+	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
+		set_userspace_pm $ns2
+		pm_nl_set_limits $ns1 0 1
+		speed=10 \
+			run_tests $ns1 $ns2 10.0.1.1 &
+		local tests_pid=$!
+		wait_mpj $ns2
+		userspace_pm_add_sf $ns2 10.0.3.2 20
+		chk_join_nr 1 1 1
+		chk_mptcp_info subflows 1 subflows 1
+		chk_subflows_total 2 2
+		userspace_pm_rm_sf $ns2 10.0.1.2
+		# we don't look at the counter linked to the RM_ADDR but
+		# to the one linked to the subflows that have been removed
+		chk_rm_nr 0 1
+		chk_rst_nr 0 0 invert
+		chk_mptcp_info subflows 1 subflows 1
+		chk_subflows_total 1 1
+		kill_events_pids
+		wait $tests_pid
+	fi
 }
 
 endpoint_tests()
-- 
2.35.3

[PATCH mptcp-next v16 6/8] mptcp: userspace pm send RM_ADDR for ID 0

[Geliang Tang]

This patch adds the ability to send RM_ADDR for local ID 0. Check
whether id 0 address is removed, if not, put id 0 into a removing
list, pass it to mptcp_pm_remove_addr() to remove id 0 address.

There is no reason not to allow the userspace to remove the initial
address (ID 0). This special case was not taken into account not
letting the userspace to delete all addresses as announced.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/379
Fixes: d9a4594edabf ("mptcp: netlink: Add MPTCP_PM_CMD_REMOVE")
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/pm_userspace.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 6b8083650bc1..ea50e694125d 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -211,6 +211,40 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+static int mptcp_userspace_remove_id_zero_address(struct mptcp_sock *msk,
+						  struct genl_info *info)
+{
+	struct mptcp_rm_list list = { .nr = 0 };
+	struct mptcp_subflow_context *subflow;
+	struct sock *sk = (struct sock *)msk;
+	bool has_id_0 = false;
+	int err = -EINVAL;
+
+	lock_sock(sk);
+	mptcp_for_each_subflow(msk, subflow) {
+		if (subflow->local_id == 0) {
+			has_id_0 = true;
+			break;
+		}
+	}
+	if (!has_id_0) {
+		GENL_SET_ERR_MSG(info, "address with id 0 not found");
+		goto remove_err;
+	}
+
+	list.ids[list.nr++] = 0;
+
+	spin_lock_bh(&msk->pm.lock);
+	mptcp_pm_remove_addr(msk, &list);
+	spin_unlock_bh(&msk->pm.lock);
+
+	err = 0;
+
+remove_err:
+	release_sock(sk);
+	return err;
+}
+
 int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
@@ -245,6 +279,11 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 		goto remove_err;
 	}
 
+	if (id_val == 0) {
+		err = mptcp_userspace_remove_id_zero_address(msk, info);
+		goto remove_err;
+	}
+
 	lock_sock(sk);
 
 	list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
-- 
2.35.3

[PATCH mptcp-next v16 7/8] mptcp: userspace pm rename remove_err to out

[Geliang Tang]

The value of 'err' will be not only '-EINVAL', but alse '0' most of the
time. So it's better to rename the lable 'remove_err' to 'out'.

Suggested-by: Matthieu Baerts <matttbe@kernel.org>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/pm_userspace.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index ea50e694125d..cdff3e631d2d 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -276,12 +276,12 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 	if (!mptcp_pm_is_userspace(msk)) {
 		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
-		goto remove_err;
+		goto out;
 	}
 
 	if (id_val == 0) {
 		err = mptcp_userspace_remove_id_zero_address(msk, info);
-		goto remove_err;
+		goto out;
 	}
 
 	lock_sock(sk);
@@ -296,7 +296,7 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	if (!match) {
 		GENL_SET_ERR_MSG(info, "address with specified id not found");
 		release_sock(sk);
-		goto remove_err;
+		goto out;
 	}
 
 	list_move(&match->list, &free_list);
@@ -310,7 +310,7 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	err = 0;
- remove_err:
+out:
 	sock_put(sk);
 	return err;
 }
-- 
2.35.3

[PATCH mptcp-next v16 8/8] selftests: mptcp: userspace pm send RM_ADDR for ID 0

[Geliang Tang]

This patch adds a selftest for userpsace PM to remove id 0 address.
Use userspace_pm_add_addr() helper to add a id 10 address, then use
userspace_pm_rm_addr() helper to remove id 0 address.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index dcb8464a5f65..5d2015f16028 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3531,6 +3531,33 @@ userspace_tests()
 		kill_events_pids
 		wait $tests_pid
 	fi
+
+	# userspace pm send RM_ADDR for ID 0
+	if reset_with_events "userspace pm send RM_ADDR for ID 0" &&
+	   continue_if mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
+		set_userspace_pm $ns1
+		pm_nl_set_limits $ns2 1 1
+		speed=10 \
+			run_tests $ns1 $ns2 10.0.1.1 &
+		local tests_pid=$!
+		wait_mpj $ns1
+		userspace_pm_add_addr $ns1 10.0.2.1 10
+		chk_join_nr 1 1 1
+		chk_add_nr 1 1
+		chk_mptcp_info subflows 1 subflows 1
+		chk_subflows_total 2 2
+		chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
+		userspace_pm_rm_addr $ns1 0
+		wait_rm_addr $ns2 1
+		# we don't look at the counter linked to the subflows that
+		# have been removed but to the one linked to the RM_ADDR
+		chk_rm_nr 1 0 invert
+		chk_rst_nr 0 0 invert
+		chk_mptcp_info subflows 1 subflows 1
+		chk_subflows_total 1 1
+		kill_events_pids
+		wait $tests_pid
+	fi
 }
 
 endpoint_tests()
-- 
2.35.3

Re: selftests: mptcp: userspace pm send RM_ADDR for ID 0: Tests Results

[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):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6399951429697536
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6399951429697536/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4992576546144256
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4992576546144256/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/5555526499565568
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5555526499565568/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6118476452986880
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6118476452986880/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/6458a1f8c548


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-debug

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 (Tessares)

Re: selftests: mptcp: userspace pm send RM_ADDR for ID 0: Tests Results

[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):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6633647055306752
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6633647055306752/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/5155903427575808
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5155903427575808/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4592953474154496
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4592953474154496/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5718853380997120
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5718853380997120/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d7237b02f9dc


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-debug

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 (Tessares)

Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address

[Matthieu Baerts]

Hi Geliang, Paolo,

On 13/10/2023 07:46, Geliang Tang wrote:
> v16:
>  - address Matt's comments in v15.

Thank you for the new version! It looks good to me!

Reviewed-by: Matthieu Baerts <matttbe@kernel.org>

I plan to apply 'mptcp: userspace pm send RM_ADDR for ID 0' to the
"-net" part of the tree. I'm not sure about "mptcp: add
__mptcp_subflow_disconnect helper".

@Paolo: are you still OK with "mptcp: add __mptcp_subflow_disconnect
helper" commit?

Do you think we should send that to "-net"? The patch looks OK to me but
on the other hand, it is not a big issue to reset the initial subflow
(but not ideal). WDYT?

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

Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address

[Matthieu Baerts]

Hi Geliang,

On 13/10/2023 10:31, Matthieu Baerts wrote:
> Hi Geliang, Paolo,
> 
> On 13/10/2023 07:46, Geliang Tang wrote:
>> v16:
>>  - address Matt's comments in v15.
> 
> Thank you for the new version! It looks good to me!
> 
> Reviewed-by: Matthieu Baerts <matttbe@kernel.org>

Mmh, I missed the errors reported by the CI:

not ok 112 - mptcp_join: userspace pm add & remove address
not ok 113 - mptcp_join: userspace pm create destroy subflow
not ok 115 - mptcp_join: userspace pm remove initial subflow
not ok 116 - mptcp_join: userspace pm send RM_ADDR for ID 0

Do you mind looking at that, please?

I wonder if I should not revert some patches (for the moment) or disable
some checks: these errors are visible on other builds, and we are
missing real issues.

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

Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address

[Geliang Tang]

On Fri, Oct 13, 2023 at 11:36:36AM +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 13/10/2023 10:31, Matthieu Baerts wrote:
> > Hi Geliang, Paolo,
> > 
> > On 13/10/2023 07:46, Geliang Tang wrote:
> >> v16:
> >>  - address Matt's comments in v15.
> > 
> > Thank you for the new version! It looks good to me!
> > 
> > Reviewed-by: Matthieu Baerts <matttbe@kernel.org>
> 
> Mmh, I missed the errors reported by the CI:
> 
> not ok 112 - mptcp_join: userspace pm add & remove address
> not ok 113 - mptcp_join: userspace pm create destroy subflow
> not ok 115 - mptcp_join: userspace pm remove initial subflow
> not ok 116 - mptcp_join: userspace pm send RM_ADDR for ID 0
> 
> Do you mind looking at that, please?

I can't reproduce these errors. All tests passed on my side.

I run this command on my Thinkpad laptop:

docker run \
	-e INPUT_BUILD_SKIP_PERF=1 \
	-e INPUT_BUILD_SKIP_PACKETDRILL=1 \
	-v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
	auto-debug

And this is my .virtme-exec-run:

  run_selftest_one ./mptcp_join.sh -u

Every tests passed.

Is there a difference between my test environment and CI?

Thanks,
-Geliang

> 
> I wonder if I should not revert some patches (for the moment) or disable
> some checks: these errors are visible on other builds, and we are
> missing real issues.
> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net

Re: [PATCH mptcp-next v16 0/8] userspace pm remove id 0 subflow & address

[Matthieu Baerts]

Hi Geliang,

On 13/10/2023 13:30, Geliang Tang wrote:
> On Fri, Oct 13, 2023 at 11:36:36AM +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 13/10/2023 10:31, Matthieu Baerts wrote:
>>> Hi Geliang, Paolo,
>>>
>>> On 13/10/2023 07:46, Geliang Tang wrote:
>>>> v16:
>>>>  - address Matt's comments in v15.
>>>
>>> Thank you for the new version! It looks good to me!
>>>
>>> Reviewed-by: Matthieu Baerts <matttbe@kernel.org>
>>
>> Mmh, I missed the errors reported by the CI:
>>
>> not ok 112 - mptcp_join: userspace pm add & remove address
>> not ok 113 - mptcp_join: userspace pm create destroy subflow
>> not ok 115 - mptcp_join: userspace pm remove initial subflow
>> not ok 116 - mptcp_join: userspace pm send RM_ADDR for ID 0
>>
>> Do you mind looking at that, please?
> 
> I can't reproduce these errors. All tests passed on my side.
> 
> I run this command on my Thinkpad laptop:
> 
> docker run \
> 	-e INPUT_BUILD_SKIP_PERF=1 \
> 	-e INPUT_BUILD_SKIP_PACKETDRILL=1 \
> 	-v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
>         --pull always mptcp/mptcp-upstream-virtme-docker:latest \
> 	auto-debug
> 
> And this is my .virtme-exec-run:
> 
>   run_selftest_one ./mptcp_join.sh -u
> 
> Every tests passed.

Thank you for looking at that!

Maybe try to reproduce the issue in a loop using:

  run_loop run_selftest_one ./mptcp_join.sh -u

> Is there a difference between my test environment and CI?

The public CI is very likely a slower environment that what you have on
your side. To help to simulate such environment, when the tests start
(because not needed before), I usually run 'stress-ng' in parallel:

  nproc=$(nproc); nproc2=$((nproc * 2))
  stress-ng --cpu "${nproc2}" --iomix "${nproc2}" --vm "${nproc2}"
--vm-bytes 1G --timeout 60m

If it is not enough, I also change the priority of the VM:

  sudo renice 20 <pid of qemu>

I hope it will help!

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