[ovs-dev] [PATCH ovn v2 2/4] Honor allow priority when stateless present

[Ihar Hrachyshka]

For each allow-stateless ACL, a rule was added earlier in the pipeline
that circumvented setting REGBIT_CONNTRACK_DEFRAG regardless of
whether other, e.g. allow ACLs with higher priority were present.

Now, when allow-stateless ACLs are present on the switch with other
allow-related ACLs, for each allow, insert an early pipeline rule that
would set DEFRAG bit for the corresponding match and priority.

Fixes: 3187b9fef1 ("ovn-northd: introduce new allow-stateless ACL verb")

Signed-off-by: Ihar Hrachyshka <ihrachys at redhat.com>

===

v1: initial commit
v2: don't unnecessarily map .has_fair_meter
---
 northd/ovn-northd.c  |  3 ++
 northd/ovn_northd.dl |  2 +-
 tests/ovn-northd.at  | 72 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index fd57e62e8..54f75d094 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -5082,6 +5082,9 @@ build_stateful_override_filters(struct ovn_datapath *od,
     apply_to_each_acl_of_action(
         od, port_groups, lflows, "allow-related",
         build_stateful_override_filter);
+    apply_to_each_acl_of_action(
+        od, port_groups, lflows, "allow",
+        build_stateful_override_filter);
 }
 
 static void
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index a29c9fba8..3b6396e5e 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -1862,7 +1862,7 @@ for (&SwitchACL(.sw = sw@&Switch{._uuid = ls_uuid}, .acl = &acl)) {
         }
     };
     if (sw.has_stateless_acl) {
-        if (acl.action == "allow-related") {
+        if ((sw.has_stateful_acl and acl.action == "allow") or acl.action == "allow-related") {
             if (acl.direction == "from-lport") {
                 Flow(.logical_datapath = ls_uuid,
                      .stage            = s_SWITCH_IN_PRE_ACL(),
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 0ff367af6..bda11fe06 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2765,6 +2765,78 @@ output("lsp2");
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- ACL priority: allow-stateless vs allow])
+ovn_start
+
+ovn-nbctl ls-add ls
+ovn-nbctl lsp-add ls lsp1
+ovn-nbctl lsp-set-addresses lsp1 00:00:00:00:00:01
+ovn-nbctl lsp-add ls lsp2
+ovn-nbctl lsp-set-addresses lsp2 00:00:00:00:00:02
+
+for direction in from to; do
+    ovn-nbctl acl-add ls ${direction}-lport 3 "tcp" allow
+done
+ovn-nbctl --wait=sb sync
+
+flow_eth='eth.src == 00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02'
+flow_ip='ip.ttl==64 && ip4.src == 42.42.42.1 && ip4.dst == 66.66.66.66'
+flow_tcp='tcp && tcp.dst == 80'
+flow_udp='udp && udp.dst == 80'
+lsp1_inport=$(fetch_column Port_Binding tunnel_key logical_port=lsp1)
+
+# TCP packets should not go to conntrack.
+flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
+AT_CHECK_UNQUOTED([ovn-trace --minimal ls "${flow}"], [0], [dnl
+# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+output("lsp2");
+])
+
+# Add allow-stateless with lower priority.
+for direction in from to; do
+    ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
+done
+ovn-nbctl --wait=sb sync
+
+# TCP packets should still not go to conntrack (allow ACL in effect).
+AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
+# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+output("lsp2");
+])
+
+# Add allow-related to the switch.
+for direction in from to; do
+    ovn-nbctl acl-add ls ${direction}-lport 3 icmp allow-related
+done
+ovn-nbctl --wait=sb sync
+
+# TCP packets should now go to conntrack (allow ACL in effect, flipped its meaning to apply conntrack)
+AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
+# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+ct_next(ct_state=new|trk) {
+    ct_next(ct_state=new|trk) {
+        output("lsp2");
+    };
+};
+])
+
+# Add allow-stateless with higher priority.
+for direction in from to; do
+    ovn-nbctl acl-add ls ${direction}-lport 5 tcp allow-stateless
+done
+ovn-nbctl --wait=sb sync
+
+
+# TCP packets should no longer go to conntrack (the new allow-stateless ACL in effect).
+AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
+# tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+output("lsp2");
+])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ovn -- ACL allow-stateless omit conntrack - Logical_Switch])
 ovn_start
-- 
2.31.1