For localnet ports, skip_port_from_conntrack() incorrectly emits "next;"
(without ct_clear) in the egress pre-ACL/pre-LB stages whenever the
switch has a stateful ACL: a packet that went through ct_lb / ct(commit)
on ingress carries ct_state (+trk, +est, ct_label, ...) into the egress
pipeline, and out_acl_eval then matches flows (ct.est, ct.new, ct_label,
...) against state from a different zone, causing rules to fire
incorrectly.

Always emit "flags.pkt_sampled = 0; ct_clear; next;" for localnet
ports so egress ACL evaluation starts with a clean ct_state.
Behavior for other port types is unchanged.

Signed-off-by: Naveen Yerramneni <[email protected]>
Acked-by: Sragdhara Datta Chaudhuri <[email protected]>
Acked-by: Aditya Mehakare <[email protected]>
---
 northd/northd.c     | 10 +++++++---
 tests/ovn-northd.at | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 7a7866d9a..9a7adab1c 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -6445,10 +6445,14 @@ skip_port_from_conntrack(const struct ovn_datapath *od, 
struct ovn_port *op,
      * router on hostA, not hostB. This would only work with distributed
      * conntrack state across all chassis. */
 
+    /* Clear the ct_state for packets egressing through localnet ports to
+     * prevent them from matching flows in ls_out_acl_eval stage based on
+     * ct_state carried over from ingress pipeline */
     const char *ingress_action = "next;";
-    const char *egress_action = has_stateful_acl
-                                ? "next;"
-                                : "flags.pkt_sampled = 0; ct_clear; next;";
+    const char *egress_action =
+        (has_stateful_acl && !lsp_is_localnet(op->nbsp))
+        ? "next;"
+        : "flags.pkt_sampled = 0; ct_clear; next;";
 
     char *ingress_match = xasprintf("ip && inport == %s", op->json_key);
     char *egress_match = xasprintf("ip && outport == %s", op->json_key);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 6c84311ed..3556d78f8 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -19736,6 +19736,43 @@ OVN_CLEANUP_NORTHD
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([LS localnet conntrack skip with stateful ACLs])
+ovn_start
+
+check ovn-nbctl ls-add sw
+check ovn-nbctl lsp-add sw sw-p1 \
+    -- lsp-set-addresses sw-p1 "00:00:00:00:00:01 10.0.0.2"
+check ovn-nbctl lsp-add sw sw-ln
+check ovn-nbctl lsp-set-type sw-ln localnet
+check ovn-nbctl lsp-set-addresses sw-ln unknown
+check ovn-nbctl acl-add sw from-lport 1002 \
+    "ip4 && tcp && tcp.dst == 80" allow-related
+check ovn-nbctl --wait=sb sync
+
+ovn-sbctl dump-flows sw > lflows
+
+dnl Localnet port: egress pre_acl/pre_lb must use ct_clear even when a
+dnl stateful ACL is configured, so that ingress-zone ct_state does not
+dnl leak into egress ACL evaluation.
+AT_CHECK([grep 'pre_acl' lflows | grep '"sw-ln"' | ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_pre_acl      ), priority=110  , match=(ip && inport == 
"sw-ln"), action=(next;)
+  table=??(ls_out_pre_acl     ), priority=110  , match=(ip && outport == 
"sw-ln"), action=(flags.pkt_sampled = 0; ct_clear; next;)
+])
+
+AT_CHECK([grep 'pre_lb' lflows | grep '"sw-ln"' | ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_pre_lb       ), priority=110  , match=(ip && inport == 
"sw-ln"), action=(next;)
+  table=??(ls_out_pre_lb      ), priority=110  , match=(ip && outport == 
"sw-ln"), action=(flags.pkt_sampled = 0; ct_clear; next;)
+])
+
+dnl Regular VIF must not get a priority-110 skip flow in pre_acl/pre_lb;
+dnl its traffic must go through conntrack normally.
+AT_CHECK([grep 'pre_acl\|pre_lb' lflows | grep 'priority=110' | grep 
'"sw-p1"'], [1])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([Check network function])
 ovn_start
-- 
2.43.5

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to