* [PATCH mptcp-next 1/3] selftests: mptcp: use 2 different 'peek' modes
2021-04-19 20:33 [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK" Matthieu Baerts
@ 2021-04-19 20:33 ` Matthieu Baerts
2021-04-19 20:33 ` [PATCH mptcp-next 2/3] selftests: mptcp: same NS for Peek tests in IPv6 Matthieu Baerts
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2021-04-19 20:33 UTC (permalink / raw)
To: mptcp; +Cc: Yonglong Li, Matthieu Baerts
When refactoring the code around (see next patches), I noticed the same
'peek mode' was used twice.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 319d1662f4fb..28530f1bd285 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -740,7 +740,7 @@ done
testpeek=true
options_log=true
-peekmode="saveAfterPeek"
+peekmode="saveWithPeek"
run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
run_tests_lo "$ns1" $sender dead:beef:1::1 1
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH mptcp-next 2/3] selftests: mptcp: same NS for Peek tests in IPv6
2021-04-19 20:33 [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK" Matthieu Baerts
2021-04-19 20:33 ` [PATCH mptcp-next 1/3] selftests: mptcp: use 2 different 'peek' modes Matthieu Baerts
@ 2021-04-19 20:33 ` Matthieu Baerts
2021-04-19 20:33 ` [PATCH mptcp-next 3/3] selftests: mptcp: avoid non-const global variables Matthieu Baerts
2021-04-19 22:28 ` [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK" Mat Martineau
3 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2021-04-19 20:33 UTC (permalink / raw)
To: mptcp; +Cc: Yonglong Li, Matthieu Baerts
Also when refactoring the code around (see the next patch), I noticed we
were using different NS for the 'peek mode' tests in IPv6: $sender was
set in a 'for' loop before. Certainly coming from a copy-paste and not
what we want to have.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 28530f1bd285..dbb4aea8b636 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -742,12 +742,12 @@ testpeek=true
options_log=true
peekmode="saveWithPeek"
run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
-run_tests_lo "$ns1" $sender dead:beef:1::1 1
+run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1
options_log=true
peekmode="saveAfterPeek"
run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
-run_tests_lo "$ns1" $sender dead:beef:1::1 1
+run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1
time_end=$(date +%s)
time_run=$((time_end-time_start))
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH mptcp-next 3/3] selftests: mptcp: avoid non-const global variables
2021-04-19 20:33 [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK" Matthieu Baerts
2021-04-19 20:33 ` [PATCH mptcp-next 1/3] selftests: mptcp: use 2 different 'peek' modes Matthieu Baerts
2021-04-19 20:33 ` [PATCH mptcp-next 2/3] selftests: mptcp: same NS for Peek tests in IPv6 Matthieu Baerts
@ 2021-04-19 20:33 ` Matthieu Baerts
2021-04-19 22:28 ` [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK" Mat Martineau
3 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2021-04-19 20:33 UTC (permalink / raw)
To: mptcp; +Cc: Yonglong Li, Matthieu Baerts
In these scripts, we have "global variables" that are only set once when
parsing the config. That's fine, they are const.
It is often not recommended to use non-const global variables for
various reasons.
Here, we were changing the behaviour of a function by changing the value
of a global var just before calling this function and reset the global
var after for the next tests. Even if we are in a Bash script, best to
avoid this because that will certainly cause issues later.
Now we pass extra args for mptcp_connect directly through the functions
we call.
Also by avoiding code duplication, it allowed me to detect 2 issues, see
the previous patches.
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
.../selftests/net/mptcp/mptcp_connect.sh | 44 +++++++++----------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index dbb4aea8b636..9236609731b1 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -23,8 +23,6 @@ rcvbuf=0
options_log=true
do_tcp=0
filesize=0
-peekmode=""
-testpeek=false
if [ $tc_loss -eq 100 ];then
tc_loss=1%
@@ -377,7 +375,7 @@ do_transfer()
local srv_proto="$4"
local connect_addr="$5"
local local_addr="$6"
- local extra_args=""
+ local extra_args="$7"
local port
port=$((10000+$TEST_COUNT))
@@ -395,14 +393,10 @@ do_transfer()
extra_args="$extra_args -m $testmode"
fi
- if $testpeek; then
- extra_args="$extra_args -P $peekmode"
- fi
-
if [ -n "$extra_args" ] && $options_log; then
- options_log=false
echo "INFO: extra options: $extra_args"
fi
+ options_log=false
:> "$cout"
:> "$sout"
@@ -595,6 +589,7 @@ run_tests_lo()
local connector_ns="$2"
local connect_addr="$3"
local loopback="$4"
+ local extra_args="$5"
local lret=0
# skip if test programs are running inside same netns for subsequent runs.
@@ -614,7 +609,8 @@ run_tests_lo()
local_addr="0.0.0.0"
fi
- do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP ${connect_addr} ${local_addr}
+ do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP \
+ ${connect_addr} ${local_addr} "${extra_args}"
lret=$?
if [ $lret -ne 0 ]; then
ret=$lret
@@ -628,14 +624,16 @@ run_tests_lo()
fi
fi
- do_transfer ${listener_ns} ${connector_ns} MPTCP TCP ${connect_addr} ${local_addr}
+ do_transfer ${listener_ns} ${connector_ns} MPTCP TCP \
+ ${connect_addr} ${local_addr} "${extra_args}"
lret=$?
if [ $lret -ne 0 ]; then
ret=$lret
return 1
fi
- do_transfer ${listener_ns} ${connector_ns} TCP MPTCP ${connect_addr} ${local_addr}
+ do_transfer ${listener_ns} ${connector_ns} TCP MPTCP \
+ ${connect_addr} ${local_addr} "${extra_args}"
lret=$?
if [ $lret -ne 0 ]; then
ret=$lret
@@ -643,7 +641,8 @@ run_tests_lo()
fi
if [ $do_tcp -gt 1 ] ;then
- do_transfer ${listener_ns} ${connector_ns} TCP TCP ${connect_addr} ${local_addr}
+ do_transfer ${listener_ns} ${connector_ns} TCP TCP \
+ ${connect_addr} ${local_addr} "${extra_args}"
lret=$?
if [ $lret -ne 0 ]; then
ret=$lret
@@ -659,6 +658,15 @@ run_tests()
run_tests_lo $1 $2 $3 0
}
+run_tests_peekmode()
+{
+ local peekmode="$1"
+
+ echo "INFO: with peek mode: ${peekmode}"
+ run_tests_lo "$ns1" "$ns1" 10.0.1.1 1 "-P ${peekmode}"
+ run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1 "-P ${peekmode}"
+}
+
make_file "$cin" "client"
make_file "$sin" "server"
@@ -738,16 +746,8 @@ for sender in $ns1 $ns2 $ns3 $ns4;do
run_tests "$ns4" $sender dead:beef:3::1
done
-testpeek=true
-options_log=true
-peekmode="saveWithPeek"
-run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
-run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1
-
-options_log=true
-peekmode="saveAfterPeek"
-run_tests_lo "$ns1" "$ns1" 10.0.1.1 1
-run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1
+run_tests_peekmode "saveWithPeek"
+run_tests_peekmode "saveAfterPeek"
time_end=$(date +%s)
time_run=$((time_end-time_start))
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK"
2021-04-19 20:33 [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK" Matthieu Baerts
` (2 preceding siblings ...)
2021-04-19 20:33 ` [PATCH mptcp-next 3/3] selftests: mptcp: avoid non-const global variables Matthieu Baerts
@ 2021-04-19 22:28 ` Mat Martineau
2021-04-20 7:48 ` Matthieu Baerts
3 siblings, 1 reply; 6+ messages in thread
From: Mat Martineau @ 2021-04-19 22:28 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp, Yonglong Li
On Mon, 19 Apr 2021, Matthieu Baerts wrote:
> I started this because I prefer avoiding setting a global variable before
> calling a function and reset it later in order to take a specific path in the
> called function. While at it, I also reduced duplicated code.
>
> But good thing, I also fixed two unwanted behaviours, see patches 1/3 and 2/3.
>
> Matthieu Baerts (3):
> selftests: mptcp: use 2 different 'peek' modes
> selftests: mptcp: same NS for Peek tests in IPv6
> selftests: mptcp: avoid non-const global variables
>
> .../selftests/net/mptcp/mptcp_connect.sh | 44 +++++++++----------
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> --
> 2.30.2
Hi Matthieu -
These look good to squash, thanks for the fixups!
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK"
2021-04-19 22:28 ` [PATCH mptcp-next 0/3] Squash to "selftests: mptcp: add a test case for MSG_PEEK" Mat Martineau
@ 2021-04-20 7:48 ` Matthieu Baerts
0 siblings, 0 replies; 6+ messages in thread
From: Matthieu Baerts @ 2021-04-20 7:48 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp, Yonglong Li
Hi Mat,
On 20/04/2021 00:28, Mat Martineau wrote:
> On Mon, 19 Apr 2021, Matthieu Baerts wrote:
>
>> I started this because I prefer avoiding setting a global variable before
>> calling a function and reset it later in order to take a specific path
>> in the
>> called function. While at it, I also reduced duplicated code.
>>
>> But good thing, I also fixed two unwanted behaviours, see patches 1/3
>> and 2/3.
>
> Hi Matthieu -
>
> These look good to squash, thanks for the fixups!
Thank you for your review!
I just added these patches in our tree and I added your RvB tag to the
patch "selftests: mptcp: add a test case for MSG_PEEK". I hope that's
alright. I can remove it if you prefer. I guess you will replace it with
your SoB when sending them anyway :)
- 56b3332ce650: "squashed" patch 1/3 in "selftests: mptcp: add a test
case for MSG_PEEK"
- 2c2ca30de1d6: "Signed-off-by" + "Co-developed-by"
- 7d6c15c30c4c: "squashed" patch 2/3 in "selftests: mptcp: add a test
case for MSG_PEEK"
- 739a1c5fe1d6: "squashed" patch 3/3 in "selftests: mptcp: add a test
case for MSG_PEEK"
- ebcf2d3b0e42: tg:msg: add Mat's RvB tag
- Results: 04c5ff771be8..e69889ce0094
Tests + export are in progress!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 6+ messages in thread