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@xxxxxxxxx>
> >
> > [ 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?
> 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.
> 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.
thanks,
greg k-h
