Re: [PATCH net-next 2/2] selftests: net: Add a selftest for externally validated neighbor entries

[Simon Horman <horms@kernel.org>]

On Wed, Jun 11, 2025 at 05:15:51PM +0300, Ido Schimmel wrote:
> Add test cases for externally validated neighbor entries, testing both
> IPv4 and IPv6. Name the file "test_neigh.sh" so that it could be
> possibly extended in the future with more neighbor test cases.
> 
> Example output:
> 
>  # ./test_neigh.sh
>  TEST: IPv4 "extern_valid" flag: Add entry                           [ OK ]
>  TEST: IPv4 "extern_valid" flag: Add with an invalid state           [ OK ]
>  TEST: IPv4 "extern_valid" flag: Add with "use" flag                 [ OK ]
>  TEST: IPv4 "extern_valid" flag: Replace entry                       [ OK ]
>  TEST: IPv4 "extern_valid" flag: Replace entry with "managed" flag   [ OK ]
>  TEST: IPv4 "extern_valid" flag: Replace with an invalid state       [ OK ]
>  TEST: IPv4 "extern_valid" flag: Transition to "reachable" state     [ OK ]
>  TEST: IPv4 "extern_valid" flag: Transition back to "stale" state    [ OK ]
>  TEST: IPv4 "extern_valid" flag: Forced garbage collection           [ OK ]
>  TEST: IPv4 "extern_valid" flag: Periodic garbage collection         [ OK ]
>  TEST: IPv6 "extern_valid" flag: Add entry                           [ OK ]
>  TEST: IPv6 "extern_valid" flag: Add with an invalid state           [ OK ]
>  TEST: IPv6 "extern_valid" flag: Add with "use" flag                 [ OK ]
>  TEST: IPv6 "extern_valid" flag: Replace entry                       [ OK ]
>  TEST: IPv6 "extern_valid" flag: Replace entry with "managed" flag   [ OK ]
>  TEST: IPv6 "extern_valid" flag: Replace with an invalid state       [ OK ]
>  TEST: IPv6 "extern_valid" flag: Transition to "reachable" state     [ OK ]
>  TEST: IPv6 "extern_valid" flag: Transition back to "stale" state    [ OK ]
>  TEST: IPv6 "extern_valid" flag: Forced garbage collection           [ OK ]
>  TEST: IPv6 "extern_valid" flag: Periodic garbage collection         [ OK ]
> 
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>

...

> diff --git a/tools/testing/selftests/net/test_neigh.sh b/tools/testing/selftests/net/test_neigh.sh

...

> +################################################################################
> +# Utilities
> +
> +run_cmd()
> +{
> +	local cmd="$1"
> +	local out
> +	local stderr="2>/dev/null"
> +
> +	if [ "$VERBOSE" = "1" ]; then
> +		printf "COMMAND: $cmd\n"
> +		stderr=
> +	fi

Hi Ido,

shellcheck was recently added to NIPA and it warns about the above as follows.
Could you consider addressing this?

In tools/testing/selftests/net/test_neigh.sh line 21:
 		printf "COMMAND: $cmd\n"
                        ^---------------^ SC2059 (info): Don't use variables in the printf format string. Use printf '..%s..' "$foo".

> +
> +	out=$(eval $cmd $stderr)
> +	rc=$?
> +	if [ "$VERBOSE" -eq 1 ] && [ -n "$out" ]; then
> +		echo "    $out"
> +	fi
> +
> +	return $rc
> +}

...

> +################################################################################
> +# Main
> +
> +while getopts ":t:pvh" opt; do
> +	case $opt in
> +		t) TESTS=$OPTARG;;
> +		p) PAUSE_ON_FAIL=yes;;
> +		v) VERBOSE=$(($VERBOSE + 1));;
> +		h) usage; exit 0;;
> +		*) usage; exit 1;;
> +	esac
> +done

Likewise, shellcheck has the following to say about the above.

In tools/testing/selftests/net/test_neigh.sh line 318:
		v) VERBOSE=$(($VERBOSE + 1));;
                              ^------^ SC2004 (style): $/${} is unnecessary on arithmetic variables.

> +
> +require_command jq
> +
> +ip neigh help 2>&1 | grep -q "extern_valid"
> +if [ $? -ne 0 ]; then
> +   echo "SKIP: iproute2 ip too old, missing \"extern_valid\" support"
> +   exit $ksft_skip
> +fi
> +
> +trap exit_cleanup_all EXIT
> +
> +for t in $TESTS
> +do
> +	setup; $t; cleanup_all_ns;
> +done
> -- 
> 2.49.0
> 

-- 
pw-bot: cr