Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The comment in cleanup()
(tools/testing/selftests/net/srv6_end_flavors_test.sh, lines 328-330)…
--
selftests: seg6: move SRv6 End PSP setup into its own helper
This commit moves the End PSP policy setup out of setup() and into a new
helper, setup_end_flv_psp(). host_srv6_end_flv_psp_tests() now calls that
helper with set -e and SETUP_ERR wrapped around it. cleanup() also changes
so that a setup error only gives SKIP when ret is not already ksft_fail.
> diff --git a/tools/testing/selftests/net/srv6_end_flavors_test.sh
> b/tools/testing/selftests/net/srv6_end_flavors_test.sh
> index 30a939392eea4..5ebd6a0344d69 100755
> --- a/tools/testing/selftests/net/srv6_end_flavors_test.sh
> +++ b/tools/testing/selftests/net/srv6_end_flavors_test.sh
> @@ -328,7 +328,7 @@ cleanup()
> # check whether the setup phase was completed successfully or not. In
> # case of an error during the setup phase of the testing environment,
> # the selftest is considered as "skipped".
> - if [ "${SETUP_ERR}" -ne 0 ]; then
> + if [ "${SETUP_ERR}" -ne 0 ] && [ "$ret" -ne "$ksft_fail" ]; then
[Severity: Low]
This isn't a bug, but the comment above this condition in cleanup() still
says that any setup error makes the selftest "skipped". Should it be
updated to match the new condition?
With this patch, host_srv6_end_flv_psp_tests() starts a second setup
phase after router_tests and host2gateway_tests have already run.
Suppose one of those earlier tests sets ret to ksft_fail, and then
setup_end_flv_psp() fails. cleanup() then runs with SETUP_ERR=1 and ret=1.
The SKIP branch is not taken, and the script exits 1 instead of
ksft_skip.
I couldn't find a later patch in the series that updates this comment.
> echo "SKIP: Setting up the testing environment failed"
> exit "${ksft_skip}"
> fi
[ ... ]
> @@ -741,7 +744,13 @@ host2gateway_tests()
>
> host_srv6_end_flv_psp_tests()
> {
> - log_section "SRv6 connectivity test hosts (h1 <-> h2, PSP flavor)"
> + set -e
> + SETUP_ERR=1
> + setup_end_flv_psp
> + SETUP_ERR=0
> + set +e
[ ... ]
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009-srv6_flavors_endx-v2-0-56ac7f838f41%40kylinos.cn
