| Message ID | 927493b7ba79d647668e95a34007f48e87c0992a.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 '002 delete and re-add' test currently fails in the 'after > delete' testcase. I guess it only fails if you use "-i" option to use "ip mptcp" instead of "pm_nl_ctl", right? MPTCP CI doesn't launch the tests with the "-i" option. Can you mention that it fails only when using "ip mptcp" which is not the default mode please? It might be good to include that in the title too not to think the test is broken and the CI didn't complain about that. BTW, how did you launch mptcp_join.sh selftest to have this test launched as second position ("002")? With "-Ii"? (you can remove this "002": it is specific to the way you launched the test, not using the default mode) > This happens because endpoint delete includes an ip address while id is > not 0, contrary to what is indicated in the ip mptcp man page: > > "When used with the delete id operation, an IFADDR is only included when > the ID is 0." > > This fixes the issue simply not using the $addr variable in > pm_nl_del_endpoint(). If you do that, are you not going to break other tests? e.g. - "remove id 0 subflow" - "remove id 0 address" (I didn't check all possibilities, maybe not or maybe there are others) Because if you specify the ID 0, you do need to specify the address, no? > Fixes: 34aa6e3bccd8 ("selftests: mptcp: add ip mptcp wrappers") > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > --- > tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index 0ae8cafde439..5424dcacfffa 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -678,7 +678,7 @@ pm_nl_del_endpoint() > local addr=$3 > > if [ $ip_mptcp -eq 1 ]; then > - ip -n $ns mptcp endpoint delete id $id $addr > + ip -n $ns mptcp endpoint delete id $id Should you not add "${addr}" only if ${id} == 1? > else > ip netns exec $ns ./pm_nl_ctl del $id $addr > fi Cheers, Matt
On Mon, Jun 26, 2023 at 01:31:07PM +0200, Matthieu Baerts wrote: > Hi Andrea, Hi Matthieu, Thanks for your review. > > On 23/06/2023 14:19, Andrea Claudi wrote: > > mptcp_join '002 delete and re-add' test currently fails in the 'after > > delete' testcase. > > I guess it only fails if you use "-i" option to use "ip mptcp" instead > of "pm_nl_ctl", right? > Yes, exactly. > MPTCP CI doesn't launch the tests with the "-i" option. > > Can you mention that it fails only when using "ip mptcp" which is not > the default mode please? It might be good to include that in the title > too not to think the test is broken and the CI didn't complain about that. > Sure, will do that. > BTW, how did you launch mptcp_join.sh selftest to have this test > launched as second position ("002")? With "-Ii"? > Yes, that's exactly the case, I use "./mptcp_join.sh -I -i" > (you can remove this "002": it is specific to the way you launched the > test, not using the default mode) Will do. > > > This happens because endpoint delete includes an ip address while id is > > not 0, contrary to what is indicated in the ip mptcp man page: > > > > "When used with the delete id operation, an IFADDR is only included when > > the ID is 0." > > > > This fixes the issue simply not using the $addr variable in > > pm_nl_del_endpoint(). > > If you do that, are you not going to break other tests? e.g. > - "remove id 0 subflow" > - "remove id 0 address" > > (I didn't check all possibilities, maybe not or maybe there are others) > > Because if you specify the ID 0, you do need to specify the address, no? > That's right, of course. I'll fix that in v2 and make sure no other tests are impacted with a "mptcp_join.sh -i" run. > > Fixes: 34aa6e3bccd8 ("selftests: mptcp: add ip mptcp wrappers") > > Signed-off-by: Andrea Claudi <aclaudi@redhat.com> > > --- > > tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > index 0ae8cafde439..5424dcacfffa 100755 > > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > > @@ -678,7 +678,7 @@ pm_nl_del_endpoint() > > local addr=$3 > > > > if [ $ip_mptcp -eq 1 ]; then > > - ip -n $ns mptcp endpoint delete id $id $addr > > + ip -n $ns mptcp endpoint delete id $id > > Should you not add "${addr}" only if ${id} == 1? > > > else > > ip netns exec $ns ./pm_nl_ctl del $id $addr > > fi > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net >
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 0ae8cafde439..5424dcacfffa 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -678,7 +678,7 @@ pm_nl_del_endpoint() local addr=$3 if [ $ip_mptcp -eq 1 ]; then - ip -n $ns mptcp endpoint delete id $id $addr + ip -n $ns mptcp endpoint delete id $id else ip netns exec $ns ./pm_nl_ctl del $id $addr fi
mptcp_join '002 delete and re-add' test currently fails in the 'after delete' testcase. This happens because endpoint delete includes an ip address while id is not 0, contrary to what is indicated in the ip mptcp man page: "When used with the delete id operation, an IFADDR is only included when the ID is 0." This fixes the issue simply not using the $addr variable in pm_nl_del_endpoint(). Fixes: 34aa6e3bccd8 ("selftests: mptcp: add ip mptcp wrappers") Signed-off-by: Andrea Claudi <aclaudi@redhat.com> --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)