On Tue, Feb 4, 2020 at 7:37 PM Daniel Alvarez Sanchez <[email protected]> wrote: > > Can we please include this patch as part of the next release? > > This is a really important bug fix especially on systems that have a > considerable number of floating IPs where otherwise the cloud becomes > unusable.
Thanks Dumitru and Daniel for the review and testing out the patch. I applied this patch to master. Thanks Numan > > On Tue, Feb 4, 2020 at 2:28 PM Daniel Alvarez Sanchez <[email protected]> > wrote: > > > Hi Numan, > > > > Thanks for this. I have deployed a setup with master branch and this patch > > applied. The patch LGTM and I can confirm that it works. Moreover, the > > number of lflows has been dramatically reduced from around 50 > > > > With 150 FIPs: > > > > Before: ~70K lflows https://imgur.com/a/OaVelCb > > After (in the same setup): 2K lflows > > > > [root@central ~]# ovn-sbctl list logical_flow | grep _uuid -c > > 2092 > > [root@central ~]# ovn-sbctl list logical_flow | grep lr_out_egr_loop -c > > 155 > > [root@central ~]# ovn-sbctl list logical_flow | grep lr_in_ip_routing -c > > 7 > > > > And I verified that FIP to FIP traffic works via the localnet port so it > > is really distributed and does not traverse any tunnels. > > > > Thanks! > > Daniel > > > > Tested-By: Daniel Alvarez Sanchez <[email protected]> > > Acked-By: Daniel Alvarez Sanchez <[email protected]> > > > > On Tue, Feb 4, 2020 at 1:59 PM Dumitru Ceara <[email protected]> wrote: > > > >> On 2/1/20 11:53 AM, [email protected] wrote: > >> > From: Numan Siddique <[email protected]> > >> > > >> > When the commit [1] added Distributed NAT support in OVN, it didn't > >> address > >> > the requirement of making East/West NAT traffic distributed. The E/W NAT > >> > traffic was still centralized. Later a couple of patches [2], addressed > >> this > >> > requirement. But the approach taken in [2] resulted in a lot of logical > >> flows > >> > as number of dnat_and_snat entries increase, as reported in > >> @Reported-at. > >> > > >> > This patch > >> > - reverts the approch taken in [2]. > >> > - removing the flows which does the NAT direct (REGBIT_NAT_REDIRECT) > >> to > >> > the gateway chassis. > >> > - and to solve the E/W centralized NAT it does the following: > >> > * Since for each NAT entry we know the MAC binding to be used for > >> the > >> > external_ip - either the external_mac if set or the MAC of the > >> > distributed gateway router port, this patch adds the flows in the > >> > S_ROUTER_IN_ARP_RESOLVE stage to set the eth.dst to the MAC if > >> the > >> > IP destination is external_ip. > >> > * The existing flows in the S_ROUTER_OUT_EGR_LOOP are now added by > >> additional > >> > match - is_chassis_resident('P') - where 'P' is logical_port of > >> the NAT entry > >> > if set, otherwise it is the chassis resident port of distributed > >> router port. > >> > With this additional match, the packet will be loopbacked to > >> apply the unSNAT/DNAT > >> > rules on the relevant chassis. > >> > > >> > Suppose if a logical port 'P' with IP 'A' has a dnat_and_snat entry > >> with external_mac/logical_port > >> > set, and if the packet's IP destination is one of the DNAT IP - then > >> the packet will be sent out > >> > of the local chassis, since eth.dst is resolved in the > >> S_ROUTER_IN_ARP_RESOLVE stage. > >> > If the external_mac/logical_port is not in NAT entry, then the packet > >> will be redirected to > >> > the gateway chassis. > >> > > >> > With this patch, for the logical resource reported in @Reported-at, the > >> number of logical > >> > flows come down to around 45k from 650k. > >> > > >> > [1] - ceacd9d49316("ovn: distributed NAT flows") > >> > > >> > [2] - 551e3d989557("OVN: fix DVR Floating IP support") > >> > 8244c6b6bd88("OVN: do not distribute traffic for local FIP") > >> > > >> > Reported-at: > >> https://mail.openvswitch.org/pipermail/ovs-discuss/2020-January/049714.html > >> > Reported-by: Daniel Alvarez Sanchez <[email protected]> > >> > Signed-off-by: Numan Siddique <[email protected]> > >> > --- > >> > northd/ovn-northd.8.xml | 191 +++++++++-------------------- > >> > northd/ovn-northd.c | 264 ++++++---------------------------------- > >> > tests/ovn-northd.at | 8 +- > >> > 3 files changed, 99 insertions(+), 364 deletions(-) > >> > > >> > >> Hi Numan, > >> > >> The patch looks ok to me and it passes unit tests. > >> > >> Hi Daniel, > >> As this is quite a significant change it might be nice if you could also > >> try it out on your setup. > >> > >> Otherwise: > >> > >> Acked-by: Dumitru Ceara <[email protected]> > >> > >> Regards, > >> Dumitru > >> > >> > _______________________________________________ > 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
