Re: [PATCH mptcp-net v2 2/6] selftests: mptcp: join: rm: set backup flag

[Matthieu Baerts <matttbe@kernel.org>]

On 04/11/2025 08:29, Geliang Tang wrote:
> Hi Matt,
> 
> On Tue, 2025-11-04 at 07:57 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> Thank you for the review.
>>
>> 4 Nov 2025 07:06:34 Geliang Tang <geliang@kernel.org>:
>>
>>> Hi Matt,
>>>
>>> Thanks for this fix. It works indeed.
>>>
>>> On Sun, 2025-11-02 at 12:30 +0100, Matthieu Baerts (NGI0) wrote:
>>>> Some of these 'remove' tests rarely fail because a subflow has
>>>> been
>>>
>>> In my testing, only the "flush addresses" test case fails
>>> intermittently. I'm wondering which other test cases have failed in
>>> your environment?
>>
>> I took the unstable ones from the Netdev and MPTCP CIs:
>>
>> https://netdev.bots.linux.dev/contest.html?pass=0&executor=vmksft-mptcp-dbg&ld-cases=1
>>
>> https://ci-results.mptcp.dev/flakes.html
>>
>>> I'm thinking we shouldn't add the "backup" flag to every test in
>>> remove_tests, but only to the specific ones that are actually
>>> failing.
>>>
>>> Similarly for patches 3 and 4, we don't need to add
>>> "test_linkfail=128"
>>> to every test. We should only apply it to the specific ones that
>>> are
>>> actually failing.
>>
>> If my analysis is correct, it means the simple fixes I did (backup
>> and
>> longer file size) can be applied to multiple tests. Then probably
>> better
>> to avoid these other tests to fail for the same reasons even if it is
>> very
>> rare, and very hard to reproduce, no? Each false positive takes a lot
>> of
>> resources (mainly time) to debug.
> 
> I'll leave it to you to decide. However, I still believe it's better to
> apply these fixes to the tests one by one, in case we see failures in
> the future.

Because these modifications are not modifying what is being validated in
the test, but only avoid noises that would make the tests failing, I
think it is better to prevent issues and waiting for them to happen.

> I'll respond with my Reviewed-by tag in the cover letter.

Thanks!

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