On Sat, Oct 10, 2026 at 10:21:05AM +0000, [email protected] 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
