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

[Guillaume Nault <gnault@redhat.com>]

On Thu, Feb 27, 2025 at 01:33:25PM +0100, Paolo Abeni wrote:
> 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.

I considered using lib.sh, after Ido's suggestion in v2
(https://lore.kernel.org/netdev/Z7sw1PPY48pkEMxB@shredder/). But I
finally decided to keep the selftest as-is because the small gain in
code size reduction didn't seem worth and because lib.sh makes the script
slower as it mandates the use of bash
(https://lore.kernel.org/netdev/Z73dm4P+Ho4EiprQ@debian/).

> > +}
> > +
> > +# 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

That allows to fail cleanly, with the error message about environment
setup failure.

> > +}
> > +
> > +# 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
> 
> Thanks,
> 
> Paolo
>