Hi Dumitru, thanks for the review, I'll answer below)

On 17.04.2025 18:41, Dumitru Ceara wrote:

Hi Alexandra,

On 4/8/25 10:46 PM, Alexandra Rukomoinikova wrote:


[0] Removed support for using load balancers in conjunction with stateless ACL.
This commit removes the ability to use load balancers alongside stateless ACL.
If a load balancer is created, the datapath is no longer fully stateless.
Therefore, to avoid traffic being directed to the contract, it is recommended
to refrain from creating a load balancer entirely.

Commit [0] ensures the separation of stateful and stateless scenarios
in the absence of load balancers, without altering the functionality
of load balancers themselves.

When a logical switch is configured with stateless ACL and a load balancer,
the check for the `REGBIT_CONNTRACK_NAT` flag in the `pre_lb` stage of
the ingress pipeline becomes redundant. Traffic directed to the load balancer
must be processed through the conntrack.

To ensure proper load balancer operation, a rule must be added to match
the load balancer's VIP address and its protocol (if applicable). This rule
is added to the datapath group and does not negatively impact performance.
Packets matching this rule would still be directed to the contract via
lower-priority rules in the absence of stateless ACL. However, with stateless 
ACL,
this rule enables load balancing when the client balances traffic to itself.

In the egress pipeline, the stateless register should only be set if no
load balancers are present on the datapath. This maintains a clear separation
between Stateful and Stateless modes when using ACL.
If a user creates a load balancer on a logical switch, they should be aware
that the traffic will no longer be fully stateless.

Also in case of lb configured with stateless ACLs we no longer take into account
ct.inv packets in egress. They will be dropped further, at the hypervisor level.

[0] - ovn-org@a0f82ef.

Signed-off-by: Alexandra Rukomoinikova 
<arukomoinikova@k2.cloud><mailto:arukomoinikova@k2.cloud>
---
v2 --> v3:
        fixed Dumitru comments in v2.
        corrected hairpin case
        fixed failed tests
---
 northd/en-ls-stateful.c |   4 +
 northd/en-ls-stateful.h |   1 +
 northd/northd.c         |  60 ++++++++++++--
 northd/ovn-northd.8.xml |  13 ++-
 tests/ovn-northd.at     | 143 +++++++++++++++++++++++++++++---
 tests/ovn.at            |   8 +-
 tests/system-ovn.at     | 179 ++++++++++++++++++++++++++++++++++++----
 7 files changed, 372 insertions(+), 36 deletions(-)

diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
index 69cda5008..6240c1ea8 100644
--- a/northd/en-ls-stateful.c
+++ b/northd/en-ls-stateful.c
@@ -452,6 +452,10 @@ ls_stateful_record_set_acl_flags_(struct 
ls_stateful_record *ls_stateful_rec,
                 && !strcmp(acl->action, "allow-related")) {
             ls_stateful_rec->has_stateful_acl = true;
         }
+        if (!ls_stateful_rec->has_stateless_acl
+                && !strcmp(acl->action, "allow-stateless")) {
+            ls_stateful_rec->has_stateless_acl = true;
+        }
         if (ls_stateful_rec->has_stateful_acl &&
             ls_acl_tiers_are_maxed_out(
                 &ls_stateful_rec->max_acl_tier,
diff --git a/northd/en-ls-stateful.h b/northd/en-ls-stateful.h
index 18a7398a6..8516996c5 100644
--- a/northd/en-ls-stateful.h
+++ b/northd/en-ls-stateful.h
@@ -50,6 +50,7 @@ struct ls_stateful_record {
     size_t ls_index;

     bool has_stateful_acl;
+    bool has_stateless_acl;
     bool has_lb_vip;
     bool has_acls;
     struct acl_tier max_acl_tier;
diff --git a/northd/northd.c b/northd/northd.c
index 762e4e057..29c3229b3 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -5894,7 +5894,11 @@ build_stateless_filter(const struct ovn_datapath *od,
                                 action,
                                 &acl->header_,
                                 lflow_ref);
-    } else {
+    } else if (!od->nbs->n_load_balancer){



This is not correct.  A switch can have load balancers applied to it via
load balancer groups.  We need to check the ls_stateful_rec->has_lb_vip
associated to the datapath instead.

thanks, I'll correct it)





+        /* For cases when we have statefull ACLs but no load
+           balancer configured on logical switch - we should
+           completely bypass conntrack on egress, otherwise
+           it is necessary to check the balanced traffic. */



So what's the point of stateless to-lport ACLs (egress) if you still
commit to conntrack in the egress pipeline whenever there's a load
balancer configured on the switch/

I find this valid, if we want load balancers to work, then we realize that in 
egress traffic will get into the conntrack, but it will not be committed in it, 
because:
table=6 (ls_out_acl_eval    ), priority= , match=((stateless acl match)), 
action=(reg8[16] = 1; next;)

conntrack lookup time is constant, it seems to me that such behavior is valid, 
what do you think? Only connections related to the work of lb will be committed
I will correct 3 of your comments in version 4 if you generally support this 
idea.





         ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
                                 acl->priority + OVN_ACL_PRI_OFFSET,
                                 acl->match,
@@ -7355,6 +7359,23 @@ choose_max_acl_tier(const struct ls_stateful_record 
*ls_stateful_rec,
     }
 }

+/* In the case of stateless ACLs and load balancers, all traffic
+ * is passed to conntrack during egress pipeline. Conntrack will
+ * mark the packets as 'invalid,' but since stateless systems do
+ * not rely on conntrack state, these invalid packets will be
+ * discarded during processing on the hypervisor level.
+ */
+static bool
+stateless_inv_match(const struct ls_stateful_record *ls_stateful_rec)
+{
+    if (ls_stateful_rec->has_lb_vip
+            && ls_stateful_rec->has_stateless_acl) {
+        return false;
+    }
+
+    return true;
+}
+
 static void
 build_acls(const struct ls_stateful_record *ls_stateful_rec,
            const struct ovn_datapath *od,
@@ -7457,8 +7478,14 @@ build_acls(const struct ls_stateful_record 
*ls_stateful_rec,
          *
          * This is enforced at a higher priority than ACLs can be defined. */
         ds_clear(&match);
-        ds_put_format(&match, "%s(ct.est && ct.rpl && ct_mark.blocked == 1)",
-                      use_ct_inv_match ? "ct.inv || " : "");
+
+        if (use_ct_inv_match && stateless_inv_match(ls_stateful_rec)) {
+            ds_put_cstr(&match, "ct.inv || (ct.est && ct.rpl && "
+                                "ct_mark.blocked == 1)");
+        } else {
+            ds_put_cstr(&match, "ct.est && ct.rpl && ct_mark.blocked == 1");
+        }
+
         ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, UINT16_MAX - 3,
                       ds_cstr(&match), REGBIT_ACL_VERDICT_DROP " = 1; next;",
                       lflow_ref);
@@ -7752,8 +7779,7 @@ build_lb_rules_pre_stateful(struct lflow_table *lflows,
         }
         ds_put_cstr(action, "ct_lb_mark;");

-        ds_put_format(match, REGBIT_CONNTRACK_NAT" == 1 && %s.dst == %s",
-                      ip_match, lb_vip->vip_str);
+        ds_put_format(match, "%s.dst == %s", ip_match, lb_vip->vip_str);
         if (lb_vip->port_str) {
             ds_put_format(match, " && %s.dst == %s", lb->proto,
                           lb_vip->port_str);
@@ -7763,6 +7789,30 @@ build_lb_rules_pre_stateful(struct lflow_table *lflows,
             lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
             S_SWITCH_IN_PRE_STATEFUL, 120, ds_cstr(match), ds_cstr(action),
             &lb->nlb->header_, lb_dps->lflow_ref);
+
+        /* Pass all VIP traffic to the conntrack to support load balancers
+           in the case of stateless acl. */
+        const char *hairpin_snat_ip = smap_get(&lb->nlb->options,
+                                               "hairpin_snat_ip");



We shouldn't be parsing LB options here.  We should do that when we're
building the parsed 'lb', in ovn_northd_lb_init().

thanks, I'll correct it)





+        bool valid_snat_ip = false;
+        if (hairpin_snat_ip) {
+            ovs_be32 ip;
+            valid_snat_ip = ip_parse(hairpin_snat_ip, &ip);
+        }
+
+        if ((!lb_vip->port_str && valid_snat_ip) || lb_vip->port_str) {



I'm not sure I understand the condition here.  Why do we check
valid_snat_ip only if the VIP has no port?

my mistake, I'll correct it)





+            ds_clear(action);
+            ds_clear(match);
+
+            ds_put_format(match, "%s && %s.dst == %s", lb->proto, ip_match,
+                          valid_snat_ip ? hairpin_snat_ip : lb_vip->vip_str);
+            ds_put_cstr(action, "ct_lb_mark;");
+
+            ovn_lflow_add_with_dp_group(
+                lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths),
+                S_SWITCH_IN_PRE_STATEFUL, 105, ds_cstr(match), ds_cstr(action),
+                &lb->nlb->header_, lb_dps->lflow_ref);
+        }
     }
 }

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 1a70ba579..600eea824 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -507,7 +507,7 @@
       <ref table="Logical_Switch_Port" db="OVN_Northbound"/>.  Multicast, IPv6
       Neighbor Discovery and MLD traffic also skips stateful ACLs. For
       "allow-stateless" ACLs, a flow is added to bypass setting the hint for
-      connection tracker processing when there are stateful ACLs or LB rules;
+      connection tracker processing when there are stateful ACLs without LB;
       <code>REGBIT_ACL_STATELESS</code> is set for traffic matching stateless
       ACL flows.
     </p>
@@ -624,6 +624,14 @@
          <code>ct_lb_mark;</code> action.
       </li>

+      <li>
+         A priority-115 flow sends all packet directed to VIP to connection
+         tracker. Packets that match this rule would still be subject to
+         connection tracking via lower-priority rules in the absence of
+         stateless ACLs. However, with stateless ACLs in place, this rule
+         enables load balancing when the client balances traffic to itself.
+      </li>
+
       <li>
          A priority-100 flow sends the packets to connection tracker based
          on a hint provided by the previous tables
@@ -770,7 +778,8 @@
       </li>
       <li>
         <code>allow-stateless</code> ACLs translate into logical flows that set
-        the allow bit and advance to the next table.
+        the allow bit and advance to the next table. In case of load balancers
+        with stateless ACLs in this table we allow ct.inv packets go further.
       </li>
       <li>
         <code>reject</code> ACLs translate into logical flows with that set the
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 8ee3c977f..ca1d1a8b1 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3747,10 +3747,18 @@ for direction in from to; do
 done
 check ovn-nbctl --wait=sb sync

-# TCP packets should not go to conntrack for load balancing.
+# TCP packets should go to conntrack for load balancing.
 flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
 AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls "${flow}"], [0], 
[dnl
-output("lsp2");
+ct_lb_mark {
+    ct_lb_mark {
+        reg0[[6]] = 0;
+        reg0[[12]] = 0;
+        ct_lb_mark /* default (use --ct to customize) */ {
+            output("lsp2");
+        };
+    };
+};
 ])

 # UDP packets still go to conntrack.
@@ -3883,10 +3891,18 @@ for direction in from to; do
 done
 check ovn-nbctl --wait=sb sync

-# TCP packets should not go to conntrack for load balancing.
+# TCP packets should go to conntrack for load balancing.
 flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
 AT_CHECK_UNQUOTED([ovn_trace --ct new --ct new --minimal ls "${flow}"], [0], 
[dnl
-output("lsp2");
+ct_lb_mark {
+    ct_lb_mark {
+        reg0[[6]] = 0;
+        reg0[[12]] = 0;
+        ct_lb_mark /* default (use --ct to customize) */ {
+            output("lsp2");
+        };
+    };
+};
 ])

 # UDP packets still go to conntrack.
@@ -4775,10 +4791,12 @@ check_stateful_flows() {
     AT_CHECK([grep "ls_in_pre_stateful" sw0flows | ovn_strip_lflows], [0], [dnl
   table=??(ls_in_pre_stateful ), priority=0    , match=(1), action=(next;)
   table=??(ls_in_pre_stateful ), priority=100  , match=(reg0[[0]] == 1), 
action=(ct_next;)
+  table=??(ls_in_pre_stateful ), priority=105  , match=(tcp && ip4.dst == 
10.0.0.10), action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=105  , match=(tcp && ip4.dst == 
10.0.0.20), action=(ct_lb_mark;)
   table=??(ls_in_pre_stateful ), priority=110  , match=(reg0[[2]] == 1), 
action=(ct_lb_mark;)
   table=??(ls_in_pre_stateful ), priority=115  , match=(reg0[[2]] == 1 && 
ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
-  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] 
= 80; ct_lb_mark;)
-  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
ip4.dst == 10.0.0.20 && tcp.dst == 80), action=(reg4 = 10.0.0.20; reg2[[0..15]] 
= 80; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 10.0.0.10 
&& tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] = 80; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 10.0.0.20 
&& tcp.dst == 80), action=(reg4 = 10.0.0.20; reg2[[0..15]] = 80; ct_lb_mark;)
 ])

     AT_CHECK([grep "ls_in_lb " sw0flows | ovn_strip_lflows], [0], [dnl
@@ -5072,16 +5090,16 @@ AT_CAPTURE_FILE([sw0flows])

 AT_CHECK([grep -w "ls_in_acl_eval" sw0flows | grep 6553 | ovn_strip_lflows], 
[0], [dnl
   table=??(ls_in_acl_eval     ), priority=65532, match=(!ct.est && ct.rel && 
!ct.new && ct_mark.blocked == 0), action=(reg0[[17]] = 1; reg8[[16]] = 1; 
ct_commit_nat;)
-  table=??(ls_in_acl_eval     ), priority=65532, match=((ct.est && ct.rpl && 
ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=65532, match=(ct.est && !ct.rel && 
!ct.new && ct.rpl && ct_mark.blocked == 0), action=(reg0[[9]] = 0; reg0[[10]] = 
0; reg0[[17]] = 1; reg8[[16]] = 1; next;)
+  table=??(ls_in_acl_eval     ), priority=65532, match=(ct.est && ct.rpl && 
ct_mark.blocked == 1), action=(reg8[[17]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=65532, match=(ct.est && 
ct_mark.allow_established == 1), action=(reg0[[21]] = 1; reg8[[16]] = 1; next;)
   table=??(ls_in_acl_eval     ), priority=65532, match=(nd || nd_ra || nd_rs 
|| mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
 ])

 AT_CHECK([grep -w "ls_out_acl_eval" sw0flows | grep 6553 | ovn_strip_lflows], 
[0], [dnl
   table=??(ls_out_acl_eval    ), priority=65532, match=(!ct.est && ct.rel && 
!ct.new && ct_mark.blocked == 0), action=(reg8[[16]] = 1; ct_commit_nat;)
-  table=??(ls_out_acl_eval    ), priority=65532, match=((ct.est && ct.rpl && 
ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;)
   table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && !ct.rel && 
!ct.new && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; next;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && ct.rpl && 
ct_mark.blocked == 1), action=(reg8[[17]] = 1; next;)
   table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && 
ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;)
   table=??(ls_out_acl_eval    ), priority=65532, match=(nd || nd_ra || nd_rs 
|| mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
 ])
@@ -14348,7 +14366,7 @@ ovn-sbctl dump-flows s1 > s1flows
 AT_CAPTURE_FILE([s1flows])

 AT_CHECK([grep "ls_in_pre_stateful" s1flows | ovn_strip_lflows | grep 
"30.0.0.1"], [0], [dnl
-  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
ip4.dst == 30.0.0.1), action=(reg4 = 30.0.0.1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 30.0.0.1), 
action=(reg4 = 30.0.0.1; ct_lb_mark;)
 ])
 AT_CHECK([grep "ls_in_lb" s1flows | ovn_strip_lflows | grep "30.0.0.1"], [0], 
[dnl
   table=??(ls_in_lb           ), priority=110  , match=(ct.new && ip4.dst == 
30.0.0.1), action=(reg4 = 30.0.0.1; 
ct_lb_mark(backends=172.16.0.103,172.16.0.102,172.16.0.101);)
@@ -14462,7 +14480,7 @@ ovn-sbctl dump-flows s1 > s1flows
 AT_CAPTURE_FILE([s1flows])

 AT_CHECK([grep "ls_in_pre_stateful" s1flows | ovn_strip_lflows | grep 
"2001:db8:cccc::1"], [0], [dnl
-  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
ip6.dst == 2001:db8:cccc::1), action=(xxreg1 = 2001:db8:cccc::1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip6.dst == 
2001:db8:cccc::1), action=(xxreg1 = 2001:db8:cccc::1; ct_lb_mark;)
 ])
 AT_CHECK([grep "ls_in_lb" s1flows | ovn_strip_lflows | grep 
"2001:db8:cccc::1"], [0], [dnl
   table=??(ls_in_lb           ), priority=110  , match=(ct.new && ip6.dst == 
2001:db8:cccc::1), action=(xxreg1 = 2001:db8:cccc::1; 
ct_lb_mark(backends=2001:db8:aaaa:3::103,2001:db8:aaaa:3::102,2001:db8:aaaa:3::101);)
@@ -14573,7 +14591,7 @@ ovn-sbctl dump-flows s1 > s1flows
 AT_CAPTURE_FILE([s1flows])

 AT_CHECK([grep "ls_in_pre_stateful" s1flows | ovn_strip_lflows | grep 
"30.0.0.1"], [0], [dnl
-  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
ip4.dst == 30.0.0.1), action=(reg4 = 30.0.0.1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 30.0.0.1), 
action=(reg4 = 30.0.0.1; ct_lb_mark;)
 ])
 AT_CHECK([grep "ls_in_lb" s1flows | ovn_strip_lflows | grep "30.0.0.1"], [0], 
[dnl
   table=??(ls_in_lb           ), priority=110  , match=(ct.new && ip4.dst == 
30.0.0.1), action=(reg4 = 30.0.0.1; 
ct_lb_mark(backends=172.16.0.103,172.16.0.102,172.16.0.101);)
@@ -16909,5 +16927,108 @@ AT_CHECK([ovn_strip_lflows < lrflows | grep 
priority=105 | grep -c "flags.force_
 1
 ])

+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Load Balancers with stateless ACLs])
+ovn_start ovn-northd
+
+AS_BOX([create logical switches and ports])
+check ovn-nbctl ls-add sw0
+check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 \
+"00:00:00:00:00:02 10.0.0.2"
+
+check ovn-nbctl --wait=sb lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 \
+"00:00:00:00:00:03 10.0.0.3"
+
+AS_BOX([create stateless ACLs])
+check ovn-nbctl acl-add sw0 from-lport 1001 "ip" allow-stateless
+check ovn-nbctl acl-add sw0 to-lport 1001 "ip" allow-stateless
+
+AS_BOX([create stateful ACLs])
+# Check if allow-stateless acls have higher priority we skip conntrack.
+check ovn-nbctl acl-add sw0 from-lport 1000 "ip" allow-related
+check ovn-nbctl acl-add sw0 to-lport 1000 "ip" allow-related
+
+ovn-sbctl dump-flows sw0 > sw0flows
+
+AT_CHECK(
+  [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], 
[0], [dnl
+  table=??(ls_in_pre_acl      ), priority=100  , match=(ip), action=(reg0[[0]] 
= 1; next;)
+  table=??(ls_in_pre_acl      ), priority=2001 , match=(ip), 
action=(reg0[[16]] = 1; next;)
+  table=??(ls_out_pre_acl     ), priority=100  , match=(ip), action=(reg0[[0]] 
= 1; next;)
+  table=??(ls_out_pre_acl     ), priority=2001 , match=(ip), 
action=(reg0[[16]] = 1; next;)
+])
+
+AT_CHECK(
+  [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], 
[dnl
+  table=??(ls_out_acl_eval    ), priority=65532, match=(!ct.est && ct.rel && 
!ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; 
ct_commit_nat;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && !ct.rel && 
!ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; 
next;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && 
ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.inv || (ct.est && 
ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(nd || nd_ra || nd_rs 
|| mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
+])
+
+AS_BOX([create Load Balancer])
+check ovn-nbctl lb-add lb1 10.0.0.4:80 10.0.0.2:80,10.0.0.3:80
+check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
+
+ovn-sbctl dump-flows sw0 > sw0flows
+
+AT_CHECK([grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | 
ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_pre_acl      ), priority=100  , match=(ip), action=(reg0[[0]] 
= 1; next;)
+  table=??(ls_in_pre_acl      ), priority=2001 , match=(ip), 
action=(reg0[[16]] = 1; next;)
+  table=??(ls_out_pre_acl     ), priority=100  , match=(ip), action=(reg0[[0]] 
= 1; next;)
+])
+
+# We do not match conntrack invalide packets in case of load balancers with 
stateless ACLs.
+AT_CHECK(
+  [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], 
[dnl
+  table=??(ls_out_acl_eval    ), priority=65532, match=(!ct.est && ct.rel && 
!ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; 
ct_commit_nat;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && !ct.rel && 
!ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; 
next;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && ct.rpl && 
ct_mark.blocked == 1), action=(reg8[[17]] = 1; next;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(ct.est && 
ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;)
+  table=??(ls_out_acl_eval    ), priority=65532, match=(nd || nd_ra || nd_rs 
|| mldv1 || mldv2), action=(reg8[[16]] = 1; next;)
+])
+
+AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_pre_stateful ), priority=0    , match=(1), action=(next;)
+  table=??(ls_in_pre_stateful ), priority=100  , match=(reg0[[0]] == 1), 
action=(ct_next;)
+  table=??(ls_in_pre_stateful ), priority=105  , match=(tcp && ip4.dst == 
10.0.0.4), action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=110  , match=(reg0[[2]] == 1), 
action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=115  , match=(reg0[[2]] == 1 && 
ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 10.0.0.4 && 
tcp.dst == 80), action=(reg4 = 10.0.0.4; reg2[[0..15]] = 80; ct_lb_mark;)
+])
+
+AS_BOX([create Load Balancer without port])
+check ovn-nbctl --wait=sb ls-lb-del sw0 lb1
+check ovn-nbctl lb-add lb2 10.0.0.5 10.0.0.2,10.0.0.3
+check ovn-nbctl --wait=sb ls-lb-add sw0 lb2
+
+ovn-sbctl dump-flows sw0 > sw0flows
+
+AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_pre_stateful ), priority=0    , match=(1), action=(next;)
+  table=??(ls_in_pre_stateful ), priority=100  , match=(reg0[[0]] == 1), 
action=(ct_next;)
+  table=??(ls_in_pre_stateful ), priority=110  , match=(reg0[[2]] == 1), 
action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=115  , match=(reg0[[2]] == 1 && 
ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 10.0.0.5), 
action=(reg4 = 10.0.0.5; ct_lb_mark;)
+])
+
+AS_BOX([set hairpin_snat_ip to Load Balancer without port])
+check ovn-nbctl --wait=sb set load_balancer lb2 
options:hairpin_snat_ip="10.0.0.6"
+
+ovn-sbctl dump-flows sw0 > sw0flows
+AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_pre_stateful ), priority=0    , match=(1), action=(next;)
+  table=??(ls_in_pre_stateful ), priority=100  , match=(reg0[[0]] == 1), 
action=(ct_next;)
+  table=??(ls_in_pre_stateful ), priority=105  , match=(tcp && ip4.dst == 
10.0.0.6), action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=110  , match=(reg0[[2]] == 1), 
action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=115  , match=(reg0[[2]] == 1 && 
ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 10.0.0.5), 
action=(reg4 = 10.0.0.5; ct_lb_mark;)
+])
+
+
 AT_CLEANUP
 ])
diff --git a/tests/ovn.at b/tests/ovn.at
index bbaa653a8..da6714a65 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -26366,7 +26366,7 @@ OVS_WAIT_FOR_OUTPUT(
   [ovn-sbctl dump-flows > sbflows
    ovn-sbctl dump-flows sw0 | grep ct_lb_mark | grep priority=120 | sed 
's/table=..//'], 0,
   [dnl
-  (ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && ip4.dst == 
10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] = 80; 
ct_lb_mark;)
+  (ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 10.0.0.10 && 
tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] = 80; ct_lb_mark;)
   (ls_in_lb           ), priority=120  , match=(ct.new && ip4.dst == 10.0.0.10 
&& tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] = 80; 
ct_lb_mark(backends=10.0.0.3:80,20.0.0.3:80; 
hash_fields="ip_dst,ip_src,tcp_dst,tcp_src");)
 ])

@@ -26411,7 +26411,7 @@ AT_CHECK(
   [grep "ip4.dst == 10.0.0.10 && tcp.dst == 80" sbflows3 | grep priority=120 |\
    ovn_strip_lflows], [0], [dnl
   table=??(ls_in_lb           ), priority=120  , match=(ct.new && ip4.dst == 
10.0.0.10 && tcp.dst == 80), action=(drop;)
-  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] 
= 80; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 10.0.0.10 
&& tcp.dst == 80), action=(reg4 = 10.0.0.10; reg2[[0..15]] = 80; ct_lb_mark;)
 ])

 AT_CAPTURE_FILE([sbflows4])
@@ -26572,7 +26572,7 @@ OVS_WAIT_FOR_OUTPUT(
   [ovn-sbctl dump-flows > sbflows
    ovn-sbctl dump-flows sw0 | grep ct_lb_mark | grep priority=120 | sed 
's/table=..//'], 0,
   [dnl
-  (ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && ip6.dst == 
2002::a && tcp.dst == 80), action=(xxreg1 = 2002::a; reg2[[0..15]] = 80; 
ct_lb_mark;)
+  (ls_in_pre_stateful ), priority=120  , match=(ip6.dst == 2002::a && tcp.dst 
== 80), action=(xxreg1 = 2002::a; reg2[[0..15]] = 80; ct_lb_mark;)
   (ls_in_lb           ), priority=120  , match=(ct.new && ip6.dst == 2002::a 
&& tcp.dst == 80), action=(xxreg1 = 2002::a; reg2[[0..15]] = 80; 
ct_lb_mark(backends=[[2001::3]]:80,[[2002::3]]:80; 
hash_fields="ipv6_dst,ipv6_src,tcp_dst,tcp_src");)
 ])

@@ -26616,7 +26616,7 @@ AT_CHECK(
   [grep "ip6.dst == 2002::a && tcp.dst == 80" sbflows3 | grep priority=120 |\
    ovn_strip_lflows], [0], [dnl
   table=??(ls_in_lb           ), priority=120  , match=(ct.new && ip6.dst == 
2002::a && tcp.dst == 80), action=(drop;)
-  table=??(ls_in_pre_stateful ), priority=120  , match=(reg0[[2]] == 1 && 
ip6.dst == 2002::a && tcp.dst == 80), action=(xxreg1 = 2002::a; reg2[[0..15]] = 
80; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip6.dst == 2002::a && 
tcp.dst == 80), action=(xxreg1 = 2002::a; reg2[[0..15]] = 80; ct_lb_mark;)
 ])

 AT_CAPTURE_FILE([sbflows4])
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 7097e4155..515967520 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -5184,6 +5184,110 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port 
patch-.*/d
 AT_CLEANUP
 ])

+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Load Balancer LS hairpin IPv4 with stateless ACLs])
+
+ovn_start
+
+OVS_TRAFFIC_VSWITCHD_START()
+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:
+# One logical switch with IPv4 load balancers that hairpin the traffic.
+check ovn-nbctl ls-add sw
+check ovn-nbctl lsp-add sw lsp -- lsp-set-addresses lsp 00:00:00:00:00:01
+check ovn-nbctl lb-add lb-ipv4-tcp 88.88.88.88:8080 42.42.42.1:4041 tcp
+check ovn-nbctl ls-lb-add sw lb-ipv4-tcp
+
+check ovn-nbctl lr-add rtr
+check ovn-nbctl lrp-add rtr rtr-sw 00:00:00:00:01:00 42.42.42.254/24
+check ovn-nbctl lsp-add sw sw-rtr                       \
+    -- lsp-set-type sw-rtr router                 \
+    -- lsp-set-addresses sw-rtr 00:00:00:00:01:00 \
+    -- lsp-set-options sw-rtr router-port=rtr-sw
+
+ADD_NAMESPACES(lsp)
+ADD_VETH(lsp, lsp, br-int, "42.42.42.1/24", "00:00:00:00:00:01", \
+         "42.42.42.254")
+
+check ovn-nbctl --wait=hv -t 3 sync
+
+# Start IPv4 TCP server on lsp.
+NETNS_DAEMONIZE([lsp], [nc -l -k 42.42.42.1 4041], [lsp0.pid])
+
+
+# Check that IPv4 TCP hairpin connection succeeds on VIP.
+NS_CHECK_EXEC([lsp], [nc 88.88.88.88 8080 -z], [0], [ignore], [ignore])
+
+check ovn-nbctl acl-add sw to-lport 2000 'ip' allow-stateless
+check ovn-nbctl acl-add sw from-lport 2000 'ip' allow-stateless
+
+NS_CHECK_EXEC([lsp], [timeout 10 nc -z 88.88.88.88 8080 || echo "Connection 
timed out after 10 seconds"], [0], [ignore], [ignore])
+
+# send syn
+# 42.42.42.1:y -> 88.88.88.88:8080
+# snat to lb vip
+# 88.88.88.88:x - 42.42.42.1:4041 syn (we pass this packet to conntrack in 
egress)
+# so we send syn + ack
+# 42.42.42.1:4041 -> 88.88.88.88:x (here we need fourth flow to pass this 
packet to conntrack)
+# snat to client ip
+# send syn + ack
+# 88.88.88.88:8080 -> 42.42.42.1:y (we pass this packet to conntrack in egress)
+# send ack
+# 42.42.42.1:y -> 88.88.88.88:8080 (this package matches the fourth flow)
+# snat to vip
+# send ack
+# 88.88.88.88:x - 42.42.42.1:4041 (here we need third flow to pass this packet 
to conntrack)
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_pre_stateful | ovn_strip_lflows 
], [0], [dnl
+  table=??(ls_in_pre_stateful ), priority=0    , match=(1), action=(next;)
+  table=??(ls_in_pre_stateful ), priority=100  , match=(reg0[[0]] == 1), 
action=(ct_next;)
+  table=??(ls_in_pre_stateful ), priority=105  , match=(tcp && ip4.dst == 
88.88.88.88), action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=110  , match=(reg0[[2]] == 1), 
action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=115  , match=(reg0[[2]] == 1 && 
ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 88.88.88.88 
&& tcp.dst == 8080), action=(reg4 = 88.88.88.88; reg2[[0..15]] = 8080; 
ct_lb_mark;)
+])
+
+check ovn-nbctl set load_balancer lb-ipv4-tcp 
options:hairpin_snat_ip="88.88.88.89"
+AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_pre_stateful | 
ovn_strip_lflows], [0], [dnl
+  table=??(ls_in_pre_stateful ), priority=0    , match=(1), action=(next;)
+  table=??(ls_in_pre_stateful ), priority=100  , match=(reg0[[0]] == 1), 
action=(ct_next;)
+  table=??(ls_in_pre_stateful ), priority=105  , match=(tcp && ip4.dst == 
88.88.88.89), action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=110  , match=(reg0[[2]] == 1), 
action=(ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=115  , match=(reg0[[2]] == 1 && 
ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;)
+  table=??(ls_in_pre_stateful ), priority=120  , match=(ip4.dst == 88.88.88.88 
&& tcp.dst == 8080), action=(reg4 = 88.88.88.88; reg2[[0..15]] = 8080; 
ct_lb_mark;)
+])
+
+# Check that IPv4 TCP hairpin connection succeeds on VIP.
+NS_CHECK_EXEC([lsp], [nc 88.88.88.88 8080 -z], [0], [ignore], [ignore])
+
+OVN_CLEANUP_CONTROLLER([hv1])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([Load Balancer LS hairpin IPv6])
 AT_SKIP_IF([test $HAVE_NC = no])
@@ -10071,7 +10175,7 @@ 
tcp,orig=(src=192.168.1.2,dst=30.30.30.30,sport=<cleared>,dport=<cleared>),reply

 AT_CHECK([ovs-appctl dpctl/flush-conntrack])

-# remove lb
+# Remove Load Balancer.
 check ovn-nbctl ls-lb-del foo lb1

 # add stateless acl
@@ -10088,7 +10192,29 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl

 AT_CHECK([ovs-appctl dpctl/flush-conntrack])

-# add lb back
+# Delete ACLs.
+check ovn-nbctl acl-del foo
+
+# Add stateful ACLs.
+check ovn-nbctl acl-add foo from-lport 1 1 allow-related
+check ovn-nbctl --wait=hv acl-add foo to-lport 1 1 allow-related
+
+# Add stateless ACLs.
+check ovn-nbctl acl-add foo from-lport 2 1 allow-stateless
+check ovn-nbctl --wait=hv acl-add foo to-lport 2 1 allow-stateless
+
+AT_CHECK([ip netns exec foo1 wget   192.168.2.2 -t 1 -T 1], [0], [ignore], 
[ignore])
+
+# Check conntrack zone has no entry.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
+FORMAT_CT(192.168.2.2) | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+
+# When lb configured: pass conntrack for vip traffic.
+# Add Load Balancer back.
 check ovn-nbctl ls-lb-add foo lb1

 # Wait for ovn-controller to catch up.
@@ -10097,13 +10223,14 @@ check ovn-nbctl --wait=hv sync
 OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | \
 grep 'nat(dst=192.168.2.2:80)'])

-# should not dnat so will not be able to connect
-AT_CHECK([ip netns exec foo1 curl 30.30.30.30 --retry 3 --max-time 1], [28], 
[ignore], [ignore])
+# Now check with VIP.
+AT_CHECK([ip netns exec foo1 wget   30.30.30.30  -t 3 -T 1], [0], [ignore], 
[ignore])

-# check conntrack zone has no tcp entry
+# Check conntrack zone has tcp entry.
 AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
 FORMAT_CT(30.30.30.30) | \
 sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+tcp,orig=(src=192.168.1.2,dst=30.30.30.30,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
 ])

 AT_CHECK([ovs-appctl dpctl/flush-conntrack])
@@ -10203,7 +10330,7 @@ 
tcp,orig=(src=fd11::2,dst=fd12::2,sport=<cleared>,dport=<cleared>),reply=(src=fd

 AT_CHECK([ovs-appctl dpctl/flush-conntrack])

-# now check with VIP
+# Now check with VIP.
 AT_CHECK([ip netns exec foo1 wget  http://[[fd30::2]]  -t 3 -T 1], [0], 
[ignore], [ignore])

 # check conntrack zone has tcp entry
@@ -10215,16 +10342,39 @@ 
tcp,orig=(src=fd11::2,dst=fd30::2,sport=<cleared>,dport=<cleared>),reply=(src=fd

 AT_CHECK([ovs-appctl dpctl/flush-conntrack])

-# remove lb
+# Remove Load_Balancer.
 check ovn-nbctl ls-lb-del foo lb1

-# add stateless acl
+# Delete ACLs.
+check ovn-nbctl acl-del foo
+
+# Add stateful ACLs.
+check ovn-nbctl acl-add foo from-lport 1 1 allow-related
+check ovn-nbctl --wait=hv acl-add foo to-lport 1 1 allow-related
+
+# Add stateless ACLs.
+check ovn-nbctl acl-add foo from-lport 2 1 allow-stateless
+check ovn-nbctl --wait=hv acl-add foo to-lport 2 1 allow-stateless
+
+AT_CHECK([ip netns exec foo1  wget http://[[fd12::2]] -t 1 -T 1], [0], 
[ignore], [ignore])
+
+# Check conntrack zone has no tcp entry.
+AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
+FORMAT_CT(fd12::2) |  grep -v fe80 | \
+sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+
+check ovn-nbctl acl-del foo
+
+# Add stateless ACLs.
 check ovn-nbctl acl-add foo from-lport 1 1 allow-stateless
 check ovn-nbctl --wait=hv acl-add foo to-lport 1 1 allow-stateless

 AT_CHECK([ip netns exec foo1  wget http://[[fd12::2]] -t 3 -T 1], [0], 
[ignore], [ignore])

-# check conntrack zone has no tcp entry
+# Check conntrack zone has no tcp entry.
 AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
 FORMAT_CT(fd12::2) |  grep -v fe80 | \
 sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
@@ -10232,7 +10382,7 @@ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl

 AT_CHECK([ovs-appctl dpctl/flush-conntrack])

-# add lb back
+# Add Load Balancer back.
 check ovn-nbctl ls-lb-add foo lb1

 # Wait for ovn-controller to catch up.
@@ -10241,13 +10391,14 @@ check ovn-nbctl --wait=hv sync
 OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-groups br-int | \
 grep 'nat(dst=\[[fd12::2\]]:80)'])

-# should not dnat so will not be able to connect
-AT_CHECK([ip netns exec foo1 curl http://[[fd30::2]] --retry 3 --max-time 1], 
[28], [ignore], [ignore])
-#
-# check conntrack zone has no tcp entry
+# Now check with VIP.
+AT_CHECK([ip netns exec foo1 wget  http://[[fd30::2]]  -t 3 -T 1], [0], 
[ignore], [ignore])
+
+# Check conntrack zone has tcp entry.
 AT_CHECK([ovs-appctl dpctl/dump-conntrack zone=$zone_id | \
 FORMAT_CT(fd30::2) | grep -v fe80 | \
 sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
+tcp,orig=(src=fd11::2,dst=fd30::2,sport=<cleared>,dport=<cleared>),reply=(src=fd12::2,dst=fd11::2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>)
 ])

 AT_CHECK([ovs-appctl dpctl/flush-conntrack])



Regards,
Dumitru





--
regards,
Alexandra.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to