On Thu, Jun 13, 2019 at 3:51 AM Ankur Sharma <[email protected]> wrote:
> Hi, > > Just adding a minor clarification inline. > > Regards, > Ankur > > > > *From:* Numan Siddique <[email protected]> > *Sent:* Wednesday, June 12, 2019 12:29 AM > *To:* Han Zhou <[email protected]> > *Cc:* ovs dev <[email protected]>; Ankur Sharma < > [email protected]> > *Subject:* Re: [ovs-dev] [PATCH] ovn: Send GARP for the router ports > connected to localnet switches > > > > > > > > On Tue, Jun 11, 2019 at 9:37 PM Han Zhou <[email protected]> wrote: > > > > > > On Mon, Jun 10, 2019 at 10:27 AM Numan Siddique <[email protected]> > wrote: > > > > > > On Fri, Jun 7, 2019 at 5:18 AM Han Zhou <[email protected]> wrote: > > > > On Wed, May 1, 2019 at 9:04 AM <[email protected]> wrote: > > > > From: Numan Siddique <[email protected]> > > > > With the commit [1], the routing for the provider logical switches > > connected to a router is centralized on the master gateway chassis > > (if the option - reside-on-redirect-chassis) is set. When the > > failover happens and a standby gateway chassis becomes master, > > it should send GARPs for the router port macs. Without this, the > > physical switch doesn't learn the new location of the router port macs > > immediately and this could result in traffic disruption. > > > > This patch addresses this issue so that the ovn-controller which claims > the > > distributed gatweway router port sends out the GARPs. > > > > [1] - 85706c34d53d ("ovn: Avoid tunneling for VLAN packets redirected to > a gateway chassis") > > > > Signed-off-by: Numan Siddique <[email protected]> > > --- > > ovn/northd/ovn-northd.c | 21 +++++++++++++++ > > tests/ovn.at [ovn.at] > <https://urldefense.proofpoint.com/v2/url?u=http-3A__ovn.at&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=NiZlnfKLpFZTne6nLvW2LY-mMfT1ROA_XO3vDUxswGA&s=UbZ9hOu17Ha6b2AEfkP9hmScErxLKfSpkmLB33spkiw&e=> > | 58 +++++++++++++++++++++++++++++++++++++++-- > > 2 files changed, 77 insertions(+), 2 deletions(-) > > > Hi Numan, > > Thanks for the fix. I have 2 comments: > > 1. The title is general which seems to address the problem for all "router > ports connected to localnet switches". However, the commit message body and > the code seems only handling the "reside-on-redirect-chassis" scenario, > without taking care of the more common scenario - the gateway port GARP. > > > > Agree. I think Ankur (CC'ed) is planning to handle this scenario - i.e > sending the GAR for the gateway ports. > > > > > > > > 2. The fix seems all related to "nat_addresses" handling, but this problem > is not directly related to "NAT". After reading more context of the code, I > realized that it is the right place to fix, but it is really not > straightforward to understand. Of course this is not a problem introduced > by current patch. It would be better if we had named the option > "garp_addresses" instead of "nat_addresses", but I think it may be hard to > rename at this stage because it will introduce compatibility problem. So > probably we can add some more comments just to make the context more clear > for readers. > > > > How about adding a new column "garp_addresses" ? This column can be used > both for the gw port GARP and for the "reside-on-redirect-chassis" ? > > > > Hi Numan, I am not sure if adding a new column is better than clarifying > with some documentation. If we add a new column, we should deprecate the > old "nat_addresses" column (and kept there only for ovsdb upgrading). I > don't think there is a need to distinguish in the schema if we are sending > GARP for NAT or for GW port. > > > > > > That's what even I thought. Ankur has some other requirements for sending > the GARPs for the gateway router port IPs. He wants the GARPs to be sent > periodically. I think where as now, the GARPs are sent as a burst whenever > a chassis claims a > > gw router port. > > > > Do you think it's better to change the GARP interval as per this patch - > https://patchwork.ozlabs.org/patch/1107466/ > [patchwork.ozlabs.org] > <https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_1107466_&d=DwMFaQ&c=s883GpUCOChKOHiocYtGcg&r=mZwX9gFQgeJHzTg-68aCJgsODyUEVsHGFOfL90J6MJY&m=NiZlnfKLpFZTne6nLvW2LY-mMfT1ROA_XO3vDUxswGA&s=cGB3-m1D9uO-iDu65mItknPBQwEMNe7noJBtRKNaoBY&e=> > > which Ankur has proposed for all the addresses - router addresses and NAT > addresses ? Please grep for "add_garp" in > > the above patch for more details. > > I have submitted the v2 here - https://patchwork.ozlabs.org/project/openvswitch/list/?series=113880. Request to please take a look > [ANKUR]: > a. We should still be sending burst and by burst I mean sending multiple > GARP packets with increasing inter packet interval. > i.e existing behavior is correct. > > b. In addition to above I wanted to add 2 more changes. > > > c. Send the GARP for a gateway chassis attached router port, even if NAT > is not configured on it. > For vlan backed networks, NAT is not mandatory to do North South > communication. > The patch 3 of the series i submitted will take care of it. > > d. For c. above do a. periodically. This is more being safe solution. > Reason being that, in the absence of encapsulation, > > redirection to gateway chassis will happen over L2, using the attached > router port mac as destination mac. > Hence, we have to ensure that this mac is always in learnt state in > the Physical switch, else redirection would end > > up causing flooding. > > > It should be fine to do (d) not just for (c) but for all the NAT addresses in the Port_Binding.nat_addresses column right ? Thoughts ? I mean there is no harm I suppose to send the GARPs periodically in bursts for all the GARP packets which ovn-controllers generate. @Ankur - I would suggest that you modify the existing garp code in ovn-controller to enhance to do (d). Thanks Numan > Thanks > > Numan > > > > > > > > If this makes sense, I will work on it. > > @Ankur - I will submit a patch with a new column (which will send GARPs > for both the gw router ports and other router ports with the option - > reside-on-redirect-chassis ) and and then you can enhance it to send the > GARPs in periodic interval instead of initial bursts (which the present > code does for NAT addresses ) ? Does this sound good ? > > > > Thanks > > Numan > > > > > > > > Thanks, > > Han > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
