[PATCH net v3 0/2] gre: Fix regressions in IPv6 link-local address generation.

[PATCH net v3 0/2] gre: Fix regressions in IPv6 link-local address generation.

[Guillaume Nault]

IPv6 link-local address generation has some special cases for GRE
devices. This has led to several regressions in the past, and some of
them are still not fixed. This series fixes the remaining problems,
like the ipv6.conf.<dev>.addr_gen_mode sysctl being ignored and the
router discovery process not being started (see details in patch 1).

To avoid any further regressions, patch 2 adds selftests covering
IPv4 and IPv6 gre/gretap devices with all combinations of currently
supported addr_gen_mode values.

v3: Rework patch 1's commit message (typos + GRE device types
    clarifications).
v2: Add Makefile entry for the new selftest (patch 2).

Guillaume Nault (2):
  gre: Fix IPv6 link-local address generation.
  selftests: Add IPv6 link-local address generation tests for GRE
    devices.

 net/ipv6/addrconf.c                           |  15 +-
 tools/testing/selftests/net/Makefile          |   1 +
 .../testing/selftests/net/gre_ipv6_lladdr.sh  | 227 ++++++++++++++++++
 3 files changed, 237 insertions(+), 6 deletions(-)
 create mode 100755 tools/testing/selftests/net/gre_ipv6_lladdr.sh

-- 
2.39.2

[PATCH net v3 1/2] gre: Fix IPv6 link-local address generation.

[Guillaume Nault]

Use addrconf_addr_gen() to generate IPv6 link-local addresses on GRE
devices in most cases and fall back to using add_v4_addrs() only in
case the GRE configuration is incompatible with addrconf_addr_gen().

GRE used to use addrconf_addr_gen() until commit e5dd729460ca
("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL
address") restricted this use to gretap and ip6gretap devices, and
created add_v4_addrs() (borrowed from SIT) for non-Ethernet GRE ones.

The original problem came when commit 9af28511be10 ("addrconf: refuse
isatap eui64 for INADDR_ANY") made __ipv6_isatap_ifid() fail when its
addr parameter was 0. The commit says that this would create an invalid
address, however, I couldn't find any RFC saying that the generated
interface identifier would be wrong. Anyway, since gre over IPv4
devices pass their local tunnel address to __ipv6_isatap_ifid(), that
commit broke their IPv6 link-local address generation when the local
address was unspecified.

Then commit e5dd729460ca ("ip/ip6_gre: use the same logic as SIT
interfaces when computing v6LL address") tried to fix that case by
defining add_v4_addrs() and calling it to generate the IPv6 link-local
address instead of using addrconf_addr_gen() (apart for gretap and
ip6gretap devices, which would still use the regular
addrconf_addr_gen(), since they have a MAC address).

That broke several use cases because add_v4_addrs() isn't properly
integrated into the rest of IPv6 Neighbor Discovery code. Several of
these shortcomings have been fixed over time, but add_v4_addrs()
remains broken on several aspects. In particular, it doesn't send any
Router Sollicitations, so the SLAAC process doesn't start until the
interface receives a Router Advertisement. Also, add_v4_addrs() mostly
ignores the address generation mode of the interface
(/proc/sys/net/ipv6/conf/*/addr_gen_mode), thus breaking the
IN6_ADDR_GEN_MODE_RANDOM and IN6_ADDR_GEN_MODE_STABLE_PRIVACY cases.

Fix the situation by using add_v4_addrs() only in the specific scenario
where the normal method would fail. That is, for interfaces that have
all of the following characteristics:

  * run over IPv4,
  * transport IP packets directly, not Ethernet (that is, not gretap
    interfaces),
  * tunnel endpoint is INADDR_ANY (that is, 0),
  * device address generation mode is EUI64.

In all other cases, revert back to the regular addrconf_addr_gen().

Also, remove the special case for ip6gre interfaces in add_v4_addrs(),
since ip6gre devices now always use addrconf_addr_gen() instead.

Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address")
Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
v3: Rework commit message to make it clearer which types of GRE devices
    we're talking about (Ido).
v2: No changes.

 net/ipv6/addrconf.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ac8cc1076536..8b6258819dad 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3209,16 +3209,13 @@ static void add_v4_addrs(struct inet6_dev *idev)
 	struct in6_addr addr;
 	struct net_device *dev;
 	struct net *net = dev_net(idev->dev);
-	int scope, plen, offset = 0;
+	int scope, plen;
 	u32 pflags = 0;
 
 	ASSERT_RTNL();
 
 	memset(&addr, 0, sizeof(struct in6_addr));
-	/* in case of IP6GRE the dev_addr is an IPv6 and therefore we use only the last 4 bytes */
-	if (idev->dev->addr_len == sizeof(struct in6_addr))
-		offset = sizeof(struct in6_addr) - 4;
-	memcpy(&addr.s6_addr32[3], idev->dev->dev_addr + offset, 4);
+	memcpy(&addr.s6_addr32[3], idev->dev->dev_addr, 4);
 
 	if (!(idev->dev->flags & IFF_POINTOPOINT) && idev->dev->type == ARPHRD_SIT) {
 		scope = IPV6_ADDR_COMPATv4;
@@ -3529,7 +3526,13 @@ static void addrconf_gre_config(struct net_device *dev)
 		return;
 	}
 
-	if (dev->type == ARPHRD_ETHER) {
+	/* Generate the IPv6 link-local address using addrconf_addr_gen(),
+	 * unless we have an IPv4 GRE device not bound to an IP address and
+	 * which is in EUI64 mode (as __ipv6_isatap_ifid() would fail in this
+	 * case). Such devices fall back to add_v4_addrs() instead.
+	 */
+	if (!(dev->type == ARPHRD_IPGRE && *(__be32 *)dev->dev_addr == 0 &&
+	      idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_EUI64)) {
 		addrconf_addr_gen(idev, true);
 		return;
 	}
-- 
2.39.2

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

[Guillaume Nault]

GRE devices have their special code for IPv6 link-local address
generation that has been the source of several regressions in the past.

Add selftest to check that all gre, ip6gre, gretap and ip6gretap get an
IPv6 link-link local address in accordance with the
net.ipv6.conf.<dev>.addr_gen_mode sysctl.

Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Guillaume Nault <gnault@redhat.com>
---
v3: No changes.
v2: Add selftest to Makefile.

 tools/testing/selftests/net/Makefile          |   1 +
 .../testing/selftests/net/gre_ipv6_lladdr.sh  | 227 ++++++++++++++++++
 2 files changed, 228 insertions(+)
 create mode 100755 tools/testing/selftests/net/gre_ipv6_lladdr.sh

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 73ee88d6b043..5916f3b81c39 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -31,6 +31,7 @@ TEST_PROGS += veth.sh
 TEST_PROGS += ioam6.sh
 TEST_PROGS += gro.sh
 TEST_PROGS += gre_gso.sh
+TEST_PROGS += gre_ipv6_lladdr.sh
 TEST_PROGS += cmsg_so_mark.sh
 TEST_PROGS += cmsg_so_priority.sh
 TEST_PROGS += cmsg_time.sh cmsg_ipv6.sh
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}"
+}
+
+# Create the network namespaces used by the script (NS0)
+#
+create_namespaces()
+{
+	ip netns add "${NS0}" || exit_cleanup
+}
+
+# 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"
+	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
+	fi
+}
+
+
+# Create a GRE device and verify that it gets an IPv6 link-local address as
+# expected.
+#
+# Parameters:
+#
+#   * $1: The device type (gre, ip6gre, gretap or ip6gretap)
+#   * $2: The local underlay IP address (can be an IPv4, an IPv6 or "any")
+#   * $3: The remote underlay IP address (can be an IPv4, an IPv6 or "any")
+#   * $4: The IPv6 interface identifier generation mode to use for the GRE
+#         device (eui64, none, stable-privacy or random).
+#
+test_gre_device()
+{
+	local GRE_TYPE="$1"
+	local LOCAL_IP="$2"
+	local REMOTE_IP="$3"
+	local MODE="$4"
+	local ADDR_GEN_MODE
+	local MATCH_REGEXP
+	local MSG
+
+	ip link add netns "${NS0}" name gretest type "${GRE_TYPE}" local "${LOCAL_IP}" remote "${REMOTE_IP}"
+
+	case "${MODE}" in
+	    "eui64")
+		ADDR_GEN_MODE=0
+		MATCH_REGEXP=""
+		MSG="${GRE_TYPE}, mode: 0 (EUI64), ${LOCAL_IP} -> ${REMOTE_IP}"
+		XRET=0
+		;;
+	    "none")
+		ADDR_GEN_MODE=1
+		MATCH_REGEXP=""
+		MSG="${GRE_TYPE}, mode: 1 (none), ${LOCAL_IP} -> ${REMOTE_IP}"
+		XRET=1 # No link-local address should be generated
+		;;
+	    "stable-privacy")
+		ADDR_GEN_MODE=2
+		MATCH_REGEXP="stable-privacy"
+		MSG="${GRE_TYPE}, mode: 2 (stable privacy), ${LOCAL_IP} -> ${REMOTE_IP}"
+		XRET=0
+		# Initialise stable_secret (required for stable-privacy mode)
+		ip netns exec "${NS0}" sysctl -qw net.ipv6.conf.gretest.stable_secret="2001:db8::abcd"
+		;;
+	    "random")
+		ADDR_GEN_MODE=3
+		MATCH_REGEXP="stable-privacy"
+		MSG="${GRE_TYPE}, mode: 3 (random), ${LOCAL_IP} -> ${REMOTE_IP}"
+		XRET=0
+		;;
+	esac
+
+	# Check that IPv6 link-local address is generated when device goes up
+	ip netns exec "${NS0}" sysctl -qw net.ipv6.conf.gretest.addr_gen_mode="${ADDR_GEN_MODE}"
+	ip -netns "${NS0}" link set dev gretest up
+	check_ipv6_ll_addr gretest "${MATCH_REGEXP}" "${XRET}" "config: ${MSG}"
+
+	# Now disable link-local address generation
+	ip -netns "${NS0}" link set dev gretest down
+	ip netns exec "${NS0}" sysctl -qw net.ipv6.conf.gretest.addr_gen_mode=1
+	ip -netns "${NS0}" link set dev gretest up
+
+	# Check that link-local address generation works when re-enabled while
+	# the device is already up
+	ip netns exec "${NS0}" sysctl -qw net.ipv6.conf.gretest.addr_gen_mode="${ADDR_GEN_MODE}"
+	check_ipv6_ll_addr gretest "${MATCH_REGEXP}" "${XRET}" "update: ${MSG}"
+
+	ip -netns "${NS0}" link del dev gretest
+}
+
+test_gre4()
+{
+	local GRE_TYPE
+	local MODE
+
+	for GRE_TYPE in "gre" "gretap"; do
+		printf "\n####\nTesting IPv6 link-local address generation on ${GRE_TYPE} devices\n####\n\n"
+
+		for MODE in "eui64" "none" "stable-privacy" "random"; do
+			test_gre_device "${GRE_TYPE}" 192.0.2.10 192.0.2.11 "${MODE}"
+			test_gre_device "${GRE_TYPE}" any 192.0.2.11 "${MODE}"
+			test_gre_device "${GRE_TYPE}" 192.0.2.10 any "${MODE}"
+		done
+	done
+}
+
+test_gre6()
+{
+	local GRE_TYPE
+	local MODE
+
+	for GRE_TYPE in "ip6gre" "ip6gretap"; do
+		printf "\n####\nTesting IPv6 link-local address generation on ${GRE_TYPE} devices\n####\n\n"
+
+		for MODE in "eui64" "none" "stable-privacy" "random"; do
+			test_gre_device "${GRE_TYPE}" 2001:db8::10 2001:db8::11 "${MODE}"
+			test_gre_device "${GRE_TYPE}" any 2001:db8::11 "${MODE}"
+			test_gre_device "${GRE_TYPE}" 2001:db8::10 any "${MODE}"
+		done
+	done
+}
+
+usage()
+{
+	echo "Usage: $0 [-p]"
+	exit 1
+}
+
+while getopts :p o
+do
+	case $o in
+		p) PAUSE_ON_FAIL="yes";;
+		*) usage;;
+	esac
+done
+
+# Create namespaces before setting up the exit trap.
+# Otherwise, exit_cleanup_all() could delete namespaces that were not created
+# by this script.
+create_namespaces
+
+set -e
+trap exit_cleanup_all EXIT
+
+setup_basenet
+
+test_gre4
+test_gre6
+
+if [ "${ERR}" -eq 1 ]; then
+	echo "Some tests failed." >&2
+else
+	ERR=0
+fi
-- 
2.39.2

Re: [PATCH net v3 1/2] gre: Fix IPv6 link-local address generation.

[Ido Schimmel]

On Tue, Feb 25, 2025 at 03:43:20PM +0100, Guillaume Nault wrote:
> Use addrconf_addr_gen() to generate IPv6 link-local addresses on GRE
> devices in most cases and fall back to using add_v4_addrs() only in
> case the GRE configuration is incompatible with addrconf_addr_gen().
> 
> GRE used to use addrconf_addr_gen() until commit e5dd729460ca
> ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL
> address") restricted this use to gretap and ip6gretap devices, and
> created add_v4_addrs() (borrowed from SIT) for non-Ethernet GRE ones.
> 
> The original problem came when commit 9af28511be10 ("addrconf: refuse
> isatap eui64 for INADDR_ANY") made __ipv6_isatap_ifid() fail when its
> addr parameter was 0. The commit says that this would create an invalid
> address, however, I couldn't find any RFC saying that the generated
> interface identifier would be wrong. Anyway, since gre over IPv4
> devices pass their local tunnel address to __ipv6_isatap_ifid(), that
> commit broke their IPv6 link-local address generation when the local
> address was unspecified.
> 
> Then commit e5dd729460ca ("ip/ip6_gre: use the same logic as SIT
> interfaces when computing v6LL address") tried to fix that case by
> defining add_v4_addrs() and calling it to generate the IPv6 link-local
> address instead of using addrconf_addr_gen() (apart for gretap and
> ip6gretap devices, which would still use the regular
> addrconf_addr_gen(), since they have a MAC address).
> 
> That broke several use cases because add_v4_addrs() isn't properly
> integrated into the rest of IPv6 Neighbor Discovery code. Several of
> these shortcomings have been fixed over time, but add_v4_addrs()
> remains broken on several aspects. In particular, it doesn't send any
> Router Sollicitations, so the SLAAC process doesn't start until the
> interface receives a Router Advertisement. Also, add_v4_addrs() mostly
> ignores the address generation mode of the interface
> (/proc/sys/net/ipv6/conf/*/addr_gen_mode), thus breaking the
> IN6_ADDR_GEN_MODE_RANDOM and IN6_ADDR_GEN_MODE_STABLE_PRIVACY cases.
> 
> Fix the situation by using add_v4_addrs() only in the specific scenario
> where the normal method would fail. That is, for interfaces that have
> all of the following characteristics:
> 
>   * run over IPv4,
>   * transport IP packets directly, not Ethernet (that is, not gretap
>     interfaces),
>   * tunnel endpoint is INADDR_ANY (that is, 0),
>   * device address generation mode is EUI64.
> 
> In all other cases, revert back to the regular addrconf_addr_gen().
> 
> Also, remove the special case for ip6gre interfaces in add_v4_addrs(),
> since ip6gre devices now always use addrconf_addr_gen() instead.
> 
> Fixes: e5dd729460ca ("ip/ip6_gre: use the same logic as SIT interfaces when computing v6LL address")
> Signed-off-by: Guillaume Nault <gnault@redhat.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

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

[Paolo Abeni]

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

Thanks,

Paolo

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

[Petr Machata]

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.

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

[Guillaume Nault]

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
>