[ovs-dev] [PATCH ovn v2 2/2] ovn-northd: Multiple distributed gateway port support.

Han Zhou hzhou at ovn.org
Wed Aug 11 22:48:22 UTC 2021


On Fri, Aug 6, 2021 at 2:13 PM Numan Siddique <numans at ovn.org> wrote:
>
> On Thu, Aug 5, 2021 at 11:40 AM Han Zhou <hzhou at ovn.org> wrote:
> >
> > From: Ankur Sharma <ankurmnnit2004 at gmail.com>
> >
> > By default, OVN support only one DGP (distributed gateway port) per
> > logical router. While a single DGP port suffices for most of the North
> > South connectivity, there are requirements where a logical router could
> > be connected to multiple external networks and based on routing decision
> > packet could go to different ones.
> >
> > This patch adds flexibility of having multiple DGPs per logical router.
> >
> > Changes can classified as following:
> > a. Data structure changes to allow multiple DGPs per ovn_datapath.
> >
> > b. Consumption of new data structure in logical flows for
> >    individual features.
> >
> > c. Features that require changes are:
> >    i. Regular NS traffic flow.
> >   ii. Network Address Translation.
> >  iii. Load Balancer
> >   iv. Gateway_mtu.
> >    v. reside-on-redirect-chassis
> >   vi. Misc code sections that assumed a single DGP.
> >
> > d. Except for reside-on-redirect-chassis all the other features
> >    could be extended to multiple DGPs. Reside on redirect
> >    chassis with its current specification could not be extended
> >    and hence should be used only with the logical router that
> >    has a single DGP.
> >
> > This patch doesn't support NAT & load-balancer features for multiple
> > DGPs yet, but added validations that disables NAT/load-balancer
> > features when there are more than one DGP configured per router.
> >
> > Signed-off-by: Ankur Sharma <ankurmnnit2004 at gmail.com>
> > Co-authored-by: Dhathri Purohith <dhathri.purohith at nutanix.com>
> > Signed-off-by: Dhathri Purohith <dhathri.purohith at nutanix.com>
> > Co-authored-by: Abhiram Sangana <sangana.abhiram at nutanix.com>
> > Signed-off-by: Abhiram Sangana <sangana.abhiram at nutanix.com>
> > Co-authored-by: Han Zhou <hzhou at ovn.org>
> > Signed-off-by: Han Zhou <hzhou at ovn.org>
>
>
> Hi Han,
>
> Thanks for v2.   I did some testing with this patch in a simple 2 node
> setup (using ovn-fake-multinode)
>
> Below are the logical resources created
>
> -----------------------------
> [root at ovn-central ~]# ovn-nbctl show
> switch 3a3c2522-fcce-49e5-8334-8a72547e7da6 (sw0)
>     port sw0-port4
>         addresses: ["50:54:00:00:00:06 dynamic"]
>     port sw0-port1
>         addresses: ["50:54:00:00:00:03 10.0.0.3 1000::3"]
>     port sw0-port2
>         addresses: ["50:54:00:00:00:04 10.0.0.4 1000::4"]
>     port sw0-lr0
>         type: router
>         router-port: lr0-sw0
>     port sw0-port3
>         addresses: ["50:54:00:00:00:05 dynamic"]
> switch 0573bbd7-fca7-4f06-84ac-1939f879fd5f (sw1)
>     port sw1-lr0
>         type: router
>         router-port: lr0-sw1
>     port sw1-port1
>         addresses: ["40:54:00:00:00:03 20.0.0.3 2000::3"]
> router c7cd8dab-4e6d-45f3-a8f4-3653d25ab476 (lr0)
>     port lr0-sw1
>         mac: "00:00:00:00:ff:02"
>         networks: ["20.0.0.1/24", "2000::a/64"]
>     port lr0-sw0
>         mac: "00:00:00:00:ff:01"
>         networks: ["10.0.0.1/24", "1000::a/64"]
>         gateway chassis: [ovn-chassis-1 ovn-chassis-2]
> [root at ovn-central ~]#
> [root at ovn-central ~]#
> [root at ovn-central ~]# ovn-sbctl show
> Chassis ovn-chassis-1
>     hostname: ovn-chassis-1
>     Encap geneve
>         ip: "170.168.0.4"
>         options: {csum="true"}
>     Port_Binding sw0-port1
>     Port_Binding sw0-port3
> Chassis ovn-gw-1
>     hostname: ovn-gw-1
>     Encap geneve
>         ip: "170.168.0.3"
>         options: {csum="true"}
> Chassis ovn-chassis-2
>     hostname: ovn-chassis-2
>     Encap geneve
>         ip: "170.168.0.5"
>         options: {csum="true"}
>     Port_Binding sw1-port1
>     Port_Binding sw0-port4
>     Port_Binding cr-lr0-sw0
> ---------
>
>
> As you can see the logical router port lr0-sw0 is a distributed gw
> port scheduled on chassis - ovn-chassis-2.
>
> The issue I see is :  I'm not able to ping from sw0-port1 (10.0.0.3,
> claimed by chassis ovn-chassis-1) to 10.0.0.1 and I'm not able to ping
> to sw1-port1 (20.0.0.3).
> I'm able to ping the same from sw0-port4 to 10.0.0.1 and 20.0.0.3
> (claimed by chassis ovn-chassis-2).
>
> If I move cr-lr0-sw0 to ovn-chassis-1, then ping from sw0-port1 works
> but not from sw0-port4.
>
> Is this expected ?
>

Yes, this is expected. I think what you are experiencing is that when
sw0-port1 is pinging 10.0.0.1 or 20.0.0.3 it need s to send a ARP to
resolve 10.0.0.1 first and by design of the chassis-redirect port it
doesn't send out ARP response if is_chassis_resident() check fails for the
DGP. I think this is not a problem. In addition, as you see your test only
has a single DGP, so it is not related to this patch which just adds
multiple DGP support and doesn't change this behavior.

> Does it mean that if a logical router has multiple gateway router
> ports connecting to geneve logical switches, then all the logical
> ports of those switches
> should be only bound on the corresponding gateway chassis ?  I
> understand this is the case with ovn-k8s.
>
> If this is a restriction, I think we should document it.

Yes, maybe it is helpful to make it more clear in the documentation for DGP
(regardless of one DGP or multiple DGPs). I can add it in a separate patch
since it is not related to this patch.

>
> Otherwise the patch LGTM and I'm fine with the feature.  The only
> issue I see is that of semantics.  If router port is a gateway router
> port, then its
> a peer's logical switch is ideally expected to have a localnet port.
> But in the case of ovn-k8s, that's not the case.  It is for this
> reason I thought
> pinning the logical switch to a particular chassis could be better.
>
> If you think having multiple gw router ports is better,  then I'd
> suggest documenting the limitations I mentioned above.  Also please
> see
> a small nit below.  And you can consider my Ack with these addressed.
>
> Acked-by: Numan Siddique <numans at ovn.org>
>
> Thanks
> Numan
>

Thanks Numan!
I applied the patch as is. Please see my explanation below.

> >
> > +static bool
> > +is_l3dgw_port(const struct ovn_port *op)
> > +{
> > +    return op->cr_port;
>
> Since the return type is bool,   I'd suggest - return !!op->cr_port;
>

For my understanding "!!" is not needed here since we don't require it to
be 0/1. In addition, there was a discussion regarding "!!" recently that
even if we need it to be 0/1, there is no need to add "!!" any more:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382820.html

Thanks,
Han


More information about the dev mailing list