Re: [ovs-dev] [PATCH v3 ovn] northd: add empty_lb controller_event for logical router

2019-09-16 Thread Numan Siddique
On Sat, Sep 14, 2019 at 2:03 AM Mark Michelson  wrote:

> Acked-by: Mark Michelson 
>
> On 9/10/19 1:00 PM, Lorenzo Bianconi wrote:
> > Add empty load balancer controller_event support to logical router
> > pipeline. Update northd documentation even for logical switch pipeline
> >
> > Signed-off-by: Lorenzo Bianconi 
>

Thanks.
I applied this patch to master.

Numan


> > ---
> > Changes since v2:
> > - improve code readability
> > Changes since v1:
> > - rebase on-top of current ovn master branch
> > ---
> >   northd/ovn-northd.8.xml | 10 
> >   northd/ovn-northd.c | 24 --
> >   tests/ovn.at| 56 +++--
> >   3 files changed, 76 insertions(+), 14 deletions(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index b34ef687a..0f4f1c112 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1785,6 +1785,16 @@ icmp6 {
> >   
> >
> >   
> > +  
> > +If controller_event has been enabled for all the configured load
> > +balancing rules for a Gateway router or Router with gateway port
> > +in OVN_Northbound database that does not have
> configured
> > +backends, a priority-130 flow is added to trigger
> ovn-controller events
> > +whenever the chassis receives a packet for that particular VIP.
> > +If event-elb meter has been previously created, it
> will be
> > +associated to the empty_lb logical flow
> > +  
> > +
> > 
> >   For all the configured load balancing rules for a Gateway
> router or
> >   Router with gateway port in OVN_Northbound
> database that
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index c24e4d864..f393cebb8 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6148,9 +6148,17 @@ get_force_snat_ip(struct ovn_datapath *od, const
> char *key_type, ovs_be32 *ip)
> >   static void
> >   add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
> >  struct ds *match, struct ds *actions, int priority,
> > -   const char *lb_force_snat_ip, char *backend_ips,
> > -   bool is_udp, int addr_family)
> > +   const char *lb_force_snat_ip, struct smap_node
> *lb_info,
> > +   bool is_udp, int addr_family, char *ip_addr,
> > +   uint16_t l4_port, struct nbrec_load_balancer *lb,
> > +   struct shash *meter_groups)
> >   {
> > +char *backend_ips = lb_info->value;
> > +
> > +build_empty_lb_event_flow(od, lflows, lb_info, ip_addr, lb,
> > +  l4_port, addr_family, S_ROUTER_IN_DNAT,
> > +  meter_groups);
> > +
> >   /* A match and actions for new connections. */
> >   char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
> >   if (lb_force_snat_ip) {
> > @@ -6308,7 +6316,7 @@ copy_ra_to_sb(struct ovn_port *op, const char
> *address_mode)
> >
> >   static void
> >   build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> > -struct hmap *lflows)
> > +struct hmap *lflows, struct shash *meter_groups)
> >   {
> >   /* This flow table structure is documented in ovn-northd(8), so
> please
> >* update ovn-northd.8.xml if you change anything. */
> > @@ -7525,7 +7533,6 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
> >   ds_put_format(&match, "ip && ip6.dst == %s",
> >   ip_address);
> >   }
> > -free(ip_address);
> >
> >   int prio = 110;
> >   bool is_udp = lb->protocol && !strcmp(lb->protocol,
> "udp") ?
> > @@ -7546,8 +7553,11 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
> > od->l3redirect_port->json_key);
> >   }
> >   add_router_lb_flow(lflows, od, &match, &actions, prio,
> > -   lb_force_snat_ip, node->value,
> is_udp,
> > -   addr_family);
> > +   lb_force_snat_ip, node, is_udp,
> > +   addr_family, ip_address, port, lb,
> > +   meter_groups);
> > +
> > +free(ip_address);
> >   }
> >   }
> >   sset_destroy(&all_ips);
> > @@ -8328,7 +8338,7 @@ build_lflows(struct northd_context *ctx, struct
> hmap *datapaths,
> >
> >   build_lswitch_flows(datapaths, ports, port_groups, &lflows,
> mcgroups,
> >   igmp_groups, meter_groups);
> > -build_lrouter_flows(datapaths, ports, &lflows);
> > +build_lrouter_flows(datapaths, ports, &lflows, meter_groups);
> >
> >   /* Push changes to the Logical_Flow table to database. */
> >   const struct 

Re: [ovs-dev] [PATCH v3 ovn] northd: add empty_lb controller_event for logical router

2019-09-13 Thread Mark Michelson

Acked-by: Mark Michelson 

On 9/10/19 1:00 PM, Lorenzo Bianconi wrote:

Add empty load balancer controller_event support to logical router
pipeline. Update northd documentation even for logical switch pipeline

Signed-off-by: Lorenzo Bianconi 
---
Changes since v2:
- improve code readability
Changes since v1:
- rebase on-top of current ovn master branch
---
  northd/ovn-northd.8.xml | 10 
  northd/ovn-northd.c | 24 --
  tests/ovn.at| 56 +++--
  3 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b34ef687a..0f4f1c112 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1785,6 +1785,16 @@ icmp6 {
  
  
  

+  
+If controller_event has been enabled for all the configured load
+balancing rules for a Gateway router or Router with gateway port
+in OVN_Northbound database that does not have configured
+backends, a priority-130 flow is added to trigger ovn-controller events
+whenever the chassis receives a packet for that particular VIP.
+If event-elb meter has been previously created, it will be
+associated to the empty_lb logical flow
+  
+

  For all the configured load balancing rules for a Gateway router or
  Router with gateway port in OVN_Northbound database that
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c24e4d864..f393cebb8 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6148,9 +6148,17 @@ get_force_snat_ip(struct ovn_datapath *od, const char 
*key_type, ovs_be32 *ip)
  static void
  add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
 struct ds *match, struct ds *actions, int priority,
-   const char *lb_force_snat_ip, char *backend_ips,
-   bool is_udp, int addr_family)
+   const char *lb_force_snat_ip, struct smap_node *lb_info,
+   bool is_udp, int addr_family, char *ip_addr,
+   uint16_t l4_port, struct nbrec_load_balancer *lb,
+   struct shash *meter_groups)
  {
+char *backend_ips = lb_info->value;
+
+build_empty_lb_event_flow(od, lflows, lb_info, ip_addr, lb,
+  l4_port, addr_family, S_ROUTER_IN_DNAT,
+  meter_groups);
+
  /* A match and actions for new connections. */
  char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
  if (lb_force_snat_ip) {
@@ -6308,7 +6316,7 @@ copy_ra_to_sb(struct ovn_port *op, const char 
*address_mode)
  
  static void

  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
-struct hmap *lflows)
+struct hmap *lflows, struct shash *meter_groups)
  {
  /* This flow table structure is documented in ovn-northd(8), so please
   * update ovn-northd.8.xml if you change anything. */
@@ -7525,7 +7533,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
  ds_put_format(&match, "ip && ip6.dst == %s",
  ip_address);
  }
-free(ip_address);
  
  int prio = 110;

  bool is_udp = lb->protocol && !strcmp(lb->protocol, "udp") ?
@@ -7546,8 +7553,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
od->l3redirect_port->json_key);
  }
  add_router_lb_flow(lflows, od, &match, &actions, prio,
-   lb_force_snat_ip, node->value, is_udp,
-   addr_family);
+   lb_force_snat_ip, node, is_udp,
+   addr_family, ip_address, port, lb,
+   meter_groups);
+
+free(ip_address);
  }
  }
  sset_destroy(&all_ips);
@@ -8328,7 +8338,7 @@ build_lflows(struct northd_context *ctx, struct hmap 
*datapaths,
  
  build_lswitch_flows(datapaths, ports, port_groups, &lflows, mcgroups,

  igmp_groups, meter_groups);
-build_lrouter_flows(datapaths, ports, &lflows);
+build_lrouter_flows(datapaths, ports, &lflows, meter_groups);
  
  /* Push changes to the Logical_Flow table to database. */

  const struct sbrec_logical_flow *sbflow, *next_sbflow;
diff --git a/tests/ovn.at b/tests/ovn.at
index de1b3b3ba..2749184eb 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14683,9 +14683,22 @@ ovn_start
  # Create hypervisors hv[12].
  # Add vif1[12] to hv1, vif2[12] to hv2
  # Add all of the vifs to a single logical switch sw0.
+# Create logical router lr0
  
  net_add n1

-ovn-nbctl ls-add sw0
+
+ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
+for i in 0 1; do
+idx=$((i+1))
+ovn-nbctl ls-add sw$i
+

[ovs-dev] [PATCH v3 ovn] northd: add empty_lb controller_event for logical router

2019-09-10 Thread Lorenzo Bianconi
Add empty load balancer controller_event support to logical router
pipeline. Update northd documentation even for logical switch pipeline

Signed-off-by: Lorenzo Bianconi 
---
Changes since v2:
- improve code readability
Changes since v1:
- rebase on-top of current ovn master branch
---
 northd/ovn-northd.8.xml | 10 
 northd/ovn-northd.c | 24 --
 tests/ovn.at| 56 +++--
 3 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b34ef687a..0f4f1c112 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1785,6 +1785,16 @@ icmp6 {
 
 
 
+  
+If controller_event has been enabled for all the configured load
+balancing rules for a Gateway router or Router with gateway port
+in OVN_Northbound database that does not have configured
+backends, a priority-130 flow is added to trigger ovn-controller events
+whenever the chassis receives a packet for that particular VIP.
+If event-elb meter has been previously created, it will be
+associated to the empty_lb logical flow
+  
+
   
 For all the configured load balancing rules for a Gateway router or
 Router with gateway port in OVN_Northbound database that
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c24e4d864..f393cebb8 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6148,9 +6148,17 @@ get_force_snat_ip(struct ovn_datapath *od, const char 
*key_type, ovs_be32 *ip)
 static void
 add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
struct ds *match, struct ds *actions, int priority,
-   const char *lb_force_snat_ip, char *backend_ips,
-   bool is_udp, int addr_family)
+   const char *lb_force_snat_ip, struct smap_node *lb_info,
+   bool is_udp, int addr_family, char *ip_addr,
+   uint16_t l4_port, struct nbrec_load_balancer *lb,
+   struct shash *meter_groups)
 {
+char *backend_ips = lb_info->value;
+
+build_empty_lb_event_flow(od, lflows, lb_info, ip_addr, lb,
+  l4_port, addr_family, S_ROUTER_IN_DNAT,
+  meter_groups);
+
 /* A match and actions for new connections. */
 char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
 if (lb_force_snat_ip) {
@@ -6308,7 +6316,7 @@ copy_ra_to_sb(struct ovn_port *op, const char 
*address_mode)
 
 static void
 build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
-struct hmap *lflows)
+struct hmap *lflows, struct shash *meter_groups)
 {
 /* This flow table structure is documented in ovn-northd(8), so please
  * update ovn-northd.8.xml if you change anything. */
@@ -7525,7 +7533,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 ds_put_format(&match, "ip && ip6.dst == %s",
 ip_address);
 }
-free(ip_address);
 
 int prio = 110;
 bool is_udp = lb->protocol && !strcmp(lb->protocol, "udp") ?
@@ -7546,8 +7553,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
   od->l3redirect_port->json_key);
 }
 add_router_lb_flow(lflows, od, &match, &actions, prio,
-   lb_force_snat_ip, node->value, is_udp,
-   addr_family);
+   lb_force_snat_ip, node, is_udp,
+   addr_family, ip_address, port, lb,
+   meter_groups);
+
+free(ip_address);
 }
 }
 sset_destroy(&all_ips);
@@ -8328,7 +8338,7 @@ build_lflows(struct northd_context *ctx, struct hmap 
*datapaths,
 
 build_lswitch_flows(datapaths, ports, port_groups, &lflows, mcgroups,
 igmp_groups, meter_groups);
-build_lrouter_flows(datapaths, ports, &lflows);
+build_lrouter_flows(datapaths, ports, &lflows, meter_groups);
 
 /* Push changes to the Logical_Flow table to database. */
 const struct sbrec_logical_flow *sbflow, *next_sbflow;
diff --git a/tests/ovn.at b/tests/ovn.at
index de1b3b3ba..2749184eb 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14683,9 +14683,22 @@ ovn_start
 # Create hypervisors hv[12].
 # Add vif1[12] to hv1, vif2[12] to hv2
 # Add all of the vifs to a single logical switch sw0.
+# Create logical router lr0
 
 net_add n1
-ovn-nbctl ls-add sw0
+
+ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
+for i in 0 1; do
+idx=$((i+1))
+ovn-nbctl ls-add sw$i
+ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24
+ovn-nbctl \
+-- lsp-add sw$i lrp$i-atta