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

On 24/07/2026 11:52, [email protected] wrote:
> July 24, 2026 at 5:27 PM, "Matthieu Baerts" <[email protected] mailto:[email protected]?to=%22Matthieu%20Baerts%22%20%3Cmatttbe%40kernel.org%3E > wrote:
> 
> 
>>
>> Hi Gang,
>>
>> On 24/07/2026 08:52, Gang Yan wrote:
>>
>>>
>>> From: Gang Yan <[email protected]>
>>>  
>>>  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 <[email protected]>
>>>  ---
>>>  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.