linux-kernel - Re: [PATCH 5.10 51/84] selftests: mptcp: add ADD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Jul 2022 10:51:24 +0200
From:   Matthieu Baerts <matthieu.baerts@...sares.net>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Sasha Levin <sashal@...nel.org>, stable@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>,
        Geliang Tang <geliangtang@...il.com>,
        Mat Martineau <mathew.j.martineau@...ux.intel.com>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
        MPTCP Upstream <mptcp@...ts.linux.dev>
Subject: Re: [PATCH 5.10 51/84] selftests: mptcp: add ADD_ADDR timeout test
 case

Hi Greg,

On 05/07/2022 18:31, Greg Kroah-Hartman wrote:
> On Tue, Jul 05, 2022 at 05:59:22PM +0200, Matthieu Baerts wrote:
>> Hi Greg, Sasha,
>>
>> (+ MPTCP upstream ML)
>>
>> First, thank you again for maintaining the stable branches!
>>
>> On 05/07/2022 13:58, Greg Kroah-Hartman wrote:
>>> From: Geliang Tang <geliangtang@...il.com>
>>>
>>> [ Upstream commit 8d014eaa9254a9b8e0841df40dd36782b451579a ]
>>>
>>> This patch added the test case for retransmitting ADD_ADDR when timeout
>>> occurs. It set NS1's add_addr_timeout to 1 second, and drop NS2's ADD_ADDR
>>> echo packets.
>> TL;DR: Could it be possible to drop all selftests MPTCP patches from
>> v5.10 queue please?
>>
>>
>> I was initially reacting on this patch because it looks like it depends on:
>>
>>   93f323b9cccc ("mptcp: add a new sysctl add_addr_timeout")
>>
>> and indirectly to:
>>
>>   9ce7deff92e8 ("docs: networking: mptcp: Add MPTCP sysctl entries")
>>
>> to have "net.mptcp.add_addr_timeout" sysctl knob needed for this new
>> selftest.
>>
>> But then I tried to understand why this current patch ("selftests:
>> mptcp: add ADD_ADDR timeout test case") has been selected for 5.10. I
>> guess it was to ease the backport of another one, right?
>> Looking at the 'series' file in 5.10 queue, it seems the new
>> "selftests-mptcp-more-stable-diag-tests" patch requires 5 other patches:
>>
>> -> selftests-mptcp-more-stable-diag-tests.patch
>>  -> selftests-mptcp-fix-diag-instability.patch
>>   -> selftests-mptcp-launch-mptcp_connect-with-timeout.patch
>>    -> selftests-mptcp-add-add_addr-ipv6-test-cases.patch
>>     -> selftests-mptcp-add-link-failure-test-case.patch
>>      -> selftests-mptcp-add-add_addr-timeout-test-case.patch
>>
>>
>> When looking at these patches in more detail, it looks like "selftests:
>> mptcp: add ADD_ADDR IPv6 test cases" depends on a new feature only
>> available from v5.11: ADD_ADDR for IPv6.
>>
>>
>> Could it be possible to drop all these patches from v5.10 then please?
> 
> Sure, but leave them in for 5.15.y and 5.18.y?

(@Mat: Thank you for having replied to this part: yes, please leave them
there)

>> The two recent fixes for the "diag" selftest mainly helps on slow / busy
>> CI. I think it is not worth backporting them to v5.10.
>>
>>
>> (Note that if we want "selftests: mptcp: fix diag instability" patch, we
>> also need 2e580a63b5c2 ("selftests: mptcp: add cfg_do_w for cfg_remove")
>> and the top part of 8da6229b9524 ("selftests: mptcp: timeout testcases
>> for multi addresses"): the list starts to be long.)
>>
>>
>> One last thing: it looks like when Sasha adds patches to a stable queue,
>> a notification is sent to less people than when Greg adds patches. For
>> example here, I have not been notified for this patch when added to the
>> queue while I was one of the reviewers. I already got notifications from
>> Greg when I was a reviewer on other patches.
>> Is it normal? Do you only cc people who signed off on the patch?
> 
> I cc: everyone on the commit, Sasha should also do that but sometimes
> his script acts up.

All good, thank you!

>> It looks like you don't cc maintainers from the MAINTAINERS file but
>> that's probably on purpose. I didn't get cc for all MPTCP patches of the
>> series here but I guess I can always subscribe to 'stable' ML for that.
> 
> No, we don't use the MAINTAINERS file, that would just be too noisy as
> ideally who ever the MAINTAINER was, they already saw this as the commit
> is already in Linus's tree.

I understand, thank you for the explanation.

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