Re: [PATCH net v3 2/2] selftests: Add IPv6 link-local address generation tests for GRE devices.

[Petr Machata <petrm@nvidia.com>]

Paolo Abeni <pabeni@redhat.com> writes:

> On 2/25/25 3:43 PM, Guillaume Nault wrote:
>> diff --git a/tools/testing/selftests/net/gre_ipv6_lladdr.sh b/tools/testing/selftests/net/gre_ipv6_lladdr.sh
>> new file mode 100755
>> index 000000000000..85e40b6df55e
>> --- /dev/null
>> +++ b/tools/testing/selftests/net/gre_ipv6_lladdr.sh
>> @@ -0,0 +1,227 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +ERR=4 # Return 4 by default, which is the SKIP code for kselftest
>> +PAUSE_ON_FAIL="no"
>> +
>> +readonly NS0=$(mktemp -u ns0-XXXXXXXX)
>> +
>> +# Exit the script after having removed the network namespaces it created
>> +#
>> +# Parameters:
>> +#
>> +#   * The list of network namespaces to delete before exiting.
>> +#
>> +exit_cleanup()
>> +{
>> +	for ns in "$@"; do
>> +		ip netns delete "${ns}" 2>/dev/null || true
>> +	done
>> +
>> +	if [ "${ERR}" -eq 4 ]; then
>> +		echo "Error: Setting up the testing environment failed." >&2
>> +	fi
>> +
>> +	exit "${ERR}"
>
> I'm sorry for the late feedback, but if you use the helper from lib.sh
> you could avoid some code duplication for ns setup and cleanup.
>
>> +}
>> +
>> +# Create the network namespaces used by the script (NS0)
>> +#
>> +create_namespaces()
>> +{
>> +	ip netns add "${NS0}" || exit_cleanup
>
> Also no need to check for failures at this point. If there is no
> namespace support most/all selftests will fail badly
>
>> +}
>> +
>> +# The trap function handler
>> +#
>> +exit_cleanup_all()
>> +{
>> +	exit_cleanup "${NS0}"
>> +}
>> +
>> +# Add fake IPv4 and IPv6 networks on the loopback device, to be used as
>> +# underlay by future GRE devices.
>> +#
>> +setup_basenet()
>> +{
>> +	ip -netns "${NS0}" link set dev lo up
>> +	ip -netns "${NS0}" address add dev lo 192.0.2.10/24
>> +	ip -netns "${NS0}" address add dev lo 2001:db8::10/64 nodad
>> +}
>> +
>> +# Check if network device has an IPv6 link-local address assigned.
>> +#
>> +# Parameters:
>> +#
>> +#   * $1: The network device to test
>> +#   * $2: An extra regular expression that should be matched (to verify the
>> +#         presence of extra attributes)
>> +#   * $3: The expected return code from grep (to allow checking the abscence of
>> +#         a link-local address)
>> +#   * $4: The user visible name for the scenario being tested
>> +#
>> +check_ipv6_ll_addr()
>> +{
>> +	local DEV="$1"
>> +	local EXTRA_MATCH="$2"
>> +	local XRET="$3"
>> +	local MSG="$4"
>> +	local RET
>> +
>> +	printf "%-75s  " "${MSG}"
>> +
>> +	set +e
>> +	ip -netns "${NS0}" -6 address show dev "${DEV}" scope link | grep "fe80::" | grep -q "${EXTRA_MATCH}"
>> +	RET=$?
>> +	set -e
>> +
>> +	if [ "${RET}" -eq "${XRET}" ]; then
>> +		printf "[ OK ]\n"
>
> You can use check_err / log_test from lib.sh to reduce code duplication
> with other tests and more consistent output.
>
>> +	else
>> +		ERR=1
>> +		printf "[FAIL]\n"
>> +		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
>> +			printf "\nHit enter to continue, 'q' to quit\n"
>> +			read -r a
>> +			if [ "$a" = "q" ]; then
>> +				exit 1
>> +			fi
>> +		fi
>
> I guess something like this could be placed into lib.sh, but that would
> be net-next material

The pause-on-fail bits? lib.sh has them as pause_on_fail(). log_test()
invokes them on FAIL an XFAIL results.

FWIW, this is untested, but with lib.sh, I think it would be:

check_ipv6_ll_addr()
{
	local DEV="$1"
	local EXTRA_MATCH="$2"
	local XRET="$3"
	local MSG="$4"

	RET=0
        set +e
	ip -netns "${NS0}" -6 address show dev "${DEV}" scope link | grep "fe80::" | grep -q "${EXTRA_MATCH}"
        check_err_fail $XRET $? ""
        log_test "${MSG}"
        set -e
}

The set +e domain needs to extend over log_test() as well, because that
at one point calls log_test_result() with one fewer argument, and the
shift in there produces a non-zero exit code.