[PATCH mptcp-next 0/2] selftests: mptcp: some improvement for selftests

[PATCH mptcp-next 0/2] selftests: mptcp: some improvement for selftests

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

This series contains two independent selftest fixes. Both are about
making failures correctly handled instead of being silently masked or
leaving resources behind. They are unrelated and can be applied/merged
independently.

Gang Yan (2):
  selftests: mptcp: run fail_test in main shell during join test
  selftests: mptcp: simult_flows: set EXIT trap earlier

 tools/testing/selftests/net/mptcp/mptcp_join.sh   | 4 ++--
 tools/testing/selftests/net/mptcp/simult_flows.sh | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH mptcp-next 1/2] selftests: mptcp: run fail_test in main shell during join test

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

check_transfer() compares the input and output files byte-by-byte using
`cmp -l "$in" "$out" | while read ...`. Because the while-loop body runs
in a subshell (the script sets neither lastpipe nor pipefail), the
fail_test call inside it -- which sets the global ret/last_test_failed --
and the `return 1` both act on the subshell, not on check_transfer().
check_transfer() thus always falls through to `return 0`, and any data
corruption affecting only the payload (leaving the subflow/PM counters
untouched) is silently reported as PASS.

Assisted-by: Codex: GLM-5.2
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 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 550a6b6117a9..2413c832af03 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -584,7 +584,7 @@ check_transfer()
 		mv "$tmpfile" "$out"
 		tmpfile=""
 	fi
-	cmp -l "$in" "$out" | while read -r i a b; do
+	while read -r i a b; do
 		local sum=$((0${a} + 0${b}))
 		if [ $check_invert -eq 0 ] || [ $sum -ne $((0xff)) ]; then
 			fail_test "$what does not match (in, out):"
@@ -595,7 +595,7 @@ check_transfer()
 		else
 			print_info "$what has inverted byte at ${i}"
 		fi
-	done
+	done < <(cmp -l "$in" "$out")
 
 	return 0
 }
-- 
2.43.0

Re: [PATCH mptcp-next 1/2] selftests: mptcp: run fail_test in main shell during join test

[Matthieu Baerts]

Hi Gang,

On 24/07/2026 08:52, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> check_transfer() compares the input and output files byte-by-byte using
> `cmp -l "$in" "$out" | while read ...`. Because the while-loop body runs
> in a subshell (the script sets neither lastpipe nor pipefail), the
> fail_test call inside it -- which sets the global ret/last_test_failed --
> and the `return 1` both act on the subshell, not on check_transfer().
> check_transfer() thus always falls through to `return 0`, and any data
> corruption affecting only the payload (leaving the subflow/PM counters
> untouched) is silently reported as PASS.

Good catch!

This looks like a fix that should be backported. Do you have a Fixes tag
to add here?

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

Re: [PATCH mptcp-next 1/2] selftests: mptcp: run fail_test in main shell during join test

[gang.yan]

July 24, 2026 at 5:28 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:


> 
> Hi Gang,
> 
> On 24/07/2026 08:52, Gang Yan wrote:
> 
> > 
> > From: Gang Yan <yangang@kylinos.cn>
> >  
> >  check_transfer() compares the input and output files byte-by-byte using
> >  `cmp -l "$in" "$out" | while read ...`. Because the while-loop body runs
> >  in a subshell (the script sets neither lastpipe nor pipefail), the
> >  fail_test call inside it -- which sets the global ret/last_test_failed --
> >  and the `return 1` both act on the subshell, not on check_transfer().
> >  check_transfer() thus always falls through to `return 0`, and any data
> >  corruption affecting only the payload (leaving the subflow/PM counters
> >  untouched) is silently reported as PASS.
> > 
> Good catch!
> 
> This looks like a fix that should be backported. Do you have a Fixes tag
> to add here?

Hi Matt,

Sorry for that, I originally thought only commits touching ./net/mptcp needed
a Fixes tag. This fix should be:

Fixes: 8117dac3e7c3 ("selftests: mptcp: add invert check in check_transfer")

Thanks
Gang


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

Re: [PATCH mptcp-next 1/2] selftests: mptcp: run fail_test in main shell during join test

[Matthieu Baerts]

On 24/07/2026 11:39, gang.yan@linux.dev wrote:
> July 24, 2026 at 5:28 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
> 
> 
>>
>> Hi Gang,
>>
>> On 24/07/2026 08:52, Gang Yan wrote:
>>
>>>
>>> From: Gang Yan <yangang@kylinos.cn>
>>>  
>>>  check_transfer() compares the input and output files byte-by-byte using
>>>  `cmp -l "$in" "$out" | while read ...`. Because the while-loop body runs
>>>  in a subshell (the script sets neither lastpipe nor pipefail), the
>>>  fail_test call inside it -- which sets the global ret/last_test_failed --
>>>  and the `return 1` both act on the subshell, not on check_transfer().
>>>  check_transfer() thus always falls through to `return 0`, and any data
>>>  corruption affecting only the payload (leaving the subflow/PM counters
>>>  untouched) is silently reported as PASS.
>>>
>> Good catch!
>>
>> This looks like a fix that should be backported. Do you have a Fixes tag
>> to add here?
> 
> Hi Matt,
> 
> Sorry for that, I originally thought only commits touching ./net/mptcp needed
> a Fixes tag. This fix should be:

"Important" selftests fixes don't need to wait by going to the next
version. If they fix something that can help users and/or CI, they are
as essential as fixes in net/mptcp.

We should just avoid fixes that don't affect results, e.g. the second
patch of this series. But that's also valid for fixes in net/mptcp, e.g.
a fix for a condition that can never happen with the current code, but
will be the case with pending new features.

> Fixes: 8117dac3e7c3 ("selftests: mptcp: add invert check in check_transfer")

Thanks!

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

Re: [PATCH mptcp-next 1/2] selftests: mptcp: run fail_test in main shell during join test

[gang.yan]

July 24, 2026 at 5:48 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:


> 
> On 24/07/2026 11:39, gang.yan@linux.dev wrote:
> 
> > 
> > July 24, 2026 at 5:28 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
> >  
> >  
> > 
> > > 
> > > Hi Gang,
> > > 
> > >  On 24/07/2026 08:52, Gang Yan wrote:
> > > 
> >  From: Gang Yan <yangang@kylinos.cn>
> >  
> >  check_transfer() compares the input and output files byte-by-byte using
> >  `cmp -l "$in" "$out" | while read ...`. Because the while-loop body runs
> >  in a subshell (the script sets neither lastpipe nor pipefail), the
> >  fail_test call inside it -- which sets the global ret/last_test_failed --
> >  and the `return 1` both act on the subshell, not on check_transfer().
> >  check_transfer() thus always falls through to `return 0`, and any data
> >  corruption affecting only the payload (leaving the subflow/PM counters
> >  untouched) is silently reported as PASS.
> > 
> > > 
> > > Good catch!
> > > 
> > >  This looks like a fix that should be backported. Do you have a Fixes tag
> > >  to add here?
> > > 
> >  
> >  Hi Matt,
> >  
> >  Sorry for that, I originally thought only commits touching ./net/mptcp needed
> >  a Fixes tag. This fix should be:
> > 
> "Important" selftests fixes don't need to wait by going to the next
> version. If they fix something that can help users and/or CI, they are
> as essential as fixes in net/mptcp.
> 
> We should just avoid fixes that don't affect results, e.g. the second
> patch of this series. But that's also valid for fixes in net/mptcp, e.g.
> a fix for a condition that can never happen with the current code, but
> will be the case with pending new features.
> 

Thank you for your kind reminder. :) 

Cherrs
Gang

> > 
> > Fixes: 8117dac3e7c3 ("selftests: mptcp: add invert check in check_transfer")
> > 
> Thanks!
> 
> Cheers,
> Matt
> -- 
> Sponsored by the NGI0 Core fund.
>

Re: [PATCH mptcp-next 1/2] selftests: mptcp: run fail_test in main shell during join test

[Matthieu Baerts]

Hi Gang,

On 24/07/2026 08:52, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> check_transfer() compares the input and output files byte-by-byte using
> `cmp -l "$in" "$out" | while read ...`. Because the while-loop body runs
> in a subshell (the script sets neither lastpipe nor pipefail), the
> fail_test call inside it -- which sets the global ret/last_test_failed --
> and the `return 1` both act on the subshell, not on check_transfer().
> check_transfer() thus always falls through to `return 0`, and any data
> corruption affecting only the payload (leaving the subflow/PM counters
> untouched) is silently reported as PASS.

Thanks, now in our tree (I changed the commit title, I hope that's OK)

New patches for t/upstream-net and t/upstream:
- 3e32d738332f: selftests: mptcp: join: mark tests with data corruption
as failed
- Results: c00da97f2a8c..14dbb670bb6e (export-net)
- Results: 95ac470148fe..ddf9f880a057 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/2c0ef2a0cdbea15d71a70a858397322358520133/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/fdfb144e9842f54cc7d121e468375e2b91aa8575/checks

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

[PATCH mptcp-next 2/2] selftests: mptcp: simult_flows: set EXIT trap earlier

[Gang Yan]

From: Gang Yan <yangang@kylinos.cn>

Set the EXIT trap for cleanup immediately after creating the temporary
file variables, before the dd commands that fill them and the namespace
init, to ensure cleanup runs on any failure or interruption during the
early setup phase. If either dd fails (e.g. ENOSPC), the temp files
already created are not cleaned up.

Assisted-by: Codex: GLM-5.2
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/simult_flows.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 3ea3d1efe32e..b40dea5f9b30 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -62,13 +62,12 @@ setup()
 	sout=$(mktemp)
 	cout=$(mktemp)
 	capout=$(mktemp)
+	trap cleanup EXIT
 	size=$((2 * 2048 * 4096))
 
 	dd if=/dev/zero of=$small bs=4096 count=20 >/dev/null 2>&1
 	dd if=/dev/zero of=$large bs=4096 count=$((size / 4096)) >/dev/null 2>&1
 
-	trap cleanup EXIT
-
 	mptcp_lib_ns_init ns1 ns2 ns3
 
 	if $capture; then
-- 
2.43.0

Re: [PATCH mptcp-next 2/2] selftests: mptcp: simult_flows: set EXIT trap earlier

[Matthieu Baerts]

Hi Gang,

On 24/07/2026 08:52, Gang Yan wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> Set the EXIT trap for cleanup immediately after creating the temporary
> file variables, before the dd commands that fill them and the namespace
> init, to ensure cleanup runs on any failure or interruption during the
> early setup phase. If either dd fails (e.g. ENOSPC), the temp files
> already created are not cleaned up.

From what I see, if one of the 'dd' command fails, the errors are simply
ignored.

> Assisted-by: Codex: GLM-5.2
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/simult_flows.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index 3ea3d1efe32e..b40dea5f9b30 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -62,13 +62,12 @@ setup()
>  	sout=$(mktemp)
>  	cout=$(mktemp)
>  	capout=$(mktemp)
> +	trap cleanup EXIT
>  	size=$((2 * 2048 * 4096))
>  
>  	dd if=/dev/zero of=$small bs=4096 count=20 >/dev/null 2>&1
>  	dd if=/dev/zero of=$large bs=4096 count=$((size / 4096)) >/dev/null 2>&1

This 'fix' is only interesting when the script is abort while one of the
dd command is in progress, to create files of 80k and 16M in RAM. I
guess this is very unlikely, no?

If you already managed to end-up in this situation by accident, OK to
accept the change, but the commit message needs to be updated.

If it is a potential fix for an unlikely situation with the selftests,
we probably don't need that.

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

Re: [PATCH mptcp-next 2/2] selftests: mptcp: simult_flows: set EXIT trap earlier

[gang.yan]

July 24, 2026 at 5:27 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:


> 
> Hi Gang,
> 
> On 24/07/2026 08:52, Gang Yan wrote:
> 
> > 
> > From: Gang Yan <yangang@kylinos.cn>
> >  
> >  Set the EXIT trap for cleanup immediately after creating the temporary
> >  file variables, before the dd commands that fill them and the namespace
> >  init, to ensure cleanup runs on any failure or interruption during the
> >  early setup phase. If either dd fails (e.g. ENOSPC), the temp files
> >  already created are not cleaned up.
> > 
> From what I see, if one of the 'dd' command fails, the errors are simply
> ignored.
> 
> > 
> > Assisted-by: Codex: GLM-5.2
> >  Signed-off-by: Gang Yan <yangang@kylinos.cn>
> >  ---
> >  tools/testing/selftests/net/mptcp/simult_flows.sh | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >  
> >  diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> >  index 3ea3d1efe32e..b40dea5f9b30 100755
> >  --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> >  +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> >  @@ -62,13 +62,12 @@ setup()
> >  sout=$(mktemp)
> >  cout=$(mktemp)
> >  capout=$(mktemp)
> >  + trap cleanup EXIT
> >  size=$((2 * 2048 * 4096))
> >  
> >  dd if=/dev/zero of=$small bs=4096 count=20 >/dev/null 2>&1
> >  dd if=/dev/zero of=$large bs=4096 count=$((size / 4096)) >/dev/null 2>&1
> > 
> This 'fix' is only interesting when the script is abort while one of the
> dd command is in progress, to create files of 80k and 16M in RAM. I
> guess this is very unlikely, no?

Hi Matt,

This change was suggested by AI, referencing commit c8da80af2838 ("selftests: mptcp:
sockopt: set EXIT trap earlier").

I agree the problematic case is unlikely to hit in practice. My main motivation is
to keep the selftest scripts consistent.

If left unfixed, someone might spot this later and send another similar patch,
bringing extra review overhead in the future.

But I am still okay dropping this patch.

Cheers,
Gang


> 
> If you already managed to end-up in this situation by accident, OK to
> accept the change, but the commit message needs to be updated.
> 
> If it is a potential fix for an unlikely situation with the selftests,
> we probably don't need that.
> 
> Cheers,
> Matt
> -- 
> Sponsored by the NGI0 Core fund.
>

Re: [PATCH mptcp-next 2/2] selftests: mptcp: simult_flows: set EXIT trap earlier

[Matthieu Baerts]

On 24/07/2026 11:52, gang.yan@linux.dev wrote:
> July 24, 2026 at 5:27 PM, "Matthieu Baerts" <matttbe@kernel.org mailto:matttbe@kernel.org?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
> 
> 
>>
>> Hi Gang,
>>
>> On 24/07/2026 08:52, Gang Yan wrote:
>>
>>>
>>> From: Gang Yan <yangang@kylinos.cn>
>>>  
>>>  Set the EXIT trap for cleanup immediately after creating the temporary
>>>  file variables, before the dd commands that fill them and the namespace
>>>  init, to ensure cleanup runs on any failure or interruption during the
>>>  early setup phase. If either dd fails (e.g. ENOSPC), the temp files
>>>  already created are not cleaned up.
>>>
>> From what I see, if one of the 'dd' command fails, the errors are simply
>> ignored.
>>
>>>
>>> Assisted-by: Codex: GLM-5.2
>>>  Signed-off-by: Gang Yan <yangang@kylinos.cn>
>>>  ---
>>>  tools/testing/selftests/net/mptcp/simult_flows.sh | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>  
>>>  diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
>>>  index 3ea3d1efe32e..b40dea5f9b30 100755
>>>  --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
>>>  +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
>>>  @@ -62,13 +62,12 @@ setup()
>>>  sout=$(mktemp)
>>>  cout=$(mktemp)
>>>  capout=$(mktemp)
>>>  + trap cleanup EXIT
>>>  size=$((2 * 2048 * 4096))
>>>  
>>>  dd if=/dev/zero of=$small bs=4096 count=20 >/dev/null 2>&1
>>>  dd if=/dev/zero of=$large bs=4096 count=$((size / 4096)) >/dev/null 2>&1
>>>
>> This 'fix' is only interesting when the script is abort while one of the
>> dd command is in progress, to create files of 80k and 16M in RAM. I
>> guess this is very unlikely, no?
> 
> Hi Matt,
> 
> This change was suggested by AI, referencing commit c8da80af2838 ("selftests: mptcp:
> sockopt: set EXIT trap earlier").

This commit caused some issues when upstreaming it, and that's one of
the reasons why I reacted here.

Still, this other one was more likely, because the netns init can be
slow on debug kernels.

> I agree the problematic case is unlikely to hit in practice. My main motivation is
> to keep the selftest scripts consistent.
> 
> If left unfixed, someone might spot this later and send another similar patch,
> bringing extra review overhead in the future.
> 
> But I am still okay dropping this patch.
At Netconf, there were discussions suggesting not to take fixes for
issues that cannot be reproduced. I agree that here, it is small
possible to have the issue, but that's in the selftests and harmless.
Because of that, I think it is best to avoid such patches, as there are
already too much traffic on netdev, and little time to review more
important fixes and features. So if that's OK for you, let's drop it.

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

Re: [PATCH mptcp-next 0/2] selftests: mptcp: some improvement for selftests

[MPTCP CI]

Hi Gang,

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! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/30074599252

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


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)