netlink-conntrack: Fix partial match of entries with SCTP.

The SCTP protocol ports were excluded from the netlink encoding. In that case the nl_ct_flush_tuple() would return EOPNOTSUPP, that could result in some CT entries not being properly flushed if we would hit SCTP entry earlier than others. This at the same time allows to flush SCTP on its own in during partial match. This should still be considered a bug, because OvS currently supports SCTP CT entries, and it should also support partial flush for them the same way it supports partial flush for TCP/UDP. Reported-at: https://bugzilla.redhat.com/2228037 Signed-off-by: Ales Musil <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
ovsrobot · Sep 8, 2023 · bac34b2 · bac34b2
1 parent 563c50f
commit bac34b2
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 5 deletions.
diff --git a/lib/netlink-conntrack.c b/lib/netlink-conntrack.c
@@ -579,7 +579,8 @@ nl_ct_put_tuple_proto(struct ofpbuf *buf, const struct ct_dpif_tuple *tuple)
         nl_msg_put_u8(buf, CTA_PROTO_ICMPV6_TYPE, tuple->icmp_type);
         nl_msg_put_u8(buf, CTA_PROTO_ICMPV6_CODE, tuple->icmp_code);
     } else if (tuple->ip_proto == IPPROTO_TCP ||
-               tuple->ip_proto == IPPROTO_UDP) {
+               tuple->ip_proto == IPPROTO_UDP ||
+               tuple->ip_proto == IPPROTO_SCTP) {
         nl_msg_put_be16(buf, CTA_PROTO_SRC_PORT, tuple->src_port);
         nl_msg_put_be16(buf, CTA_PROTO_DST_PORT, tuple->dst_port);
     } else {

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
@@ -2516,6 +2516,7 @@ AT_CLEANUP
 
 AT_SETUP([conntrack - ct flush])
 CHECK_CONNTRACK()
+CHECK_CONNTRACK_SCTP()
 OVS_TRAFFIC_VSWITCHD_START()
 
 ADD_NAMESPACES(at_ns0, at_ns1)
@@ -2526,10 +2527,8 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
 AT_DATA([flows.txt], [dnl
 priority=1,action=drop
 priority=10,arp,action=normal
-priority=100,in_port=1,udp,action=ct(commit),2
-priority=100,in_port=2,udp,action=ct(zone=5,commit),1
-priority=100,in_port=1,icmp,action=ct(commit),2
-priority=100,in_port=2,icmp,action=ct(zone=5,commit),1
+priority=100,in_port=1,ip,action=ct(commit),2
+priority=100,in_port=2,ip,action=ct(zone=5,commit),1
 ])
 
 AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
@@ -2692,6 +2691,25 @@ udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.
 
 AT_CHECK([FLUSH_CMD])
 
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+
+dnl Test SCTP flush based on port.
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500003400010000408464410a0101010a01010200010002000000009178f7d30100001470e18ccc00000000000a000a00000000 actions=resubmit(,0)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000950540000000a08004500003400010000408464410a0101020a010101000200010000000098f29e470100001470e18ccc00000000000a000a00000000 actions=resubmit(,0)"])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sed "s/,protoinfo=.*$//" | sort], [0], [dnl
+sctp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
+sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
+])
+
+AT_CHECK([FLUSH_CMD 'ct_nw_src=10.1.1.1,ct_nw_proto=132,ct_tp_src=1,ct_tp_dst=2'])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sed "s/,protoinfo=.*$//" | sort], [0], [dnl
+sctp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
+])
+
+AT_CHECK([FLUSH_CMD 'ct_nw_src=10.1.1.2,ct_nw_proto=132,ct_tp_src=2,ct_tp_dst=1'])
+
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
 ])