On Mon, Jun 10, 2019 at 10:27 AM Numan Siddique <nusid...@redhat.com> wrote:
>
>
> On Fri, Jun 7, 2019 at 5:18 AM Han Zhou <zhou...@gmail.com> wrote:
>
>>
>>
>> On Wed, May 1, 2019 at 9:04 AM <nusid...@redhat.com> wrote:
>> >
>> > From: Numan Siddique <nusid...@redhat.com>
>> >
>> > 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 <nusid...@redhat.com>
>> > ---
>> > 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.
>
> 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
