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