[PATCH net-next v8] selftests/net/openvswitch: add SCTP flow key support and test

[PATCH net-next v8] selftests/net/openvswitch: add SCTP flow key support and test

[Minxi Hou]

The ovskey flow-string parser has no OVS_KEY_ATTR_SCTP entry, so a
flow string containing sctp(src=.../dst=...) parses without error but
silently drops the L4 key. The resulting flow carries only
ipv4(proto=132), and the kernel rejects it: match_validate() in
flow_netlink.c requires OVS_KEY_ATTR_SCTP when the IP protocol is
IPPROTO_SCTP and returns -EINVAL for the missing key.

Register OVS_KEY_ATTR_SCTP in the parse table and add a matching
selftest that verifies SCTP flow key matching (sctp src/dst port).

Also enable CONFIG_IP_SCTP in the selftest kernel config.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
---
Changes from v7:
  - Drop all unit tests per Ilya's feedback (tests for test code
    feels excessive); merge patches back into one.
  - Enable CONFIG_IP_SCTP in the selftest kernel config.
  - Rebase onto latest net-next; add base-commit header.
v7: https://lore.kernel.org/netdev/20260729064549.3647518-1-houminxi@gmail.com/
v6: https://lore.kernel.org/netdev/20260724150624.3457427-1-houminxi@gmail.com/
v5: https://lore.kernel.org/netdev/20260723215630.2502169-1-houminxi@gmail.com/
v4: https://lore.kernel.org/netdev/20260723162503.1790998-1-houminxi@gmail.com/
v3: https://lore.kernel.org/netdev/20260722214915.4128292-1-houminxi@gmail.com/
v2: https://lore.kernel.org/netdev/20260721190648.2713156-1-houminxi@gmail.com/
v1: https://lore.kernel.org/netdev/20260718215437.3257200-1-houminxi@gmail.com/

 .../testing/selftests/net/openvswitch/config  |   1 +
 .../selftests/net/openvswitch/openvswitch.sh  | 121 ++++++++++++++++++
 .../selftests/net/openvswitch/ovs-dpctl.py    |   5 +
 3 files changed, 127 insertions(+)

diff --git a/tools/testing/selftests/net/openvswitch/config b/tools/testing/selftests/net/openvswitch/config
index c659749cd086..754297b1644e 100644
--- a/tools/testing/selftests/net/openvswitch/config
+++ b/tools/testing/selftests/net/openvswitch/config
@@ -1,6 +1,7 @@
 CONFIG_GENEVE=m
 CONFIG_INET_DIAG=y
 CONFIG_IPV6=y
+CONFIG_IP_SCTP=y
 CONFIG_NETFILTER=y
 CONFIG_NET_IPGRE=m
 CONFIG_NET_IPGRE_DEMUX=m
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 853dbc1b00d7..b448324527d8 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -34,6 +34,7 @@ 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
 	psample					psample: Sampling packets with psample"
 
 info() {
@@ -611,6 +612,126 @@ test_icmpv6() {
 	return 0
 }
 
+# Check for an SCTP endpoint via /proc, which works without sctp_diag.
+sctp_eps_has() {
+	ip netns exec "$1" awk -v p="$2" '$6==p' /proc/net/sctp/eps | grep -q .
+}
+
+# sctp_connect_v4 test
+# - sctp(dst=4443) matches client-to-server INIT
+# - sctp(src=4443) matches server-to-client INIT-ACK
+# - remove flows and verify connection fails, reinstall and recover
+test_sctp_connect_v4() {
+	local t="test_sctp_connect_v4"
+	local srv_ip=172.31.110.20
+
+	modprobe -q sctp 2>/dev/null || return "$ksft_skip"
+	socat -V 2>&1 | grep -q "define WITH_SCTP" || return "$ksft_skip"
+
+	sbx_add "$t" || return $?
+	ovs_add_dp "$t" sctp4 || return 1
+
+	info "create namespaces"
+	for ns in client server; do
+		ovs_add_netns_and_veths "$t" "sctp4" "$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 "${srv_ip}/24" dev s1
+	ip netns exec server ip link set s1 up
+
+	# Probe: check if kernel supports sctp flow key.
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
+	    '2' &>/dev/null
+	if [ $? -ne 0 ]; then
+		info "no support for sctp key - skipping"
+		ovs_exit_sig
+		return $ksft_skip
+	fi
+	ovs_del_flows "$t" sctp4
+
+	# ARP forwarding
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(1),eth(),eth_type(0x0806),arp()' \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(2),eth(),eth_type(0x0806),arp()' \
+	    '1' || return 1
+
+	# SCTP port matching: dst for request, src for reply
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)' \
+	    '1' || return 1
+
+	ovs_netns_spawn_daemon "$t" "server" \
+	    socat -u SCTP4-LISTEN:4443 STDOUT
+	local server_pid="$pid"
+	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 "SCTP4-CONNECT:${srv_ip}:4443" </dev/null \
+	    || return 1
+
+	ovs_del_flows "$t" sctp4
+
+	info "verify connection fails without flows"
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(1),eth(),eth_type(0x0806),arp()' \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(2),eth(),eth_type(0x0806),arp()' \
+	    '1' || return 1
+
+	kill -TERM "$server_pid" 2>/dev/null
+	local i=0
+	while kill -0 "$server_pid" 2>/dev/null && [ "$i" -lt 5 ]; do
+	    sleep 0.2
+	    i=$((i + 1))
+	done
+	ovs_netns_spawn_daemon "$t" "server" \
+	    socat -u SCTP4-LISTEN:4443 STDOUT
+	server_pid="$pid"
+	ovs_wait sctp_eps_has server 4443 || return 1
+
+	ovs_sbx "$t" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP4-CONNECT:${srv_ip}: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" sctp4 \
+	    'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
+	    '2' || return 1
+	ovs_add_flow "$t" sctp4 \
+	    'in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)' \
+	    '1' || return 1
+
+	kill -TERM "$server_pid" 2>/dev/null
+	i=0
+	while kill -0 "$server_pid" 2>/dev/null && [ "$i" -lt 5 ]; do
+	    sleep 0.2
+	    i=$((i + 1))
+	done
+	ovs_netns_spawn_daemon "$t" "server" \
+	    socat -u SCTP4-LISTEN:4443 STDOUT
+	server_pid="$pid"
+	ovs_wait sctp_eps_has server 4443 || return 1
+
+	ovs_sbx "$t" ip netns exec client \
+	    timeout 3 socat -u STDIN "SCTP4-CONNECT:${srv_ip}:4443" </dev/null \
+	    || return 1
+
+	return 0
+}
+
 # psample test
 # - use psample to observe packets
 test_psample() {
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index f3edd198223f..ce790d936832 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -1984,6 +1984,11 @@ class ovskey(nla):
                 "udp",
                 ovskey.ovs_key_udp,
             ),
+            (
+                "OVS_KEY_ATTR_SCTP",
+                "sctp",
+                ovskey.ovs_key_sctp,
+            ),
             (
                 "OVS_KEY_ATTR_ICMP",
                 "icmp",

base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51
-- 
2.55.0

Re: [PATCH net-next v8] selftests/net/openvswitch: add SCTP flow key support and test

[Aaron Conole]

Hi Minxi,

Minxi Hou <houminxi@gmail.com> writes:

> The ovskey flow-string parser has no OVS_KEY_ATTR_SCTP entry, so a
> flow string containing sctp(src=.../dst=...) parses without error but
> silently drops the L4 key. The resulting flow carries only
> ipv4(proto=132), and the kernel rejects it: match_validate() in
> flow_netlink.c requires OVS_KEY_ATTR_SCTP when the IP protocol is
> IPPROTO_SCTP and returns -EINVAL for the missing key.
>
> Register OVS_KEY_ATTR_SCTP in the parse table and add a matching
> selftest that verifies SCTP flow key matching (sctp src/dst port).
>
> Also enable CONFIG_IP_SCTP in the selftest kernel config.
>
> Signed-off-by: Minxi Hou <houminxi@gmail.com>
> ---
>
> Changes from v7:
>   - Drop all unit tests per Ilya's feedback (tests for test code
>     feels excessive); merge patches back into one.
>   - Enable CONFIG_IP_SCTP in the selftest kernel config.
>   - Rebase onto latest net-next; add base-commit header.
> v7: https://lore.kernel.org/netdev/20260729064549.3647518-1-houminxi@gmail.com/
> v6: https://lore.kernel.org/netdev/20260724150624.3457427-1-houminxi@gmail.com/
> v5: https://lore.kernel.org/netdev/20260723215630.2502169-1-houminxi@gmail.com/
> v4: https://lore.kernel.org/netdev/20260723162503.1790998-1-houminxi@gmail.com/
> v3: https://lore.kernel.org/netdev/20260722214915.4128292-1-houminxi@gmail.com/
> v2: https://lore.kernel.org/netdev/20260721190648.2713156-1-houminxi@gmail.com/
> v1: https://lore.kernel.org/netdev/20260718215437.3257200-1-houminxi@gmail.com/
>
>  .../testing/selftests/net/openvswitch/config  |   1 +
>  .../selftests/net/openvswitch/openvswitch.sh  | 121 ++++++++++++++++++
>  .../selftests/net/openvswitch/ovs-dpctl.py    |   5 +
>  3 files changed, 127 insertions(+)
>
> diff --git a/tools/testing/selftests/net/openvswitch/config b/tools/testing/selftests/net/openvswitch/config
> index c659749cd086..754297b1644e 100644
> --- a/tools/testing/selftests/net/openvswitch/config
> +++ b/tools/testing/selftests/net/openvswitch/config
> @@ -1,6 +1,7 @@
>  CONFIG_GENEVE=m
>  CONFIG_INET_DIAG=y
>  CONFIG_IPV6=y
> +CONFIG_IP_SCTP=y
>  CONFIG_NETFILTER=y
>  CONFIG_NET_IPGRE=m
>  CONFIG_NET_IPGRE_DEMUX=m
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index 853dbc1b00d7..b448324527d8 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -34,6 +34,7 @@ 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
>  	psample					psample: Sampling packets with psample"
>  
>  info() {
> @@ -611,6 +612,126 @@ test_icmpv6() {
>  	return 0
>  }
>  
> +# Check for an SCTP endpoint via /proc, which works without sctp_diag.
> +sctp_eps_has() {
> +	ip netns exec "$1" awk -v p="$2" '$6==p' /proc/net/sctp/eps | grep -q .
> +}
> +
> +# sctp_connect_v4 test
> +# - sctp(dst=4443) matches client-to-server INIT
> +# - sctp(src=4443) matches server-to-client INIT-ACK
> +# - remove flows and verify connection fails, reinstall and recover
> +test_sctp_connect_v4() {
> +	local t="test_sctp_connect_v4"
> +	local srv_ip=172.31.110.20
> +
> +	modprobe -q sctp 2>/dev/null || return "$ksft_skip"
> +	socat -V 2>&1 | grep -q "define WITH_SCTP" || return "$ksft_skip"

socat has the option to run the listener with ',fork' option, which I'll
talk about below.

> +	sbx_add "$t" || return $?
> +	ovs_add_dp "$t" sctp4 || return 1
> +
> +	info "create namespaces"
> +	for ns in client server; do
> +		ovs_add_netns_and_veths "$t" "sctp4" "$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 "${srv_ip}/24" dev s1
> +	ip netns exec server ip link set s1 up
> +
> +	# Probe: check if kernel supports sctp flow key.
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
> +	    '2' &>/dev/null
> +	if [ $? -ne 0 ]; then
> +		info "no support for sctp key - skipping"
> +		ovs_exit_sig
> +		return $ksft_skip
> +	fi
> +	ovs_del_flows "$t" sctp4
> +
> +	# ARP forwarding
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(1),eth(),eth_type(0x0806),arp()' \
> +	    '2' || return 1
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(2),eth(),eth_type(0x0806),arp()' \
> +	    '1' || return 1
> +
> +	# SCTP port matching: dst for request, src for reply
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
> +	    '2' || return 1
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)' \
> +	    '1' || return 1
> +
> +	ovs_netns_spawn_daemon "$t" "server" \
> +	    socat -u SCTP4-LISTEN:4443 STDOUT
> +	local server_pid="$pid"
> +	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 "SCTP4-CONNECT:${srv_ip}:4443" </dev/null \
> +	    || return 1

We spawn a socat instance to listen, and then after we determine the
connection has succeeded, we kill everything, and create new flows.
BUT, as sashiko points out, we do this multiple times, and don't even
check whether the listen socket successfully comes up in the second /
third ones.

I think we could use a single SCTP listen instance with the fork option
as an optimization.  In that case, it would better test that the flows
are working versus just colliding on some possible listener race issue.
What I mean is the pattern:

 -- setup flows
 -- connect
 -- tear down flows

seems fine to me, but the respawning of the listen side feels excessive.

Sashiko seems to imply that we could also do the wait-test with longer
timeout, in the case that we respawn the listener.  There again I'd
argue that it's better just not restarting all of the listening side
each time.

> +	ovs_del_flows "$t" sctp4
> +
> +	info "verify connection fails without flows"
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(1),eth(),eth_type(0x0806),arp()' \
> +	    '2' || return 1
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(2),eth(),eth_type(0x0806),arp()' \
> +	    '1' || return 1
> +
> +	kill -TERM "$server_pid" 2>/dev/null
> +	local i=0
> +	while kill -0 "$server_pid" 2>/dev/null && [ "$i" -lt 5 ]; do
> +	    sleep 0.2
> +	    i=$((i + 1))
> +	done
> +	ovs_netns_spawn_daemon "$t" "server" \
> +	    socat -u SCTP4-LISTEN:4443 STDOUT
> +	server_pid="$pid"
> +	ovs_wait sctp_eps_has server 4443 || return 1
> +
> +	ovs_sbx "$t" ip netns exec client \
> +	    timeout 3 socat -u STDIN "SCTP4-CONNECT:${srv_ip}: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" sctp4 \
> +	    'in_port(1),eth(),eth_type(0x0800),ipv4(proto=132),sctp(dst=4443)' \
> +	    '2' || return 1
> +	ovs_add_flow "$t" sctp4 \
> +	    'in_port(2),eth(),eth_type(0x0800),ipv4(proto=132),sctp(src=4443)' \
> +	    '1' || return 1
> +
> +	kill -TERM "$server_pid" 2>/dev/null
> +	i=0
> +	while kill -0 "$server_pid" 2>/dev/null && [ "$i" -lt 5 ]; do
> +	    sleep 0.2
> +	    i=$((i + 1))
> +	done
> +	ovs_netns_spawn_daemon "$t" "server" \
> +	    socat -u SCTP4-LISTEN:4443 STDOUT
> +	server_pid="$pid"
> +	ovs_wait sctp_eps_has server 4443 || return 1
> +
> +	ovs_sbx "$t" ip netns exec client \
> +	    timeout 3 socat -u STDIN "SCTP4-CONNECT:${srv_ip}:4443" </dev/null \
> +	    || return 1
> +
> +	return 0
> +}
> +
>  # psample test
>  # - use psample to observe packets
>  test_psample() {
> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> index f3edd198223f..ce790d936832 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -1984,6 +1984,11 @@ class ovskey(nla):
>                  "udp",
>                  ovskey.ovs_key_udp,
>              ),
> +            (
> +                "OVS_KEY_ATTR_SCTP",
> +                "sctp",
> +                ovskey.ovs_key_sctp,
> +            ),
>              (
>                  "OVS_KEY_ATTR_ICMP",
>                  "icmp",
>
> base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51

Re: [PATCH net-next v8] selftests/net/openvswitch: add SCTP flow key support and test

[Minxi Hou]

Agreed.  v9 uses a single

  socat -u SCTP4-LISTEN:4443,fork STDOUT

for the whole test, so the flow rules are the only thing that changes
between the three phases.  Both respawn blocks go away.

> BUT, as sashiko points out, we do this multiple times, and don't even
> check whether the listen socket successfully comes up in the second /
> third ones.

The check is there -- each respawn is followed by

  ovs_wait sctp_eps_has server 4443

For reference, the sashiko run on v8 reports no open concerns; the
listener restart and that wait both appear under dismissed concerns.

What is fair to say is that the check is weaker than it looks.
sctp_eps_has matches on the local port alone, so across a restart it
can assert only that some listener holds 4443, not that it is the new
one.  With a single forking listener that question does not arise,
which is a better reason to make the change than the line count.

> Sashiko seems to imply that we could also do the wait-test with longer
> timeout, in the case that we respawn the listener.

Not needed either way.  Two of those dismissed concerns are about the
restart itself, so dropping it removes the code they were about rather
than widening a window around it.

> https://netdev-ctrl.bots.linux.dev/logs/build/1137903/14722442/check_selftest/

That failure is unrelated to the listener.  The selftest config check
strips underscores before comparing keys, so CONFIG_IP_SCTP has to
sort before CONFIG_IPV6 even though a plain sort puts it after.
Fixed in v9, and validate_config_format.py is clean on the new file.

Thanks for the review.