[ovs-dev] [PATCH] ovn: Send GARP for the router ports connected to localnet switches

Numan Siddique nusiddiq at redhat.com
Wed Jun 12 07:29:25 UTC 2019


On Tue, Jun 11, 2019 at 9:37 PM Han Zhou <zhouhan at gmail.com> wrote:

>
>
> On Mon, Jun 10, 2019 at 10:27 AM Numan Siddique <nusiddiq at redhat.com>
> wrote:
>
>>
>>
>> On Fri, Jun 7, 2019 at 5:18 AM Han Zhou <zhouhan at gmail.com> wrote:
>>
>>>
>>>
>>> On Wed, May 1, 2019 at 9:04 AM <nusiddiq at redhat.com> wrote:
>>> >
>>> > From: Numan Siddique <nusiddiq at 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 <nusiddiq at 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.
>
>>
>>
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
>>>
>>


More information about the dev mailing list