Re: [ovs-dev] [PATCH ovn v4] OVN: Multiple distributed gateway port support
On Wed, Mar 24, 2021 at 1:57 PM Ben Pfaff wrote: > > On Wed, Mar 24, 2021 at 08:27:07PM +, Dhathri Purohith wrote: > > Will work on the ddlog changes and update the patch. > > I'm available to help with the ddlog changes, especially if you have > trouble figuring it out yourself or if you have any questions. > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Hi Dhathir, Ankur, Any followup on this patch? It is quite useful but now requires a big rebase. Let me know if any help is needed. Thanks, Han ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4] OVN: Multiple distributed gateway port support
On Wed, Mar 24, 2021 at 08:27:07PM +, Dhathri Purohith wrote: > Will work on the ddlog changes and update the patch. I'm available to help with the ddlog changes, especially if you have trouble figuring it out yourself or if you have any questions. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v4] OVN: Multiple distributed gateway port support
On Thu, Mar 4, 2021 at 6:49 AM svc.eng.git-pa...@nutanix.com wrote: > > From: Ankur Sharma > > By default, OVN support only one distributed gateway > port (we will call it l3dgw port for further reference) > per logical router. While a single l3dgw port suffices > for most of the North South connectivity, however there > are requirements where a logical router could be connected > to multiple physical networks and based on routing decision > packet could go to vlan X or vlan Y. Additionally, packet > may or may not get NATed based on the configuration. > > This patch adds flexibility of having multiple l3dgw ports > per logical router. > > Changes can classified as following: > a. Data structure changes to allow multiple l3dgw ports 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 l3dgw port. > > d. ovn-nbctl cli change to allow multiple external ips >for a logical ip for same type. > > e. Except for reside-on-redirect-chassis all the other features >could be extended to multiple l3dgw ports. 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 l3dgw port. > > FUTURE WORK: > CT ZONES are still common for traffic from different physical networks. > This adds a restriction/assumption that same 5 tuple will not come > from different l3dgw ports. > A cleaner approach would be have different ct zones for each l3dgw port. > Changing the CT ZONE assignment is one of the enhancements we are > considering as next step. > > Signed-off-by: Ankur Sharma > Signed-off-by: Dhathri Purohith > Co-authored-by: Dhathri Purohith Hi Ankur, Thanks for v4. I'm sorry that it took a while to review this patch. This patch needs a rebase. Overall the patch LGTM. I have few comments. Please see below. If you can address those and add ddlog support in v5, I think it is good to go. ) This patch needs to update the documentation for distributed gateway ports in ovn-nb.xml (mainly here - https://github.com/ovn-org/ovn/blob/master/ovn-nb.xml#L2274 and in ovn-architecture.7.xml. Please do consider adding a section for multiple gw ports in ovn-architecture.7.xml if you can. Please add an entry in the NEWS file about this feature. Please see below for additional comments. > --- > northd/ovn-northd.c | 521 +++--- > ovn-nb.xml| 4 +- > tests/ovn-nbctl.at| 38 ++- > tests/ovn-northd.at | 465 - > tests/ovn.at | 310 - > utilities/ovn-nbctl.c | 147 +++- > 6 files changed, 1276 insertions(+), 209 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index ac872aade..cc228fd96 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -602,6 +602,19 @@ ovn_mcast_group_allocate_key(struct mcast_info > *mcast_info) >&mcast_info->group_tnlid_hint); > } > > +struct ovn_l3dgw_port { > +/* OVN northd only needs to know about the logical router gateway port > for > + * NAT on a distributed router. This "distributed gateway port" is > + * populated only when there is a gateway chassis or ha chassis group > + * specified for one of the ports on the logical router. Otherwise this > + * will be NULL. */ > +struct ovn_port *dgw_port; > + > +/* The "derived" OVN port representing the instance of l3dgw_port on > + * the gateway chassis. */ > +struct ovn_port *redirect_port; > +}; > + > /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or > * sb->external_ids:logical-switch. */ > struct ovn_datapath { > @@ -633,14 +646,9 @@ struct ovn_datapath { > /* Multicast data. */ > struct mcast_info mcast_info; > > -/* OVN northd only needs to know about the logical router gateway port > for > - * NAT on a distributed router. This "distributed gateway port" is > - * populated only when there is a gateway chassis specified for one of > - * the ports on the logical router. Otherwise this will be NULL. */ > -struct ovn_port *l3dgw_port; > -/* The "derived" OVN port representing the instance of l3dgw_port on > - * the gateway chassis. */ > -struct ovn_port *l3redirect_port; > +/* L3 distributed gateway ports */ > +struct ovn_l3dgw_port *l3dgw_ports; > +size_t n_l3dgw_ports; > > /* NAT entries configured on the router. */ > struct ovn_nat *nat_entries; > @@ -863,6 +871,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct > ovn_datapath *od) > ovn_destroy_tnlids(&od->port_tnlids); > destroy_ipam_info(&od->ipam
Re: [ovs-dev] [PATCH ovn v4] OVN: Multiple distributed gateway port support
References: <20210304011709.221041-1-svc.eng.git-pa...@nutanix.com> Bleep bloop. Greetings svc.eng.git-pa...@nutanix.com, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line lacks whitespace around operator #2046 FILE: utilities/ovn-nbctl.c:784: lr-nat-del ROUTER [TYPE [IP [EXTERNAL_IP]]]\n\ Lines checked: 2245, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v4] OVN: Multiple distributed gateway port support
From: Ankur Sharma By default, OVN support only one distributed gateway port (we will call it l3dgw port for further reference) per logical router. While a single l3dgw port suffices for most of the North South connectivity, however there are requirements where a logical router could be connected to multiple physical networks and based on routing decision packet could go to vlan X or vlan Y. Additionally, packet may or may not get NATed based on the configuration. This patch adds flexibility of having multiple l3dgw ports per logical router. Changes can classified as following: a. Data structure changes to allow multiple l3dgw ports 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 l3dgw port. d. ovn-nbctl cli change to allow multiple external ips for a logical ip for same type. e. Except for reside-on-redirect-chassis all the other features could be extended to multiple l3dgw ports. 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 l3dgw port. FUTURE WORK: CT ZONES are still common for traffic from different physical networks. This adds a restriction/assumption that same 5 tuple will not come from different l3dgw ports. A cleaner approach would be have different ct zones for each l3dgw port. Changing the CT ZONE assignment is one of the enhancements we are considering as next step. Signed-off-by: Ankur Sharma Signed-off-by: Dhathri Purohith Co-authored-by: Dhathri Purohith --- northd/ovn-northd.c | 521 +++--- ovn-nb.xml| 4 +- tests/ovn-nbctl.at| 38 ++- tests/ovn-northd.at | 465 - tests/ovn.at | 310 - utilities/ovn-nbctl.c | 147 +++- 6 files changed, 1276 insertions(+), 209 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index ac872aade..cc228fd96 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -602,6 +602,19 @@ ovn_mcast_group_allocate_key(struct mcast_info *mcast_info) &mcast_info->group_tnlid_hint); } +struct ovn_l3dgw_port { +/* OVN northd only needs to know about the logical router gateway port for + * NAT on a distributed router. This "distributed gateway port" is + * populated only when there is a gateway chassis or ha chassis group + * specified for one of the ports on the logical router. Otherwise this + * will be NULL. */ +struct ovn_port *dgw_port; + +/* The "derived" OVN port representing the instance of l3dgw_port on + * the gateway chassis. */ +struct ovn_port *redirect_port; +}; + /* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or * sb->external_ids:logical-switch. */ struct ovn_datapath { @@ -633,14 +646,9 @@ struct ovn_datapath { /* Multicast data. */ struct mcast_info mcast_info; -/* OVN northd only needs to know about the logical router gateway port for - * NAT on a distributed router. This "distributed gateway port" is - * populated only when there is a gateway chassis specified for one of - * the ports on the logical router. Otherwise this will be NULL. */ -struct ovn_port *l3dgw_port; -/* The "derived" OVN port representing the instance of l3dgw_port on - * the gateway chassis. */ -struct ovn_port *l3redirect_port; +/* L3 distributed gateway ports */ +struct ovn_l3dgw_port *l3dgw_ports; +size_t n_l3dgw_ports; /* NAT entries configured on the router. */ struct ovn_nat *nat_entries; @@ -863,6 +871,7 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) ovn_destroy_tnlids(&od->port_tnlids); destroy_ipam_info(&od->ipam_info); free(od->router_ports); +free(od->l3dgw_ports); destroy_nat_entries(od); free(od->nat_entries); free(od->localnet_ports); @@ -1416,6 +1425,82 @@ struct ovn_port { struct ovs_list list; /* In list of similar records. */ }; +/* Get the l3dgw port corresponding to a logical router port.*/ +static inline struct ovn_l3dgw_port* +ovn_get_l3dgw_port_from_lrp(const struct ovn_port *op) +{ +struct ovn_datapath *od = op->od; + +if (!op || !op->nbrp) { +return NULL; +} + +for (size_t i = 0; i < od->n_l3dgw_ports; i++) { +struct ovn_l3dgw_port *l3dgw_port = +&(od->l3dgw_ports[i]); +if (op == l3dgw_port->dgw_port) { +return l3dgw_port; +} +} + +return NULL; +} + +/* Get the l3dgw port corresponding to a logical router port + * with input ip */ +static struct ovn_l3dgw_port* +ov