Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 0 · Low: 1
- [Low] test_sctp_connect_v6() gates itself on `[ -e /proc/sys/net/ipv6
]` (line 717) and on socat's `WITH_SCTP` define (line 716) only, and…
--
selftests/net/openvswitch: add SCTP flow key test over IPv6
Adds test_sctp_connect_v6 to the openvswitch selftest, mirroring the
existing IPv4 SCTP test: icmpv6() flows forward neighbour discovery,
ipv6(proto=132),sctp() flows gate an SCTP association over IPv6, a keyless
ipv6(proto=132) install is expected to be refused with EINVAL, and a payload
is pushed across the datapath.
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index a31f7fb6882d..1160f8ed7d8a 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
[ ... ]
> @@ -700,6 +701,113 @@ test_sctp_connect_v4() {
> return 0
> }
>
> +# sctp_connect_v6 test
> +# - sctp(dst=4443) matches client-to-server INIT
> +# - sctp(src=4443) matches server-to-client INIT-ACK
> +# - icmpv6 NS/NA flows forward neighbour discovery
> +# - remove flows and verify connection fails, reinstall and recover
> +test_sctp_connect_v6() {
> + local t="test_sctp_connect_v6"
> + local v6="eth_type(0x86dd),ipv6(proto=132)"
> + local payload="SCTP6_DATA_OK"
> + local rxfile="${ovs_base}/${t}/sctp-rx.txt"
> +
> + modprobe -q sctp 2>/dev/null || return "$ksft_skip"
> + socat -V 2>&1 | grep -q "define WITH_SCTP" || return "$ksft_skip"
> + [ -e /proc/sys/net/ipv6 ] || return "$ksft_skip"
> +
> + sbx_add "$t" || return $?
> + ovs_add_dp "$t" sctp6 || return 1
> +
> + info "create namespaces"
> + for ns in client server; do
> + ovs_add_netns_and_veths "$t" "sctp6" "$ns" \
> + "${ns:0:1}0" "${ns:0:1}1" || return 1
> + done
> +
> + ip netns exec client ip addr add fd00::1/64 dev c1 nodad
> + ip netns exec client ip link set c1 up
> + ip netns exec server ip addr add fd00::2/64 dev s1 nodad
> + ip netns exec server ip link set s1 up
[Severity: Low]
Two questions about the skip/setup handling in test_sctp_connect_v6().
First, the capability probe only greps socat -V for "define WITH_SCTP", while
this test uses the SCTP6-LISTEN and SCTP6-CONNECT addresses, which also need
socat's IPv6 support (WITH_IP6). On a socat built with SCTP but without IPv6,
the probe passes, the listener never comes up, and:
ovs_wait sctp_eps_has server 4443 || return 1
times out and reports FAIL for the SCTP flow-key path rather than skipping for
a missing userspace prerequisite. Would it make sense to also grep for the
IPv6 define here?
Second, the four addressing commands above discard their exit status. The rest
of the test does not depend on addressing until the association attempt, so if
"ip addr add fd00::1/64 ... nodad" fails (for example an iproute2 that rejects
nodad, or IPv6 disabled per-device in the fresh netns via
net.core.devconf_inherit_init_net), the first visible error is:
ovs_sbx "$t" ip netns exec client \
timeout 3 socat -u STDIN "SCTP6-CONNECT:[fd00::2]:4443" </dev/null \
|| return 1
which attributes a setup failure to the SCTP flow-key path. The file-level
"trap ovs_exit_sig EXIT TERM INT ERR" does not catch these, since set -e/set -E
are not in use and run_test() runs the body in a subshell via
"eval test_${tname}". Should these four commands carry "|| return 1" so the
failure is reported where it happens? The pre-existing test_icmpv6() and
test_sctp_connect_v4() use the same unchecked pattern, so this is the current
convention in the file rather than something new here.
> +
> + # NS/NA forwarding
[ ... ]
> + ovs_netns_spawn_daemon "$t" "server" \
> + socat -u -t 1 SCTP6-LISTEN:4443,fork \
> + OPEN:"$rxfile",creat,append
> + ovs_wait sctp_eps_has server 4443 || return 1
[ ... ]
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909122521.2687193-1-houminxi%40gmail.com
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
