gre: Fix regressions in IPv6 link-local address generation.

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 <[email protected]>

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +15 -0

new file mode 100755
index 000000000000..85e40b6df55e
@@ -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()

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +37 -0

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

Quoted 21 lines · Guillaume Nault

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +15 -0

new file mode 100755
index 000000000000..85e40b6df55e
@@ -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()

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 25

Quoted 10 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +10 -0

+{
+	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.

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 32

Quoted 7 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +7 -0

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

Quoted 37 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +37 -0

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

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 79

Quoted 10 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +10 -0

+
+	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.

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 89

Quoted 10 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +10 -0

+	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

Quoted 26 lines · Guillaume Nault

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +20 -0

new file mode 100755
index 000000000000..85e40b6df55e
@@ -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
+

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 25

Quoted 5 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +5 -0

+	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.

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 32

Quoted 7 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +7 -0

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

Quoted 37 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +37 -0

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

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 79

Quoted 10 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +10 -0

+
+	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.

tools/testing/selftests/net/gre_ipv6_lladdr.sh line 89

Quoted 10 lines

tools/testing/selftests/net/gre_ipv6_lladdr.sh new file +10 -0

+	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