Hi Dumitru,

thanks for your time! I will answer below.
On 22.04.2025 14:11, Dumitru Ceara wrote:

On 4/18/25 2:42 PM, Rukomoinikova Aleksandra wrote:


+        /* 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.


Hi Alexandra,

Side note: I think there's something weird going on with your email
client - at least with the html version of this email - the reply above
has white font color which on my white background is "hidden" :)

Sorry, I'm fixing my difficult relationship with my email client. )

To your comment:

Conntrack lookup is "constant" but quite costly.  It actually translates
to a recirculation in the datapath.  After conntrack state has been
populated the packet is reinjected in the OVS datapath with that
information attached.  That means packet headers need to be reparsed and
a new datapath flow lookup must happen.

regarding the fact that conntrack lookup is an expensive operation due to 
recirculation: but this is the price we pay for using balancers under the 
condition of stateless acl) a lookup by contract in egress will not occur if we 
do not have balancers, it will only occur if we have balancers under the 
condition of stateless acl. this is a decent payment for the work of balancers, 
as it seems to me. also, in this mode we return to the case as it was before 
the commit ([0] - ovn-org/ovn@a0f82ef.) before this commit we checked for 
conntrack all packets that got through according to stateless rules.

Another thing that's important is that without conntrack commit those
packets are forwarded by a datapath flow that matches on ct_state=+inv
which is not offloadable on certain HW (e.g., as far as I know, NVidia
NICs).

regarding hardware offloading: if I understand everything correctly, then 
hardware offloading on NICs is not supported during matching on the ct_state 
field +inv or -inv but I remove the match on the invalid flow from the 
pipelines, in this case I will show below (this is from my tests in ovn-northd):

before we add lb (have only stateful acl + stateless):
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;)
])

in the stateless_inv_match function I just check the need to add this flow:

after adding load balancers:

# 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;)
])

that is, it seems to me that this should not break offloading.

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>
+


One more thing I still don't completely understand is the actual use
case: do we also want to cover general load balancing in the presence of
stateless ACLs?  The doc update above talks about "load balancing when
the client balances traffic to itself".  Isn't your patch enabling load
balancing in general even when stateless ACLs are used?


yes, of course, it doesn't break any cases, it's just that this flow is 
necessary for the hairpin case, I'll make a clearer description and correct the 
priority (I forgot to change it to 105 in the documentation in v3)

Thanks,
Dumitru



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

Reply via email to