* [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back"
@ 2021-07-21 8:56 Geliang Tang
2021-07-22 8:55 ` Matthieu Baerts
0 siblings, 1 reply; 5+ messages in thread
From: Geliang Tang @ 2021-07-21 8:56 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
Drop the debug logs and make the output aligned:
link ns1eth3 down
01 multiple flows, signal, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale[ ok ]
Created /tmp/tmp.dltHx8oBcT (size 2048 KB) containing data sent by server
link ns1eth3 down
02 multiple flows, signal, bidirectional, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale[ ok ]
link ns1eth1 down
03 backup subflow unused with link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
link usage [ ok ]
link ns1eth1 down
link ns1eth2 down
04 backup subflow used due to multiple links failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale[ ok ]
link usage [ ok ]
link ns1eth1 down
link ns1eth2 down
05 backup subflow in use, bidirectional, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale[ ok ]
link usage [ ok ]
->
01 multiple flows, signal, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale [ ok ]
Created /tmp/tmp.JYR5OkgZ9C (size 6144 KB) containing data sent by server
02 multi flows, signal, bidi, link fail syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale [ ok ]
03 backup subflow unused, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
link usage [ ok ]
04 backup flow used, multi links fail syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale [ ok ]
link usage [ ok ]
05 backup flow used, bidi, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
add[ ok ] - echo [ ok ]
stale [ ok ]
link usage [ ok ]
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
tools/testing/selftests/net/mptcp/mptcp_join.sh | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 220154cb92a7..d0518487b874 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -228,7 +228,6 @@ link_failure()
for l in $FAILING_LINKS; do
veth="ns1eth$l"
- echo "link $veth down" 1>&2
ip -net "$ns" link set "$veth" down
done
}
@@ -634,7 +633,7 @@ chk_stale_nr()
local stale_nr
local recover_nr
- printf "%-39s %s" " " "stale"
+ printf "%-39s %s" " " "stale "
stale_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowStale | awk '{print $2}'`
[ -z "$stale_nr" ] && stale_nr=0
recover_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowRecover | awk '{print $2}'`
@@ -878,7 +877,7 @@ chk_link_usage()
local tx_rate=$((tx_link * 100 / $tx_total))
local tolerance=5
- printf "%-39s %s" " " "link usage "
+ printf "%-39s %s" " " "link usage "
if [ $tx_rate -lt $((expected_rate - $tolerance)) -o \
$tx_rate -gt $((expected_rate + $tolerance)) ]; then
echo "[fail] got $tx_rate% usage, expected $expected_rate%"
@@ -1039,7 +1038,7 @@ link_failure_tests()
ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 dev ns2eth3 flags subflow
ip netns exec $ns2 ./pm_nl_ctl add 10.0.4.2 dev ns2eth4 flags subflow
run_tests $ns1 $ns2 10.0.1.1 2
- chk_join_nr "multiple flows, signal, bidirectional, link failure" 3 3 3
+ chk_join_nr "multi flows, signal, bidi, link fail" 3 3 3
chk_add_nr 1 1
chk_stale_nr $ns2 1 -1 1
@@ -1053,7 +1052,7 @@ link_failure_tests()
export FAILING_LINKS="1"
ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 dev ns2eth3 flags subflow,backup
run_tests $ns1 $ns2 10.0.1.1 1
- chk_join_nr "backup subflow unused with link failure" 2 2 2
+ chk_join_nr "backup subflow unused, link failure" 2 2 2
chk_add_nr 1 1
chk_link_usage $ns2 ns2eth3 $cinsent 0
@@ -1067,7 +1066,7 @@ link_failure_tests()
ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 dev ns2eth3 flags subflow,backup
export FAILING_LINKS="1 2"
run_tests $ns1 $ns2 10.0.1.1 1
- chk_join_nr "backup subflow used due to multiple links failure" 2 2 2
+ chk_join_nr "backup flow used, multi links fail" 2 2 2
chk_add_nr 1 1
chk_stale_nr $ns2 2 4 2
chk_link_usage $ns2 ns2eth3 $cinsent 50
@@ -1081,7 +1080,7 @@ link_failure_tests()
ip netns exec $ns2 ./pm_nl_ctl limits 1 3
ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 dev ns2eth3 flags subflow,backup
run_tests $ns1 $ns2 10.0.1.1 2
- chk_join_nr "backup subflow in use, bidirectional, link failure" 2 2 2
+ chk_join_nr "backup flow used, bidi, link failure" 2 2 2
chk_add_nr 1 1
chk_stale_nr $ns2 1 -1 2
chk_link_usage $ns2 ns2eth3 $cinsent 50
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back"
2021-07-21 8:56 [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back" Geliang Tang
@ 2021-07-22 8:55 ` Matthieu Baerts
2021-07-22 9:28 ` Geliang Tang
2021-07-22 9:29 ` Matthieu Baerts
0 siblings, 2 replies; 5+ messages in thread
From: Matthieu Baerts @ 2021-07-22 8:55 UTC (permalink / raw)
To: Geliang Tang, mptcp
Hi Geliang,
On 21/07/2021 10:56, Geliang Tang wrote:
> Drop the debug logs and make the output aligned:
Good idea!
> 05 backup flow used, bidi, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
> add[ ok ] - echo [ ok ]
> stale [ ok ]
> link usage [ ok ]
I wonder if it would not be even clearer to display one item per line, e.g.:
05 backup flow used, bidi, link failure:
syn [ ok ]
synack [ ok ]
ack [ ok ]
add [ ok ]
echo [ ok ]
stale [ ok ]
link usage [ ok ]
06 (...)
But that is for another patch anyway. I can look at that later when
trying to make this test more reliable by removing fixed values for
"sleep" and others if nobody is already looking at that.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 220154cb92a7..d0518487b874 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -228,7 +228,6 @@ link_failure()
>
> for l in $FAILING_LINKS; do
> veth="ns1eth$l"
> - echo "link $veth down" 1>&2
> ip -net "$ns" link set "$veth" down
> done
> }
> @@ -634,7 +633,7 @@ chk_stale_nr()
> local stale_nr
> local recover_nr
>
> - printf "%-39s %s" " " "stale"
> + printf "%-39s %s" " " "stale "
> stale_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowStale | awk '{print $2}'`
> [ -z "$stale_nr" ] && stale_nr=0
> recover_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowRecover | awk '{print $2}'`
> @@ -878,7 +877,7 @@ chk_link_usage()
> local tx_rate=$((tx_link * 100 / $tx_total))
> local tolerance=5
>
> - printf "%-39s %s" " " "link usage "
> + printf "%-39s %s" " " "link usage "
Do you mind if I change this when applying the patch to avoid plenty of
spaces?
printf "%-39s %-18s" " " "stale"
printf "%-39s %-18s" " " "link usage"
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back"
2021-07-22 8:55 ` Matthieu Baerts
@ 2021-07-22 9:28 ` Geliang Tang
2021-07-22 12:50 ` Matthieu Baerts
2021-07-22 9:29 ` Matthieu Baerts
1 sibling, 1 reply; 5+ messages in thread
From: Geliang Tang @ 2021-07-22 9:28 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
Hi Matt,
Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年7月22日周四 下午4:55写道:
>
> Hi Geliang,
>
> On 21/07/2021 10:56, Geliang Tang wrote:
> > Drop the debug logs and make the output aligned:
>
> Good idea!
>
> > 05 backup flow used, bidi, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
> > add[ ok ] - echo [ ok ]
> > stale [ ok ]
> > link usage [ ok ]
>
> I wonder if it would not be even clearer to display one item per line, e.g.:
>
>
> 05 backup flow used, bidi, link failure:
> syn [ ok ]
> synack [ ok ]
> ack [ ok ]
> add [ ok ]
> echo [ ok ]
> stale [ ok ]
> link usage [ ok ]
> 06 (...)
>
I think the original display is much better than one item per line. Since
it put the related items in one line, and do the different tests in each
line. I think that's more clearer than one item per line. Do you agree?
> But that is for another patch anyway. I can look at that later when
> trying to make this test more reliable by removing fixed values for
> "sleep" and others if nobody is already looking at that.
>
> >
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> > ---
> > tools/testing/selftests/net/mptcp/mptcp_join.sh | 13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index 220154cb92a7..d0518487b874 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -228,7 +228,6 @@ link_failure()
> >
> > for l in $FAILING_LINKS; do
> > veth="ns1eth$l"
> > - echo "link $veth down" 1>&2
> > ip -net "$ns" link set "$veth" down
> > done
> > }
> > @@ -634,7 +633,7 @@ chk_stale_nr()
> > local stale_nr
> > local recover_nr
> >
> > - printf "%-39s %s" " " "stale"
> > + printf "%-39s %s" " " "stale "
> > stale_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowStale | awk '{print $2}'`
> > [ -z "$stale_nr" ] && stale_nr=0
> > recover_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowRecover | awk '{print $2}'`
> > @@ -878,7 +877,7 @@ chk_link_usage()
> > local tx_rate=$((tx_link * 100 / $tx_total))
> > local tolerance=5
> >
> > - printf "%-39s %s" " " "link usage "
> > + printf "%-39s %s" " " "link usage "
>
> Do you mind if I change this when applying the patch to avoid plenty of
> spaces?
>
> printf "%-39s %-18s" " " "stale"
> printf "%-39s %-18s" " " "link usage"
It's much better to avoid spaces like this, thanks Matt.
-Geliang
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back"
2021-07-22 8:55 ` Matthieu Baerts
2021-07-22 9:28 ` Geliang Tang
@ 2021-07-22 9:29 ` Matthieu Baerts
1 sibling, 0 replies; 5+ messages in thread
From: Matthieu Baerts @ 2021-07-22 9:29 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
Hi Geliang,
On 22/07/2021 10:55, Matthieu Baerts wrote:
> On 21/07/2021 10:56, Geliang Tang wrote:
>> Drop the debug logs and make the output aligned:
(...)
>> @@ -878,7 +877,7 @@ chk_link_usage()
>> local tx_rate=$((tx_link * 100 / $tx_total))
>> local tolerance=5
>>
>> - printf "%-39s %s" " " "link usage "
>> + printf "%-39s %s" " " "link usage "
>
> Do you mind if I change this when applying the patch to avoid plenty of
> spaces?
Thank you for the patch! Just applied with this additional small
modification.
- bef6bd8eb873: "squashed" in "selftests: mptcp: add testcase for
active-back"
- 8135e0128562: "Signed-off-by" + "Co-developed-by"
- 85c4567fba08: selftests: mptcp join: avoid multiple spaces
- Results: 3a1215522891..1180d21b724d
Builds and tests are now in progress:
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210722T092910
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210722T092910
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back"
2021-07-22 9:28 ` Geliang Tang
@ 2021-07-22 12:50 ` Matthieu Baerts
0 siblings, 0 replies; 5+ messages in thread
From: Matthieu Baerts @ 2021-07-22 12:50 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
Hi Geliang,
On 22/07/2021 11:28, Geliang Tang wrote:
> Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年7月22日周四 下午4:55写道:
>> 05 backup flow used, bidi, link failure:
>> syn [ ok ]
>> synack [ ok ]
>> ack [ ok ]
>> add [ ok ]
>> echo [ ok ]
>> stale [ ok ]
>> link usage [ ok ]
>> 06 (...)
>>
>
> I think the original display is much better than one item per line. Since
> it put the related items in one line, and do the different tests in each
> line. I think that's more clearer than one item per line. Do you agree?
If everything was on one line, ideally with one global result at the
end, that would be fine for me. But here, when you have a long list of
tests with multiple '[ ok ]' on the same line but in different columns,
it is "hard" to spot the errors I think.
Here you can see the output when it fails:
https://api.cirrus-ci.com/v1/artifact/task/5542024628666368/tap_result/selftest_mptcp_join.tap
There are three columns and I think it is not "direct" to spot where is
the error.
If you have: plenty of "[ ok ]" with nothing after, it is easy to spot a
"[FAIL]", especially if something more is written on the right of it I
think.
An alternative would be to have one global "[ OK ]", e.g.
03 backup subflow unused, link failure: [ OK ]
04 backup flow used, multi links fail: [FAIL]
syn [ ok ]
synack [ ok ]
ack [ ok ]
add [ ok ]
echo [FAIL] # extra info
stale [ ok ]
link usage [ ok ]
05 backup flow used, bidi, link failure: [ OK ]
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-22 12:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 8:56 [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back" Geliang Tang
2021-07-22 8:55 ` Matthieu Baerts
2021-07-22 9:28 ` Geliang Tang
2021-07-22 12:50 ` Matthieu Baerts
2021-07-22 9:29 ` Matthieu Baerts
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).