On Fri, Aug 6, 2021 at 2:13 PM Numan Siddique <[email protected]> wrote: > > On Thu, Aug 5, 2021 at 11:40 AM Han Zhou <[email protected]> wrote: > > > > From: Ankur Sharma <[email protected]> > > > > 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 <[email protected]> > > Co-authored-by: Dhathri Purohith <[email protected]> > > Signed-off-by: Dhathri Purohith <[email protected]> > > Co-authored-by: Abhiram Sangana <[email protected]> > > Signed-off-by: Abhiram Sangana <[email protected]> > > Co-authored-by: Han Zhou <[email protected]> > > Signed-off-by: Han Zhou <[email protected]> > > > 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@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@ovn-central ~]# > [root@ovn-central ~]# > [root@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 <[email protected]> > > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
