Re: [ovs-dev] [PATCH ovn v4] OVN: Multiple distributed gateway port support

2021-07-02 Thread Han Zhou
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

2021-03-24 Thread Ben Pfaff
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

2021-03-24 Thread Numan Siddique
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

2021-03-03 Thread 0-day Robot
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

2021-03-03 Thread svc.eng.git-pa...@nutanix.com
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