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

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


On Wed, Aug 11, 2021 at 3:48 PM Han Zhou <hzhou at ovn.org> wrote:
>
>
>
> 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.

+ Abhiram

Thanks Ankur, Dhathri and Abhiram for co-authoring!

>
> > >
> > > +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