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" ? 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
