| Message ID | 70e1c8044096af86ed19ee5b4068dd8ce15aad30.1687522138.git.aclaudi@redhat.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: fix mptcp_join test | expand |
Hi Andrea, On 23/06/2023 14:19, Andrea Claudi wrote: > mptcp_join '001 implicit EP' test currently fails because of two > reasons: Same as on patch 1/2: can you remove "001" and mention it is only failing when using "ip mptcp" ("-i" option) please? > - iproute v6.3.0 does not support the implicit flag, fixed with > iproute2-next commit 3a2535a41854 ("mptcp: add support for implicit > flag") Thank you for that! Out of curiosity: why is it in iproute2-next (following net-next tree, for v6.5) and not in iproute2 tree (following -net / Linus tree: for v6.4)? > - 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: 69c6ce7b6eca ("selftests: mptcp: add implicit endpoint test case") > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > --- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index 5424dcacfffa..6c3525e42273 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -768,10 +768,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" It looks good, no need to change anything here but later (not for -net anyway), we should probably parse the JSON output of "ip -j mptcp" instead. Cheers, Matt
On Mon, Jun 26, 2023 at 01:32:17PM +0200, Matthieu Baerts wrote: > Hi Andrea, > > On 23/06/2023 14:19, Andrea Claudi wrote: > > mptcp_join '001 implicit EP' test currently fails because of two > > reasons: > > Same as on patch 1/2: can you remove "001" and mention it is only > failing when using "ip mptcp" ("-i" option) please? > Sure, no problem. > > - iproute v6.3.0 does not support the implicit flag, fixed with > > iproute2-next commit 3a2535a41854 ("mptcp: add support for implicit > > flag") > > Thank you for that! > > Out of curiosity: why is it in iproute2-next (following net-next tree, > for v6.5) and not in iproute2 tree (following -net / Linus tree: for v6.4)? > I usually target fixes to iproute2 and new stuff to iproute2-next, no other reason than that. But I see your point here, having this on -net may end up in the commit not landing in the same release cycle. Should I send v2 for this series to mptcp-next, then? Andrea > > - 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: 69c6ce7b6eca ("selftests: mptcp: add implicit endpoint test case") > > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > > --- > > tools/testing/selftests/net/mptcp/mptcp_join.sh | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > index 5424dcacfffa..6c3525e42273 100755 > > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > @@ -768,10 +768,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" > > It looks good, no need to change anything here but later (not for -net > anyway), we should probably parse the JSON output of "ip -j mptcp" instead. > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net >
Hi Andrea, Thank you for your replies! On 27/06/2023 15:43, Andrea Claudi wrote: > On Mon, Jun 26, 2023 at 01:32:17PM +0200, Matthieu Baerts wrote: >> On 23/06/2023 14:19, Andrea Claudi wrote: (...) >> Out of curiosity: why is it in iproute2-next (following net-next tree, >> for v6.5) and not in iproute2 tree (following -net / Linus tree: for v6.4)? >> > > I usually target fixes to iproute2 and new stuff to iproute2-next, no > other reason than that. But I see your point here, having this on -net > may end up in the commit not landing in the same release cycle. I see, thank you for the explanation. If I'm not mistaken, a big difference with how the 'net' tree is handled -- i.e. only bug fixes -- 'iproute2' tree accepts new features as long as the kernel using the 'net' tree supports these new features. If it depends on features only in 'net-next', then the patches should target 'iproute2-next'. > Should I send v2 for this series to mptcp-next, then? Yes please, only to the MPTCP list without netdev list and maintainers if you don't mind, just to avoid bothering too many people with MPTCP specific stuff :) Cheers, Matt
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 5424dcacfffa..6c3525e42273 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -768,10 +768,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 '001 implicit EP' test currently fails 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: 69c6ce7b6eca ("selftests: mptcp: add implicit endpoint test case") Signed-off-by: Andrea Claudi <aclaudi@redhat.com> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)