netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/4] selftests: Use busywait() in a couple places
@ 2020-03-02 17:56 Petr Machata
  2020-03-02 17:56 ` [PATCH net-next 1/4] selftests: forwarding: lib: Add tc_rule_handle_stats_get() Petr Machata
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-02 17:56 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel, Petr Machata, David Miller

From: Petr Machata <petrm@mellanox.com>

Two helper function for active waiting for an event were recently
introduced: busywait() as the active-waiting tool, and until_counter_is()
as a configurable predicate that can be plugged into busywait(). Use these
in tc_common and mlxsw's qos_defprio instead of hand-coding equivalents.

Patches #1 and #2 extend lib.sh facilities to make the transition possible.
Patch #3 converts tc_common, and patch #4 qos_defprio.

Petr Machata (4):
  selftests: forwarding: lib: Add tc_rule_handle_stats_get()
  selftests: forwarding: Convert until_counter_is() to take expression
  selftests: forwarding: tc_common: Convert to use busywait
  selftests: mlxsw: qos_defprio: Use until_counter_is

 .../drivers/net/mlxsw/qos_defprio.sh          | 18 +++--------
 .../drivers/net/mlxsw/sch_red_core.sh         |  6 ++--
 tools/testing/selftests/net/forwarding/lib.sh | 17 ++++++++--
 .../selftests/net/forwarding/tc_common.sh     | 32 +++----------------
 4 files changed, 25 insertions(+), 48 deletions(-)

-- 
2.20.1


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

* [PATCH net-next 1/4] selftests: forwarding: lib: Add tc_rule_handle_stats_get()
  2020-03-02 17:56 [PATCH net-next 0/4] selftests: Use busywait() in a couple places Petr Machata
@ 2020-03-02 17:56 ` Petr Machata
  2020-03-02 17:56 ` [PATCH net-next 2/4] selftests: forwarding: Convert until_counter_is() to take expression Petr Machata
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-02 17:56 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel, Petr Machata, David Miller, Amit Cohen

From: Petr Machata <petrm@mellanox.com>

The function tc_rule_stats_get() fetches a given statistic of a TC rule
given the rule preference. Another common way to reference a rule is using
its handle. Introduce a dual to the aforementioned function that gets a
statistic given rule handle.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Amit Cohen <amitc@mellanox.com>
---
 tools/testing/selftests/net/forwarding/lib.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 83fd15e3e545..de57e8887a7c 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -626,6 +626,17 @@ tc_rule_stats_get()
 	    | jq ".[1].options.actions[].stats$selector"
 }
 
+tc_rule_handle_stats_get()
+{
+	local id=$1; shift
+	local handle=$1; shift
+	local selector=${1:-.packets}; shift
+
+	tc -j -s filter show $id \
+	    | jq ".[] | select(.options.handle == $handle) | \
+		  .options.actions[0].stats$selector"
+}
+
 ethtool_stats_get()
 {
 	local dev=$1; shift
-- 
2.20.1


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

* [PATCH net-next 2/4] selftests: forwarding: Convert until_counter_is() to take expression
  2020-03-02 17:56 [PATCH net-next 0/4] selftests: Use busywait() in a couple places Petr Machata
  2020-03-02 17:56 ` [PATCH net-next 1/4] selftests: forwarding: lib: Add tc_rule_handle_stats_get() Petr Machata
@ 2020-03-02 17:56 ` Petr Machata
  2020-03-02 17:56 ` [PATCH net-next 3/4] selftests: forwarding: tc_common: Convert to use busywait Petr Machata
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-02 17:56 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel, Petr Machata, David Miller, Amit Cohen

From: Petr Machata <petrm@mellanox.com>

until_counter_is() currently takes as an argument a number and the
condition holds when the current counter value is >= that number. Make the
function more generic by taking a partial expression instead of just the
number.

Convert the two existing users.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Amit Cohen <amitc@mellanox.com>
---
 tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh | 6 +++---
 tools/testing/selftests/net/forwarding/lib.sh             | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
index ebf7752f6d93..8f833678ac4d 100644
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
@@ -351,7 +351,7 @@ build_backlog()
 	local i=0
 
 	while :; do
-		local cur=$(busywait 1100 until_counter_is $((cur + 1)) \
+		local cur=$(busywait 1100 until_counter_is "> $cur" \
 					    get_qdisc_backlog $vlan)
 		local diff=$((size - cur))
 		local pkts=$(((diff + 7999) / 8000))
@@ -481,14 +481,14 @@ do_mc_backlog_test()
 	start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) bc
 	start_tcp_traffic $h2.$vlan $(ipaddr 2 $vlan) $(ipaddr 3 $vlan) bc
 
-	qbl=$(busywait 5000 until_counter_is 500000 \
+	qbl=$(busywait 5000 until_counter_is ">= 500000" \
 		       get_qdisc_backlog $vlan)
 	check_err $? "Could not build MC backlog"
 
 	# Verify that we actually see the backlog on BUM TC. Do a busywait as
 	# well, performance blips might cause false fail.
 	local ebl
-	ebl=$(busywait 5000 until_counter_is 500000 \
+	ebl=$(busywait 5000 until_counter_is ">= 500000" \
 		       get_mc_transmit_queue $vlan)
 	check_err $? "MC backlog reported by qdisc not visible in ethtool"
 
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index de57e8887a7c..7ecce65d08f9 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -277,11 +277,11 @@ wait_for_offload()
 
 until_counter_is()
 {
-	local value=$1; shift
+	local expr=$1; shift
 	local current=$("$@")
 
 	echo $((current))
-	((current >= value))
+	((current $expr))
 }
 
 busywait_for_counter()
@@ -290,7 +290,7 @@ busywait_for_counter()
 	local delta=$1; shift
 
 	local base=$("$@")
-	busywait "$timeout" until_counter_is $((base + delta)) "$@"
+	busywait "$timeout" until_counter_is ">= $((base + delta))" "$@"
 }
 
 setup_wait_dev()
-- 
2.20.1


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

* [PATCH net-next 3/4] selftests: forwarding: tc_common: Convert to use busywait
  2020-03-02 17:56 [PATCH net-next 0/4] selftests: Use busywait() in a couple places Petr Machata
  2020-03-02 17:56 ` [PATCH net-next 1/4] selftests: forwarding: lib: Add tc_rule_handle_stats_get() Petr Machata
  2020-03-02 17:56 ` [PATCH net-next 2/4] selftests: forwarding: Convert until_counter_is() to take expression Petr Machata
@ 2020-03-02 17:56 ` Petr Machata
  2020-03-02 17:56 ` [PATCH net-next 4/4] selftests: mlxsw: qos_defprio: Use until_counter_is Petr Machata
  2020-03-04  1:04 ` [PATCH net-next 0/4] selftests: Use busywait() in a couple places David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-02 17:56 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel, Petr Machata, David Miller, Amit Cohen

From: Petr Machata <petrm@mellanox.com>

A function busywait() was recently added based on the logic in
__tc_check_packets(). Convert the code in tc_common to use the new
function.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Amit Cohen <amitc@mellanox.com>
---
 .../selftests/net/forwarding/tc_common.sh     | 32 +++----------------
 1 file changed, 4 insertions(+), 28 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/tc_common.sh b/tools/testing/selftests/net/forwarding/tc_common.sh
index 64f652633585..0e18e8be6e2a 100644
--- a/tools/testing/selftests/net/forwarding/tc_common.sh
+++ b/tools/testing/selftests/net/forwarding/tc_common.sh
@@ -6,39 +6,14 @@ CHECK_TC="yes"
 # Can be overridden by the configuration file. See lib.sh
 TC_HIT_TIMEOUT=${TC_HIT_TIMEOUT:=1000} # ms
 
-__tc_check_packets()
-{
-	local id=$1
-	local handle=$2
-	local count=$3
-	local operator=$4
-
-	start_time="$(date -u +%s%3N)"
-	while true
-	do
-		cmd_jq "tc -j -s filter show $id" \
-		       ".[] | select(.options.handle == $handle) | \
-			    select(.options.actions[0].stats.packets $operator $count)" \
-		    &> /dev/null
-		ret=$?
-		if [[ $ret -eq 0 ]]; then
-			return $ret
-		fi
-		current_time="$(date -u +%s%3N)"
-		diff=$(expr $current_time - $start_time)
-		if [ "$diff" -gt "$TC_HIT_TIMEOUT" ]; then
-			return 1
-		fi
-	done
-}
-
 tc_check_packets()
 {
 	local id=$1
 	local handle=$2
 	local count=$3
 
-	__tc_check_packets "$id" "$handle" "$count" "=="
+	busywait "$TC_HIT_TIMEOUT" until_counter_is "== $count" \
+		 tc_rule_handle_stats_get "$id" "$handle" > /dev/null
 }
 
 tc_check_packets_hitting()
@@ -46,5 +21,6 @@ tc_check_packets_hitting()
 	local id=$1
 	local handle=$2
 
-	__tc_check_packets "$id" "$handle" 0 ">"
+	busywait "$TC_HIT_TIMEOUT" until_counter_is "> 0" \
+		 tc_rule_handle_stats_get "$id" "$handle" > /dev/null
 }
-- 
2.20.1


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

* [PATCH net-next 4/4] selftests: mlxsw: qos_defprio: Use until_counter_is
  2020-03-02 17:56 [PATCH net-next 0/4] selftests: Use busywait() in a couple places Petr Machata
                   ` (2 preceding siblings ...)
  2020-03-02 17:56 ` [PATCH net-next 3/4] selftests: forwarding: tc_common: Convert to use busywait Petr Machata
@ 2020-03-02 17:56 ` Petr Machata
  2020-03-04  1:04 ` [PATCH net-next 0/4] selftests: Use busywait() in a couple places David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-02 17:56 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel, Petr Machata, David Miller, Amit Cohen

From: Petr Machata <petrm@mellanox.com>

Instead of hand-coding the busywait() predicate, use the until_counter_is()
introduced recently.

Signed-off-by: Petr Machata <petrm@mellanox.com>
Reviewed-by: Amit Cohen <amitc@mellanox.com>
---
 .../selftests/drivers/net/mlxsw/qos_defprio.sh | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_defprio.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_defprio.sh
index eff6393ce974..71066bc4b886 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_defprio.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_defprio.sh
@@ -114,23 +114,12 @@ ping_ipv4()
 	ping_test $h1 192.0.2.2
 }
 
-wait_for_packets()
-{
-	local t0=$1; shift
-	local prio_observe=$1; shift
-
-	local t1=$(ethtool_stats_get $swp1 rx_frames_prio_$prio_observe)
-	local delta=$((t1 - t0))
-	echo $delta
-	((delta >= 10))
-}
-
 __test_defprio()
 {
 	local prio_install=$1; shift
 	local prio_observe=$1; shift
-	local delta
 	local key
+	local t1
 	local i
 
 	RET=0
@@ -139,9 +128,10 @@ __test_defprio()
 
 	local t0=$(ethtool_stats_get $swp1 rx_frames_prio_$prio_observe)
 	mausezahn -q $h1 -d 100m -c 10 -t arp reply
-	delta=$(busywait "$HIT_TIMEOUT" wait_for_packets $t0 $prio_observe)
+	t1=$(busywait "$HIT_TIMEOUT" until_counter_is ">= $((t0 + 10))" \
+		ethtool_stats_get $swp1 rx_frames_prio_$prio_observe)
 
-	check_err $? "Default priority $prio_install/$prio_observe: Expected to capture 10 packets, got $delta."
+	check_err $? "Default priority $prio_install/$prio_observe: Expected to capture 10 packets, got $((t1 - t0))."
 	log_test "Default priority $prio_install/$prio_observe"
 
 	defprio_uninstall $swp1 $prio_install
-- 
2.20.1


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

* Re: [PATCH net-next 0/4] selftests: Use busywait() in a couple places
  2020-03-02 17:56 [PATCH net-next 0/4] selftests: Use busywait() in a couple places Petr Machata
                   ` (3 preceding siblings ...)
  2020-03-02 17:56 ` [PATCH net-next 4/4] selftests: mlxsw: qos_defprio: Use until_counter_is Petr Machata
@ 2020-03-04  1:04 ` David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-03-04  1:04 UTC (permalink / raw)
  To: me; +Cc: netdev, idosch, petrm

From: Petr Machata <me@pmachata.org>
Date: Mon,  2 Mar 2020 19:56:01 +0200

> From: Petr Machata <petrm@mellanox.com>
> 
> Two helper function for active waiting for an event were recently
> introduced: busywait() as the active-waiting tool, and until_counter_is()
> as a configurable predicate that can be plugged into busywait(). Use these
> in tc_common and mlxsw's qos_defprio instead of hand-coding equivalents.
> 
> Patches #1 and #2 extend lib.sh facilities to make the transition possible.
> Patch #3 converts tc_common, and patch #4 qos_defprio.

Series applied, thanks.

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

end of thread, other threads:[~2020-03-04  1:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-02 17:56 [PATCH net-next 0/4] selftests: Use busywait() in a couple places Petr Machata
2020-03-02 17:56 ` [PATCH net-next 1/4] selftests: forwarding: lib: Add tc_rule_handle_stats_get() Petr Machata
2020-03-02 17:56 ` [PATCH net-next 2/4] selftests: forwarding: Convert until_counter_is() to take expression Petr Machata
2020-03-02 17:56 ` [PATCH net-next 3/4] selftests: forwarding: tc_common: Convert to use busywait Petr Machata
2020-03-02 17:56 ` [PATCH net-next 4/4] selftests: mlxsw: qos_defprio: Use until_counter_is Petr Machata
2020-03-04  1:04 ` [PATCH net-next 0/4] selftests: Use busywait() in a couple places David Miller

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