| Message ID | cb461bd9ebbbfda680c2d1fea3bc7467596e3d5d.1627391588.git.geliangtang@xiaomi.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | fullmesh path manager support | expand |
On Tue, 2021-07-27 at 21:24 +0800, Geliang Tang wrote: > From: Geliang Tang <geliangtang@xiaomi.com> > > This patch added the testcases for the fullmesh address flag of the path > manager. > > Reuse the above 10 address numbers for the fullmesh test. > > Signed-off-by: Geliang Tang <geliangtang@xiaomi.com> > --- > .../testing/selftests/net/mptcp/mptcp_join.sh | 57 ++++++++++++++++++- > 1 file changed, 54 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index 937e861e9490..2a27d6240f5b 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -367,7 +367,13 @@ do_transfer() > fi > > if [ $addr_nr_ns2 -gt 0 ]; then > - let add_nr_ns2=addr_nr_ns2 > + if [ $addr_nr_ns2 -gt 10 ]; then > + let add_nr_ns2=addr_nr_ns2-10 > + flags=subflow,fullmesh > + else > + let add_nr_ns2=addr_nr_ns2 > + flags=subflow > + fi > counter=3 > sleep 1 > while [ $add_nr_ns2 -gt 0 ]; do > @@ -377,7 +383,7 @@ do_transfer() > else > addr="10.0.$counter.2" > fi > - ip netns exec $ns2 ./pm_nl_ctl add $addr flags subflow > + ip netns exec $ns2 ./pm_nl_ctl add $addr flags $flags > let counter+=1 > let add_nr_ns2-=1 > done > @@ -1697,6 +1703,46 @@ deny_join_id0_tests() > chk_join_nr "subflow and address allow join id0 2" 1 1 1 > } > > +fullmesh_tests() > +{ > + # fullmesh 1 > + reset > + ip netns exec $ns1 ./pm_nl_ctl limits 0 4 > + ip netns exec $ns2 ./pm_nl_ctl limits 1 4 > + ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow,fullmesh > + ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow,fullmesh > + run_tests $ns1 $ns2 10.0.1.1 0 1 0 slow > + chk_join_nr "fullmesh test 1" 4 4 4 > + chk_add_nr 1 1 > + > + # fullmesh 2 > + reset > + ip netns exec $ns1 ./pm_nl_ctl limits 1 3 > + ip netns exec $ns2 ./pm_nl_ctl limits 1 3 > + ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal > + run_tests $ns1 $ns2 10.0.1.1 0 0 11 slow It still took me a bit follow topology creation here. I think a comment is deserved, the first time this kind of argument is used. Additionally, I suggest to use the "12" as an argument even in the previous test, dropping the explicit endpoint creation, for consistency. All the above could use a squash-to patch, as this is the only minor thing I have to comment for the whole series. Thanks! Paolo
Hi Geliang, Paolo, Thank you for the new version. On 28/07/2021 19:21, Paolo Abeni wrote: > On Tue, 2021-07-27 at 21:24 +0800, Geliang Tang wrote: >> From: Geliang Tang <geliangtang@xiaomi.com> >> >> This patch added the testcases for the fullmesh address flag of the path >> manager. >> >> Reuse the above 10 address numbers for the fullmesh test. >> >> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com> >> --- >> .../testing/selftests/net/mptcp/mptcp_join.sh | 57 ++++++++++++++++++- >> 1 file changed, 54 insertions(+), 3 deletions(-) >> >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh >> index 937e861e9490..2a27d6240f5b 100755 >> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh >> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh >> @@ -367,7 +367,13 @@ do_transfer() >> fi >> >> if [ $addr_nr_ns2 -gt 0 ]; then >> - let add_nr_ns2=addr_nr_ns2 >> + if [ $addr_nr_ns2 -gt 10 ]; then Maybe more explicit if we pass 'fullmesh-X' where X is the number of addresses we need with the fullmesh flag. local flags="subflow" (...) if [[ "${addr_nr_ns2}" = "fullmesh-"* ]]; then flags="${flags},fullmesh" addr_nr_ns2=${addr_nr_ns2:9} fi if [ $addr_nr_ns2 -gt 0 ]; then counter=3 (...) This can also be fixed with a squash-to patch ;) >> + let add_nr_ns2=addr_nr_ns2-10 >> + flags=subflow,fullmesh >> + else >> + let add_nr_ns2=addr_nr_ns2 >> + flags=subflow >> + fi >> counter=3 >> sleep 1 >> while [ $add_nr_ns2 -gt 0 ]; do >> @@ -377,7 +383,7 @@ do_transfer() >> else >> addr="10.0.$counter.2" >> fi >> - ip netns exec $ns2 ./pm_nl_ctl add $addr flags subflow >> + ip netns exec $ns2 ./pm_nl_ctl add $addr flags $flags >> let counter+=1 >> let add_nr_ns2-=1 >> done >> @@ -1697,6 +1703,46 @@ deny_join_id0_tests() >> chk_join_nr "subflow and address allow join id0 2" 1 1 1 >> } >> >> +fullmesh_tests() >> +{ >> + # fullmesh 1 >> + reset >> + ip netns exec $ns1 ./pm_nl_ctl limits 0 4 >> + ip netns exec $ns2 ./pm_nl_ctl limits 1 4 >> + ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow,fullmesh >> + ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow,fullmesh >> + run_tests $ns1 $ns2 10.0.1.1 0 1 0 slow >> + chk_join_nr "fullmesh test 1" 4 4 4 >> + chk_add_nr 1 1 >> + >> + # fullmesh 2 >> + reset >> + ip netns exec $ns1 ./pm_nl_ctl limits 1 3 >> + ip netns exec $ns2 ./pm_nl_ctl limits 1 3 >> + ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal >> + run_tests $ns1 $ns2 10.0.1.1 0 0 11 slow > > It still took me a bit follow topology creation here. I think a comment > is deserved, the first time this kind of argument is used. Good idea! It could be good to also rename the tests: "fullmesh test [1234]" are not very explicit. It could be nice to put the topology there, e.g. "2x2", "2x2 w/ signal"? :) Cheers, Matt
Hi Matt, Paolo, Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年7月29日周四 上午1:41写道： > > Hi Geliang, Paolo, > > Thank you for the new version. > > On 28/07/2021 19:21, Paolo Abeni wrote: > > On Tue, 2021-07-27 at 21:24 +0800, Geliang Tang wrote: > >> From: Geliang Tang <geliangtang@xiaomi.com> > >> > >> This patch added the testcases for the fullmesh address flag of the path > >> manager. > >> > >> Reuse the above 10 address numbers for the fullmesh test. > >> > >> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com> > >> --- > >> .../testing/selftests/net/mptcp/mptcp_join.sh | 57 ++++++++++++++++++- > >> 1 file changed, 54 insertions(+), 3 deletions(-) > >> > >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > >> index 937e861e9490..2a27d6240f5b 100755 > >> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > >> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > >> @@ -367,7 +367,13 @@ do_transfer() > >> fi > >> > >> if [ $addr_nr_ns2 -gt 0 ]; then > >> - let add_nr_ns2=addr_nr_ns2 > >> + if [ $addr_nr_ns2 -gt 10 ]; then > > Maybe more explicit if we pass 'fullmesh-X' where X is the number of > addresses we need with the fullmesh flag. > > local flags="subflow" > > (...) > > if [[ "${addr_nr_ns2}" = "fullmesh-"* ]]; then It doesn't work. addr_nr_ns2 is expected as a integer, we can't pass a string to it. Otherwise, we will get this error: ./mptcp_join.sh: line 370: [: fullmesh-1: integer expression expected. > flags="${flags},fullmesh" > addr_nr_ns2=${addr_nr_ns2:9} > fi > > if [ $addr_nr_ns2 -gt 0 ]; then > counter=3 > (...) > > This can also be fixed with a squash-to patch ;) I have same new changes in this series, so I'll sent a v7. All other comments will be added in v7. Thanks, -Geliang > > >> + let add_nr_ns2=addr_nr_ns2-10 > >> + flags=subflow,fullmesh > >> + else > >> + let add_nr_ns2=addr_nr_ns2 > >> + flags=subflow > >> + fi > >> counter=3 > >> sleep 1 > >> while [ $add_nr_ns2 -gt 0 ]; do > >> @@ -377,7 +383,7 @@ do_transfer() > >> else > >> addr="10.0.$counter.2" > >> fi > >> - ip netns exec $ns2 ./pm_nl_ctl add $addr flags subflow > >> + ip netns exec $ns2 ./pm_nl_ctl add $addr flags $flags > >> let counter+=1 > >> let add_nr_ns2-=1 > >> done > >> @@ -1697,6 +1703,46 @@ deny_join_id0_tests() > >> chk_join_nr "subflow and address allow join id0 2" 1 1 1 > >> } > >> > >> +fullmesh_tests() > >> +{ > >> + # fullmesh 1 > >> + reset > >> + ip netns exec $ns1 ./pm_nl_ctl limits 0 4 > >> + ip netns exec $ns2 ./pm_nl_ctl limits 1 4 > >> + ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow,fullmesh > >> + ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow,fullmesh > >> + run_tests $ns1 $ns2 10.0.1.1 0 1 0 slow > >> + chk_join_nr "fullmesh test 1" 4 4 4 > >> + chk_add_nr 1 1 > >> + > >> + # fullmesh 2 > >> + reset > >> + ip netns exec $ns1 ./pm_nl_ctl limits 1 3 > >> + ip netns exec $ns2 ./pm_nl_ctl limits 1 3 > >> + ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal > >> + run_tests $ns1 $ns2 10.0.1.1 0 0 11 slow > > > > It still took me a bit follow topology creation here. I think a comment > > is deserved, the first time this kind of argument is used. > > Good idea! > > It could be good to also rename the tests: "fullmesh test [1234]" are > not very explicit. > It could be nice to put the topology there, e.g. "2x2", "2x2 w/ signal"? :) > > 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 937e861e9490..2a27d6240f5b 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -367,7 +367,13 @@ do_transfer() fi if [ $addr_nr_ns2 -gt 0 ]; then - let add_nr_ns2=addr_nr_ns2 + if [ $addr_nr_ns2 -gt 10 ]; then + let add_nr_ns2=addr_nr_ns2-10 + flags=subflow,fullmesh + else + let add_nr_ns2=addr_nr_ns2 + flags=subflow + fi counter=3 sleep 1 while [ $add_nr_ns2 -gt 0 ]; do @@ -377,7 +383,7 @@ do_transfer() else addr="10.0.$counter.2" fi - ip netns exec $ns2 ./pm_nl_ctl add $addr flags subflow + ip netns exec $ns2 ./pm_nl_ctl add $addr flags $flags let counter+=1 let add_nr_ns2-=1 done @@ -1697,6 +1703,46 @@ deny_join_id0_tests() chk_join_nr "subflow and address allow join id0 2" 1 1 1 } +fullmesh_tests() +{ + # fullmesh 1 + reset + ip netns exec $ns1 ./pm_nl_ctl limits 0 4 + ip netns exec $ns2 ./pm_nl_ctl limits 1 4 + ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow,fullmesh + ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow,fullmesh + run_tests $ns1 $ns2 10.0.1.1 0 1 0 slow + chk_join_nr "fullmesh test 1" 4 4 4 + chk_add_nr 1 1 + + # fullmesh 2 + reset + ip netns exec $ns1 ./pm_nl_ctl limits 1 3 + ip netns exec $ns2 ./pm_nl_ctl limits 1 3 + ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal + run_tests $ns1 $ns2 10.0.1.1 0 0 11 slow + chk_join_nr "fullmesh test 2" 3 3 3 + chk_add_nr 1 1 + + # fullmesh 3 + reset + ip netns exec $ns1 ./pm_nl_ctl limits 2 5 + ip netns exec $ns2 ./pm_nl_ctl limits 1 5 + ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal + run_tests $ns1 $ns2 10.0.1.1 0 0 12 slow + chk_join_nr "fullmesh test 3" 5 5 5 + chk_add_nr 1 1 + + # fullmesh 4 + reset + ip netns exec $ns1 ./pm_nl_ctl limits 2 4 + ip netns exec $ns2 ./pm_nl_ctl limits 1 4 + ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal + run_tests $ns1 $ns2 10.0.1.1 0 0 12 slow + chk_join_nr "fullmesh test 4" 4 4 4 + chk_add_nr 1 1 +} + all_tests() { subflows_tests @@ -1712,6 +1758,7 @@ all_tests() syncookies_tests checksum_tests deny_join_id0_tests + fullmesh_tests } usage() @@ -1730,6 +1777,7 @@ usage() echo " -k syncookies_tests" echo " -S checksum_tests" echo " -d deny_join_id0_tests" + echo " -m fullmesh_tests" echo " -c capture pcap files" echo " -C enable data checksum" echo " -h help" @@ -1765,7 +1813,7 @@ if [ $do_all_tests -eq 1 ]; then exit $ret fi -while getopts 'fsltra64bpkdchCS' opt; do +while getopts 'fsltra64bpkdmchCS' opt; do case $opt in f) subflows_tests @@ -1806,6 +1854,9 @@ while getopts 'fsltra64bpkdchCS' opt; do d) deny_join_id0_tests ;; + m) + fullmesh_tests + ;; c) ;; C)