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

Ankur Sharma ankur.sharma at nutanix.com
Wed Jun 12 22:20:54 UTC 2019


Hi,

Just adding a minor clarification inline.

Regards,
Ankur

From: Numan Siddique <nusiddiq at redhat.com>
Sent: Wednesday, June 12, 2019 12:29 AM
To: Han Zhou <zhouhan at gmail.com>
Cc: ovs dev <dev at openvswitch.org>; Ankur Sharma <ankur.sharma at nutanix.com>
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 <zhouhan at gmail.com<mailto:zhouhan at gmail.com>> wrote:


On Mon, Jun 10, 2019 at 10:27 AM Numan Siddique <nusiddiq at redhat.com<mailto:nusiddiq at redhat.com>> wrote:


On Fri, Jun 7, 2019 at 5:18 AM Han Zhou <zhouhan at gmail.com<mailto:zhouhan at gmail.com>> wrote:


On Wed, May 1, 2019 at 9:04 AM <nusiddiq at redhat.com<mailto:nusiddiq at redhat.com>> wrote:
>
> From: Numan Siddique <nusiddiq at redhat.com<mailto: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<mailto:nusiddiq at redhat.com>>
> ---
>  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.

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

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.

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