* [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout
@ 2025-09-04 11:38 Matthieu Baerts (NGI0)
2025-09-04 11:38 ` [PATCH mptcp-next v2 1/3] Squash to "selftests: mptcp: remove add_addr_timeout settings" Matthieu Baerts (NGI0)
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-04 11:38 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts (NGI0), Geliang Tang
The recent patch "mptcp: make ADD_ADDR retransmission timeout adaptive"
seems causing some unstable tests, see the links below.
The modification in the selftest is reverted, and more tolerance is
added instead.
While at it, slow down some signalling tests.
Link: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/17391054770
Link: https://netdev-3.bots.linux.dev/vmksft-mptcp-dbg/results/280241/1-mptcp-join-sh/stdout
Link: https://netdev-3.bots.linux.dev/vmksft-mptcp-dbg/results/279881/1-mptcp-join-sh/stdout
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v2:
- Patch 2: tolerate more ADD_ADDR, except if no ADD_ADDR are expected.
- Add Geliang's RvB tags (thanks!)
- Link to v1: https://lore.kernel.org/r/20250902-sft-mptcp-join-add_addr-no-retrans-v1-0-956a315d6a5a@kernel.org
---
Matthieu Baerts (NGI0) (3):
Squash to "selftests: mptcp: remove add_addr_timeout settings"
selftests: mptcp: join: tolerate more ADD_ADDR
selftests: mptcp: join: allow more time to send ADD_ADDR
tools/testing/selftests/net/mptcp/mptcp_join.sh | 28 ++++++++++++-------------
1 file changed, 14 insertions(+), 14 deletions(-)
---
base-commit: 8984ec428a58d1d82ba89db40e462ad1e8dd317e
change-id: 20250902-sft-mptcp-join-add_addr-no-retrans-345b5db46fac
Best regards,
--
Matthieu Baerts (NGI0) <matttbe@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH mptcp-next v2 1/3] Squash to "selftests: mptcp: remove add_addr_timeout settings" 2025-09-04 11:38 [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout Matthieu Baerts (NGI0) @ 2025-09-04 11:38 ` Matthieu Baerts (NGI0) 2025-09-04 11:38 ` [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR Matthieu Baerts (NGI0) ` (3 subsequent siblings) 4 siblings, 0 replies; 9+ messages in thread From: Matthieu Baerts (NGI0) @ 2025-09-04 11:38 UTC (permalink / raw) To: mptcp; +Cc: Matthieu Baerts (NGI0), Geliang Tang This reverts the patch: when add_addr_timeout is set to 1, chk_add_nr() and chk_add_tx_nr() are more tolerant with retransmissions, see commit 6ef84b1517e0 ("selftests: mptcp: more robust signal race test"). But the main reason is that this revert breaks the selftests running on old kernels. It is then important to keep this. Plus it still acts as a maximum, just in case everything is very slow, we still have retransmissions in time, especially with the new exponential backoff. Reviewed-by: Geliang Tang <geliang@kernel.org> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index a97b568104bc284f050b2f0e09fe3fdd3341c5cb..2f046167a0b6cc6fb5531a033d8d95c9ea399cf9 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -358,6 +358,8 @@ reset_with_add_addr_timeout() tables="${ip6tables}" fi + ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1 + if ! ip netns exec $ns2 $tables -A OUTPUT -p tcp \ -m tcp --tcp-option 30 \ -m bpf --bytecode \ @@ -2303,6 +2305,7 @@ signal_address_tests() pm_nl_add_endpoint $ns2 10.0.4.2 flags signal # the peer could possibly miss some addr notification, allow retransmission + ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1 speed=slow \ run_tests $ns1 $ns2 10.0.1.1 -- 2.51.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR 2025-09-04 11:38 [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout Matthieu Baerts (NGI0) 2025-09-04 11:38 ` [PATCH mptcp-next v2 1/3] Squash to "selftests: mptcp: remove add_addr_timeout settings" Matthieu Baerts (NGI0) @ 2025-09-04 11:38 ` Matthieu Baerts (NGI0) 2025-09-04 14:54 ` Matthieu Baerts 2025-09-04 11:38 ` [PATCH mptcp-next v2 3/3] selftests: mptcp: join: allow more time to send ADD_ADDR Matthieu Baerts (NGI0) ` (2 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Matthieu Baerts (NGI0) @ 2025-09-04 11:38 UTC (permalink / raw) To: mptcp; +Cc: Matthieu Baerts (NGI0) ADD_ADDR can be retransmitted, and with, the parent commit, these retransmissions can be sent quicker: from 2 minutes to less than one second. To avoid false positives where retransmitted ADD_ADDR causes higher counters than expected, it is required to be more tolerant. Errors are now only reported when fewer ADD_ADDRs have been sent/received, except if no ADD_ADDR are expected. Before the parent commit, the tolerance was present for each tests where the ADD_ADDR could be retransmitted in a reasonable time (1 sec). Now that all tests can have retransmitted ADD_ADDR, it is normal to apply the same tolerance for all tests. An alternative could be to disable the ADD_ADDR retransmissions by default, but that's changing the default kernel behaviour. Plus, ADD_ADDR retransmissions can be required for some tests. To avoid adding exceptions to many tests, it seems better to increase the tolerance. Later, we could add a new MIB counter to identify the ADD_ADDR retransmissions, and remove the tolerance when this counter is available. Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 2f046167a0b6cc6fb5531a033d8d95c9ea399cf9..be49712f91155a1f14f6cc21776764d65a897c27 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -358,6 +358,7 @@ reset_with_add_addr_timeout() tables="${ip6tables}" fi + # set a maximum, to avoid too long timeout with exponential backoff ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1 if ! ip netns exec $ns2 $tables -A OUTPUT -p tcp \ @@ -1669,7 +1670,6 @@ chk_add_nr() local tx="" local rx="" local count - local timeout if [[ $ns_invert = "invert" ]]; then ns_tx=$ns2 @@ -1678,15 +1678,13 @@ chk_add_nr() rx=" server" fi - timeout=$(ip netns exec ${ns_tx} sysctl -n net.mptcp.add_addr_timeout) - print_check "add addr rx${rx}" count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtAddAddr") if [ -z "$count" ]; then print_skip - # if the test configured a short timeout tolerate greater then expected - # add addrs options, due to retransmissions - elif [ "$count" != "$add_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_nr" ]; }; then + # Tolerate more ADD_ADDR then expected (if any), due to retransmissions + elif [ "$count" -lt "$add_nr" ] || + { [ "$count" != "$add_nr" ] && [ "$add_nr" != 0 ]; }; then fail_test "got $count ADD_ADDR[s] expected $add_nr" else print_ok @@ -1774,18 +1772,15 @@ chk_add_tx_nr() { local add_tx_nr=$1 local echo_tx_nr=$2 - local timeout local count - timeout=$(ip netns exec $ns1 sysctl -n net.mptcp.add_addr_timeout) - print_check "add addr tx" count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtAddAddrTx") if [ -z "$count" ]; then print_skip - # if the test configured a short timeout tolerate greater then expected - # add addrs options, due to retransmissions - elif [ "$count" != "$add_tx_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_tx_nr" ]; }; then + # Tolerate more ADD_ADDR then expected (if any), due to retransmissions + elif [ "$count" -lt "$add_tx_nr" ] || + { [ "$count" != "$add_tx_nr" ] && [ "$add_tx_nr" != 0 ]; }; then fail_test "got $count ADD_ADDR[s] TX, expected $add_tx_nr" else print_ok -- 2.51.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR 2025-09-04 11:38 ` [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR Matthieu Baerts (NGI0) @ 2025-09-04 14:54 ` Matthieu Baerts 2025-09-05 8:16 ` Geliang Tang 0 siblings, 1 reply; 9+ messages in thread From: Matthieu Baerts @ 2025-09-04 14:54 UTC (permalink / raw) To: MPTCP Linux On 04/09/2025 13:38, Matthieu Baerts (NGI0) wrote: > ADD_ADDR can be retransmitted, and with, the parent commit, these > retransmissions can be sent quicker: from 2 minutes to less than one > second. > > To avoid false positives where retransmitted ADD_ADDR causes higher > counters than expected, it is required to be more tolerant. Errors are > now only reported when fewer ADD_ADDRs have been sent/received, except > if no ADD_ADDR are expected. > > Before the parent commit, the tolerance was present for each tests where > the ADD_ADDR could be retransmitted in a reasonable time (1 sec). Now > that all tests can have retransmitted ADD_ADDR, it is normal to apply > the same tolerance for all tests. > > An alternative could be to disable the ADD_ADDR retransmissions by > default, but that's changing the default kernel behaviour. Plus, > ADD_ADDR retransmissions can be required for some tests. To avoid adding > exceptions to many tests, it seems better to increase the tolerance. > > Later, we could add a new MIB counter to identify the ADD_ADDR > retransmissions, and remove the tolerance when this counter is > available. > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 19 +++++++------------ > 1 file changed, 7 insertions(+), 12 deletions(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index 2f046167a0b6cc6fb5531a033d8d95c9ea399cf9..be49712f91155a1f14f6cc21776764d65a897c27 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -358,6 +358,7 @@ reset_with_add_addr_timeout() > tables="${ip6tables}" > fi > > + # set a maximum, to avoid too long timeout with exponential backoff > ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1 > > if ! ip netns exec $ns2 $tables -A OUTPUT -p tcp \ > @@ -1669,7 +1670,6 @@ chk_add_nr() > local tx="" > local rx="" > local count > - local timeout > > if [[ $ns_invert = "invert" ]]; then > ns_tx=$ns2 > @@ -1678,15 +1678,13 @@ chk_add_nr() > rx=" server" > fi > > - timeout=$(ip netns exec ${ns_tx} sysctl -n net.mptcp.add_addr_timeout) > - > print_check "add addr rx${rx}" > count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtAddAddr") > if [ -z "$count" ]; then > print_skip > - # if the test configured a short timeout tolerate greater then expected > - # add addrs options, due to retransmissions > - elif [ "$count" != "$add_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_nr" ]; }; then > + # Tolerate more ADD_ADDR then expected (if any), due to retransmissions > + elif [ "$count" -lt "$add_nr" ] || > + { [ "$count" != "$add_nr" ] && [ "$add_nr" != 0 ]; }; then Arf, it should be: "$add_nr" == 0 > fail_test "got $count ADD_ADDR[s] expected $add_nr" > else > print_ok > @@ -1774,18 +1772,15 @@ chk_add_tx_nr() > { > local add_tx_nr=$1 > local echo_tx_nr=$2 > - local timeout > local count > > - timeout=$(ip netns exec $ns1 sysctl -n net.mptcp.add_addr_timeout) > - > print_check "add addr tx" > count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtAddAddrTx") > if [ -z "$count" ]; then > print_skip > - # if the test configured a short timeout tolerate greater then expected > - # add addrs options, due to retransmissions > - elif [ "$count" != "$add_tx_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_tx_nr" ]; }; then > + # Tolerate more ADD_ADDR then expected (if any), due to retransmissions > + elif [ "$count" -lt "$add_tx_nr" ] || > + { [ "$count" != "$add_tx_nr" ] && [ "$add_tx_nr" != 0 ]; }; then Same here: "$add_tx_nr" == 0 If there is nothing else, I can fix that when applying the patches. > fail_test "got $count ADD_ADDR[s] TX, expected $add_tx_nr" > else > print_ok > Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR 2025-09-04 14:54 ` Matthieu Baerts @ 2025-09-05 8:16 ` Geliang Tang 2025-09-05 9:52 ` Matthieu Baerts 0 siblings, 1 reply; 9+ messages in thread From: Geliang Tang @ 2025-09-05 8:16 UTC (permalink / raw) To: Matthieu Baerts, MPTCP Linux Hi Matt, Thanks for this new version, and thanks for the explanation in v1. On Thu, 2025-09-04 at 16:54 +0200, Matthieu Baerts wrote: > On 04/09/2025 13:38, Matthieu Baerts (NGI0) wrote: > > ADD_ADDR can be retransmitted, and with, the parent commit, these > > retransmissions can be sent quicker: from 2 minutes to less than > > one > > second. > > > > To avoid false positives where retransmitted ADD_ADDR causes higher > > counters than expected, it is required to be more tolerant. Errors > > are > > now only reported when fewer ADD_ADDRs have been sent/received, > > except > > if no ADD_ADDR are expected. > > > > Before the parent commit, the tolerance was present for each tests > > where > > the ADD_ADDR could be retransmitted in a reasonable time (1 sec). > > Now > > that all tests can have retransmitted ADD_ADDR, it is normal to > > apply > > the same tolerance for all tests. > > > > An alternative could be to disable the ADD_ADDR retransmissions by > > default, but that's changing the default kernel behaviour. Plus, > > ADD_ADDR retransmissions can be required for some tests. To avoid > > adding > > exceptions to many tests, it seems better to increase the > > tolerance. > > > > Later, we could add a new MIB counter to identify the ADD_ADDR > > retransmissions, and remove the tolerance when this counter is > > available. > > > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > > --- > > tools/testing/selftests/net/mptcp/mptcp_join.sh | 19 +++++++------ > > ------ > > 1 file changed, 7 insertions(+), 12 deletions(-) > > > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh > > b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > index > > 2f046167a0b6cc6fb5531a033d8d95c9ea399cf9..be49712f91155a1f14f6cc217 > > 76764d65a897c27 100755 > > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > @@ -358,6 +358,7 @@ reset_with_add_addr_timeout() > > tables="${ip6tables}" > > fi > > > > + # set a maximum, to avoid too long timeout with > > exponential backoff > > ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1 > > > > if ! ip netns exec $ns2 $tables -A OUTPUT -p tcp \ > > @@ -1669,7 +1670,6 @@ chk_add_nr() > > local tx="" > > local rx="" > > local count > > - local timeout > > > > if [[ $ns_invert = "invert" ]]; then > > ns_tx=$ns2 > > @@ -1678,15 +1678,13 @@ chk_add_nr() > > rx=" server" > > fi > > > > - timeout=$(ip netns exec ${ns_tx} sysctl -n > > net.mptcp.add_addr_timeout) > > - > > print_check "add addr rx${rx}" > > count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtAddAddr") > > if [ -z "$count" ]; then > > print_skip > > - # if the test configured a short timeout tolerate greater > > then expected > > - # add addrs options, due to retransmissions > > - elif [ "$count" != "$add_nr" ] && { [ "$timeout" -gt 1 ] > > || [ "$count" -lt "$add_nr" ]; }; then > > + # Tolerate more ADD_ADDR then expected (if any), due to > > retransmissions > > + elif [ "$count" -lt "$add_nr" ] || > > + { [ "$count" != "$add_nr" ] && [ "$add_nr" != 0 ]; }; > > then > > Arf, it should be: > > "$add_nr" == 0 It should indeed be "$add_nr" == 0. nit: Is it possible to keep the same order as before when checking "$timeout", just replace [ "$timeout" -gt 1 ] with [ "$add_nr" == 0 ]? That is: elif [ "$count" != "$add_nr" ] && { [ "$add_nr" == 0 ] || [ "$count" - lt "$add_nr" ]; }; then Although they are actually the same, I usually try to keep it consistent with the original code. > > fail_test "got $count ADD_ADDR[s] expected > > $add_nr" > > else > > print_ok > > @@ -1774,18 +1772,15 @@ chk_add_tx_nr() > > { > > local add_tx_nr=$1 > > local echo_tx_nr=$2 > > - local timeout > > local count > > > > - timeout=$(ip netns exec $ns1 sysctl -n > > net.mptcp.add_addr_timeout) > > - > > print_check "add addr tx" > > count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtAddAddrTx") > > if [ -z "$count" ]; then > > print_skip > > - # if the test configured a short timeout tolerate greater > > then expected > > - # add addrs options, due to retransmissions > > - elif [ "$count" != "$add_tx_nr" ] && { [ "$timeout" -gt 1 > > ] || [ "$count" -lt "$add_tx_nr" ]; }; then > > + # Tolerate more ADD_ADDR then expected (if any), due to > > retransmissions > > + elif [ "$count" -lt "$add_tx_nr" ] || > > + { [ "$count" != "$add_tx_nr" ] && [ "$add_tx_nr" != 0 > > ]; }; then > > Same here: > > "$add_tx_nr" == 0 > > If there is nothing else, I can fix that when applying the patches. No need to send a v3, please fix it when applying it. Reviewed-by: Geliang Tang <geliang@kernel.org> Thanks, -Geliang > > > fail_test "got $count ADD_ADDR[s] TX, expected > > $add_tx_nr" > > else > > print_ok > > > > Cheers, > Matt ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR 2025-09-05 8:16 ` Geliang Tang @ 2025-09-05 9:52 ` Matthieu Baerts 0 siblings, 0 replies; 9+ messages in thread From: Matthieu Baerts @ 2025-09-05 9:52 UTC (permalink / raw) To: Geliang Tang, MPTCP Linux Hi Geliang, On 05/09/2025 10:16, Geliang Tang wrote: > Hi Matt, > > Thanks for this new version, and thanks for the explanation in v1. > > On Thu, 2025-09-04 at 16:54 +0200, Matthieu Baerts wrote: >> On 04/09/2025 13:38, Matthieu Baerts (NGI0) wrote: >>> ADD_ADDR can be retransmitted, and with, the parent commit, these >>> retransmissions can be sent quicker: from 2 minutes to less than >>> one >>> second. >>> >>> To avoid false positives where retransmitted ADD_ADDR causes higher >>> counters than expected, it is required to be more tolerant. Errors >>> are >>> now only reported when fewer ADD_ADDRs have been sent/received, >>> except >>> if no ADD_ADDR are expected. >>> >>> Before the parent commit, the tolerance was present for each tests >>> where >>> the ADD_ADDR could be retransmitted in a reasonable time (1 sec). >>> Now >>> that all tests can have retransmitted ADD_ADDR, it is normal to >>> apply >>> the same tolerance for all tests. >>> >>> An alternative could be to disable the ADD_ADDR retransmissions by >>> default, but that's changing the default kernel behaviour. Plus, >>> ADD_ADDR retransmissions can be required for some tests. To avoid >>> adding >>> exceptions to many tests, it seems better to increase the >>> tolerance. >>> >>> Later, we could add a new MIB counter to identify the ADD_ADDR >>> retransmissions, and remove the tolerance when this counter is >>> available. >>> >>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >>> --- >>> tools/testing/selftests/net/mptcp/mptcp_join.sh | 19 +++++++------ >>> ------ >>> 1 file changed, 7 insertions(+), 12 deletions(-) >>> >>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh >>> b/tools/testing/selftests/net/mptcp/mptcp_join.sh >>> index >>> 2f046167a0b6cc6fb5531a033d8d95c9ea399cf9..be49712f91155a1f14f6cc217 >>> 76764d65a897c27 100755 >>> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh >>> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh >>> @@ -358,6 +358,7 @@ reset_with_add_addr_timeout() >>> tables="${ip6tables}" >>> fi >>> >>> + # set a maximum, to avoid too long timeout with >>> exponential backoff >>> ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1 >>> >>> if ! ip netns exec $ns2 $tables -A OUTPUT -p tcp \ >>> @@ -1669,7 +1670,6 @@ chk_add_nr() >>> local tx="" >>> local rx="" >>> local count >>> - local timeout >>> >>> if [[ $ns_invert = "invert" ]]; then >>> ns_tx=$ns2 >>> @@ -1678,15 +1678,13 @@ chk_add_nr() >>> rx=" server" >>> fi >>> >>> - timeout=$(ip netns exec ${ns_tx} sysctl -n >>> net.mptcp.add_addr_timeout) >>> - >>> print_check "add addr rx${rx}" >>> count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtAddAddr") >>> if [ -z "$count" ]; then >>> print_skip >>> - # if the test configured a short timeout tolerate greater >>> then expected >>> - # add addrs options, due to retransmissions >>> - elif [ "$count" != "$add_nr" ] && { [ "$timeout" -gt 1 ] >>> || [ "$count" -lt "$add_nr" ]; }; then >>> + # Tolerate more ADD_ADDR then expected (if any), due to >>> retransmissions >>> + elif [ "$count" -lt "$add_nr" ] || >>> + { [ "$count" != "$add_nr" ] && [ "$add_nr" != 0 ]; }; >>> then >> >> Arf, it should be: >> >> "$add_nr" == 0 > > It should indeed be "$add_nr" == 0. > > nit: > > Is it possible to keep the same order as before when checking > "$timeout", just replace [ "$timeout" -gt 1 ] with [ "$add_nr" == 0 ]? > > That is: > > elif [ "$count" != "$add_nr" ] && { [ "$add_nr" == 0 ] || [ "$count" - > lt "$add_nr" ]; }; then > > Although they are actually the same, I usually try to keep it > consistent with the original code. Good point, even if I found the check "-lt || (-gt && = 0)" maybe more logical, with the v2, we are closer to the original code, I can do this modification. > >>> fail_test "got $count ADD_ADDR[s] expected >>> $add_nr" >>> else >>> print_ok >>> @@ -1774,18 +1772,15 @@ chk_add_tx_nr() >>> { >>> local add_tx_nr=$1 >>> local echo_tx_nr=$2 >>> - local timeout >>> local count >>> >>> - timeout=$(ip netns exec $ns1 sysctl -n >>> net.mptcp.add_addr_timeout) >>> - >>> print_check "add addr tx" >>> count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtAddAddrTx") >>> if [ -z "$count" ]; then >>> print_skip >>> - # if the test configured a short timeout tolerate greater >>> then expected >>> - # add addrs options, due to retransmissions >>> - elif [ "$count" != "$add_tx_nr" ] && { [ "$timeout" -gt 1 >>> ] || [ "$count" -lt "$add_tx_nr" ]; }; then >>> + # Tolerate more ADD_ADDR then expected (if any), due to >>> retransmissions >>> + elif [ "$count" -lt "$add_tx_nr" ] || >>> + { [ "$count" != "$add_tx_nr" ] && [ "$add_tx_nr" != 0 >>> ]; }; then >> >> Same here: >> >> "$add_tx_nr" == 0 >> >> If there is nothing else, I can fix that when applying the patches. > > No need to send a v3, please fix it when applying it. > > Reviewed-by: Geliang Tang <geliang@kernel.org> Thanks! Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH mptcp-next v2 3/3] selftests: mptcp: join: allow more time to send ADD_ADDR 2025-09-04 11:38 [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout Matthieu Baerts (NGI0) 2025-09-04 11:38 ` [PATCH mptcp-next v2 1/3] Squash to "selftests: mptcp: remove add_addr_timeout settings" Matthieu Baerts (NGI0) 2025-09-04 11:38 ` [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR Matthieu Baerts (NGI0) @ 2025-09-04 11:38 ` Matthieu Baerts (NGI0) 2025-09-04 13:52 ` [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout MPTCP CI 2025-09-05 10:56 ` Matthieu Baerts 4 siblings, 0 replies; 9+ messages in thread From: Matthieu Baerts (NGI0) @ 2025-09-04 11:38 UTC (permalink / raw) To: mptcp; +Cc: Matthieu Baerts (NGI0), Geliang Tang When many ADD_ADDR need to be sent, it can take some time to send each of them, and create new subflows. Some CIs seem to occasionally have issues with these tests, especially with "debug" kernels. Two subtests will now run for a slightly longer time: the last two where 3 or more ADD_ADDR are sent during the test. Reviewed-by: Geliang Tang <geliang@kernel.org> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index be49712f91155a1f14f6cc21776764d65a897c27..dc1f7d5b7fbaf5aa71d95c0d3e0b418cf3916199 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -2268,7 +2268,8 @@ signal_address_tests() pm_nl_add_endpoint $ns1 10.0.3.1 flags signal pm_nl_add_endpoint $ns1 10.0.4.1 flags signal pm_nl_set_limits $ns2 3 3 - run_tests $ns1 $ns2 10.0.1.1 + speed=slow \ + run_tests $ns1 $ns2 10.0.1.1 chk_join_nr 3 3 3 chk_add_nr 3 3 fi @@ -2280,7 +2281,8 @@ signal_address_tests() pm_nl_add_endpoint $ns1 10.0.3.1 flags signal pm_nl_add_endpoint $ns1 10.0.14.1 flags signal pm_nl_set_limits $ns2 3 3 - run_tests $ns1 $ns2 10.0.1.1 + speed=slow \ + run_tests $ns1 $ns2 10.0.1.1 join_syn_tx=3 \ chk_join_nr 1 1 1 chk_add_nr 3 3 -- 2.51.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout 2025-09-04 11:38 [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout Matthieu Baerts (NGI0) ` (2 preceding siblings ...) 2025-09-04 11:38 ` [PATCH mptcp-next v2 3/3] selftests: mptcp: join: allow more time to send ADD_ADDR Matthieu Baerts (NGI0) @ 2025-09-04 13:52 ` MPTCP CI 2025-09-05 10:56 ` Matthieu Baerts 4 siblings, 0 replies; 9+ messages in thread From: MPTCP CI @ 2025-09-04 13:52 UTC (permalink / raw) To: Matthieu Baerts; +Cc: mptcp Hi Matthieu, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal: Unstable: 1 failed test(s): selftest_mptcp_connect 🔴 - KVM Validation: debug: Unstable: 3 failed test(s): packetdrill_add_addr selftest_mptcp_connect_checksum selftest_mptcp_join 🔴 - KVM Validation: btf-normal (only bpftest_all): Success! ✅ - KVM Validation: btf-debug (only bpftest_all): Success! ✅ - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/17463051487 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/7003d852301a Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=998827 If there are some issues, you can reproduce them using the same environment as the one used by the CI thanks to a docker image, e.g.: $ cd [kernel source code] $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \ --pull always mptcp/mptcp-upstream-virtme-docker:latest \ auto-normal For more details: https://github.com/multipath-tcp/mptcp-upstream-virtme-docker Please note that despite all the efforts that have been already done to have a stable tests suite when executed on a public CI like here, it is possible some reported issues are not due to your modifications. Still, do not hesitate to help us improve that ;-) Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (NGI0 Core) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout 2025-09-04 11:38 [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout Matthieu Baerts (NGI0) ` (3 preceding siblings ...) 2025-09-04 13:52 ` [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout MPTCP CI @ 2025-09-05 10:56 ` Matthieu Baerts 4 siblings, 0 replies; 9+ messages in thread From: Matthieu Baerts @ 2025-09-05 10:56 UTC (permalink / raw) To: mptcp; +Cc: Geliang Tang Hi Geliang, On 04/09/2025 13:38, Matthieu Baerts (NGI0) wrote: > The recent patch "mptcp: make ADD_ADDR retransmission timeout adaptive" > seems causing some unstable tests, see the links below. > > The modification in the selftest is reverted, and more tolerance is > added instead. > > While at it, slow down some signalling tests. Thank you for the review! Now in our tree, with the suggested fix: New patches for t/upstream: - a37ad3e9fe07: "squashed" patch 1/3 in "selftests: mptcp: remove add_addr_timeout settings" - 97cd737c96db: selftests: mptcp: join: tolerate more ADD_ADDR - 1782e66db572: selftests: mptcp: join: allow more time to send ADD_ADDR - Results: b908f5af710b..9335a0684b09 (export) Tests are now in progress: - export: https://github.com/multipath-tcp/mptcp_net-next/commit/f109be59e49a8e13d97704bde7c726ff26835034/checks Cheers, Matt -- Sponsored by the NGI0 Core fund. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-09-05 10:56 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-04 11:38 [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout Matthieu Baerts (NGI0) 2025-09-04 11:38 ` [PATCH mptcp-next v2 1/3] Squash to "selftests: mptcp: remove add_addr_timeout settings" Matthieu Baerts (NGI0) 2025-09-04 11:38 ` [PATCH mptcp-next v2 2/3] selftests: mptcp: join: tolerate more ADD_ADDR Matthieu Baerts (NGI0) 2025-09-04 14:54 ` Matthieu Baerts 2025-09-05 8:16 ` Geliang Tang 2025-09-05 9:52 ` Matthieu Baerts 2025-09-04 11:38 ` [PATCH mptcp-next v2 3/3] selftests: mptcp: join: allow more time to send ADD_ADDR Matthieu Baerts (NGI0) 2025-09-04 13:52 ` [PATCH mptcp-next v2 0/3] mptcp: fixes for adaptive ADD_ADDR retransmission timeout MPTCP CI 2025-09-05 10:56 ` Matthieu Baerts
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.