Re: [PATCH net-next v2 5/5] selftests: seg6: add End.X PSP selftest

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

On Sat, Oct 10, 2026 at 10:21:05AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The End.X PSP test in
>   tools/testing/selftests/net/srv6_end_flavors_test.sh does not check…

This patch is a series of the End.X PSP support. I don't think anyone will
only backport this selftest without the kernel support.

> - [Low] The patch adds an `<id:op:act>` descriptor syntax and an End.X
>   PSP scenario, but the script's documentation was not updated.

Low priority. If there is a new version. I will update the documents.

Thanks
Hangbin

> 
> --
> 
> selftests: seg6: add End.X PSP selftest
> 
> This commit adds an End.X PSP test to srv6_end_flavors_test.sh. A new
> helper, __get_srv6_rtcfg_act(), lets __setup_rt_policy() parse an
> optional action field in the router description, e.g. "4:psp:End.X,4,2".
> 
> > diff --git a/tools/testing/selftests/net/srv6_end_flavors_test.sh b/tools/testing/selftests/net/srv6_end_flavors_test.sh
> > index a57e92541a046..4ba93ace86563 100755
> > --- a/tools/testing/selftests/net/srv6_end_flavors_test.sh
> > +++ b/tools/testing/selftests/net/srv6_end_flavors_test.sh
> > @@ -397,6 +397,36 @@ __get_srv6_rtcfg_op()
> >  		xargs | sed 's/ /,/g'
> >  }
> >  
> > +# Given the description of a router <id:op:act> as an input, the function
> > +# returns the <act> token which represents the action (e.g. End, End.X,
> > +# End.T behavior with or without route table, out interface) configured
> > +# for the node.
> > +#
> > +# Support End, End.X at present. When omit, return default End behavior.
> > +# i.e. input: "1:psp:End.X,1,3"
> > +#      output: "End.X nh6 fcf0:0:1:3::3 oif veth-rt-1-3"
> > +__get_srv6_rtcfg_act()
> > +{
> > +	local element="$1"
> > +	local net_prefix
> > +	local acts
> > +	local out
> > +
> > +	acts="$(echo "${element}" | cut -d':' -f3)"
> > +	act="$(echo "${acts}" | cut -d',' -f1)"
> > +	act="${act:-"End"}"
> > +
> > +	rt="$(echo "${acts}" | cut -d',' -f2)"
> > +	neigh="$(echo "${acts}" | cut -d',' -f3)"
> > +
> > +	if [ "$act" == "End" ]; then
> > +		echo "End"
> > +	elif [ "$act" == "End.X" ]; then
> > +		net_prefix="$(get_network_prefix "${rt}" "${neigh}")"
> > +		echo "End.X nh6 ${net_prefix}::${neigh} oif veth-rt-${rt}-${neigh}"
> > +	fi
> > +}
> 
> [Severity: Low]
> What should happen when act is something other than End or End.X? The
> comment above mentions End.T, but there is no else branch. For End.T, a
> typo, or a case mismatch, the function prints an empty string, and
> __setup_rt_policy() builds:
> 
>     encap seg6local action  flavors psp
> 
> Under set -e that shows up as an environment setup SKIP, not as a clear
> error in the test description.
> 
> A few smaller points in the same helper:
> 
> act, rt and neigh are not declared local, but the unused out is.
> 
> The End.X form repeats the node id ("4:psp:End.X,4,2") and never checks
> that the two ids match.
> 
> Some existing comments in the file also look stale after this change:
> 
> The file header still says "Currently in this selftest we consider only
> the PSP flavor for the SRv6 End behavior."
> 
> The Local SID table still says "fcff:x::ef1 is associated with the SRv6
> End behavior with PSP flavor". In the new test, fcff:3::ef1 and
> fcff:4::ef1 are End.X PSP.
> 
> The $3 description for __setup_rt_policy() still talks only about
> <nodeid:op> elements and does not mention the new act field.
> 
> [ ... ]
> 
> > @@ -819,6 +875,22 @@ host_srv6_end_flv_psp_tests()
> >  	cleanup_end_flv_psp
> >  }
> >  
> > +host_srv6_endx_flv_psp_tests()
> > +{
> > +	set -e
> > +	SETUP_ERR=1
> > +	setup_endx_flv_psp
> > +	SETUP_ERR=0
> > +	set +e
> 
> [Severity: Medium]
> Does this need a kernel support check for End.X with the PSP flavor?
> The only kernel probe is "test_kernel_supp_or_ksft_skip psp", and it only
> installs "action End flavors psp". It never tries End.X, End.X with
> flavors, or End.X with oif.
> 
> Before the series commit "seg6: add RFC8986 flavor support for End.X",
> net/ipv6/seg6_local.c had:
> 
>     #define SEG6_LOCAL_END_X_FLV_SUPP_OPS	SEG6_F_LOCAL_FLV_NEXT_CSID
> 
> On those kernels parse_nla_flavors() rejects "flavors psp" on End.X with
> -EOPNOTSUPP.
> 
> When that happens, the End.X route add from setup_endx_flv_psp() fails
> under set -e and the EXIT trap runs cleanup(). The earlier test groups
> passed, so ret is still ksft_skip, and cleanup() takes this path:
> 
> 	if [ "${SETUP_ERR}" -ne 0 ] && [ "$ret" -ne "$ksft_fail" ]; then
> 		echo "SKIP: Setting up the testing environment failed"
> 		exit "${ksft_skip}"
> 	fi
> 
> Would this turn a previous PASS on older or stable kernels into a SKIP
> for the whole script? print_log_test_results() never runs, so the End
> PSP results that already passed are dropped.
> 
> On a kernel that is supposed to support End.X PSP, a regression that
> breaks this route install would also be reported as SKIP rather than
> FAIL.
> 
> Could End.X PSP support be checked up front, so that only this test
> group is skipped when the kernel lacks it?
> 
> > +
> > +	log_section "SRv6 connectivity test hosts (h1 <-> h2, End.X flavor PSP)"
> > +
> > +	check_and_log_hs_connectivity 1 2
> > +	check_and_log_hs_connectivity 2 1
> > +
> > +	cleanup_endx_flv_psp
> > +}
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009-srv6_flavors_endx-v2-0-56ac7f838f41%40kylinos.cn