* [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit
@ 2024-07-23 8:22 Hangbin Liu
2024-07-23 8:30 ` Nikolay Aleksandrov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hangbin Liu @ 2024-07-23 8:22 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Hangbin Liu, Nikolay Aleksandrov, Johannes Nixdorf,
linux-kselftest
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@gmail.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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit
2024-07-23 8:22 [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit Hangbin Liu
@ 2024-07-23 8:30 ` Nikolay Aleksandrov
2024-07-23 9:24 ` Paolo Abeni
2024-07-23 11:13 ` Johannes Nixdorf
2024-07-24 12:00 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2024-07-23 8:30 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Shuah Khan, Johannes Nixdorf, linux-kselftest
On 23/07/2024 11:22, 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@gmail.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
Isn't the selftest supposed to be added after the feature was included?
I don't understand why this one is special, we should have the same
issue with all new features.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit
2024-07-23 8:30 ` Nikolay Aleksandrov
@ 2024-07-23 9:24 ` Paolo Abeni
2024-07-23 9:43 ` Nikolay Aleksandrov
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2024-07-23 9:24 UTC (permalink / raw)
To: Nikolay Aleksandrov, Hangbin Liu, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Shuah Khan,
Johannes Nixdorf, linux-kselftest
On 7/23/24 10:30, Nikolay Aleksandrov wrote:
> On 23/07/2024 11:22, 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@gmail.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
>
> Isn't the selftest supposed to be added after the feature was included?
>
> I don't understand why this one is special, we should have the same
> issue with all new features.
I must admit I was surprised when I learned the fact, but the stable
team routinely runs up2date upstream self-tests on top of stable/older
kernels:
https://lore.kernel.org/mptcp/ZAHLYvOPEYghRcJ1@kroah.com/
The expected self-test design is to probe the tested feature and skip if
not available in the running kernel. The self-test should not break when
run on an older kernel not offering such feature.
I understand some (most?) of the self-tests do not cope with the above
perfectly, but we can improve ;).
Thanks,
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit
2024-07-23 9:24 ` Paolo Abeni
@ 2024-07-23 9:43 ` Nikolay Aleksandrov
0 siblings, 0 replies; 6+ messages in thread
From: Nikolay Aleksandrov @ 2024-07-23 9:43 UTC (permalink / raw)
To: Paolo Abeni, Hangbin Liu, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Shuah Khan,
Johannes Nixdorf, linux-kselftest
On 23/07/2024 12:24, Paolo Abeni wrote:
>
>
> On 7/23/24 10:30, Nikolay Aleksandrov wrote:
>> On 23/07/2024 11:22, 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@gmail.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
>>
>> Isn't the selftest supposed to be added after the feature was included?
>>
>> I don't understand why this one is special, we should have the same
>> issue with all new features.
>
> I must admit I was surprised when I learned the fact, but the stable team routinely runs up2date upstream self-tests on top of stable/older kernels:
>
> https://lore.kernel.org/mptcp/ZAHLYvOPEYghRcJ1@kroah.com/
>
> The expected self-test design is to probe the tested feature and skip if not available in the running kernel. The self-test should not break when run on an older kernel not offering such feature.
>
> I understand some (most?) of the self-tests do not cope with the above perfectly, but we can improve ;).
>
> Thanks,
>
> Paolo
>
>
Interesting, and unexpected at least for me. :) But okay, I'll keep it in mind
when sending/reviewing patches.
Thanks!
For the patch:
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit
2024-07-23 8:22 [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit Hangbin Liu
2024-07-23 8:30 ` Nikolay Aleksandrov
@ 2024-07-23 11:13 ` Johannes Nixdorf
2024-07-24 12:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Nixdorf @ 2024-07-23 11:13 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Shuah Khan, Nikolay Aleksandrov, linux-kselftest
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@gmail.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@avm.de>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit
2024-07-23 8:22 [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit Hangbin Liu
2024-07-23 8:30 ` Nikolay Aleksandrov
2024-07-23 11:13 ` Johannes Nixdorf
@ 2024-07-24 12:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-07-24 12:00 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, davem, edumazet, kuba, pabeni, shuah, razor, jnixdorf-oss,
linux-kselftest
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Tue, 23 Jul 2024 16:22:52 +0800 you 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@gmail.com>
>
> [...]
Here is the summary with links:
- [net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit
https://git.kernel.org/netdev/net/c/863ff546fb62
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-24 12:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-23 8:22 [PATCH net] selftests: forwarding: skip if kernel not support setting bridge fdb learning limit Hangbin Liu
2024-07-23 8:30 ` Nikolay Aleksandrov
2024-07-23 9:24 ` Paolo Abeni
2024-07-23 9:43 ` Nikolay Aleksandrov
2024-07-23 11:13 ` Johannes Nixdorf
2024-07-24 12:00 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox