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