On Tue, Mar 8, 2022 at 8:02 AM Numan Siddique <[email protected]> wrote: > > On Tue, Mar 8, 2022 at 9:51 AM Dumitru Ceara <[email protected]> wrote: > > > > On 3/7/22 18:46, [email protected] wrote: > > > From: Numan Siddique <[email protected]> > > > > > > Presently for ACLs and LBs we do the following in the logical switch > > > ingress pipeline > > > > > > 1. Send the packet to conntrack. > > > 2. Apply ACLs (from-lport) > > > 3a. If the packet is a new connection and it is destined to the LB > > > VIP, then select a backend (and commit to conntrack with DNAT). > > > 3b. If the packet is a new connection and it doesn't match 3a, then > > > commit to conntrack. > > > > > > With the above approach, we cannot address the scenario of applying > > > ACLs after the load balancing. There can be ACLs which could match > > > on the load balancer backend ips. > > > > > > This patch addresses this usecase by > > > > > > 1. Send the packet to conntrack. > > > 2. Apply ACLs (from-lport, not configured with apply-after-lb=true) > > > 3. If the packet is a new connection and it is destined to the LB > > > VIP, then select a backend (and commit to conntrack with DNAT). > > > 4. Apply ACLs (from-lport, configured with apply-after-lb=true) > > > 5. If the packet is a new connection and it didn't match (2), then > > > commit to conntrack. > > > > > > In order to support this usecase, this patch supports an option > > > "apply-after-lb=true" in the ACL table. This option is valid > > > only for "from-lport" ACLs. > > > > > > Suggested-by: Dumitru Ceara <[email protected]> > > > Signed-off-by: Numan Siddique <[email protected]> > > > --- > > > > Aside from Han's comments, the rest looks good to me, thanks! > > > > Acked-by: Dumitru Ceara <[email protected]> > > Thanks Han and Dumitru. > > I applied this patch to the main branch with the below changes.
Thanks Numan. Since this is an important feature, would it be ok to backport to branch-21.12? cc @Mark Michelson <[email protected]> Han > > -- > > diff --git a/ovn-nb.xml b/ovn-nb.xml > index cea105b545..4d7a23c527 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2040,19 +2040,20 @@ > </p> > > <p> > - The main usecase of this option is to support ACLs matching on > + The main use case of this option is to support ACLs matching on > the destination IP address of the packet for the backend IPs > of load balancers. > </p> > > <p> > - <code>OVN</code> will apply the <code>from-lport</code>ACLs in two > + <code>OVN</code> will apply the <code>from-lport</code> ACLs in two > stages. ACLs without this option <code>apply-after-lb</code> > set, will be applied before the load balancer stage and ACLs > with this option set will be applied after the load balancer > - stage. Hence CMS should be extra careful when using this option > - and should carefully evaluate the priorities of all the ACLs and > - the default deny/allow ACLs if any. > + stage. The priorities are indepedent between these stages and > + may not be obvious to the CMS. Hence CMS should be extra careful > + when using this option and should carefully evaluate the priorities > + of all the ACLs and the default deny/allow ACLs if any. > </p> > </column> > </group> > > > > Numan > > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
