Message ID | 5c70132fb9512ecc42fea01c3118dc9a292791ab.1690454072.git.aclaudi@redhat.com (mailing list archive) |
---|---|
State | Accepted, archived |
Commit | b2b73765f5219e22aedb4148bd7e2b09ad3350dc |
Delegated to: | Matthieu Baerts |
Headers | show |
Series | selftests: fix mptcp_join test | expand |
Context | Check | Description |
---|---|---|
matttbe/build | success | Build and static analysis OK |
matttbe/checkpatch | warning | total: 0 errors, 2 warnings, 0 checks, 13 lines checked |
matttbe/KVM_Validation__normal__except_selftest_mptcp_join_ | success | Success! ✅ |
matttbe/KVM_Validation__normal__only_selftest_mptcp_join_ | success | Success! ✅ |
matttbe/KVM_Validation__debug__only_selftest_mptcp_join_ | success | Success! ✅ |
matttbe/KVM_Validation__debug__except_selftest_mptcp_join_ | success | Success! ✅ |
Hi Andrea, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal (except selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/6266739109920768 - Summary: https://api.cirrus-ci.com/v1/artifact/task/6266739109920768/summary/summary.txt - KVM Validation: debug (except selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5985264133210112 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5985264133210112/summary/summary.txt - KVM Validation: normal (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/4859364226367488 - Summary: https://api.cirrus-ci.com/v1/artifact/task/4859364226367488/summary/summary.txt - KVM Validation: debug (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5422314179788800 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5422314179788800/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/2243a05ff3d4 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-debug 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 (Tessares)
Hi Andrea, On 27/07/2023 13:08, Andrea Claudi wrote: > mptcp_join 'implicit EP' test currently fails when using ip mptcp: > > $ ./mptcp_join.sh -iI > <snip> > 001 implicit EP creation[fail] expected '10.0.2.2 10.0.2.2 id 1 implicit' found '10.0.2.2 id 1 rawflags 10 ' > Error: too many addresses or duplicate one: -22. > ID change is prevented[fail] expected '10.0.2.2 10.0.2.2 id 1 implicit' found '10.0.2.2 id 1 rawflags 10 ' > modif is allowed[fail] expected '10.0.2.2 10.0.2.2 id 1 signal' found '10.0.2.2 id 1 signal ' > > This happens because of two reasons: > - iproute v6.3.0 does not support the implicit flag, fixed with > iproute2-next commit 3a2535a41854 ("mptcp: add support for implicit > flag") > - pm_nl_check_endpoint wrongly expects the ip address to be repeated two > times in iproute output, and does not account for a final whitespace > in it. > > This fixes the issue trimming the whitespace in the output string and > removing the double address in the expected string. > > Fixes: 34aa6e3bccd8 ("selftests: mptcp: add ip mptcp wrappers") I guess it is not the right one to use, you might want to keep the one you had in v1: Fixes: 69c6ce7b6eca ("selftests: mptcp: add implicit endpoint test case") No? Cheers, Matt
On Thu, Jul 27, 2023 at 06:52:50PM +0200, Matthieu Baerts wrote: > Hi Andrea, > > On 27/07/2023 13:08, Andrea Claudi wrote: > > mptcp_join 'implicit EP' test currently fails when using ip mptcp: > > > > $ ./mptcp_join.sh -iI > > <snip> > > 001 implicit EP creation[fail] expected '10.0.2.2 10.0.2.2 id 1 implicit' found '10.0.2.2 id 1 rawflags 10 ' > > Error: too many addresses or duplicate one: -22. > > ID change is prevented[fail] expected '10.0.2.2 10.0.2.2 id 1 implicit' found '10.0.2.2 id 1 rawflags 10 ' > > modif is allowed[fail] expected '10.0.2.2 10.0.2.2 id 1 signal' found '10.0.2.2 id 1 signal ' > > > > This happens because of two reasons: > > - iproute v6.3.0 does not support the implicit flag, fixed with > > iproute2-next commit 3a2535a41854 ("mptcp: add support for implicit > > flag") > > - pm_nl_check_endpoint wrongly expects the ip address to be repeated two > > times in iproute output, and does not account for a final whitespace > > in it. > > > > This fixes the issue trimming the whitespace in the output string and > > removing the double address in the expected string. > > > > Fixes: 34aa6e3bccd8 ("selftests: mptcp: add ip mptcp wrappers") > > I guess it is not the right one to use, you might want to keep the one > you had in v1: > > Fixes: 69c6ce7b6eca ("selftests: mptcp: add implicit endpoint test case") > > No? Yes, the one in v1 is the correct one. I somehow mixed it up amending the patch, sorry for that. > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net >
Hi Andrea, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal (except selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/6040037498814464 - Summary: https://api.cirrus-ci.com/v1/artifact/task/6040037498814464/summary/summary.txt - KVM Validation: normal (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5477087545393152 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5477087545393152/summary/summary.txt - KVM Validation: debug (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/4773400103616512 - Summary: https://api.cirrus-ci.com/v1/artifact/task/4773400103616512/summary/summary.txt - KVM Validation: debug (except selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5708187203010560 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5708187203010560/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/af37d841a4c2 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-debug 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 (Tessares)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index b972feb9a3a2..4938baa4c5fe 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -769,10 +769,11 @@ pm_nl_check_endpoint() fi if [ $ip_mptcp -eq 1 ]; then + # get line and trim trailing whitespace line=$(ip -n $ns mptcp endpoint show $id) + line="${line% }" # the dump order is: address id flags port dev - expected_line="$addr" - [ -n "$addr" ] && expected_line="$expected_line $addr" + [ -n "$addr" ] && expected_line="$addr" expected_line="$expected_line $id" [ -n "$_flags" ] && expected_line="$expected_line ${_flags//","/" "}" [ -n "$dev" ] && expected_line="$expected_line $dev"
mptcp_join 'implicit EP' test currently fails when using ip mptcp: $ ./mptcp_join.sh -iI <snip> 001 implicit EP creation[fail] expected '10.0.2.2 10.0.2.2 id 1 implicit' found '10.0.2.2 id 1 rawflags 10 ' Error: too many addresses or duplicate one: -22. ID change is prevented[fail] expected '10.0.2.2 10.0.2.2 id 1 implicit' found '10.0.2.2 id 1 rawflags 10 ' modif is allowed[fail] expected '10.0.2.2 10.0.2.2 id 1 signal' found '10.0.2.2 id 1 signal ' This happens because of two reasons: - iproute v6.3.0 does not support the implicit flag, fixed with iproute2-next commit 3a2535a41854 ("mptcp: add support for implicit flag") - pm_nl_check_endpoint wrongly expects the ip address to be repeated two times in iproute output, and does not account for a final whitespace in it. This fixes the issue trimming the whitespace in the output string and removing the double address in the expected string. Fixes: 34aa6e3bccd8 ("selftests: mptcp: add ip mptcp wrappers") Signed-off-by: Andrea Claudi <aclaudi@redhat.com> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)