netdev - Re: [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zp-QZjrSYsPJ6ig0@u-jnixdorf.ads.avm.de>
Date: Tue, 23 Jul 2024 13:13:42 +0200
From: Johannes Nixdorf <jnixdorf-oss@....de>
To: Hangbin Liu <liuhangbin@...il.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Shuah Khan <shuah@...nel.org>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net] selftests: forwarding: skip if kernel not support
 setting bridge fdb learning limit

On Tue, Jul 23, 2024 at 04:22:52PM +0800, Hangbin Liu wrote:
> If the testing kernel doesn't support setting fdb_max_learned or show
> fdb_n_learned, just skip it. Or we will get errors like
> 
> ./bridge_fdb_learning_limit.sh: line 218: [: null: integer expression expected
> ./bridge_fdb_learning_limit.sh: line 225: [: null: integer expression expected
> 
> Fixes: 6f84090333bb ("selftests: forwarding: bridge_fdb_learning_limit: Add a new selftest")
> Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> ---
>  .../forwarding/bridge_fdb_learning_limit.sh    | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/forwarding/bridge_fdb_learning_limit.sh b/tools/testing/selftests/net/forwarding/bridge_fdb_learning_limit.sh
> index 0760a34b7114..a21b7085da2e 100755
> --- a/tools/testing/selftests/net/forwarding/bridge_fdb_learning_limit.sh
> +++ b/tools/testing/selftests/net/forwarding/bridge_fdb_learning_limit.sh
> @@ -178,6 +178,22 @@ fdb_del()
>  	check_err $? "Failed to remove a FDB entry of type ${type}"
>  }
>  
> +check_fdb_n_learned_support()
> +{
> +	if ! ip link help bridge 2>&1 | grep -q "fdb_max_learned"; then
> +		echo "SKIP: iproute2 too old, missing bridge max learned support"
> +		exit $ksft_skip
> +	fi
> +
> +	ip link add dev br0 type bridge
> +	local learned=$(fdb_get_n_learned)
> +	ip link del dev br0
> +	if [ "$learned" == "null" ]; then
> +		echo "SKIP: kernel too old; bridge fdb_n_learned feature not supported."
> +		exit $ksft_skip
> +	fi
> +}
> +
>  check_accounting_one_type()
>  {
>  	local type=$1 is_counted=$2 overrides_learned=$3
> @@ -274,6 +290,8 @@ check_limit()
>  	done
>  }
>  
> +check_fdb_n_learned_support
> +
>  trap cleanup EXIT
>  
>  setup_prepare
> -- 
> 2.45.0

Thanks for the fix. I also assumed that it's fine to depend on new
features after trying to find out how those feature tests are usually
done from the surrounding tests and their history.

The code looks right to me, and seems to behave as expected when feeding
it data with and without fdb_n_learned.

Reviewed-by: Johannes Nixdorf <jnixdorf-oss@....de>