diff mbox series

[net-next,v3,4/4] selftests: add ring and coalesce selftests

Message ID 20201113231655.139948-4-acardace@redhat.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series None | expand

Commit Message

Antonio Cardace Nov. 13, 2020, 11:16 p.m. UTC 
Add scripts to test ring and coalesce settings
of netdevsim.

Signed-off-by: Antonio Cardace <acardace@redhat.com>
---
 .../drivers/net/netdevsim/ethtool-coalesce.sh | 68 +++++++++++++++++++
 .../drivers/net/netdevsim/ethtool-ring.sh     | 53 +++++++++++++++
 2 files changed, 121 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh

Comments

Jakub Kicinski Nov. 17, 2020, 12:45 a.m. UTC | #1 
On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:
> Add scripts to test ring and coalesce settings
> of netdevsim.
> 
> Signed-off-by: Antonio Cardace <acardace@redhat.com>

> @@ -0,0 +1,68 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +source ethtool-common.sh
> +
> +function get_value {
> +    local key=$1
> +
> +    echo $(ethtool -c $NSIM_NETDEV | \
> +        awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> +}
> +
> +if ! ethtool -h | grep -q coalesce; then
> +    echo "SKIP: No --coalesce support in ethtool"
> +    exit 4

I think the skip exit code for selftests is 2

> +fi
> +
> +NSIM_NETDEV=$(make_netdev)
> +
> +set -o pipefail
> +
> +declare -A SETTINGS_MAP=(
> +    ["rx-frames-low"]="rx-frame-low"
> +    ["tx-frames-low"]="tx-frame-low"
> +    ["rx-frames-high"]="rx-frame-high"
> +    ["tx-frames-high"]="tx-frame-high"
> +    ["rx-usecs"]="rx-usecs"
> +    ["rx-frames"]="rx-frames"
> +    ["rx-usecs-irq"]="rx-usecs-irq"
> +    ["rx-frames-irq"]="rx-frames-irq"
> +    ["tx-usecs"]="tx-usecs"
> +    ["tx-frames"]="tx-frames"
> +    ["tx-usecs-irq"]="tx-usecs-irq"
> +    ["tx-frames-irq"]="tx-frames-irq"
> +    ["stats-block-usecs"]="stats-block-usecs"
> +    ["pkt-rate-low"]="pkt-rate-low"
> +    ["rx-usecs-low"]="rx-usecs-low"
> +    ["tx-usecs-low"]="tx-usecs-low"
> +    ["pkt-rate-high"]="pkt-rate-high"
> +    ["rx-usecs-high"]="rx-usecs-high"
> +    ["tx-usecs-high"]="tx-usecs-high"
> +    ["sample-interval"]="sample-interval"
> +)
> +
> +for key in ${!SETTINGS_MAP[@]}; do
> +    query_key=${SETTINGS_MAP[$key]}
> +    value=$((RANDOM % $((2**32-1))))
> +    ethtool -C $NSIM_NETDEV "$key" "$value"
> +    s=$(get_value "$query_key")

It would be better to validate the entire config, not just the most
recently set key. This way we would catch the cases where setting
attr breaks the value of another.

> +    check $? "$s" "$value"
> +done
Antonio Cardace Nov. 17, 2020, 11:32 a.m. UTC | #2 
On Mon, Nov 16, 2020 at 04:45:03PM -0800, Jakub Kicinski wrote:
> On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:
> > Add scripts to test ring and coalesce settings
> > of netdevsim.
> > 
> > Signed-off-by: Antonio Cardace <acardace@redhat.com>
> 
> > @@ -0,0 +1,68 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +source ethtool-common.sh
> > +
> > +function get_value {
> > +    local key=$1
> > +
> > +    echo $(ethtool -c $NSIM_NETDEV | \
> > +        awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> > +}
> > +
> > +if ! ethtool -h | grep -q coalesce; then
> > +    echo "SKIP: No --coalesce support in ethtool"
> > +    exit 4
> 
> I think the skip exit code for selftests is 2
In the ethtool-pause.sh selftest the exit code is 4 (I copied it from
there), should I change that too?
> 
> > +fi
> > +
> > +NSIM_NETDEV=$(make_netdev)
> > +
> > +set -o pipefail
> > +
> > +declare -A SETTINGS_MAP=(
> > +    ["rx-frames-low"]="rx-frame-low"
> > +    ["tx-frames-low"]="tx-frame-low"
> > +    ["rx-frames-high"]="rx-frame-high"
> > +    ["tx-frames-high"]="tx-frame-high"
> > +    ["rx-usecs"]="rx-usecs"
> > +    ["rx-frames"]="rx-frames"
> > +    ["rx-usecs-irq"]="rx-usecs-irq"
> > +    ["rx-frames-irq"]="rx-frames-irq"
> > +    ["tx-usecs"]="tx-usecs"
> > +    ["tx-frames"]="tx-frames"
> > +    ["tx-usecs-irq"]="tx-usecs-irq"
> > +    ["tx-frames-irq"]="tx-frames-irq"
> > +    ["stats-block-usecs"]="stats-block-usecs"
> > +    ["pkt-rate-low"]="pkt-rate-low"
> > +    ["rx-usecs-low"]="rx-usecs-low"
> > +    ["tx-usecs-low"]="tx-usecs-low"
> > +    ["pkt-rate-high"]="pkt-rate-high"
> > +    ["rx-usecs-high"]="rx-usecs-high"
> > +    ["tx-usecs-high"]="tx-usecs-high"
> > +    ["sample-interval"]="sample-interval"
> > +)
> > +
> > +for key in ${!SETTINGS_MAP[@]}; do
> > +    query_key=${SETTINGS_MAP[$key]}
> > +    value=$((RANDOM % $((2**32-1))))
> > +    ethtool -C $NSIM_NETDEV "$key" "$value"
> > +    s=$(get_value "$query_key")
> 
> It would be better to validate the entire config, not just the most
> recently set key. This way we would catch the cases where setting
> attr breaks the value of another.
> 
Good idea, will do.

Thanks,
Antonio
Jakub Kicinski Nov. 17, 2020, 5:15 p.m. UTC | #3 
On Tue, 17 Nov 2020 12:32:36 +0100 Antonio Cardace wrote:
> On Mon, Nov 16, 2020 at 04:45:03PM -0800, Jakub Kicinski wrote:
> > On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:  
> > > Add scripts to test ring and coalesce settings
> > > of netdevsim.
> > > 
> > > Signed-off-by: Antonio Cardace <acardace@redhat.com>  
> >   
> > > @@ -0,0 +1,68 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +source ethtool-common.sh
> > > +
> > > +function get_value {
> > > +    local key=$1
> > > +
> > > +    echo $(ethtool -c $NSIM_NETDEV | \
> > > +        awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> > > +}
> > > +
> > > +if ! ethtool -h | grep -q coalesce; then
> > > +    echo "SKIP: No --coalesce support in ethtool"
> > > +    exit 4  
> > 
> > I think the skip exit code for selftests is 2  
> In the ethtool-pause.sh selftest the exit code is 4 (I copied it from
> there), should I change that too?

Sorry I misremembered it's 4. We can leave that as is.
Willem de Bruijn Nov. 18, 2020, 9:43 p.m. UTC | #4 
On Tue, Nov 17, 2020 at 12:19 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Nov 2020 12:32:36 +0100 Antonio Cardace wrote:
> > On Mon, Nov 16, 2020 at 04:45:03PM -0800, Jakub Kicinski wrote:
> > > On Sat, 14 Nov 2020 00:16:55 +0100 Antonio Cardace wrote:
> > > > Add scripts to test ring and coalesce settings
> > > > of netdevsim.
> > > >
> > > > Signed-off-by: Antonio Cardace <acardace@redhat.com>
> > >
> > > > @@ -0,0 +1,68 @@
> > > > +#!/bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > +
> > > > +source ethtool-common.sh
> > > > +
> > > > +function get_value {
> > > > +    local key=$1
> > > > +
> > > > +    echo $(ethtool -c $NSIM_NETDEV | \
> > > > +        awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
> > > > +}
> > > > +
> > > > +if ! ethtool -h | grep -q coalesce; then
> > > > +    echo "SKIP: No --coalesce support in ethtool"
> > > > +    exit 4
> > >
> > > I think the skip exit code for selftests is 2
> > In the ethtool-pause.sh selftest the exit code is 4 (I copied it from
> > there), should I change that too?
>
> Sorry I misremembered it's 4. We can leave that as is.

Instead of having to remember, maybe we should have a file in
tools/testing/selftest to define constants?

I defined them one-off in tools/testing/selftests/net/udpgso_bench.sh

readonly KSFT_PASS=0
readonly KSFT_FAIL=1
readonly KSFT_SKIP=4

along with some other kselftest shell support infra. But having each
test figure this out independently is duplicative and error prone.
Jakub Kicinski Nov. 18, 2020, 11:19 p.m. UTC | #5 
On Wed, 18 Nov 2020 16:43:33 -0500 Willem de Bruijn wrote:
> On Tue, Nov 17, 2020 at 12:19 PM Jakub Kicinski <kuba@kernel.org> wrote:
> > Sorry I misremembered it's 4. We can leave that as is.  
> 
> Instead of having to remember, maybe we should have a file in
> tools/testing/selftest to define constants?
> 
> I defined them one-off in tools/testing/selftests/net/udpgso_bench.sh
> 
> readonly KSFT_PASS=0
> readonly KSFT_FAIL=1
> readonly KSFT_SKIP=4
> 
> along with some other kselftest shell support infra. But having each
> test figure this out independently is duplicative and error prone.

Sounds like a good idea, I was surprised it wasn't already defined in
any lib.

CCing the selftest ML.
diff mbox series

Patch

diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh
new file mode 100755
index 000000000000..3b322c99be69
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-coalesce.sh
@@ -0,0 +1,68 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+source ethtool-common.sh
+
+function get_value {
+    local key=$1
+
+    echo $(ethtool -c $NSIM_NETDEV | \
+        awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[ \t]/, "", $2); print $2}')
+}
+
+if ! ethtool -h | grep -q coalesce; then
+    echo "SKIP: No --coalesce support in ethtool"
+    exit 4
+fi
+
+NSIM_NETDEV=$(make_netdev)
+
+set -o pipefail
+
+declare -A SETTINGS_MAP=(
+    ["rx-frames-low"]="rx-frame-low"
+    ["tx-frames-low"]="tx-frame-low"
+    ["rx-frames-high"]="rx-frame-high"
+    ["tx-frames-high"]="tx-frame-high"
+    ["rx-usecs"]="rx-usecs"
+    ["rx-frames"]="rx-frames"
+    ["rx-usecs-irq"]="rx-usecs-irq"
+    ["rx-frames-irq"]="rx-frames-irq"
+    ["tx-usecs"]="tx-usecs"
+    ["tx-frames"]="tx-frames"
+    ["tx-usecs-irq"]="tx-usecs-irq"
+    ["tx-frames-irq"]="tx-frames-irq"
+    ["stats-block-usecs"]="stats-block-usecs"
+    ["pkt-rate-low"]="pkt-rate-low"
+    ["rx-usecs-low"]="rx-usecs-low"
+    ["tx-usecs-low"]="tx-usecs-low"
+    ["pkt-rate-high"]="pkt-rate-high"
+    ["rx-usecs-high"]="rx-usecs-high"
+    ["tx-usecs-high"]="tx-usecs-high"
+    ["sample-interval"]="sample-interval"
+)
+
+for key in ${!SETTINGS_MAP[@]}; do
+    query_key=${SETTINGS_MAP[$key]}
+    value=$((RANDOM % $((2**32-1))))
+    ethtool -C $NSIM_NETDEV "$key" "$value"
+    s=$(get_value "$query_key")
+    check $? "$s" "$value"
+done
+
+# bool settings which ethtool displays on the same line
+ethtool -C $NSIM_NETDEV adaptive-rx on
+s=$(ethtool -c $NSIM_NETDEV | grep -q "Adaptive RX: on  TX: off")
+check $? "$s" ""
+
+ethtool -C $NSIM_NETDEV adaptive-tx on
+s=$(ethtool -c $NSIM_NETDEV | grep -q "Adaptive RX: on  TX: on")
+check $? "$s" ""
+
+if [ $num_errors -eq 0 ]; then
+    echo "PASSED all $((num_passes)) checks"
+    exit 0
+else
+    echo "FAILED $num_errors/$((num_errors+num_passes)) checks"
+    exit 1
+fi
diff --git a/tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh b/tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh
new file mode 100755
index 000000000000..513b9875c637
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/ethtool-ring.sh
@@ -0,0 +1,53 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+source ethtool-common.sh
+
+function get_value {
+    local key=$1
+
+    echo $(ethtool -g $NSIM_NETDEV | \
+        tail -n +$CURR_SETT_LINE | \
+        awk -F':' -v pattern="$key:" '$0 ~ pattern {gsub(/[\t ]/, "", $2); print $2}')
+}
+
+if ! ethtool -h | grep -q set-ring >/dev/null; then
+    echo "SKIP: No --set-ring support in ethtool"
+    exit 4
+fi
+
+NSIM_NETDEV=$(make_netdev)
+
+set -o pipefail
+
+declare -A SETTINGS_MAP=(
+    ["rx"]="RX"
+    ["rx-mini"]="RX Mini"
+    ["rx-jumbo"]="RX Jumbo"
+    ["tx"]="TX"
+)
+
+MAX_VALUE=$((RANDOM % $((2**32-1))))
+RING_MAX_LIST=$(ls $NSIM_DEV_DFS/ethtool/ring/)
+
+for ring_max_entry in $RING_MAX_LIST; do
+    echo $MAX_VALUE > $NSIM_DEV_DFS/ethtool/ring/$ring_max_entry
+done
+
+CURR_SETT_LINE=$(ethtool -g $NSIM_NETDEV | grep -i -m1 -n 'Current hardware settings' | cut -f1 -d:)
+
+for key in ${!SETTINGS_MAP[@]}; do
+    query_key=${SETTINGS_MAP[$key]}
+    value=$((RANDOM % $MAX_VALUE))
+    ethtool -G $NSIM_NETDEV "$key" "$value"
+    s=$(get_value "$query_key")
+    check $? "$s" "$value"
+done
+
+if [ $num_errors -eq 0 ]; then
+    echo "PASSED all $((num_passes)) checks"
+    exit 0
+else
+    echo "FAILED $num_errors/$((num_errors+num_passes)) checks"
+    exit 1
+fi