Re: [PATCH net-next 3/5] selftests: seg6: move SRv6 End PSP setup into its own helper

[Hangbin Liu <hangbin.liu@linux.dev>]

On Thu, Sep 24, 2026 at 11:57:58AM +0000, netdev-bot+sashiko@kernel.org wrote:
> 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] Mid-run re-arming of the SETUP_ERR latch converts already-
>   recorded test FAILures into an overall SKIP.

OK, I will update the commit description.

> - [Low] setup_end_flv_psp()
>   (tools/testing/selftests/net/srv6_end_flavors_test.sh:651-670) is…

ack, I will move the ret guard to this patch.

Thanks
Hangbin
> 
> --
> 
> selftests: seg6: move SRv6 End PSP setup into its own helper
> 
> This moves the End PSP policy installation out of setup() into a new
> setup_end_flv_psp() helper, and calls that helper from
> host_srv6_end_flv_psp_tests() with errexit enabled around it. The intent
> is to decouple the base topology setup from the flavor-specific test case.
> 
> > diff --git a/tools/testing/selftests/net/srv6_end_flavors_test.sh b/tools/testing/selftests/net/srv6_end_flavors_test.sh
> > index 318487eda671b..56a1c5f865a77 100755
> > --- a/tools/testing/selftests/net/srv6_end_flavors_test.sh
> > +++ b/tools/testing/selftests/net/srv6_end_flavors_test.sh
> > @@ -645,6 +645,12 @@ setup()
> >  	setup_rt_local_sids 3 "1 2 4"
> >  	setup_rt_local_sids 4 "1 2 3"
> >  
> > +	# testing environment was set up successfully
> > +	SETUP_ERR=0
> > +}
> > +
> > +setup_end_flv_psp()
> > +{
> >  	# set up SRv6 policies
> >  	# create a connection between hosts hs-1 and hs-2.
> >  	# The path between hs-1 and hs-2 traverses SRv6 aware routers.
> 
> [ ... ]
> 
> >  	#  - rt-1 (SRv6 End flavor PSP with SL=1)
> >  	setup_rt_policy_ipv6 2 1 "3:noflv 4:psp 2:psp"
> >  	setup_rt_policy_ipv6 1 2 "1:psp"
> > -
> > -	# testing environment was set up successfully
> > -	SETUP_ERR=0
> >  }
> 
> [Severity: Low]
> The commit message says the split lets "each PSP test case can setup and
> later cleanup independently", but this commit only adds the setup half.
> setup_end_flv_psp() installs the localsid seg6local routes, the cafe::<dst>
> H.Insert route and the proxy NDP entries through __setup_rt_policy(), and
> nothing releases them except the global EXIT trap cleanup(), which just
> does:
> 
> 	for i in ${ROUTERS}; do
> 		nsname="$(get_rtname "${i}")"
> 		ip netns del "${nsname}" &>/dev/null || true
> 	done
> 
> Would it make sense to mention in the commit message that the matching
> cleanup_end_flv_psp() arrives in the following patch ("selftest: seg6: add
> explicit cleanup for SRv6 End PSP test")? As it stands the description
> promises a teardown path that does not exist yet at this commit. There is no
> functional problem here, since the two PSP policies are the only flavor
> state installed and netns deletion reclaims everything.
> 
> >  
> >  check_rt_connectivity()
> > @@ -758,7 +761,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
> 
> [Severity: Low]
> Can re-arming SETUP_ERR here turn already-recorded failures into a skip?
> 
> By the time host_srv6_end_flv_psp_tests() runs, router_tests() and
> host2gateway_tests() have already executed and may have set ret=1 and
> incremented nfail:
> 
> 	setup
> 	set +e
> 
> 	router_tests
> 	host2gateway_tests
> 	host_srv6_end_flv_psp_tests
> 
> 	print_log_test_results
> 
> If any ip command inside setup_end_flv_psp() fails, for example the
> "encap seg6local action End flavors psp" route add, errexit aborts the
> script while SETUP_ERR is still 1, and the EXIT trap takes this path in
> cleanup():
> 
> 	if [ "${SETUP_ERR}" -ne 0 ]; then
> 		echo "SKIP: Setting up the testing environment failed"
> 		exit "${ksft_skip}"
> 	fi
> 
> 	exit "${ret}"
> 
> so the script exits 4 (skip) instead of reaching exit ${ret} with ret=1, and
> print_log_test_results is never reached, meaning the tests passed/failed
> summary is lost too.
> 
> The final patch of this series ("selftests: seg6: add End.X PSP selftest")
> changes the guard to also require [ "$ret" -ne "$ksft_fail" ], which removes
> the fail-to-skip masking. Could that guard change be moved earlier in the
> series, so the intermediate commits do not carry the masking behaviour?
> 
> > +
> > +	log_section "SRv6 connectivity test hosts (h1 <-> h2, End flavor PSP)"
> >  
> >  	check_and_log_hs_connectivity 1 2
> >  	check_and_log_hs_connectivity 2 1
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922-srv6_flavors_endx-v1-0-4d01c05c47b7%40kylinos.cn