This commit resulted in significant decreased dataplane performance when
testing a dense OpenShift cluster. This was pinpointed to be due to an
extra ct(nat(src)) that this commit added.
For now, revert this commit.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992012
Signed-off-by: Mark Michelson <[email protected]>
---
include/ovn/actions.h | 1 -
lib/actions.c | 31 ------
tests/ovn.at | 2 +-
tests/system-common-macros.at | 4 -
tests/system-ovn.at | 190 ----------------------------------
5 files changed, 1 insertion(+), 227 deletions(-)
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index f023a37b9..b2f2f57c6 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -25,7 +25,6 @@
#include "openvswitch/hmap.h"
#include "openvswitch/uuid.h"
#include "util.h"
-#include "ovn/features.h"
struct expr;
struct lexer;
diff --git a/lib/actions.c b/lib/actions.c
index c572e88ae..f0291afef 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -742,22 +742,6 @@ encode_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc,
ct->zone_src.ofs = 0;
ct->zone_src.n_bits = 16;
- /* If the datapath supports all-zero SNAT then use it to avoid tuple
- * collisions at commit time between NATed and firewalled-only sessions.
- */
-
- if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) {
- size_t nat_offset = ofpacts->size;
- ofpbuf_pull(ofpacts, nat_offset);
-
- struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
- nat->flags = 0;
- nat->range_af = AF_UNSPEC;
- nat->flags |= NX_NAT_F_SRC;
- ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
- ct = ofpacts->header;
- }
-
size_t set_field_offset = ofpacts->size;
ofpbuf_pull(ofpacts, set_field_offset);
@@ -808,21 +792,6 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on,
ct->zone_src.ofs = 0;
ct->zone_src.n_bits = 16;
- /* If the datapath supports all-zero SNAT then use it to avoid tuple
- * collisions at commit time between NATed and firewalled-only sessions.
- */
- if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) {
- size_t nat_offset = ofpacts->size;
- ofpbuf_pull(ofpacts, nat_offset);
-
- struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
- nat->flags = 0;
- nat->range_af = AF_UNSPEC;
- nat->flags |= NX_NAT_F_SRC;
- ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
- ct = ofpacts->header;
- }
-
size_t set_field_offset = ofpacts->size;
ofpbuf_pull(ofpacts, set_field_offset);
diff --git a/tests/ovn.at b/tests/ovn.at
index 4957a1063..fbdffc0ef 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -23643,7 +23643,7 @@ AT_CHECK([
for hv in 1 2; do
grep table=15 hv${hv}flows | \
grep "priority=100" | \
- grep -c
"ct(commit,zone=NXM_NX_REG11\\[[0..15\\]],.*exec(move:NXM_OF_ETH_SRC\\[[\\]]->NXM_NX_CT_LABEL\\[[32..79\\]],load:0x[[0-9]]->NXM_NX_CT_LABEL\\[[80..95\\]]))"
+ grep -c
"ct(commit,zone=NXM_NX_REG11\\[[0..15\\]],exec(move:NXM_OF_ETH_SRC\\[[\\]]->NXM_NX_CT_LABEL\\[[32..79\\]],load:0x[[0-9]]->NXM_NX_CT_LABEL\\[[80..95\\]]))"
grep table=22 hv${hv}flows | \
grep "priority=200" | \
diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 616a87fcf..c23804f6f 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -330,7 +330,3 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP],
# OVS_CHECK_CT_CLEAR()
m4_define([OVS_CHECK_CT_CLEAR],
[AT_SKIP_IF([! grep -q "Datapath supports ct_clear action"
ovs-vswitchd.log])])
-
-# OVS_CHECK_CT_ZERO_SNAT()
-m4_define([OVS_CHECK_CT_ZERO_SNAT],
- [AT_SKIP_IF([! grep -q "Datapath supports ct_zero_snat"
ovs-vswitchd.log])]))
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index aadd68634..9487dde49 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -5319,196 +5319,6 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port
patch-.*/d
AT_CLEANUP
])
-OVN_FOR_EACH_NORTHD([
-AT_SETUP([load-balancer and firewall tuple conflict IPv4])
-AT_SKIP_IF([test $HAVE_NC = no])
-AT_KEYWORDS([ovnlb])
-
-CHECK_CONNTRACK()
-CHECK_CONNTRACK_NAT()
-ovn_start
-OVS_TRAFFIC_VSWITCHD_START()
-OVS_CHECK_CT_ZERO_SNAT()
-ADD_BR([br-int])
-
-# Set external-ids in br-int needed for ovn-controller
-ovs-vsctl \
- -- set Open_vSwitch . external-ids:system-id=hv1 \
- -- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
- -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
- -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
- -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
-
-# Start ovn-controller
-start_daemon ovn-controller
-
-# Logical network:
-# 1 logical switch connetected to one logical router.
-# 2 VMs, one used as backend for a load balancer.
-
-check ovn-nbctl \
- -- lr-add rtr \
- -- lrp-add rtr rtr-ls 00:00:00:00:01:00 42.42.42.1/24 \
- -- ls-add ls \
- -- lsp-add ls ls-rtr \
- -- lsp-set-addresses ls-rtr 00:00:00:00:01:00 \
- -- lsp-set-type ls-rtr router \
- -- lsp-set-options ls-rtr router-port=rtr-ls \
- -- lsp-add ls vm1 -- lsp-set-addresses vm1 00:00:00:00:00:01 \
- -- lsp-add ls vm2 -- lsp-set-addresses vm2 00:00:00:00:00:02 \
- -- lb-add lb-test 66.66.66.66:666 42.42.42.2:4242 tcp \
- -- ls-lb-add ls lb-test
-
-ADD_NAMESPACES(vm1)
-ADD_VETH(vm1, vm1, br-int, "42.42.42.2/24", "00:00:00:00:00:01", "42.42.42.1")
-
-ADD_NAMESPACES(vm2)
-ADD_VETH(vm2, vm2, br-int, "42.42.42.3/24", "00:00:00:00:00:02", "42.42.42.1")
-
-# Wait for ovn-controller to catch up.
-wait_for_ports_up
-check ovn-nbctl --wait=hv sync
-
-# Start IPv4 TCP server on vm1.
-NETNS_DAEMONIZE([vm1], [nc -k -l 42.42.42.2 4242], [nc-vm1.pid])
-
-# Make sure connecting to the VIP works.
-NS_CHECK_EXEC([vm2], [nc 66.66.66.66 666 -p 2000 -z])
-
-# Start IPv4 TCP connection to VIP from vm2.
-NS_CHECK_EXEC([vm2], [nc 66.66.66.66 666 -p 2001 -z])
-
-# Check conntrack. We expect two entries:
-# - one in vm1's zone (firewall)
-# - one in vm2's zone (dnat)
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 | \
-grep "orig=.src=42\.42\.42\.3" | \
-sed -e 's/port=2001/port=<clnt_s_port>/g' \
- -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
- -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g' \
- -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
-tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=<clnt_s_port>,dport=4242),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
-tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=<clnt_s_port>,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
-])
-
-# Start IPv4 TCP connection to backend IP from vm2 which would require
-# additional source port translation to avoid a tuple conflict.
-NS_CHECK_EXEC([vm2], [nc 42.42.42.2 4242 -p 2001 -z])
-
-# Check conntrack. We expect three entries:
-# - one in vm1's zone (firewall) - reused from the previous connection.
-# - one in vm2's zone (dnat) - still in TIME_WAIT after the previous
connection.
-# - one in vm2's zone (firewall + additional all-zero SNAT)
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 | \
-grep "orig=.src=42\.42\.42\.3" | \
-sed -e 's/port=2001/port=<clnt_s_port>/g' \
- -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
- -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g' \
- -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
-tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=<clnt_s_port>,dport=4242),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
-tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=<clnt_s_port>,dport=4242),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<rnd_port>),zone=<cleared>,protoinfo=(state=<cleared>)
-tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=<clnt_s_port>,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
-])
-
-AT_CLEANUP
-])
-
-OVN_FOR_EACH_NORTHD([
-AT_SETUP([load-balancer and firewall tuple conflict IPv6])
-AT_SKIP_IF([test $HAVE_NC = no])
-AT_KEYWORDS([ovnlb])
-
-CHECK_CONNTRACK()
-CHECK_CONNTRACK_NAT()
-ovn_start
-OVS_TRAFFIC_VSWITCHD_START()
-OVS_CHECK_CT_ZERO_SNAT()
-ADD_BR([br-int])
-
-# Set external-ids in br-int needed for ovn-controller
-ovs-vsctl \
- -- set Open_vSwitch . external-ids:system-id=hv1 \
- -- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
- -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
- -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
- -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
-
-# Start ovn-controller
-start_daemon ovn-controller
-
-# Logical network:
-# 1 logical switch connetected to one logical router.
-# 2 VMs, one used as backend for a load balancer.
-
-check ovn-nbctl \
- -- lr-add rtr \
- -- lrp-add rtr rtr-ls 00:00:00:00:01:00 4242::1/64 \
- -- ls-add ls \
- -- lsp-add ls ls-rtr \
- -- lsp-set-addresses ls-rtr 00:00:00:00:01:00 \
- -- lsp-set-type ls-rtr router \
- -- lsp-set-options ls-rtr router-port=rtr-ls \
- -- lsp-add ls vm1 -- lsp-set-addresses vm1 00:00:00:00:00:01 \
- -- lsp-add ls vm2 -- lsp-set-addresses vm2 00:00:00:00:00:02 \
- -- lb-add lb-test [[6666::1]]:666 [[4242::2]]:4242 tcp \
- -- ls-lb-add ls lb-test
-
-ADD_NAMESPACES(vm1)
-ADD_VETH(vm1, vm1, br-int, "4242::2/64", "00:00:00:00:00:01", "4242::1")
-OVS_WAIT_UNTIL([test "$(ip netns exec vm1 ip a | grep 4242::2 | grep tentative)" =
""])
-
-ADD_NAMESPACES(vm2)
-ADD_VETH(vm2, vm2, br-int, "4242::3/64", "00:00:00:00:00:02", "4242::1")
-OVS_WAIT_UNTIL([test "$(ip netns exec vm2 ip a | grep 4242::3 | grep tentative)" =
""])
-
-# Wait for ovn-controller to catch up.
-wait_for_ports_up
-check ovn-nbctl --wait=hv sync
-
-# Start IPv6 TCP server on vm1.
-NETNS_DAEMONIZE([vm1], [nc -k -l 4242::2 4242], [nc-vm1.pid])
-
-# Make sure connecting to the VIP works.
-NS_CHECK_EXEC([vm2], [nc 6666::1 666 -p 2000 -z])
-
-# Start IPv6 TCP connection to VIP from vm2.
-NS_CHECK_EXEC([vm2], [nc 6666::1 666 -p 2001 -z])
-
-# Check conntrack. We expect two entries:
-# - one in vm1's zone (firewall)
-# - one in vm2's zone (dnat)
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 | \
-grep "orig=.src=4242::3" | \
-sed -e 's/port=2001/port=<clnt_s_port>/g' \
- -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
- -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g' \
- -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
-tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
-tcp,orig=(src=4242::3,dst=6666::1,sport=<clnt_s_port>,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
-])
-
-# Start IPv6 TCP connection to backend IP from vm2 which would require
-# additional source port translation to avoid a tuple conflict.
-NS_CHECK_EXEC([vm2], [nc 4242::2 4242 -p 2001 -z])
-
-# Check conntrack. We expect three entries:
-# - one in vm1's zone (firewall) - reused from the previous connection.
-# - one in vm2's zone (dnat) - still in TIME_WAIT after the previous
connection.
-# - one in vm2's zone (firewall + additional all-zero SNAT)
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 | \
-grep "orig=.src=4242::3" | \
-sed -e 's/port=2001/port=<clnt_s_port>/g' \
- -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
- -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g' \
- -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
-tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
-tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<rnd_port>),zone=<cleared>,protoinfo=(state=<cleared>)
-tcp,orig=(src=4242::3,dst=6666::1,sport=<clnt_s_port>,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
-])
-
-AT_CLEANUP
-])
-
# When a lport is released on a chassis, ovn-controller was
# not clearing some of the flowss in the table 33 leading
# to packet drops if ct() is hit.
