[PATCH] nvme: add optional ipv6 test flavor

[Shinichiro Kawasaki]

Hi Sagi,

I tried to run nvme tests with nvme_adrfam=ipv6 and confirmed it works.
Please find my minor comments in line.

On Jun 26, 2022 / 17:32, Sagi Grimberg wrote:
> Allow Setting nvme_adrfam=[ipv4|ipv6] and running tests on either
> address family. Ignored for non IP transports.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>  common/multipath-over-rdma | 10 ++++++++++
>  tests/nvme/rc              | 31 ++++++++++++++++++++++++++++---
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
> index f9d7b9a236ce..4307ceb4aae1 100644
> --- a/common/multipath-over-rdma
> +++ b/common/multipath-over-rdma
> @@ -74,6 +74,16 @@ get_ipv6_addr() {
>  		sed -n 's/.*[[:blank:]]inet6[[:blank:]]*\([^[:blank:]/]*\).*/\1/p'
>  }
>  
> +# grab only the link local address
> +get_ipv6_ll_addr() {
> +	if [ ! -e "/sys/class/net/$1" ]; then
> +		echo "get_ipv6_addr(): $1 is not a network interface" 1>&2
> +	fi
> +	ll_addr=$(ip -6 -o addr show dev "$1" | grep "scope link" |
> +		sed -n 's/.*[[:blank:]]inet6[[:blank:]]*\([^[:blank:]/]*\).*/\1/p')
> +	echo "$ll_addr%$1"
> +}
> +
>  # Whether or not $1 is a number.
>  is_number() {
>  	[ "$1" -eq "0$1" ] 2>/dev/null
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 998b18164306..b36cc19ec3c1 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -38,6 +38,24 @@ _nvme_requires() {
>  		SKIP_REASON="unsupported nvme_trtype=${nvme_trtype}"
>  		return 1
>  	esac
> +
> +	if [[ ! -z ${nvme_adrfam} ]]; then

Shellcheck complains about the line above:

$ make check
shellcheck -x -e SC2119 -f gcc check new common/* \
        tests/*/rc tests/*/[0-9]*[0-9]
tests/nvme/rc:42:8: note: Use -n instead of ! -z. [SC2236]
make: *** [Makefile:21: check] Error 1

> +		case ${nvme_adrfam} in
> +		ipv6)
> +			def_traddr="::1"
> +			def_adrfam="ipv6"
> +			;;
> +		ipv4)
> +			;; # was already set
> +		*)
> +			# ignore for non ip transports
> +			if [[ "${nvme_trtype}" == "tcp" || "${nvme_trtype}" == "rdma" ]]; then

The line above exceeds the 80 characters per line limit. I suggest to make it
two lines splitting after the || operator.

I think the line below is ok to exceed the 80 chars to keep the string within
one line.

> +				SKIP_REASON="unsupported nvme_adrfam=${nvme_adrfam}"
> +				return 1
> +			fi
> +		esac
> +	fi
> +
>  	return 0
>  }
>  
> @@ -148,9 +166,16 @@ _setup_nvmet() {
>  		start_soft_rdma
>  		for i in $(rdma_network_interfaces)
>  		do
> -			ipv4_addr=$(get_ipv4_addr "$i")
> -			if [ -n "${ipv4_addr}" ]; then
> -				def_traddr=${ipv4_addr}
> +			if [[ "${nvme_adrfam}" == "ipv6" ]]; then
> +				ipv6_addr=$(get_ipv6_ll_addr "$i")
> +				if [ -n "${ipv6_addr}" ]; then

I don't care much but double brackets [[ ]] and single brackets [ ] are mixed.
Double brackets are rather preferred within blktests.

> +					def_traddr=${ipv6_addr}
> +				fi
> +			else
> +				ipv4_addr=$(get_ipv4_addr "$i")
> +				if [ -n "${ipv4_addr}" ]; then
> +					def_traddr=${ipv4_addr}
> +				fi
>  			fi
>  		done
>  	fi
> -- 
> 2.34.1
> 

-- 
Shin'ichiro Kawasaki