[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z8CXEngzSuTuWylx@debian>
Date: Thu, 27 Feb 2025 17:47:14 +0100
From: Guillaume Nault <gnault@...hat.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: David Miller <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
Simon Horman <horms@...nel.org>, David Ahern <dsahern@...nel.org>,
Antonio Quartulli <antonio@...delbit.com>,
Ido Schimmel <idosch@...sch.org>
Subject: Re: [PATCH net v3 2/2] selftests: Add IPv6 link-local address
generation tests for GRE devices.
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
>
Powered by blists - more mailing lists