netdev - Re: [RFC net-next 3/3] selftests: rds: add testing infrastructure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240607202402.GI27689@kernel.org>
Date: Fri, 7 Jun 2024 21:24:02 +0100
From: Simon Horman <horms@...nel.org>
To: allison.henderson@...cle.com
Cc: netdev@...r.kernel.org
Subject: Re: [RFC net-next 3/3] selftests: rds: add testing infrastructure

On Thu, Jun 06, 2024 at 05:36:31PM -0700, allison.henderson@...cle.com wrote:
> From: Vegard Nossum <vegard.nossum@...cle.com>
> 
> This adds some basic self-testing infrastructure for RDS.
> 
> Signed-off-by: Vegard Nossum <vegard.nossum@...cle.com>
> Signed-off-by: Allison Henderson <allison.henderson@...cle.com>
> Signed-off-by: Chuck Lever <chuck.lever@...cle.com>

Hi Allison,

Some minor nits from my side.

> diff --git a/tools/testing/selftests/net/rds/config.sh b/tools/testing/selftests/net/rds/config.sh
> new file mode 100755
> index 000000000000..c2c36756ba1f
> --- /dev/null
> +++ b/tools/testing/selftests/net/rds/config.sh
> @@ -0,0 +1,33 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#! /bin/bash

#! line needs to be the first line for it to take effect

Flagged by shellcheck.

https://www.shellcheck.net/wiki/SC1128

> +
> +set -e
> +set -u
> +set -x
> +
> +unset KBUILD_OUTPUT
> +
> +# start with a default config
> +make defconfig
> +
> +# no modules
> +scripts/config --disable CONFIG_MODULES
> +
> +# enable RDS
> +scripts/config --enable CONFIG_RDS
> +scripts/config --enable CONFIG_RDS_TCP
> +
> +# instrument RDS and only RDS
> +scripts/config --enable CONFIG_GCOV_KERNEL
> +scripts/config --disable GCOV_PROFILE_ALL
> +scripts/config --enable GCOV_PROFILE_RDS
> +
> +# need network namespaces to run tests with veth network interfaces
> +scripts/config --enable CONFIG_NET_NS
> +scripts/config --enable CONFIG_VETH
> +
> +# simulate packet loss
> +scripts/config --enable CONFIG_NET_SCH_NETEM
> +
> +# generate real .config without asking any questions
> +make olddefconfig
> diff --git a/tools/testing/selftests/net/rds/init.sh b/tools/testing/selftests/net/rds/init.sh
> new file mode 100755
> index 000000000000..a29e3de81ed5
> --- /dev/null
> +++ b/tools/testing/selftests/net/rds/init.sh
> @@ -0,0 +1,49 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +set -u
> +
> +LOG_DIR=/tmp
> +PY_CMD="/usr/bin/python3"
> +while getopts "d:p:" opt; do
> +  case ${opt} in
> +    d)
> +      LOG_DIR=${OPTARG}
> +      ;;
> +    p)
> +      PY_CMD=${OPTARG}
> +      ;;
> +    :)
> +      echo "USAGE: init.sh [-d logdir] [-p python_cmd]"
> +      exit 1
> +      ;;
> +    ?)
> +      echo "Invalid option: -${OPTARG}."
> +      exit 1
> +      ;;
> +  esac
> +done
> +
> +LOG_FILE=$LOG_DIR/rds-strace.txt
> +
> +mount -t proc none /proc
> +mount -t sysfs none /sys
> +mount -t tmpfs none /var/run
> +mount -t debugfs none /sys/kernel/debug
> +
> +echo running RDS tests...
> +echo Traces will be logged to $LOG_FILE
> +rm -f $LOG_FILE
> +strace -T -tt -o "$LOG_FILE" $PY_CMD $(dirname "$0")/test.py -d "$LOG_DIR" || true

Perhaps it can't occur, but I don't think this will behave as
expected if the out put of dirname includes spaces.

Flagged by shellecheck.
https://www.shellcheck.net/wiki/SC2046

Also, $LOG_DIR is quoted here, but not elsewhere, which seems inconsistent.

> +
> +echo saving coverage data...
> +(set +x; cd /sys/kernel/debug/gcov; find * -name '*.gcda' | \

shellcheck warns that:

SC2035 (info): Use ./*glob* or -- *glob* so names with dashes won't become options. 

https://www.shellcheck.net/wiki/SC2035

Although I guess in practice there are no filenames with dashes in
that directory.

> +while read f
> +do
> +	cat < /sys/kernel/debug/gcov/$f > /$f

Again, I guess it doesn't occur in practice.
But should $f be quoted in case it includes whitespace?

> +done)
> +
> +dmesg > $LOG_DIR/dmesg.out
> +
> +/usr/sbin/poweroff --no-wtmp --force

...