All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] selftests: Add debugging options to pmtu.sh
@ 2019-04-04  1:18 David Ahern
  2019-04-04 12:16 ` Stefano Brivio
  0 siblings, 1 reply; 3+ messages in thread
From: David Ahern @ 2019-04-04  1:18 UTC (permalink / raw)
  To: davem; +Cc: netdev, sbrivio, David Ahern

From: David Ahern <dsahern@gmail.com>

pmtu.sh script runs a number of tests and dumps a summary of pass/fail.
If a test fails, it is near impossible to debug why. For example:

    TEST: ipv6: PMTU exceptions                       [FAIL]

There are a lot of commands run behind the scenes for this test. Which
one is failing?

Add a VERBOSE option to show commands that are run and any output from
those commands. Add a PAUSE_ON_FAIL option to halt the script if a test
fails allowing users to poke around with the setup in the failed state.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/testing/selftests/net/pmtu.sh | 215 +++++++++++++++++++++---------------
 1 file changed, 127 insertions(+), 88 deletions(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 912b2dc50be3..28e8c97b5c9e 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -116,6 +116,9 @@
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
 
+PAUSE_ON_FAIL=no
+VERBOSE=0
+
 # Some systems don't have a ping6 binary anymore
 which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
 
@@ -222,6 +225,26 @@ err_flush() {
 	err_buf=
 }
 
+run_cmd() {
+	local cmd="$*"
+	local out
+	local stderr="2>/dev/null"
+
+	if [ "$VERBOSE" = "1" ]; then
+		printf "    COMMAND: $cmd\n"
+		stderr=
+	fi
+
+	out=$(eval $cmd $stderr)
+	rc=$?
+	if [ "$VERBOSE" = "1" -a -n "$out" ]; then
+		echo "    $out"
+		echo
+	fi
+
+	return $rc
+}
+
 # Find the auto-generated name for this namespace
 nsname() {
 	eval echo \$NS_$1
@@ -258,22 +281,22 @@ setup_fou_or_gue() {
 		fi
 	fi
 
-	${ns_a} ip fou add port 5555 ipproto ${ipproto} || return 2
-	${ns_a} ip link add ${encap}_a type ${type} ${mode} local ${a_addr} remote ${b_addr} encap ${encap} encap-sport auto encap-dport 5556 || return 2
+	run_cmd ${ns_a} ip fou add port 5555 ipproto ${ipproto} || return 2
+	run_cmd ${ns_a} ip link add ${encap}_a type ${type} ${mode} local ${a_addr} remote ${b_addr} encap ${encap} encap-sport auto encap-dport 5556 || return 2
 
-	${ns_b} ip fou add port 5556 ipproto ${ipproto}
-	${ns_b} ip link add ${encap}_b type ${type} ${mode} local ${b_addr} remote ${a_addr} encap ${encap} encap-sport auto encap-dport 5555
+	run_cmd ${ns_b} ip fou add port 5556 ipproto ${ipproto}
+	run_cmd ${ns_b} ip link add ${encap}_b type ${type} ${mode} local ${b_addr} remote ${a_addr} encap ${encap} encap-sport auto encap-dport 5555
 
 	if [ "${inner}" = "4" ]; then
-		${ns_a} ip addr add ${tunnel4_a_addr}/${tunnel4_mask} dev ${encap}_a
-		${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev ${encap}_b
+		run_cmd ${ns_a} ip addr add ${tunnel4_a_addr}/${tunnel4_mask} dev ${encap}_a
+		run_cmd ${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev ${encap}_b
 	else
-		${ns_a} ip addr add ${tunnel6_a_addr}/${tunnel6_mask} dev ${encap}_a
-		${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev ${encap}_b
+		run_cmd ${ns_a} ip addr add ${tunnel6_a_addr}/${tunnel6_mask} dev ${encap}_a
+		run_cmd ${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev ${encap}_b
 	fi
 
-	${ns_a} ip link set ${encap}_a up
-	${ns_b} ip link set ${encap}_b up
+	run_cmd ${ns_a} ip link set ${encap}_a up
+	run_cmd ${ns_b} ip link set ${encap}_b up
 }
 
 setup_fou44() {
@@ -319,17 +342,17 @@ setup_namespaces() {
 }
 
 setup_veth() {
-	${ns_a} ip link add veth_a type veth peer name veth_b || return 1
-	${ns_a} ip link set veth_b netns ${NS_B}
+	run_cmd ${ns_a} ip link add veth_a type veth peer name veth_b || return 1
+	run_cmd ${ns_a} ip link set veth_b netns ${NS_B}
 
-	${ns_a} ip addr add ${veth4_a_addr}/${veth4_mask} dev veth_a
-	${ns_b} ip addr add ${veth4_b_addr}/${veth4_mask} dev veth_b
+	run_cmd ${ns_a} ip addr add ${veth4_a_addr}/${veth4_mask} dev veth_a
+	run_cmd ${ns_b} ip addr add ${veth4_b_addr}/${veth4_mask} dev veth_b
 
-	${ns_a} ip addr add ${veth6_a_addr}/${veth6_mask} dev veth_a
-	${ns_b} ip addr add ${veth6_b_addr}/${veth6_mask} dev veth_b
+	run_cmd ${ns_a} ip addr add ${veth6_a_addr}/${veth6_mask} dev veth_a
+	run_cmd ${ns_b} ip addr add ${veth6_b_addr}/${veth6_mask} dev veth_b
 
-	${ns_a} ip link set veth_a up
-	${ns_b} ip link set veth_b up
+	run_cmd ${ns_a} ip link set veth_a up
+	run_cmd ${ns_b} ip link set veth_b up
 }
 
 setup_vti() {
@@ -342,14 +365,14 @@ setup_vti() {
 
 	[ ${proto} -eq 6 ] && vti_type="vti6" || vti_type="vti"
 
-	${ns_a} ip link add vti${proto}_a type ${vti_type} local ${veth_a_addr} remote ${veth_b_addr} key 10 || return 1
-	${ns_b} ip link add vti${proto}_b type ${vti_type} local ${veth_b_addr} remote ${veth_a_addr} key 10
+	run_cmd ${ns_a} ip link add vti${proto}_a type ${vti_type} local ${veth_a_addr} remote ${veth_b_addr} key 10 || return 1
+	run_cmd ${ns_b} ip link add vti${proto}_b type ${vti_type} local ${veth_b_addr} remote ${veth_a_addr} key 10
 
-	${ns_a} ip addr add ${vti_a_addr}/${vti_mask} dev vti${proto}_a
-	${ns_b} ip addr add ${vti_b_addr}/${vti_mask} dev vti${proto}_b
+	run_cmd ${ns_a} ip addr add ${vti_a_addr}/${vti_mask} dev vti${proto}_a
+	run_cmd ${ns_b} ip addr add ${vti_b_addr}/${vti_mask} dev vti${proto}_b
 
-	${ns_a} ip link set vti${proto}_a up
-	${ns_b} ip link set vti${proto}_b up
+	run_cmd ${ns_a} ip link set vti${proto}_a up
+	run_cmd ${ns_b} ip link set vti${proto}_b up
 }
 
 setup_vti4() {
@@ -375,17 +398,17 @@ setup_vxlan_or_geneve() {
 		opts_b=""
 	fi
 
-	${ns_a} ip link add ${type}_a type ${type} id 1 ${opts_a} remote ${b_addr} ${opts} || return 1
-	${ns_b} ip link add ${type}_b type ${type} id 1 ${opts_b} remote ${a_addr} ${opts}
+	run_cmd ${ns_a} ip link add ${type}_a type ${type} id 1 ${opts_a} remote ${b_addr} ${opts} || return 1
+	run_cmd ${ns_b} ip link add ${type}_b type ${type} id 1 ${opts_b} remote ${a_addr} ${opts}
 
-	${ns_a} ip addr add ${tunnel4_a_addr}/${tunnel4_mask} dev ${type}_a
-	${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev ${type}_b
+	run_cmd ${ns_a} ip addr add ${tunnel4_a_addr}/${tunnel4_mask} dev ${type}_a
+	run_cmd ${ns_b} ip addr add ${tunnel4_b_addr}/${tunnel4_mask} dev ${type}_b
 
-	${ns_a} ip addr add ${tunnel6_a_addr}/${tunnel6_mask} dev ${type}_a
-	${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev ${type}_b
+	run_cmd ${ns_a} ip addr add ${tunnel6_a_addr}/${tunnel6_mask} dev ${type}_a
+	run_cmd ${ns_b} ip addr add ${tunnel6_b_addr}/${tunnel6_mask} dev ${type}_b
 
-	${ns_a} ip link set ${type}_a up
-	${ns_b} ip link set ${type}_b up
+	run_cmd ${ns_a} ip link set ${type}_a up
+	run_cmd ${ns_b} ip link set ${type}_b up
 }
 
 setup_geneve4() {
@@ -409,15 +432,15 @@ setup_xfrm() {
 	veth_a_addr="${2}"
 	veth_b_addr="${3}"
 
-	${ns_a} ip -${proto} xfrm state add src ${veth_a_addr} dst ${veth_b_addr} spi 0x1000 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel || return 1
-	${ns_a} ip -${proto} xfrm state add src ${veth_b_addr} dst ${veth_a_addr} spi 0x1001 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
-	${ns_a} ip -${proto} xfrm policy add dir out mark 10 tmpl src ${veth_a_addr} dst ${veth_b_addr} proto esp mode tunnel
-	${ns_a} ip -${proto} xfrm policy add dir in mark 10 tmpl src ${veth_b_addr} dst ${veth_a_addr} proto esp mode tunnel
+	run_cmd "${ns_a} ip -${proto} xfrm state add src ${veth_a_addr} dst ${veth_b_addr} spi 0x1000 proto esp aead 'rfc4106(gcm(aes))' 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel" || return 1
+	run_cmd "${ns_a} ip -${proto} xfrm state add src ${veth_b_addr} dst ${veth_a_addr} spi 0x1001 proto esp aead 'rfc4106(gcm(aes))' 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel"
+	run_cmd "${ns_a} ip -${proto} xfrm policy add dir out mark 10 tmpl src ${veth_a_addr} dst ${veth_b_addr} proto esp mode tunnel"
+	run_cmd "${ns_a} ip -${proto} xfrm policy add dir in mark 10 tmpl src ${veth_b_addr} dst ${veth_a_addr} proto esp mode tunnel"
 
-	${ns_b} ip -${proto} xfrm state add src ${veth_a_addr} dst ${veth_b_addr} spi 0x1000 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
-	${ns_b} ip -${proto} xfrm state add src ${veth_b_addr} dst ${veth_a_addr} spi 0x1001 proto esp aead "rfc4106(gcm(aes))" 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel
-	${ns_b} ip -${proto} xfrm policy add dir out mark 10 tmpl src ${veth_b_addr} dst ${veth_a_addr} proto esp mode tunnel
-	${ns_b} ip -${proto} xfrm policy add dir in mark 10 tmpl src ${veth_a_addr} dst ${veth_b_addr} proto esp mode tunnel
+	run_cmd "${ns_b} ip -${proto} xfrm state add src ${veth_a_addr} dst ${veth_b_addr} spi 0x1000 proto esp aead 'rfc4106(gcm(aes))' 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel"
+	run_cmd "${ns_b} ip -${proto} xfrm state add src ${veth_b_addr} dst ${veth_a_addr} spi 0x1001 proto esp aead 'rfc4106(gcm(aes))' 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f 128 mode tunnel"
+	run_cmd "${ns_b} ip -${proto} xfrm policy add dir out mark 10 tmpl src ${veth_b_addr} dst ${veth_a_addr} proto esp mode tunnel"
+	run_cmd "${ns_b} ip -${proto} xfrm policy add dir in mark 10 tmpl src ${veth_a_addr} dst ${veth_b_addr} proto esp mode tunnel"
 }
 
 setup_xfrm4() {
@@ -597,8 +620,8 @@ test_pmtu_ipvX() {
 	mtu "${ns_b}"  veth_B-R2 1500
 
 	# Create route exceptions
-	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst1} > /dev/null
-	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst2} > /dev/null
+	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst1}
+	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1800 ${dst2}
 
 	# Check that exceptions have been created with the correct PMTU
 	pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
@@ -630,7 +653,7 @@ test_pmtu_ipvX() {
 	# Decrease remote MTU on path via R2, get new exception
 	mtu "${ns_r2}" veth_R2-B 400
 	mtu "${ns_b}"  veth_B-R2 400
-	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2} > /dev/null
+	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2}
 	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
 	check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
 
@@ -647,7 +670,7 @@ test_pmtu_ipvX() {
 	check_pmtu_value "1500" "${pmtu_2}" "increasing local MTU" || return 1
 
 	# Get new exception
-	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2} > /dev/null
+	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s 1400 ${dst2}
 	pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
 	check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
 }
@@ -696,7 +719,7 @@ test_pmtu_ipvX_over_vxlanY_or_geneveY_exception() {
 
 	mtu "${ns_a}" ${type}_a $((${ll_mtu} + 1000))
 	mtu "${ns_b}" ${type}_b $((${ll_mtu} + 1000))
-	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null
+	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst}
 
 	# Check that exception was created
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})"
@@ -776,7 +799,7 @@ test_pmtu_ipvX_over_fouY_or_gueY() {
 
 	mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
 	mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
-	${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst} > /dev/null
+	run_cmd ${ns_a} ${ping} -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${dst}
 
 	# Check that exception was created
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst})"
@@ -834,13 +857,13 @@ test_pmtu_vti4_exception() {
 
 	# Send DF packet without exceeding link layer MTU, check that no
 	# exception is created
-	${ns_a} ping -q -M want -i 0.1 -w 1 -s ${ping_payload} ${tunnel4_b_addr} > /dev/null
+	run_cmd ${ns_a} ping -q -M want -i 0.1 -w 1 -s ${ping_payload} ${tunnel4_b_addr}
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})"
 	check_pmtu_value "" "${pmtu}" "sending packet smaller than PMTU (IP payload length ${esp_payload_rfc4106})" || return 1
 
 	# Now exceed link layer MTU by one byte, check that exception is created
 	# with the right PMTU value
-	${ns_a} ping -q -M want -i 0.1 -w 1 -s $((ping_payload + 1)) ${tunnel4_b_addr} > /dev/null
+	run_cmd ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((ping_payload + 1)) ${tunnel4_b_addr}
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel4_b_addr})"
 	check_pmtu_value "${esp_payload_rfc4106}" "${pmtu}" "exceeding PMTU (IP payload length $((esp_payload_rfc4106 + 1)))"
 }
@@ -856,7 +879,7 @@ test_pmtu_vti6_exception() {
 	mtu "${ns_b}" veth_b 4000
 	mtu "${ns_a}" vti6_a 5000
 	mtu "${ns_b}" vti6_b 5000
-	${ns_a} ${ping6} -q -i 0.1 -w 1 -s 60000 ${tunnel6_b_addr} > /dev/null
+	run_cmd ${ns_a} ${ping6} -q -i 0.1 -w 1 -s 60000 ${tunnel6_b_addr}
 
 	# Check that exception was created
 	pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${tunnel6_b_addr})"
@@ -902,9 +925,9 @@ test_pmtu_vti6_default_mtu() {
 test_pmtu_vti4_link_add_mtu() {
 	setup namespaces || return 2
 
-	${ns_a} ip link add vti4_a type vti local ${veth4_a_addr} remote ${veth4_b_addr} key 10
+	run_cmd ${ns_a} ip link add vti4_a type vti local ${veth4_a_addr} remote ${veth4_b_addr} key 10
 	[ $? -ne 0 ] && err "  vti not supported" && return 2
-	${ns_a} ip link del vti4_a
+	run_cmd ${ns_a} ip link del vti4_a
 
 	fail=0
 
@@ -912,7 +935,7 @@ test_pmtu_vti4_link_add_mtu() {
 	max=$((65535 - 20))
 	# Check invalid values first
 	for v in $((min - 1)) $((max + 1)); do
-		${ns_a} ip link add vti4_a mtu ${v} type vti local ${veth4_a_addr} remote ${veth4_b_addr} key 10 2>/dev/null
+		run_cmd ${ns_a} ip link add vti4_a mtu ${v} type vti local ${veth4_a_addr} remote ${veth4_b_addr} key 10
 		# This can fail, or MTU can be adjusted to a proper value
 		[ $? -ne 0 ] && continue
 		mtu="$(link_get_mtu "${ns_a}" vti4_a)"
@@ -920,14 +943,14 @@ test_pmtu_vti4_link_add_mtu() {
 			err "  vti tunnel created with invalid MTU ${mtu}"
 			fail=1
 		fi
-		${ns_a} ip link del vti4_a
+		run_cmd ${ns_a} ip link del vti4_a
 	done
 
 	# Now check valid values
 	for v in ${min} 1300 ${max}; do
-		${ns_a} ip link add vti4_a mtu ${v} type vti local ${veth4_a_addr} remote ${veth4_b_addr} key 10
+		run_cmd ${ns_a} ip link add vti4_a mtu ${v} type vti local ${veth4_a_addr} remote ${veth4_b_addr} key 10
 		mtu="$(link_get_mtu "${ns_a}" vti4_a)"
-		${ns_a} ip link del vti4_a
+		run_cmd ${ns_a} ip link del vti4_a
 		if [ "${mtu}" != "${v}" ]; then
 			err "  vti MTU ${mtu} doesn't match configured value ${v}"
 			fail=1
@@ -940,9 +963,9 @@ test_pmtu_vti4_link_add_mtu() {
 test_pmtu_vti6_link_add_mtu() {
 	setup namespaces || return 2
 
-	${ns_a} ip link add vti6_a type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10
+	run_cmd ${ns_a} ip link add vti6_a type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10
 	[ $? -ne 0 ] && err "  vti6 not supported" && return 2
-	${ns_a} ip link del vti6_a
+	run_cmd ${ns_a} ip link del vti6_a
 
 	fail=0
 
@@ -950,7 +973,7 @@ test_pmtu_vti6_link_add_mtu() {
 	max=$((65535 - 40))
 	# Check invalid values first
 	for v in $((min - 1)) $((max + 1)); do
-		${ns_a} ip link add vti6_a mtu ${v} type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10 2>/dev/null
+		run_cmd ${ns_a} ip link add vti6_a mtu ${v} type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10
 		# This can fail, or MTU can be adjusted to a proper value
 		[ $? -ne 0 ] && continue
 		mtu="$(link_get_mtu "${ns_a}" vti6_a)"
@@ -958,14 +981,14 @@ test_pmtu_vti6_link_add_mtu() {
 			err "  vti6 tunnel created with invalid MTU ${v}"
 			fail=1
 		fi
-		${ns_a} ip link del vti6_a
+		run_cmd ${ns_a} ip link del vti6_a
 	done
 
 	# Now check valid values
 	for v in 68 1280 1300 $((65535 - 40)); do
-		${ns_a} ip link add vti6_a mtu ${v} type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10
+		run_cmd ${ns_a} ip link add vti6_a mtu ${v} type vti6 local ${veth6_a_addr} remote ${veth6_b_addr} key 10
 		mtu="$(link_get_mtu "${ns_a}" vti6_a)"
-		${ns_a} ip link del vti6_a
+		run_cmd ${ns_a} ip link del vti6_a
 		if [ "${mtu}" != "${v}" ]; then
 			err "  vti6 MTU ${mtu} doesn't match configured value ${v}"
 			fail=1
@@ -978,19 +1001,19 @@ test_pmtu_vti6_link_add_mtu() {
 test_pmtu_vti6_link_change_mtu() {
 	setup namespaces || return 2
 
-	${ns_a} ip link add dummy0 mtu 1500 type dummy
+	run_cmd ${ns_a} ip link add dummy0 mtu 1500 type dummy
 	[ $? -ne 0 ] && err "  dummy not supported" && return 2
-	${ns_a} ip link add dummy1 mtu 3000 type dummy
-	${ns_a} ip link set dummy0 up
-	${ns_a} ip link set dummy1 up
+	run_cmd ${ns_a} ip link add dummy1 mtu 3000 type dummy
+	run_cmd ${ns_a} ip link set dummy0 up
+	run_cmd ${ns_a} ip link set dummy1 up
 
-	${ns_a} ip addr add ${dummy6_0_addr}/${dummy6_mask} dev dummy0
-	${ns_a} ip addr add ${dummy6_1_addr}/${dummy6_mask} dev dummy1
+	run_cmd ${ns_a} ip addr add ${dummy6_0_addr}/${dummy6_mask} dev dummy0
+	run_cmd ${ns_a} ip addr add ${dummy6_1_addr}/${dummy6_mask} dev dummy1
 
 	fail=0
 
 	# Create vti6 interface bound to device, passing MTU, check it
-	${ns_a} ip link add vti6_a mtu 1300 type vti6 remote ${dummy6_0_addr} local ${dummy6_0_addr}
+	run_cmd ${ns_a} ip link add vti6_a mtu 1300 type vti6 remote ${dummy6_0_addr} local ${dummy6_0_addr}
 	mtu="$(link_get_mtu "${ns_a}" vti6_a)"
 	if [ ${mtu} -ne 1300 ]; then
 		err "  vti6 MTU ${mtu} doesn't match configured value 1300"
@@ -999,7 +1022,7 @@ test_pmtu_vti6_link_change_mtu() {
 
 	# Move to another device with different MTU, without passing MTU, check
 	# MTU is adjusted
-	${ns_a} ip link set vti6_a type vti6 remote ${dummy6_1_addr} local ${dummy6_1_addr}
+	run_cmd ${ns_a} ip link set vti6_a type vti6 remote ${dummy6_1_addr} local ${dummy6_1_addr}
 	mtu="$(link_get_mtu "${ns_a}" vti6_a)"
 	if [ ${mtu} -ne $((3000 - 40)) ]; then
 		err "  vti MTU ${mtu} is not dummy MTU 3000 minus IPv6 header length"
@@ -1007,7 +1030,7 @@ test_pmtu_vti6_link_change_mtu() {
 	fi
 
 	# Move it back, passing MTU, check MTU is not overridden
-	${ns_a} ip link set vti6_a mtu 1280 type vti6 remote ${dummy6_0_addr} local ${dummy6_0_addr}
+	run_cmd ${ns_a} ip link set vti6_a mtu 1280 type vti6 remote ${dummy6_0_addr} local ${dummy6_0_addr}
 	mtu="$(link_get_mtu "${ns_a}" vti6_a)"
 	if [ ${mtu} -ne 1280 ]; then
 		err "  vti6 MTU ${mtu} doesn't match configured value 1280"
@@ -1052,7 +1075,7 @@ test_cleanup_vxlanX_exception() {
 	# Fill exception cache for multiple CPUs (2)
 	# we can always use inner IPv4 for that
 	for cpu in ${cpu_list}; do
-		taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null
+		run_cmd taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr}
 	done
 
 	${ns_a} ip link del dev veth_A-R1 &
@@ -1084,29 +1107,35 @@ usage() {
 	exit 1
 }
 
+################################################################################
+#
+tracing=0
 exitcode=0
 desc=0
+
+while getopts :ptv o
+do
+	case $o in
+	p) PAUSE_ON_FAIL=yes;;
+	v) VERBOSE=1;;
+	t) if which tcpdump > /dev/null 2>&1; then
+		tracing=1
+	   else
+		echo "=== tcpdump not available, tracing disabled"
+	   fi
+	   ;;
+	*) usage;;
+	esac
+done
+shift $(($OPTIND-1))
+
 IFS="	
 "
 
-tracing=0
 for arg do
-	if [ "${arg}" != "${arg#--*}" ]; then
-		opt="${arg#--}"
-		if [ "${opt}" = "trace" ]; then
-			if which tcpdump > /dev/null 2>&1; then
-				tracing=1
-			else
-				echo "=== tcpdump not available, tracing disabled"
-			fi
-		else
-			usage
-		fi
-	else
-		# Check first that all requested tests are available before
-		# running any
-		command -v > /dev/null "test_${arg}" || { echo "=== Test ${arg} not found"; usage; }
-	fi
+	# Check first that all requested tests are available before
+	# running any
+	command -v > /dev/null "test_${arg}" || { echo "=== Test ${arg} not found"; usage; }
 done
 
 trap cleanup EXIT
@@ -1124,6 +1153,11 @@ for t in ${tests}; do
 
 	(
 		unset IFS
+
+		if [ "$VERBOSE" = "1" ]; then
+			printf "\n##########################################################################\n\n"
+		fi
+
 		eval test_${name}
 		ret=$?
 		cleanup
@@ -1132,6 +1166,11 @@ for t in ${tests}; do
 			printf "TEST: %-60s  [ OK ]\n" "${t}"
 		elif [ $ret -eq 1 ]; then
 			printf "TEST: %-60s  [FAIL]\n" "${t}"
+			if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
+				echo
+				echo "Pausing. Hit enter to continue"
+				read a
+			fi
 			err_flush
 			exit 1
 		elif [ $ret -eq 2 ]; then
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH net-next] selftests: Add debugging options to pmtu.sh
  2019-04-04  1:18 [PATCH net-next] selftests: Add debugging options to pmtu.sh David Ahern
@ 2019-04-04 12:16 ` Stefano Brivio
  2019-04-04 15:24   ` David Ahern
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Brivio @ 2019-04-04 12:16 UTC (permalink / raw)
  To: David Ahern; +Cc: davem, netdev, David Ahern

Hi David,

On Wed,  3 Apr 2019 18:18:24 -0700
David Ahern <dsahern@kernel.org> wrote:

> From: David Ahern <dsahern@gmail.com>
> 
> pmtu.sh script runs a number of tests and dumps a summary of pass/fail.
> If a test fails, it is near impossible to debug why. For example:
> 
>     TEST: ipv6: PMTU exceptions                       [FAIL]
> 
> There are a lot of commands run behind the scenes for this test. Which
> one is failing?
> 
> Add a VERBOSE option to show commands that are run and any output from
> those commands. Add a PAUSE_ON_FAIL option to halt the script if a test
> fails allowing users to poke around with the setup in the failed state.

Thanks for doing this, I have to admit I occasionally had to sprinkle
this script with sleep/read/exit in the past. A few comments:

> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  tools/testing/selftests/net/pmtu.sh | 215 +++++++++++++++++++++---------------
>  1 file changed, 127 insertions(+), 88 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 912b2dc50be3..28e8c97b5c9e 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -116,6 +116,9 @@
>  # Kselftest framework requirement - SKIP code is 4.
>  ksft_skip=4
>  
> +PAUSE_ON_FAIL=no
> +VERBOSE=0

For consistency, I'd also rename 'tracing' below to TRACING and assign
it here.

>  # Some systems don't have a ping6 binary anymore
>  which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
>  
> @@ -222,6 +225,26 @@ err_flush() {
>  	err_buf=
>  }
>  
> +run_cmd() {
> +	local cmd="$*"
> +	local out
> +	local stderr="2>/dev/null"

'local' is not POSIX, and I think it actually breaks (at least) on
ksh93 (maybe not a big deal, but I kept everything else POSIX, so I
wouldn't break it just for this).

Besides, for 'ping' commands, it's stdout that needs to be suppressed
(we can just suppress both stdout and stderr if not in verbose mode).

> +	if [ "$VERBOSE" = "1" ]; then
> +		printf "    COMMAND: $cmd\n"
> +		stderr=
> +	fi
> +
> +	out=$(eval $cmd $stderr)

I think this needs quoting. Is eval really needed, by the way?

> +	rc=$?
> +	if [ "$VERBOSE" = "1" -a -n "$out" ]; then
> +		echo "    $out"
> +		echo
> +	fi
> +
> +	return $rc
> +}
>
> [...]
>
> +while getopts :ptv o

Oh, and this is now POSIX and well defined, I didn't know. Thanks for
making this readable :)

> +do
> +	case $o in
> +	p) PAUSE_ON_FAIL=yes;;
> +	v) VERBOSE=1;;
> +	t) if which tcpdump > /dev/null 2>&1; then
> +		tracing=1
> +	   else
> +		echo "=== tcpdump not available, tracing disabled"
> +	   fi
> +	   ;;
> +	*) usage;;
> +	esac
> +done
> +shift $(($OPTIND-1))
> +
>  IFS="	
>  "
>  
> -tracing=0
>  for arg do
> -	if [ "${arg}" != "${arg#--*}" ]; then
> -		opt="${arg#--}"
> -		if [ "${opt}" = "trace" ]; then
> -			if which tcpdump > /dev/null 2>&1; then
> -				tracing=1
> -			else
> -				echo "=== tcpdump not available, tracing disabled"
> -			fi
> -		else
> -			usage
> -		fi
> -	else
> -		# Check first that all requested tests are available before
> -		# running any
> -		command -v > /dev/null "test_${arg}" || { echo "=== Test ${arg} not found"; usage; }
> -	fi
> +	# Check first that all requested tests are available before
> +	# running any

This fits on a single line now.

> +	command -v > /dev/null "test_${arg}" || { echo "=== Test ${arg} not found"; usage; }
>  done
>  
>  trap cleanup EXIT
> @@ -1124,6 +1153,11 @@ for t in ${tests}; do
>  
>  	(
>  		unset IFS
> +
> +		if [ "$VERBOSE" = "1" ]; then
> +			printf "\n##########################################################################\n\n"
> +		fi
> +
>  		eval test_${name}
>  		ret=$?
>  		cleanup
> @@ -1132,6 +1166,11 @@ for t in ${tests}; do
>  			printf "TEST: %-60s  [ OK ]\n" "${t}"
>  		elif [ $ret -eq 1 ]; then
>  			printf "TEST: %-60s  [FAIL]\n" "${t}"
> +			if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> +				echo
> +				echo "Pausing. Hit enter to continue"
> +				read a
> +			fi
>  			err_flush
>  			exit 1
>  		elif [ $ret -eq 2 ]; then

-- 
Stefano

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH net-next] selftests: Add debugging options to pmtu.sh
  2019-04-04 12:16 ` Stefano Brivio
@ 2019-04-04 15:24   ` David Ahern
  0 siblings, 0 replies; 3+ messages in thread
From: David Ahern @ 2019-04-04 15:24 UTC (permalink / raw)
  To: Stefano Brivio, David Ahern; +Cc: davem, netdev

On 4/4/19 6:16 AM, Stefano Brivio wrote:
>> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
>> index 912b2dc50be3..28e8c97b5c9e 100755
>> --- a/tools/testing/selftests/net/pmtu.sh
>> +++ b/tools/testing/selftests/net/pmtu.sh
>> @@ -116,6 +116,9 @@
>>  # Kselftest framework requirement - SKIP code is 4.
>>  ksft_skip=4
>>  
>> +PAUSE_ON_FAIL=no
>> +VERBOSE=0
> 
> For consistency, I'd also rename 'tracing' below to TRACING and assign
> it here.

ok.

> 
>>  # Some systems don't have a ping6 binary anymore
>>  which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
>>  
>> @@ -222,6 +225,26 @@ err_flush() {
>>  	err_buf=
>>  }
>>  
>> +run_cmd() {
>> +	local cmd="$*"
>> +	local out
>> +	local stderr="2>/dev/null"
> 
> 'local' is not POSIX, and I think it actually breaks (at least) on
> ksh93 (maybe not a big deal, but I kept everything else POSIX, so I
> wouldn't break it just for this).

ok. I did not realize this one is expected to be posix compliant. I have
fib_test.sh using /bin/bash

> 
> Besides, for 'ping' commands, it's stdout that needs to be suppressed
> (we can just suppress both stdout and stderr if not in verbose mode).

sure

> 
>> +	if [ "$VERBOSE" = "1" ]; then
>> +		printf "    COMMAND: $cmd\n"
>> +		stderr=
>> +	fi
>> +
>> +	out=$(eval $cmd $stderr)
> 
> I think this needs quoting. Is eval really needed, by the way?

old habits. Works fine with bash; not sure about others.

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-04 15:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-04  1:18 [PATCH net-next] selftests: Add debugging options to pmtu.sh David Ahern
2019-04-04 12:16 ` Stefano Brivio
2019-04-04 15:24   ` David Ahern

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.