[PATCH net-next v2 0/2] selftests: openvswitch: SCTP flow key coverage (IPv6 + NAT)

[PATCH net-next v2 0/2] selftests: openvswitch: SCTP flow key coverage (IPv6 + NAT)

[Minxi Hou]

The SCTP flow key test merged in August covers only IPv4 and only the
plain forwarding path. This series extends coverage to the two paths a
regression would otherwise ship silently on: SCTP over IPv6, and SCTP
across conntrack NAT (where OVS re-extracts L3/L4 on the recirculated
skb).

Both tests verify the association and then push a known payload across
and confirm the listener received it, so a flow-key bug that matches
handshake packets but breaks DATA chunks is caught rather than passing
on the handshake alone.

Full OVS selftest suite runs 19/19 green on a freshly built kernel.

v2:
  - no code changes; rebased onto current net-next after the tree
    reopened (v1 was deferred while net-next was closed)

Minxi Hou (2):
  selftests/net/openvswitch: add SCTP flow key test over IPv6
  selftests/net/openvswitch: add SCTP flow key test across conntrack NAT

 .../selftests/net/openvswitch/openvswitch.sh  | 183 +++++++++++++++++-
 1 file changed, 182 insertions(+), 1 deletion(-)


base-commit: d0ec95a8a4e79f2fd6063fc8932415db8c227689
-- 
2.55.0

[PATCH net-next v2 1/2] selftests/net/openvswitch: add SCTP flow key test over IPv6

[Minxi Hou]

The merged SCTP test covers only IPv4. The IPv6 extraction path
(parse_ipv6hdr walking extension headers, then the proto=132 branch in
key_extract) and the v6 side of the SCTP netlink validation have no
selftest coverage at all, so a regression there would ship silently.

Add test_sctp_connect_v6 mirroring the v4 test: NS/NA forwarding flows
stand in for ARP, and ipv6(proto=132),sctp(dst=4443)/sctp(src=4443)
flows gate the association in the same three phases (flows installed,
flows removed, flows reinstalled). After the association succeeds the
test also pushes a known payload across and verifies the listener
received it, so a flow-key bug that matches handshake packets but
breaks DATA chunks is caught too. Skips when the sctp module is
missing, socat lacks SCTP support, or IPv6 is unavailable; an
association or DATA failure with the flows installed fails the test.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
---
v2: rebased onto current net-next, no code changes
 .../selftests/net/openvswitch/openvswitch.sh  | 97 ++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index a31f7fb6882dc..06bd98d80a084 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -33,7 +33,8 @@ tests="
 	action_set				set: SET action rewrites fields
 	trunc					trunc: output truncation
 	icmpv6					icmpv6: ICMPv6 echo type match
-	sctp_connect_v4				sctp: SCTP flow key matching
+	sctp_connect_v4			sctp: SCTP flow key matching
+	sctp_connect_v6			sctp6: SCTP flow key matching over IPv6
 	psample					psample: Sampling packets with psample"
 
 info() {
@@ -700,6 +701,100 @@ 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
+
+	# NS/NA forwarding
+	ovs_add_flow "$t" sctp6 \
+	    'in_port(1),eth(),eth_type(0x86dd),ipv6(proto=58),icmpv6()' \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp6 \
+	    'in_port(2),eth(),eth_type(0x86dd),ipv6(proto=58),icmpv6()' \
+	    '1' || return 1
+
+	# SCTP port matching: dst for request, src for reply
+	ovs_add_flow "$t" sctp6 \
+	    "in_port(1),eth(),$v6,sctp(dst=4443)" \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp6 \
+	    "in_port(2),eth(),$v6,sctp(src=4443)" \
+	    '1' || return 1
+
+	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
+
+	info "verify SCTP association with port-keyed flows"
+	ovs_sbx "$t" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP6-CONNECT:[fd00::2]:4443" </dev/null \
+	    || return 1
+
+	info "verify SCTP DATA chunk crosses the datapath"
+	ovs_sbx "$t" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP6-CONNECT:[fd00::2]:4443" \
+	    <<< "$payload" || return 1
+	grep -q "$payload" "$rxfile" 2>/dev/null \
+	    || { info "server did not receive SCTP DATA payload"
+	         return 1; }
+
+	ovs_del_flows "$t" sctp6
+
+	info "verify connection fails without flows"
+	ovs_add_flow "$t" sctp6 \
+	    'in_port(1),eth(),eth_type(0x86dd),ipv6(proto=58),icmpv6()' \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp6 \
+	    'in_port(2),eth(),eth_type(0x86dd),ipv6(proto=58),icmpv6()' \
+	    '1' || return 1
+
+	ovs_sbx "$t" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP6-CONNECT:[fd00::2]:4443" </dev/null \
+	    >/dev/null 2>&1 \
+	    && { info "connection should fail without flows"
+	         return 1; }
+
+	info "reinstall flows and verify recovery"
+	ovs_add_flow "$t" sctp6 \
+	    "in_port(1),eth(),$v6,sctp(dst=4443)" \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp6 \
+	    "in_port(2),eth(),$v6,sctp(src=4443)" \
+	    '1' || return 1
+
+	ovs_sbx "$t" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP6-CONNECT:[fd00::2]:4443" </dev/null \
+	    || return 1
+
+	return 0
+}
+
 # psample test
 # - use psample to observe packets
 test_psample() {

base-commit: d0ec95a8a4e79f2fd6063fc8932415db8c227689
-- 
2.55.0

[PATCH net-next v2 2/2] selftests/net/openvswitch: add SCTP flow key test across conntrack NAT

[Minxi Hou]

Conntrack NAT rewrites the packet between the first-pass and recirc
passes, and OVS re-extracts L3/L4 on the recirculated skb via
ovs_flow_key_update_l3l4(). Nothing in the suite exercises SCTP key
extraction on that post-NAT path: the existing nat test drives TCP
over nc, and the merged SCTP test has no conntrack in the path.

Add test_sctp_nat_connect_v4, reusing the nat4 fixture with the SCTP
socket pair from the v4 SCTP test: untracked ipv4 traffic hits
ct(commit,nat(...)),recirc, and the post-recirc flows match
ct_state(+trk) with the extracted sctp(dst=4443)/sctp(src=4443) ports.
A probe for the SCTP conntrack sysctl turns a kernel without working
SCTP conntrack into a skip instead of an opaque timeout. After the
association succeeds the test pushes a known payload across and
verifies the listener received it, so a flow-key bug that matches
handshake packets but breaks DATA chunks is caught too. If ct+nat is
accepted but the association or DATA fails, the test fails.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
---
v2: rebased onto current net-next, no code changes
 .../selftests/net/openvswitch/openvswitch.sh  | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 06bd98d80a084..d5acdca0a0ac1 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -35,6 +35,7 @@ tests="
 	icmpv6					icmpv6: ICMPv6 echo type match
 	sctp_connect_v4			sctp: SCTP flow key matching
 	sctp_connect_v6			sctp6: SCTP flow key matching over IPv6
+	sctp_nat_connect_v4		sctpnat4: SCTP flow key across conntrack NAT
 	psample					psample: Sampling packets with psample"
 
 info() {
@@ -1186,6 +1187,91 @@ test_nat_connect_v4 () {
 	return 0
 }
 
+# sctp_nat_connect_v4 test
+#  - SCTP association crosses a ct(commit,nat(dst=...)) translation
+#  - post-recirc flows match ct_state(+trk) plus the extracted SCTP ports
+test_sctp_nat_connect_v4 () {
+	local t="test_sctp_nat_connect_v4"
+	local payload="SCTP_NAT_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"
+	# SCTP conntrack is compiled into nf_conntrack.ko, so check that
+	# loading it actually exposed the SCTP conntrack sysctls.
+	modprobe -q nf_conntrack 2>/dev/null || return "$ksft_skip"
+	[ -e /proc/sys/net/netfilter/nf_conntrack_sctp_timeout_established ] \
+	    || { info "no SCTP conntrack support - skipping"
+	         return "$ksft_skip"; }
+
+	sbx_add "test_sctp_nat_connect_v4" || return $?
+
+	ovs_add_dp "test_sctp_nat_connect_v4" sctpnat4 || return 1
+	info "create namespaces"
+	for ns in client server; do
+		ovs_add_netns_and_veths "test_sctp_nat_connect_v4" "sctpnat4" \
+		    "$ns" "${ns:0:1}0" "${ns:0:1}1" || return 1
+	done
+
+	ip netns exec client ip addr add 172.31.110.10/24 dev c1
+	ip netns exec client ip link set c1 up
+	ip netns exec server ip addr add 172.31.110.20/24 dev s1
+	ip netns exec server ip link set s1 up
+
+	ip netns exec client ip route add default via 172.31.110.20
+
+	# Check if the ct action can be configured.
+	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
+		'in_port(1),eth(),eth_type(0x0800),ipv4()' \
+		'ct(commit),recirc(0x1)' &> /dev/null
+	if [ $? == 1 ]; then
+		info "no support for ct action - skipping"
+		ovs_exit_sig
+		return $ksft_skip
+	fi
+
+	ovs_del_flows "test_sctp_nat_connect_v4" sctpnat4
+
+	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
+		'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
+	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
+		'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
+	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
+		"ct_state(-trk),in_port(1),eth(),eth_type(0x0800),ipv4(dst=192.168.0.20)" \
+		"ct(commit,nat(dst=172.31.110.20)),recirc(0x1)" || return 1
+	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
+		"ct_state(-trk),in_port(2),eth(),eth_type(0x0800),ipv4()" \
+		"ct(commit,nat),recirc(0x2)" || return 1
+
+	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
+		"recirc_id(0x1),ct_state(+trk-inv),in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)" \
+		"2" || return 1
+	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
+		"recirc_id(0x2),ct_state(+trk-inv),in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)" \
+		"1" || return 1
+
+	ovs_netns_spawn_daemon "test_sctp_nat_connect_v4" "server" \
+		socat -u -t 1 SCTP4-LISTEN:4443,fork \
+		OPEN:"$rxfile",creat,append
+	ovs_wait sctp_eps_has server 4443 || return 1
+
+	info "verify SCTP association across NAT"
+	ovs_sbx "test_sctp_nat_connect_v4" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP4-CONNECT:192.168.0.20:4443" \
+	    </dev/null || return 1
+
+	info "verify SCTP DATA chunk crosses NAT"
+	ovs_sbx "test_sctp_nat_connect_v4" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP4-CONNECT:192.168.0.20:4443" \
+	    <<< "$payload" || return 1
+	grep -q "$payload" "$rxfile" 2>/dev/null \
+	    || { info "server did not receive SCTP DATA payload"
+	         return 1; }
+
+	info "done..."
+	return 0
+}
+
 # nat_related_v4 test
 #  - client->server ip packets go via SNAT
 #  - client solicits ICMP destination unreachable packet from server

base-commit: d0ec95a8a4e79f2fd6063fc8932415db8c227689
prerequisite-patch-id: a3848bf8ef3803b528f964b8e866dd3622e5b154
-- 
2.55.0

Re: [PATCH net-next v2 1/2] selftests/net/openvswitch: add SCTP flow key test over IPv6

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 2 · Low: 1

- [Medium] Commit message rationale is not supported by the code it
  claims to cover.
- [Medium] Unsynchronized cross-process assertion makes the test flaky.
- [Low] The patch contains a spurious hunk that rewrites the whitespace
  of the pre-existing `sctp_connect_v4` entry in the `tests` list…

--

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 with NS/NA forwarding flows and
ipv6(proto=132),sctp(dst=4443)/sctp(src=4443) flows gating an
association in three phases, plus a payload transfer check.

A couple of questions about the commit message and the new test below.

[Severity: Medium]
The commit message says:

    After the association succeeds the test also pushes a known payload
    across and verifies the listener received it, so a flow-key bug that
    matches handshake packets but breaks DATA chunks is caught too.

Can such a flow-key bug exist?  The SCTP extraction in
key_extract_l3l4() never looks at chunk types, it only copies the two
port fields out of the 12-byte common header:

net/openvswitch/flow.c:key_extract_l3l4() {
    ...
    } else if (key->ip.proto == NEXTHDR_SCTP) {
            if (sctphdr_ok(skb)) {
                    struct sctphdr *sctp = sctp_hdr(skb);
                    key->tp.src = sctp->source;
                    key->tp.dst = sctp->dest;
    ...
}

The IPv4 branch (key->ip.proto == IPPROTO_SCTP) does exactly the same.
So INIT, INIT-ACK, DATA and SACK all produce identical key->tp.src /
key->tp.dst, and the flow key cannot distinguish them.

The same paragraph also says:

    The IPv6 extraction path (parse_ipv6hdr walking extension headers,
    then the proto=132 branch in key_extract) and the v6 side of the SCTP
    netlink validation have no selftest coverage at all

Is that accurate for parse_ipv6hdr()?  The pre-existing test_icmpv6()
in the same script already sends IPv6 traffic through the datapath,
which runs key_extract_l3l4() -> parse_ipv6hdr().  And the new test only
configures fd00::1/fd00::2 and sends plain SCTP6 socat traffic, so no
hop-by-hop, routing or fragment header is ever generated and this loop

net/openvswitch/flow.c:parse_ipv6hdr() {
    ...
    nexthdr = ipv6_find_hdr(skb, &payload_ofs, -1, &frag_off, &flags);
    ...
}

is only exercised in the trivial nexthdr == 132 case.  Could the message
be narrowed to what is genuinely new, i.e. the NEXTHDR_SCTP branch and
the v6 SCTP netlink key?

> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index a31f7fb6882d..06bd98d80a08 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -33,7 +33,8 @@ tests="
>  	action_set				set: SET action rewrites fields
>  	trunc					trunc: output truncation
>  	icmpv6					icmpv6: ICMPv6 echo type match
> -	sctp_connect_v4				sctp: SCTP flow key matching
> +	sctp_connect_v4			sctp: SCTP flow key matching
> +	sctp_connect_v6			sctp6: SCTP flow key matching over IPv6

[Severity: Low]
This isn't a bug, but why is the pre-existing sctp_connect_v4 line
rewritten here?  The four tabs become three, which is unrelated to
adding an IPv6 test and the changelog says v2 has "no code changes".

Word splitting is unaffected because IFS is set to tab+newline before
the "for t in ${tests}" loop, but usage() prints the string verbatim:

	echo "Available tests${tests}"

With one leading tab, a 15-character name and four tabs the description
starts at column 48 like every other entry; with three tabs both the
modified sctp_connect_v4 row and the new sctp_connect_v6 row start at
column 40 and are misaligned against the rest of the help output.

>  	psample					psample: Sampling packets with psample"
>  
>  info() {

[ ... ]

> @@ -700,6 +701,100 @@ test_sctp_connect_v4() {
>  	return 0
>  }
>  
> +# sctp_connect_v6 test

[ ... ]

> +	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
> +
> +	info "verify SCTP association with port-keyed flows"
> +	ovs_sbx "$t" ip netns exec client \
> +	    timeout 3 socat -u STDIN "SCTP6-CONNECT:[fd00::2]:4443" </dev/null \
> +	    || return 1
> +
> +	info "verify SCTP DATA chunk crosses the datapath"
> +	ovs_sbx "$t" ip netns exec client \
> +	    timeout 3 socat -u STDIN "SCTP6-CONNECT:[fd00::2]:4443" \
> +	    <<< "$payload" || return 1
> +	grep -q "$payload" "$rxfile" 2>/dev/null \
> +	    || { info "server did not receive SCTP DATA payload"
> +	         return 1; }

[Severity: Medium]
Can this grep race with the listener and make the test flaky?

$rxfile is written by the forked socat child spawned in the background
by ovs_netns_spawn_daemon():

	ip netns exec $netns $*  >> $ovs_dir/stdout  2>> $ovs_dir/stderr &

The client socat exits once its local send and teardown are done, so its
exit status says nothing about the server child having read the DATA
chunk and written it out.  The grep runs immediately after, with no
barrier in between.

If the server child has not been scheduled yet, the file is either
missing (grep exit 2, hidden by 2>/dev/null) or present but empty (grep
exit 1 - the earlier payload-less association already created it via
creat), and both land in the return 1 branch, giving a [FAIL] and
exitcode=1.

Would ovs_wait be the right primitive here?  It is already used four
lines above for the listener readiness check:

	ovs_wait sctp_eps_has server 4443 || return 1

and ovs_wait() retries for WAIT_TIMEOUT seconds, doubling it when
KSFT_MACHINE_SLOW=yes, which is exactly the loaded-CI case that loses
this race.  The same unsynchronized pattern appears again in the
follow-up commit adding test_sctp_nat_connect_v4, so it is not corrected
later in the series.

> +
> +	ovs_del_flows "$t" sctp6

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902125133.1828572-1-houminxi%40gmail.com

Re: [PATCH net-next v2 2/2] selftests/net/openvswitch: add SCTP flow key test across conntrack NAT

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 3 · Low: 1

- [Medium] Commit-message/coverage mismatch: the new
  `test_sctp_nat_connect_v4` cannot detect the bug class its commit…
- [Medium] `test_sctp_nat_connect_v4` probes only the plain `ct(commit)`
  action (openvswitch.sh:1224-1231) before deciding SKIP vs. run, but…
- [Medium] Unsynchronized cross-process check makes the new test flaky.
- [Low] The Open vSwitch selftest Kconfig fragment
  (tools/testing/selftests/net/openvswitch/config) is not updated for…

--

selftests/net/openvswitch: add SCTP flow key test across conntrack NAT

This adds test_sctp_nat_connect_v4 to the openvswitch selftest suite. It
builds a client/server namespace pair, installs ct(commit,nat(...)) plus
recirc flows, and checks that an SCTP association and a DATA payload cross
the translation.

A few questions about the new test below.

> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index 06bd98d80a08..d5acdca0a0ac 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh

[ ... ]

> @@ -1186,6 +1187,91 @@ test_nat_connect_v4 () {
>  	return 0
>  }
>  
> +# sctp_nat_connect_v4 test
> +#  - SCTP association crosses a ct(commit,nat(dst=...)) translation
> +#  - post-recirc flows match ct_state(+trk) plus the extracted SCTP ports
> +test_sctp_nat_connect_v4 () {
> +	local t="test_sctp_nat_connect_v4"
> +	local payload="SCTP_NAT_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"
> +	# SCTP conntrack is compiled into nf_conntrack.ko, so check that
> +	# loading it actually exposed the SCTP conntrack sysctls.
> +	modprobe -q nf_conntrack 2>/dev/null || return "$ksft_skip"
> +	[ -e /proc/sys/net/netfilter/nf_conntrack_sctp_timeout_established ] \
> +	    || { info "no SCTP conntrack support - skipping"
> +	         return "$ksft_skip"; }

[Severity: Low]

Should tools/testing/selftests/net/openvswitch/config gain the kernel options
this new test depends on?

The sysctl probed here only exists when CONFIG_NF_CT_PROTO_SCTP is enabled,
and that option is an independent bool:

net/netfilter/Kconfig:
	config NF_CT_PROTO_SCTP
		bool 'SCTP protocol connection tracking support'
		depends on NETFILTER_ADVANCED
		default y

With NETFILTER_ADVANCED=n the dependency is unmet and the option is off in
spite of the default, and a base config that sets it to n is unaffected by
merging the fragment.

The current fragment has:

	CONFIG_IP_SCTP=y
	CONFIG_NF_CONNTRACK=m
	CONFIG_NF_CONNTRACK_OVS=y
	CONFIG_OPENVSWITCH=m

so in those builds the test returns $ksft_skip and the coverage the changelog
describes never runs. Would adding CONFIG_NF_CT_PROTO_SCTP=y (and
CONFIG_NF_NAT=y, see below) to the fragment make it deterministic?

[ ... ]

> +	# Check if the ct action can be configured.
> +	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
> +		'in_port(1),eth(),eth_type(0x0800),ipv4()' \
> +		'ct(commit),recirc(0x1)' &> /dev/null
> +	if [ $? == 1 ]; then
> +		info "no support for ct action - skipping"
> +		ovs_exit_sig
> +		return $ksft_skip
> +	fi

[Severity: Medium]

This probe tests plain ct(commit), but the flows installed below require the
nat action. Does that turn a missing CONFIG_NF_NAT into a reported failure
instead of a skip?

In net/openvswitch/conntrack.c, both the policy entry and the parse arm are
compiled conditionally:

	#if IS_ENABLED(CONFIG_NF_NAT)
		/* NAT length is checked when parsing the nested attributes. */
		[OVS_CT_ATTR_NAT]	= { .minlen = 0, .maxlen = INT_MAX },
	#endif

With NF_NAT off the entry is {0, 0}, so the nested nat attribute fails the
length check in parse_ct() and the flow add is rejected with -EINVAL, while
the ct(commit) probe above still succeeds.

That configuration is reachable from the suite's own fragment, which does not
set CONFIG_NF_NAT, and:

net/netfilter/Kconfig:
	config NF_NAT
		tristate "Network Address Translation support"
		depends on NF_CONNTRACK
		default m if NETFILTER_ADVANCED=n

CONFIG_OPENVSWITCH only does "select NF_NAT_OVS if NF_NAT", so with the usual
NETFILTER_ADVANCED=y, NF_NAT defaults to n. Would probing with
ct(commit,nat(dst=...)) instead, or skipping rather than failing on the nat
flow adds, keep this a skip?

[ ... ]

> +	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
> +		"ct_state(-trk),in_port(1),eth(),eth_type(0x0800),ipv4(dst=192.168.0.20)" \
> +		"ct(commit,nat(dst=172.31.110.20)),recirc(0x1)" || return 1
> +	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
> +		"ct_state(-trk),in_port(2),eth(),eth_type(0x0800),ipv4()" \
> +		"ct(commit,nat),recirc(0x2)" || return 1
> +
> +	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
> +		"recirc_id(0x1),ct_state(+trk-inv),in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)" \
> +		"2" || return 1
> +	ovs_add_flow "test_sctp_nat_connect_v4" sctpnat4 \
> +		"recirc_id(0x2),ct_state(+trk-inv),in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)" \
> +		"1" || return 1

[Severity: Medium]

Can these flows actually detect a post-NAT SCTP flow key bug?

nat(dst=172.31.110.20) rewrites only the IPv4 destination and never a port,
and the post-recirc flows match ipv4(proto=132) plus sctp(dst=4443) /
sctp(src=4443) - exactly the fields NAT leaves alone. The translated
addresses are not matched at all.

The post-NAT ports are patched into the key here:

net/openvswitch/conntrack.c:ovs_nat_update_key() {
	...
		if (key->ip.proto == IPPROTO_UDP)
			dst = udp_hdr(skb)->dest;
		else if (key->ip.proto == IPPROTO_TCP)
			dst = tcp_hdr(skb)->dest;
		else if (key->ip.proto == IPPROTO_SCTP)
			dst = sctp_hdr(skb)->dest;
		else
			return;

		key->tp.dst = dst;
	...
}

If the IPPROTO_SCTP arms were deleted so the function took the bare return,
key->tp.dst / key->tp.src would keep their identical first-pass values and
both flows would still match, so the test would still pass.

Would matching on distinct translated SCTP ports, and/or on the translated
addresses in the post-recirc flows, give the intended coverage?

Also on the changelog:

	Conntrack NAT rewrites the packet between the first-pass and recirc
	passes, and OVS re-extracts L3/L4 on the recirculated skb via
	ovs_flow_key_update_l3l4().

Is that the path this test exercises? ovs_flow_key_update_l3l4() in
net/openvswitch/flow.c has a single caller:

net/openvswitch/conntrack.c:ovs_ct_handle_fragments() {
	...
	/* The key extracted from the fragment that completed this datagram
	 * likely didn't have an L4 header, so regenerate it.
	 */
	ovs_flow_key_update_l3l4(skb, key);
	...
}

that is, only for datagrams reassembled from IP fragments, which this test
never produces. On the plain ct+recirc path the key is not re-extracted:

net/openvswitch/actions.c:execute_recirc() {
	...
	if (!is_flow_key_valid(key)) {
		int err;

		err = ovs_flow_key_update(skb, key);
	...
}

and the CT/NAT action does not call invalidate_flow_key().

	After the association succeeds the test pushes a known payload across and
	verifies the listener received it, so a flow-key bug that matches
	handshake packets but breaks DATA chunks is caught too.

Is there such a distinction? SCTP key extraction reads only the common
header source/destination ports, which are the same for INIT and DATA
chunks.

> +	ovs_netns_spawn_daemon "test_sctp_nat_connect_v4" "server" \
> +		socat -u -t 1 SCTP4-LISTEN:4443,fork \
> +		OPEN:"$rxfile",creat,append

[ ... ]

> +	info "verify SCTP DATA chunk crosses NAT"
> +	ovs_sbx "test_sctp_nat_connect_v4" ip netns exec client \
> +	    timeout 3 socat -u STDIN "SCTP4-CONNECT:192.168.0.20:4443" \
> +	    <<< "$payload" || return 1
> +	grep -q "$payload" "$rxfile" 2>/dev/null \
> +	    || { info "server did not receive SCTP DATA payload"
> +	         return 1; }

[Severity: Medium]

Can this grep race the server writing $rxfile?

The listener is started in the background:

ovs_netns_spawn_daemon() {
	...
		ip netns exec $netns $*  >> $ovs_dir/stdout  2>> $ovs_dir/stderr &
	pid=$!
	...
}

and ",fork" means a separate per-association child does the read() and the
write() into $rxfile. The client socat exiting only means its own socket was
closed; delivery to the server socket, the child's read() and its write() to
the file all happen afterwards in another process, and "-t 1" decouples the
child's lifetime from the client's exit.

So this interleaving looks possible:

	client socat exits
	shell runs grep -q on an empty/short $rxfile   -> return 1
	server child writes payload

Every other cross-process observation in this file goes through the retry
helper, and this same test already uses it for endpoint readiness:

	ovs_wait sctp_eps_has server 4443 || return 1

with ovs_wait() polling up to WAIT_TIMEOUT (raised to 10s when
KSFT_MACHINE_SLOW=yes). Would "ovs_wait grep -q "$payload" "$rxfile"" be
more robust here?

The same one-shot grep pattern was introduced one patch earlier in this
series, in the IPv6 SCTP test, so both places would need the same change.

> +
> +	info "done..."
> +	return 0
> +}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902125133.1828572-1-houminxi%40gmail.com