linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] selftests: pmtu.sh: improve the test result processing
@ 2020-11-05 10:50 Po-Hsu Lin
  2020-11-05 10:50 ` [PATCH 1/2] selftests: pmtu.sh: use $ksft_skip for skipped return code Po-Hsu Lin
  2020-11-05 10:50 ` [PATCH 2/2] selftests: pmtu.sh: improve the test result processing Po-Hsu Lin
  0 siblings, 2 replies; 7+ messages in thread
From: Po-Hsu Lin @ 2020-11-05 10:50 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel; +Cc: davem, kuba, skhan, po-hsu.lin

The pmtu.sh test script treats all non-zero return code as a failure,
thus it will be marked as FAILED when some sub-test got skipped.

This patchset will:
  1. Use the kselftest framework skip code $ksft_skip to replace the
     hardcoded SKIP return code.
  2. Improve the result processing, the test will be marked as PASSED
     if nothing goes wrong and not all the tests were skipped.

Po-Hsu Lin (2):
  selftests: pmtu.sh: use $ksft_skip for skipped return code
  selftests: pmtu.sh: improve the test result processing

 tools/testing/selftests/net/pmtu.sh | 83 ++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 33 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] selftests: pmtu.sh: use $ksft_skip for skipped return code
  2020-11-05 10:50 [PATCH 0/2] selftests: pmtu.sh: improve the test result processing Po-Hsu Lin
@ 2020-11-05 10:50 ` Po-Hsu Lin
  2020-11-05 10:50 ` [PATCH 2/2] selftests: pmtu.sh: improve the test result processing Po-Hsu Lin
  1 sibling, 0 replies; 7+ messages in thread
From: Po-Hsu Lin @ 2020-11-05 10:50 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel; +Cc: davem, kuba, skhan, po-hsu.lin

This test uses return code 2 as a hard-coded skipped state, let's use
the kselftest framework skip code variable $ksft_skip instead to make
it more readable and easier to maintain.

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/pmtu.sh | 64 ++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 6bbf69a..fb53987 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -355,7 +355,7 @@ setup_fou_or_gue() {
 	encap="${3}"
 
 	if [ "${outer}" = "4" ]; then
-		modprobe fou || return 2
+		modprobe fou || return $ksft_skip
 		a_addr="${prefix4}.${a_r1}.1"
 		b_addr="${prefix4}.${b_r1}.1"
 		if [ "${inner}" = "4" ]; then
@@ -366,7 +366,7 @@ setup_fou_or_gue() {
 			ipproto="41"
 		fi
 	else
-		modprobe fou6 || return 2
+		modprobe fou6 || return $ksft_skip
 		a_addr="${prefix6}:${a_r1}::1"
 		b_addr="${prefix6}:${b_r1}::1"
 		if [ "${inner}" = "4" ]; then
@@ -380,8 +380,8 @@ setup_fou_or_gue() {
 		fi
 	fi
 
-	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
+	run_cmd ${ns_a} ip fou add port 5555 ipproto ${ipproto} || return $ksft_skip
+	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 $ksft_skip
 
 	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
@@ -455,7 +455,7 @@ setup_ipvX_over_ipvY() {
 		fi
 	fi
 
-	run_cmd ${ns_a} ip link add ip_a type ${type} local ${a_addr} remote ${b_addr} mode ${mode} || return 2
+	run_cmd ${ns_a} ip link add ip_a type ${type} local ${a_addr} remote ${b_addr} mode ${mode} || return $ksft_skip
 	run_cmd ${ns_b} ip link add ip_b type ${type} local ${b_addr} remote ${a_addr} mode ${mode}
 
 	run_cmd ${ns_a} ip link set ip_a up
@@ -713,7 +713,7 @@ setup_routing() {
 }
 
 setup_bridge() {
-	run_cmd ${ns_a} ip link add br0 type bridge || return 2
+	run_cmd ${ns_a} ip link add br0 type bridge || return $ksft_skip
 	run_cmd ${ns_a} ip link set br0 up
 
 	run_cmd ${ns_c} ip link add veth_C-A type veth peer name veth_A-C
@@ -765,7 +765,7 @@ setup_ovs_vxlan6() {
 }
 
 setup_ovs_bridge() {
-	run_cmd ovs-vsctl add-br ovs_br0 || return 2
+	run_cmd ovs-vsctl add-br ovs_br0 || return $ksft_skip
 	run_cmd ip link set ovs_br0 up
 
 	run_cmd ${ns_c} ip link add veth_C-A type veth peer name veth_A-C
@@ -887,7 +887,7 @@ check_pmtu_value() {
 test_pmtu_ipvX() {
 	family=${1}
 
-	setup namespaces routing || return 2
+	setup namespaces routing || return $ksft_skip
 	trace "${ns_a}"  veth_A-R1    "${ns_r1}" veth_R1-A \
 	      "${ns_r1}" veth_R1-B    "${ns_b}"  veth_B-R1 \
 	      "${ns_a}"  veth_A-R2    "${ns_r2}" veth_R2-A \
@@ -985,11 +985,11 @@ test_pmtu_ipvX_over_vxlanY_or_geneveY_exception() {
 	ll_mtu=4000
 
 	if [ ${outer_family} -eq 4 ]; then
-		setup namespaces routing ${type}4 || return 2
+		setup namespaces routing ${type}4 || return $ksft_skip
 		#                      IPv4 header   UDP header   VXLAN/GENEVE header   Ethernet header
 		exp_mtu=$((${ll_mtu} - 20          - 8          - 8                   - 14))
 	else
-		setup namespaces routing ${type}6 || return 2
+		setup namespaces routing ${type}6 || return $ksft_skip
 		#                      IPv6 header   UDP header   VXLAN/GENEVE header   Ethernet header
 		exp_mtu=$((${ll_mtu} - 40          - 8          - 8                   - 14))
 	fi
@@ -1060,11 +1060,11 @@ test_pmtu_ipvX_over_bridged_vxlanY_or_geneveY_exception() {
 	ll_mtu=4000
 
 	if [ ${outer_family} -eq 4 ]; then
-		setup namespaces routing bridge bridged_${type}4 || return 2
+		setup namespaces routing bridge bridged_${type}4 || return $ksft_skip
 		#                      IPv4 header   UDP header   VXLAN/GENEVE header   Ethernet header
 		exp_mtu=$((${ll_mtu} - 20          - 8          - 8                   - 14))
 	else
-		setup namespaces routing bridge bridged_${type}6 || return 2
+		setup namespaces routing bridge bridged_${type}6 || return $ksft_skip
 		#                      IPv6 header   UDP header   VXLAN/GENEVE header   Ethernet header
 		exp_mtu=$((${ll_mtu} - 40          - 8          - 8                   - 14))
 	fi
@@ -1144,11 +1144,11 @@ test_pmtu_ipvX_over_ovs_vxlanY_or_geneveY_exception() {
 	ll_mtu=4000
 
 	if [ ${outer_family} -eq 4 ]; then
-		setup namespaces routing ovs_bridge ovs_${type}4 || return 2
+		setup namespaces routing ovs_bridge ovs_${type}4 || return $ksft_skip
 		#                      IPv4 header   UDP header   VXLAN/GENEVE header   Ethernet header
 		exp_mtu=$((${ll_mtu} - 20          - 8          - 8                   - 14))
 	else
-		setup namespaces routing ovs_bridge ovs_${type}6 || return 2
+		setup namespaces routing ovs_bridge ovs_${type}6 || return $ksft_skip
 		#                      IPv6 header   UDP header   VXLAN/GENEVE header   Ethernet header
 		exp_mtu=$((${ll_mtu} - 40          - 8          - 8                   - 14))
 	fi
@@ -1230,7 +1230,7 @@ test_pmtu_ipvX_over_fouY_or_gueY() {
 	encap=${3}
 	ll_mtu=4000
 
-	setup namespaces routing ${encap}${outer_family}${inner_family} || return 2
+	setup namespaces routing ${encap}${outer_family}${inner_family} || return $ksft_skip
 	trace "${ns_a}" ${encap}_a   "${ns_b}"  ${encap}_b \
 	      "${ns_a}" veth_A-R1    "${ns_r1}" veth_R1-A \
 	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B
@@ -1309,7 +1309,7 @@ test_pmtu_ipvX_over_ipvY_exception() {
 	outer=${2}
 	ll_mtu=4000
 
-	setup namespaces routing ip${inner}ip${outer} || return 2
+	setup namespaces routing ip${inner}ip${outer} || return $ksft_skip
 
 	trace "${ns_a}" ip_a         "${ns_b}"  ip_b  \
 	      "${ns_a}" veth_A-R1    "${ns_r1}" veth_R1-A \
@@ -1363,7 +1363,7 @@ test_pmtu_ipv6_ipv6_exception() {
 }
 
 test_pmtu_vti4_exception() {
-	setup namespaces veth vti4 xfrm4 || return 2
+	setup namespaces veth vti4 xfrm4 || return $ksft_skip
 	trace "${ns_a}" veth_a    "${ns_b}" veth_b \
 	      "${ns_a}" vti4_a    "${ns_b}" vti4_b
 
@@ -1393,7 +1393,7 @@ test_pmtu_vti4_exception() {
 }
 
 test_pmtu_vti6_exception() {
-	setup namespaces veth vti6 xfrm6 || return 2
+	setup namespaces veth vti6 xfrm6 || return $ksft_skip
 	trace "${ns_a}" veth_a    "${ns_b}" veth_b \
 	      "${ns_a}" vti6_a    "${ns_b}" vti6_b
 	fail=0
@@ -1423,7 +1423,7 @@ test_pmtu_vti6_exception() {
 }
 
 test_pmtu_vti4_default_mtu() {
-	setup namespaces veth vti4 || return 2
+	setup namespaces veth vti4 || return $ksft_skip
 
 	# Check that MTU of vti device is MTU of veth minus IPv4 header length
 	veth_mtu="$(link_get_mtu "${ns_a}" veth_a)"
@@ -1435,7 +1435,7 @@ test_pmtu_vti4_default_mtu() {
 }
 
 test_pmtu_vti6_default_mtu() {
-	setup namespaces veth vti6 || return 2
+	setup namespaces veth vti6 || return $ksft_skip
 
 	# Check that MTU of vti device is MTU of veth minus IPv6 header length
 	veth_mtu="$(link_get_mtu "${ns_a}" veth_a)"
@@ -1447,10 +1447,10 @@ test_pmtu_vti6_default_mtu() {
 }
 
 test_pmtu_vti4_link_add_mtu() {
-	setup namespaces || return 2
+	setup namespaces || return $ksft_skip
 
 	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
+	[ $? -ne 0 ] && err "  vti not supported" && return $ksft_skip
 	run_cmd ${ns_a} ip link del vti4_a
 
 	fail=0
@@ -1485,10 +1485,10 @@ test_pmtu_vti4_link_add_mtu() {
 }
 
 test_pmtu_vti6_link_add_mtu() {
-	setup namespaces || return 2
+	setup namespaces || return $ksft_skip
 
 	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
+	[ $? -ne 0 ] && err "  vti6 not supported" && return $ksft_skip
 	run_cmd ${ns_a} ip link del vti6_a
 
 	fail=0
@@ -1523,10 +1523,10 @@ test_pmtu_vti6_link_add_mtu() {
 }
 
 test_pmtu_vti6_link_change_mtu() {
-	setup namespaces || return 2
+	setup namespaces || return $ksft_skip
 
 	run_cmd ${ns_a} ip link add dummy0 mtu 1500 type dummy
-	[ $? -ne 0 ] && err "  dummy not supported" && return 2
+	[ $? -ne 0 ] && err "  dummy not supported" && return $ksft_skip
 	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
@@ -1579,10 +1579,10 @@ test_cleanup_vxlanX_exception() {
 	encap="vxlan"
 	ll_mtu=4000
 
-	check_command taskset || return 2
+	check_command taskset || return $ksft_skip
 	cpu_list=$(grep -m 2 processor /proc/cpuinfo | cut -d ' ' -f 2)
 
-	setup namespaces routing ${encap}${outer} || return 2
+	setup namespaces routing ${encap}${outer} || return $ksft_skip
 	trace "${ns_a}" ${encap}_a   "${ns_b}"  ${encap}_b \
 	      "${ns_a}" veth_A-R1    "${ns_r1}" veth_R1-A \
 	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B
@@ -1644,7 +1644,7 @@ run_test() {
 		fi
 		err_flush
 		exit 1
-	elif [ $ret -eq 2 ]; then
+	elif [ $ret -eq $ksft_skip ]; then
 		printf "TEST: %-60s  [SKIP]\n" "${tdesc}"
 		err_flush
 	fi
@@ -1667,7 +1667,7 @@ run_test_nh() {
 }
 
 test_list_flush_ipv4_exception() {
-	setup namespaces routing || return 2
+	setup namespaces routing || return $ksft_skip
 	trace "${ns_a}"  veth_A-R1    "${ns_r1}" veth_R1-A \
 	      "${ns_r1}" veth_R1-B    "${ns_b}"  veth_B-R1 \
 	      "${ns_a}"  veth_A-R2    "${ns_r2}" veth_R2-A \
@@ -1721,7 +1721,7 @@ test_list_flush_ipv4_exception() {
 }
 
 test_list_flush_ipv6_exception() {
-	setup namespaces routing || return 2
+	setup namespaces routing || return $ksft_skip
 	trace "${ns_a}"  veth_A-R1    "${ns_r1}" veth_R1-A \
 	      "${ns_r1}" veth_R1-B    "${ns_b}"  veth_B-R1 \
 	      "${ns_a}"  veth_A-R2    "${ns_r2}" veth_R2-A \
@@ -1840,7 +1840,7 @@ for t in ${tests}; do
 	if [ $run_this -eq 1 ]; then
 		run_test "${name}" "${desc}"
 		# if test was skipped no need to retry with nexthop objects
-		[ $? -eq 2 ] && rerun_nh=0
+		[ $? -eq $ksft_skip ] && rerun_nh=0
 
 		if [ "${rerun_nh}" = "1" ]; then
 			run_test_nh "${name}" "${desc}"
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] selftests: pmtu.sh: improve the test result processing
  2020-11-05 10:50 [PATCH 0/2] selftests: pmtu.sh: improve the test result processing Po-Hsu Lin
  2020-11-05 10:50 ` [PATCH 1/2] selftests: pmtu.sh: use $ksft_skip for skipped return code Po-Hsu Lin
@ 2020-11-05 10:50 ` Po-Hsu Lin
  2020-11-07 23:02   ` Jakub Kicinski
  1 sibling, 1 reply; 7+ messages in thread
From: Po-Hsu Lin @ 2020-11-05 10:50 UTC (permalink / raw)
  To: netdev, linux-kselftest, linux-kernel; +Cc: davem, kuba, skhan, po-hsu.lin

This test will treat all non-zero return codes as failures, it will
make the pmtu.sh test script being marked as FAILED when some
sub-test got skipped.

Improve the result processing by
  * Only mark the whole test script as SKIP when all of the
    sub-tests were skipped
  * If the sub-tests were either passed or skipped, the overall
    result will be PASS
  * If any of them has failed, the overall result will be FAIL
  * Treat other return codes (e.g. 127 for command not found) as FAIL

Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/pmtu.sh | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index fb53987..5c86fb1 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -1652,7 +1652,23 @@ run_test() {
 	return $ret
 	)
 	ret=$?
-	[ $ret -ne 0 ] && exitcode=1
+	case $ret in
+		0)
+			all_skipped=false
+			[ $exitcode=$ksft_skip ] && exitcode=0
+		;;
+		1)
+			all_skipped=false
+			exitcode=1
+		;;
+		$ksft_skip)
+			[ $all_skipped = true ] && exitcode=$ksft_skip
+		;;
+		*)
+			all_skipped=false
+			exitcode=1
+		;;
+	esac
 
 	return $ret
 }
@@ -1786,6 +1802,7 @@ usage() {
 #
 exitcode=0
 desc=0
+all_skipped=true
 
 while getopts :ptv o
 do
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] selftests: pmtu.sh: improve the test result processing
  2020-11-05 10:50 ` [PATCH 2/2] selftests: pmtu.sh: improve the test result processing Po-Hsu Lin
@ 2020-11-07 23:02   ` Jakub Kicinski
  2020-11-09  3:42     ` Po-Hsu Lin
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2020-11-07 23:02 UTC (permalink / raw)
  To: Po-Hsu Lin; +Cc: netdev, linux-kselftest, linux-kernel, davem, skhan

On Thu,  5 Nov 2020 18:50:51 +0800 Po-Hsu Lin wrote:
> This test will treat all non-zero return codes as failures, it will
> make the pmtu.sh test script being marked as FAILED when some
> sub-test got skipped.
> 
> Improve the result processing by
>   * Only mark the whole test script as SKIP when all of the
>     sub-tests were skipped
>   * If the sub-tests were either passed or skipped, the overall
>     result will be PASS
>   * If any of them has failed, the overall result will be FAIL
>   * Treat other return codes (e.g. 127 for command not found) as FAIL
> 
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>

Patch 1 looks like a cleanup while patch 2 is more of a fix, can we
separate the two and apply the former to -next and latter to 5.10?
They shouldn't conflict, right?

> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index fb53987..5c86fb1 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -1652,7 +1652,23 @@ run_test() {
>  	return $ret
>  	)
>  	ret=$?
> -	[ $ret -ne 0 ] && exitcode=1
> +	case $ret in
> +		0)
> +			all_skipped=false
> +			[ $exitcode=$ksft_skip ] && exitcode=0
> +		;;
> +		1)
> +			all_skipped=false
> +			exitcode=1
> +		;;

Does it make sense to remove this case? The handling is identical to
the default case *).

> +		$ksft_skip)
> +			[ $all_skipped = true ] && exitcode=$ksft_skip
> +		;;
> +		*)
> +			all_skipped=false
> +			exitcode=1
> +		;;
> +	esac
>  
>  	return $ret
>  }
> @@ -1786,6 +1802,7 @@ usage() {
>  #
>  exitcode=0
>  desc=0
> +all_skipped=true
>  
>  while getopts :ptv o
>  do


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] selftests: pmtu.sh: improve the test result processing
  2020-11-07 23:02   ` Jakub Kicinski
@ 2020-11-09  3:42     ` Po-Hsu Lin
  2020-11-09 18:09       ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Po-Hsu Lin @ 2020-11-09  3:42 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, linux-kselftest, linux-kernel, David Miller, Shuah Khan

On Sun, Nov 8, 2020 at 7:02 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu,  5 Nov 2020 18:50:51 +0800 Po-Hsu Lin wrote:
> > This test will treat all non-zero return codes as failures, it will
> > make the pmtu.sh test script being marked as FAILED when some
> > sub-test got skipped.
> >
> > Improve the result processing by
> >   * Only mark the whole test script as SKIP when all of the
> >     sub-tests were skipped
> >   * If the sub-tests were either passed or skipped, the overall
> >     result will be PASS
> >   * If any of them has failed, the overall result will be FAIL
> >   * Treat other return codes (e.g. 127 for command not found) as FAIL
> >
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
>
> Patch 1 looks like a cleanup while patch 2 is more of a fix, can we
> separate the two and apply the former to -next and latter to 5.10?
> They shouldn't conflict, right?
>

Hello Jakub,

Yes the first patch is just changing return code to $ksft_skip, the
real fix is the second one. However the second patch was based on the
first one, if we want to apply them separately we might need to change
this $ksft_skip handling part in the second patch.

What should I do to deal with this?
Resend the former for -next and rebase + resend the latter (plus the
fix to remove case 1) for 5.10 without the former patch?
Thanks!

> > diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> > index fb53987..5c86fb1 100755
> > --- a/tools/testing/selftests/net/pmtu.sh
> > +++ b/tools/testing/selftests/net/pmtu.sh
> > @@ -1652,7 +1652,23 @@ run_test() {
> >       return $ret
> >       )
> >       ret=$?
> > -     [ $ret -ne 0 ] && exitcode=1
> > +     case $ret in
> > +             0)
> > +                     all_skipped=false
> > +                     [ $exitcode=$ksft_skip ] && exitcode=0
> > +             ;;
> > +             1)
> > +                     all_skipped=false
> > +                     exitcode=1
> > +             ;;
>
> Does it make sense to remove this case? The handling is identical to
> the default case *).
>

Yes you're right, we can remove this part.

> > +             $ksft_skip)
> > +                     [ $all_skipped = true ] && exitcode=$ksft_skip
> > +             ;;
> > +             *)
> > +                     all_skipped=false
> > +                     exitcode=1
> > +             ;;
> > +     esac
> >
> >       return $ret
> >  }
> > @@ -1786,6 +1802,7 @@ usage() {
> >  #
> >  exitcode=0
> >  desc=0
> > +all_skipped=true
> >
> >  while getopts :ptv o
> >  do
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] selftests: pmtu.sh: improve the test result processing
  2020-11-09  3:42     ` Po-Hsu Lin
@ 2020-11-09 18:09       ` Jakub Kicinski
  2020-11-10  1:36         ` Po-Hsu Lin
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2020-11-09 18:09 UTC (permalink / raw)
  To: Po-Hsu Lin
  Cc: netdev, linux-kselftest, linux-kernel, David Miller, Shuah Khan

On Mon, 9 Nov 2020 11:42:33 +0800 Po-Hsu Lin wrote:
> On Sun, Nov 8, 2020 at 7:02 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Thu,  5 Nov 2020 18:50:51 +0800 Po-Hsu Lin wrote:  
> > > This test will treat all non-zero return codes as failures, it will
> > > make the pmtu.sh test script being marked as FAILED when some
> > > sub-test got skipped.
> > >
> > > Improve the result processing by
> > >   * Only mark the whole test script as SKIP when all of the
> > >     sub-tests were skipped
> > >   * If the sub-tests were either passed or skipped, the overall
> > >     result will be PASS
> > >   * If any of them has failed, the overall result will be FAIL
> > >   * Treat other return codes (e.g. 127 for command not found) as FAIL
> > >
> > > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>  
> >
> > Patch 1 looks like a cleanup while patch 2 is more of a fix, can we
> > separate the two and apply the former to -next and latter to 5.10?
> > They shouldn't conflict, right?
> >  
> 
> Hello Jakub,
> 
> Yes the first patch is just changing return code to $ksft_skip, the
> real fix is the second one. However the second patch was based on the
> first one, if we want to apply them separately we might need to change
> this $ksft_skip handling part in the second patch.

Ah, I misread the situation, ksft_skip is 4, not 2, so the patch is
more than just refactoring.

> What should I do to deal with this?
> Resend the former for -next and rebase + resend the latter (plus the
> fix to remove case 1) for 5.10 without the former patch?

Let's apply both of the patches to net-next if that's fine with you.
Indeed detangling them is may be more effort that it's worth.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] selftests: pmtu.sh: improve the test result processing
  2020-11-09 18:09       ` Jakub Kicinski
@ 2020-11-10  1:36         ` Po-Hsu Lin
  0 siblings, 0 replies; 7+ messages in thread
From: Po-Hsu Lin @ 2020-11-10  1:36 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, linux-kselftest, linux-kernel, David Miller, Shuah Khan

On Tue, Nov 10, 2020 at 2:09 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 9 Nov 2020 11:42:33 +0800 Po-Hsu Lin wrote:
> > On Sun, Nov 8, 2020 at 7:02 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > On Thu,  5 Nov 2020 18:50:51 +0800 Po-Hsu Lin wrote:
> > > > This test will treat all non-zero return codes as failures, it will
> > > > make the pmtu.sh test script being marked as FAILED when some
> > > > sub-test got skipped.
> > > >
> > > > Improve the result processing by
> > > >   * Only mark the whole test script as SKIP when all of the
> > > >     sub-tests were skipped
> > > >   * If the sub-tests were either passed or skipped, the overall
> > > >     result will be PASS
> > > >   * If any of them has failed, the overall result will be FAIL
> > > >   * Treat other return codes (e.g. 127 for command not found) as FAIL
> > > >
> > > > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > >
> > > Patch 1 looks like a cleanup while patch 2 is more of a fix, can we
> > > separate the two and apply the former to -next and latter to 5.10?
> > > They shouldn't conflict, right?
> > >
> >
> > Hello Jakub,
> >
> > Yes the first patch is just changing return code to $ksft_skip, the
> > real fix is the second one. However the second patch was based on the
> > first one, if we want to apply them separately we might need to change
> > this $ksft_skip handling part in the second patch.
>
> Ah, I misread the situation, ksft_skip is 4, not 2, so the patch is
> more than just refactoring.
>
> > What should I do to deal with this?
> > Resend the former for -next and rebase + resend the latter (plus the
> > fix to remove case 1) for 5.10 without the former patch?
>
> Let's apply both of the patches to net-next if that's fine with you.
> Indeed detangling them is may be more effort that it's worth.

That would be great, but allow me to resend V2 to get rid of case 1 first.
Thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-10  1:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-05 10:50 [PATCH 0/2] selftests: pmtu.sh: improve the test result processing Po-Hsu Lin
2020-11-05 10:50 ` [PATCH 1/2] selftests: pmtu.sh: use $ksft_skip for skipped return code Po-Hsu Lin
2020-11-05 10:50 ` [PATCH 2/2] selftests: pmtu.sh: improve the test result processing Po-Hsu Lin
2020-11-07 23:02   ` Jakub Kicinski
2020-11-09  3:42     ` Po-Hsu Lin
2020-11-09 18:09       ` Jakub Kicinski
2020-11-10  1:36         ` Po-Hsu Lin

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).