Re: [PATCH 5.10 51/84] selftests: mptcp: add ADD_ADDR timeout test case

[Matthieu Baerts]

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?


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?

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.


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