[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87v7svfqa0.fsf@nvidia.com>
Date: Thu, 27 Feb 2025 14:13:34 +0100
From: Petr Machata <petrm@...dia.com>
To: Paolo Abeni <pabeni@...hat.com>
CC: Guillaume Nault <gnault@...hat.com>, 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.
Paolo Abeni <pabeni@...hat.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.
Powered by blists - more mailing lists