All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Baerts <matthieu.baerts@tessares.net>
To: mptcp@lists.linux.dev
Cc: Yonglong Li <liyonglong@chinatelecom.cn>,
	Matthieu Baerts <matthieu.baerts@tessares.net>
Subject: [PATCH mptcp-next 3/3] selftests: mptcp: avoid non-const global variables
Date: Mon, 19 Apr 2021 22:33:55 +0200	[thread overview]
Message-ID: <20210419203355.3937162-4-matthieu.baerts@tessares.net> (raw)
In-Reply-To: <20210419203355.3937162-1-matthieu.baerts@tessares.net>

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


  parent reply	other threads:[~2021-04-19 20:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210419203355.3937162-4-matthieu.baerts@tessares.net \
    --to=matthieu.baerts@tessares.net \
    --cc=liyonglong@chinatelecom.cn \
    --cc=mptcp@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.