[ovs-dev] [PATCH ovn v2 2/8] northd: Add a new engine node - northd_lb_data.

2023-07-06 Thread numans 
From: Numan Siddique 

This patch separates out the 'lbs' and 'lb_groups' from the 'northd' engine
node data into a new engine node  'northd_lb_data'. This new node becomes
an input to the 'northd' node.

This makes handling the NB load balancer and load balancer group changes
easier.

Signed-off-by: Numan Siddique 
---
 lib/lb.c   | 201 +--
 lib/lb.h   |  86 +++--
 northd/automake.mk |   2 +
 northd/en-lflow.c  |   3 +-
 northd/en-northd-lb-data.c | 126 +++
 northd/en-northd-lb-data.h |  19 ++
 northd/en-northd.c |  11 +-
 northd/en-sync-sb.c|   2 +-
 northd/inc-proc-northd.c   |   8 +-
 northd/northd.c| 673 +
 northd/northd.h|  15 +-
 11 files changed, 780 insertions(+), 366 deletions(-)
 create mode 100644 northd/en-northd-lb-data.c
 create mode 100644 northd/en-northd-lb-data.h

diff --git a/lib/lb.c b/lib/lb.c
index 7afdaed65b..429dbf15af 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -26,6 +26,7 @@
 #include "openvswitch/vlog.h"
 #include "lib/bitmap.h"
 #include "lib/smap.h"
+#include "socket-util.h"
 
 VLOG_DEFINE_THIS_MODULE(lb);
 
@@ -431,11 +432,62 @@ void ovn_northd_lb_vip_init(struct ovn_northd_lb_vip 
*lb_vip_nb,
 ovn_lb_get_health_check(nbrec_lb, vip_port_str, template);
 }
 
+static void
+ovn_lb_vip_backends_health_check_init(const struct ovn_northd_lb *lb,
+  const struct ovn_lb_vip *lb_vip,
+  struct ovn_northd_lb_vip *lb_vip_nb)
+{
+struct ds key = DS_EMPTY_INITIALIZER;
+
+for (size_t j = 0; j < lb_vip->n_backends; j++) {
+struct ovn_lb_backend *backend = &lb_vip->backends[j];
+ds_clear(&key);
+ds_put_format(&key, IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)
+  ? "%s" : "[%s]", backend->ip_str);
+
+const char *s = smap_get(&lb->nlb->ip_port_mappings, ds_cstr(&key));
+if (!s) {
+continue;
+}
+
+char *svc_mon_src_ip = NULL;
+char *port_name = xstrdup(s);
+char *p = strstr(port_name, ":");
+if (p) {
+*p = 0;
+p++;
+struct sockaddr_storage svc_mon_src_addr;
+if (!inet_parse_address(p, &svc_mon_src_addr)) {
+static struct vlog_rate_limit rl =
+VLOG_RATE_LIMIT_INIT(5, 1);
+VLOG_WARN_RL(&rl, "Invalid svc mon src IP %s", p);
+} else {
+struct ds src_ip_s = DS_EMPTY_INITIALIZER;
+ss_format_address_nobracks(&svc_mon_src_addr,
+&src_ip_s);
+svc_mon_src_ip = ds_steal_cstr(&src_ip_s);
+}
+}
+
+if (svc_mon_src_ip) {
+struct ovn_northd_lb_backend *backend_nb =
+&lb_vip_nb->backends_nb[j];
+backend_nb->health_check = true;
+backend_nb->logical_port = xstrdup(port_name);
+backend_nb->svc_mon_src_ip = svc_mon_src_ip;
+}
+free(port_name);
+}
+
+ds_destroy(&key);
+}
+
 static
 void ovn_northd_lb_vip_destroy(struct ovn_northd_lb_vip *vip)
 {
 free(vip->backend_ips);
 for (size_t i = 0; i < vip->n_backends; i++) {
+free(vip->backends_nb[i].logical_port);
 free(vip->backends_nb[i].svc_mon_src_ip);
 }
 free(vip->backends_nb);
@@ -555,8 +607,7 @@ ovn_lb_get_health_check(const struct nbrec_load_balancer 
*nbrec_lb,
 }
 
 struct ovn_northd_lb *
-ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb,
- size_t n_ls_datapaths, size_t n_lr_datapaths)
+ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb)
 {
 bool template = smap_get_bool(&nbrec_lb->options, "template", false);
 bool is_udp = nullable_string_is_equal(nbrec_lb->protocol, "udp");
@@ -595,9 +646,6 @@ ovn_northd_lb_create(const struct nbrec_load_balancer 
*nbrec_lb,
 }
 lb->affinity_timeout = affinity_timeout;
 
-lb->nb_ls_map = bitmap_allocate(n_ls_datapaths);
-lb->nb_lr_map = bitmap_allocate(n_lr_datapaths);
-
 sset_init(&lb->ips_v4);
 sset_init(&lb->ips_v6);
 struct smap_node *node;
@@ -631,7 +679,12 @@ ovn_northd_lb_create(const struct nbrec_load_balancer 
*nbrec_lb,
 ovn_lb_vip6_template_format_internal(lb_vip),
 xstrdup(node->value));
 }
+
 n_vips++;
+
+if (lb_vip_nb->lb_health_check) {
+ovn_lb_vip_backends_health_check_init(lb, lb_vip, lb_vip_nb);
+}
 }
 
 /* It's possible that parsing VIPs fails.  Update the lb->n_vips to the
@@ -639,6 +692,7 @@ ovn_northd_lb_create(const struct nbrec_load_balancer 
*nbrec_lb,
  */
 lb->n_vips = n_vips;
 
+
 if (nbrec_lb->n_selection_fields) {
 char *proto = NULL;
 if (nbrec_lb->protocol && nbrec_lb->protocol[0]) {
@@ -684,24 +738,6 @@ ovn_northd_l

Re: [ovs-dev] [PATCH ovn v2 2/8] northd: Add a new engine node - northd_lb_data.

2023-07-07 Thread Mark Michelson 

Hi Numan,

I have one small nit below.

On 7/7/23 01:53, num...@ovn.org wrote:

From: Numan Siddique 

This patch separates out the 'lbs' and 'lb_groups' from the 'northd' engine
node data into a new engine node  'northd_lb_data'. This new node becomes
an input to the 'northd' node.

This makes handling the NB load balancer and load balancer group changes
easier.

Signed-off-by: Numan Siddique 
---
  lib/lb.c   | 201 +--
  lib/lb.h   |  86 +++--
  northd/automake.mk |   2 +
  northd/en-lflow.c  |   3 +-
  northd/en-northd-lb-data.c | 126 +++
  northd/en-northd-lb-data.h |  19 ++
  northd/en-northd.c |  11 +-
  northd/en-sync-sb.c|   2 +-
  northd/inc-proc-northd.c   |   8 +-
  northd/northd.c| 673 +
  northd/northd.h|  15 +-
  11 files changed, 780 insertions(+), 366 deletions(-)
  create mode 100644 northd/en-northd-lb-data.c
  create mode 100644 northd/en-northd-lb-data.h

diff --git a/lib/lb.c b/lib/lb.c
index 7afdaed65b..429dbf15af 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -26,6 +26,7 @@
  #include "openvswitch/vlog.h"
  #include "lib/bitmap.h"
  #include "lib/smap.h"
+#include "socket-util.h"
  
  VLOG_DEFINE_THIS_MODULE(lb);
  
@@ -431,11 +432,62 @@ void ovn_northd_lb_vip_init(struct ovn_northd_lb_vip *lb_vip_nb,

  ovn_lb_get_health_check(nbrec_lb, vip_port_str, template);
  }
  
+static void

+ovn_lb_vip_backends_health_check_init(const struct ovn_northd_lb *lb,
+  const struct ovn_lb_vip *lb_vip,
+  struct ovn_northd_lb_vip *lb_vip_nb)
+{
+struct ds key = DS_EMPTY_INITIALIZER;
+
+for (size_t j = 0; j < lb_vip->n_backends; j++) {
+struct ovn_lb_backend *backend = &lb_vip->backends[j];
+ds_clear(&key);
+ds_put_format(&key, IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)
+  ? "%s" : "[%s]", backend->ip_str);
+
+const char *s = smap_get(&lb->nlb->ip_port_mappings, ds_cstr(&key));
+if (!s) {
+continue;
+}
+
+char *svc_mon_src_ip = NULL;
+char *port_name = xstrdup(s);
+char *p = strstr(port_name, ":");
+if (p) {
+*p = 0;
+p++;
+struct sockaddr_storage svc_mon_src_addr;
+if (!inet_parse_address(p, &svc_mon_src_addr)) {
+static struct vlog_rate_limit rl =
+VLOG_RATE_LIMIT_INIT(5, 1);
+VLOG_WARN_RL(&rl, "Invalid svc mon src IP %s", p);
+} else {
+struct ds src_ip_s = DS_EMPTY_INITIALIZER;
+ss_format_address_nobracks(&svc_mon_src_addr,
+&src_ip_s);
+svc_mon_src_ip = ds_steal_cstr(&src_ip_s);
+}
+}
+
+if (svc_mon_src_ip) {
+struct ovn_northd_lb_backend *backend_nb =
+&lb_vip_nb->backends_nb[j];
+backend_nb->health_check = true;
+backend_nb->logical_port = xstrdup(port_name);
+backend_nb->svc_mon_src_ip = svc_mon_src_ip;
+}
+free(port_name);
+}
+
+ds_destroy(&key);
+}
+
  static
  void ovn_northd_lb_vip_destroy(struct ovn_northd_lb_vip *vip)
  {
  free(vip->backend_ips);
  for (size_t i = 0; i < vip->n_backends; i++) {
+free(vip->backends_nb[i].logical_port);
  free(vip->backends_nb[i].svc_mon_src_ip);
  }
  free(vip->backends_nb);
@@ -555,8 +607,7 @@ ovn_lb_get_health_check(const struct nbrec_load_balancer 
*nbrec_lb,
  }
  
  struct ovn_northd_lb *

-ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb,
- size_t n_ls_datapaths, size_t n_lr_datapaths)
+ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb)
  {
  bool template = smap_get_bool(&nbrec_lb->options, "template", false);
  bool is_udp = nullable_string_is_equal(nbrec_lb->protocol, "udp");
@@ -595,9 +646,6 @@ ovn_northd_lb_create(const struct nbrec_load_balancer 
*nbrec_lb,
  }
  lb->affinity_timeout = affinity_timeout;
  
-lb->nb_ls_map = bitmap_allocate(n_ls_datapaths);

-lb->nb_lr_map = bitmap_allocate(n_lr_datapaths);
-
  sset_init(&lb->ips_v4);
  sset_init(&lb->ips_v6);
  struct smap_node *node;
@@ -631,7 +679,12 @@ ovn_northd_lb_create(const struct nbrec_load_balancer 
*nbrec_lb,
  ovn_lb_vip6_template_format_internal(lb_vip),
  xstrdup(node->value));
  }
+
  n_vips++;
+
+if (lb_vip_nb->lb_health_check) {
+ovn_lb_vip_backends_health_check_init(lb, lb_vip, lb_vip_nb);
+}
  }
  
  /* It's possible that parsing VIPs fails.  Update the lb->n_vips to the

@@ -639,6 +692,7 @@ ovn_northd_lb_create(const struct nbrec_load_balancer 
*nbrec_lb,
   */
  lb->n_vips = n_vips;
  
+

  if (nbrec_lb->n_se

Re: [ovs-dev] [PATCH ovn v2 2/8] northd: Add a new engine node - northd_lb_data.

2023-07-12 Thread Han Zhou 
On Sat, Jul 8, 2023 at 3:57 AM Mark Michelson  wrote:
>
> Hi Numan,
>
> I have one small nit below.

+1
Acked-by: Han Zhou 

>
> On 7/7/23 01:53, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > This patch separates out the 'lbs' and 'lb_groups' from the 'northd'
engine
> > node data into a new engine node  'northd_lb_data'. This new node
becomes
> > an input to the 'northd' node.
> >
> > This makes handling the NB load balancer and load balancer group changes
> > easier.
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   lib/lb.c   | 201 +--
> >   lib/lb.h   |  86 +++--
> >   northd/automake.mk |   2 +
> >   northd/en-lflow.c  |   3 +-
> >   northd/en-northd-lb-data.c | 126 +++
> >   northd/en-northd-lb-data.h |  19 ++
> >   northd/en-northd.c |  11 +-
> >   northd/en-sync-sb.c|   2 +-
> >   northd/inc-proc-northd.c   |   8 +-
> >   northd/northd.c| 673 +
> >   northd/northd.h|  15 +-
> >   11 files changed, 780 insertions(+), 366 deletions(-)
> >   create mode 100644 northd/en-northd-lb-data.c
> >   create mode 100644 northd/en-northd-lb-data.h
> >
> > diff --git a/lib/lb.c b/lib/lb.c
> > index 7afdaed65b..429dbf15af 100644
> > --- a/lib/lb.c
> > +++ b/lib/lb.c
> > @@ -26,6 +26,7 @@
> >   #include "openvswitch/vlog.h"
> >   #include "lib/bitmap.h"
> >   #include "lib/smap.h"
> > +#include "socket-util.h"
> >
> >   VLOG_DEFINE_THIS_MODULE(lb);
> >
> > @@ -431,11 +432,62 @@ void ovn_northd_lb_vip_init(struct
ovn_northd_lb_vip *lb_vip_nb,
> >   ovn_lb_get_health_check(nbrec_lb, vip_port_str, template);
> >   }
> >
> > +static void
> > +ovn_lb_vip_backends_health_check_init(const struct ovn_northd_lb *lb,
> > +  const struct ovn_lb_vip *lb_vip,
> > +  struct ovn_northd_lb_vip
*lb_vip_nb)
> > +{
> > +struct ds key = DS_EMPTY_INITIALIZER;
> > +
> > +for (size_t j = 0; j < lb_vip->n_backends; j++) {
> > +struct ovn_lb_backend *backend = &lb_vip->backends[j];
> > +ds_clear(&key);
> > +ds_put_format(&key, IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)
> > +  ? "%s" : "[%s]", backend->ip_str);
> > +
> > +const char *s = smap_get(&lb->nlb->ip_port_mappings,
ds_cstr(&key));
> > +if (!s) {
> > +continue;
> > +}
> > +
> > +char *svc_mon_src_ip = NULL;
> > +char *port_name = xstrdup(s);
> > +char *p = strstr(port_name, ":");
> > +if (p) {
> > +*p = 0;
> > +p++;
> > +struct sockaddr_storage svc_mon_src_addr;
> > +if (!inet_parse_address(p, &svc_mon_src_addr)) {
> > +static struct vlog_rate_limit rl =
> > +VLOG_RATE_LIMIT_INIT(5, 1);
> > +VLOG_WARN_RL(&rl, "Invalid svc mon src IP %s", p);
> > +} else {
> > +struct ds src_ip_s = DS_EMPTY_INITIALIZER;
> > +ss_format_address_nobracks(&svc_mon_src_addr,
> > +&src_ip_s);
> > +svc_mon_src_ip = ds_steal_cstr(&src_ip_s);
> > +}
> > +}
> > +
> > +if (svc_mon_src_ip) {
> > +struct ovn_northd_lb_backend *backend_nb =
> > +&lb_vip_nb->backends_nb[j];
> > +backend_nb->health_check = true;
> > +backend_nb->logical_port = xstrdup(port_name);
> > +backend_nb->svc_mon_src_ip = svc_mon_src_ip;
> > +}
> > +free(port_name);
> > +}
> > +
> > +ds_destroy(&key);
> > +}
> > +
> >   static
> >   void ovn_northd_lb_vip_destroy(struct ovn_northd_lb_vip *vip)
> >   {
> >   free(vip->backend_ips);
> >   for (size_t i = 0; i < vip->n_backends; i++) {
> > +free(vip->backends_nb[i].logical_port);
> >   free(vip->backends_nb[i].svc_mon_src_ip);
> >   }
> >   free(vip->backends_nb);
> > @@ -555,8 +607,7 @@ ovn_lb_get_health_check(const struct
nbrec_load_balancer *nbrec_lb,
> >   }
> >
> >   struct ovn_northd_lb *
> > -ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb,
> > - size_t n_ls_datapaths, size_t n_lr_datapaths)
> > +ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb)
> >   {
> >   bool template = smap_get_bool(&nbrec_lb->options, "template",
false);
> >   bool is_udp = nullable_string_is_equal(nbrec_lb->protocol, "udp");
> > @@ -595,9 +646,6 @@ ovn_northd_lb_create(const struct
nbrec_load_balancer *nbrec_lb,
> >   }
> >   lb->affinity_timeout = affinity_timeout;
> >
> > -lb->nb_ls_map = bitmap_allocate(n_ls_datapaths);
> > -lb->nb_lr_map = bitmap_allocate(n_lr_datapaths);
> > -
> >   sset_init(&lb->ips_v4);
> >   sset_init(&lb->ips_v6);
> >   struct smap_node *node;
> > @@ -631,7 +679,12 @@ ovn_northd_lb_create(const struct
nbrec_load_balancer

Re: [ovs-dev] [PATCH ovn v2 2/8] northd: Add a new engine node - northd_lb_data.

2023-07-12 Thread Han Zhou 
On Wed, Jul 12, 2023 at 11:54 PM Han Zhou  wrote:
>
>
>
> On Sat, Jul 8, 2023 at 3:57 AM Mark Michelson  wrote:
> >
> > Hi Numan,
> >
> > I have one small nit below.
>
> +1
> Acked-by: Han Zhou 
>

Sorry that I forgot one small comment regarding the naming of the new node
(and the related files). I think it is better to be just lb_data instead of
northd_lb_data.
The node northd_data was named that way because it was initially the single
node that contains almost everything in northd. For each I-P node that is
extracted from the northd_data, the northd_ prefix becomes unnecessary.
What do you think?

Regards,
Han
> >
> > On 7/7/23 01:53, num...@ovn.org wrote:
> > > From: Numan Siddique 
> > >
> > > This patch separates out the 'lbs' and 'lb_groups' from the 'northd'
engine
> > > node data into a new engine node  'northd_lb_data'. This new node
becomes
> > > an input to the 'northd' node.
> > >
> > > This makes handling the NB load balancer and load balancer group
changes
> > > easier.
> > >
> > > Signed-off-by: Numan Siddique 
> > > ---
> > >   lib/lb.c   | 201 +--
> > >   lib/lb.h   |  86 +++--
> > >   northd/automake.mk |   2 +
> > >   northd/en-lflow.c  |   3 +-
> > >   northd/en-northd-lb-data.c | 126 +++
> > >   northd/en-northd-lb-data.h |  19 ++
> > >   northd/en-northd.c |  11 +-
> > >   northd/en-sync-sb.c|   2 +-
> > >   northd/inc-proc-northd.c   |   8 +-
> > >   northd/northd.c| 673
+
> > >   northd/northd.h|  15 +-
> > >   11 files changed, 780 insertions(+), 366 deletions(-)
> > >   create mode 100644 northd/en-northd-lb-data.c
> > >   create mode 100644 northd/en-northd-lb-data.h
> > >
> > > diff --git a/lib/lb.c b/lib/lb.c
> > > index 7afdaed65b..429dbf15af 100644
> > > --- a/lib/lb.c
> > > +++ b/lib/lb.c
> > > @@ -26,6 +26,7 @@
> > >   #include "openvswitch/vlog.h"
> > >   #include "lib/bitmap.h"
> > >   #include "lib/smap.h"
> > > +#include "socket-util.h"
> > >
> > >   VLOG_DEFINE_THIS_MODULE(lb);
> > >
> > > @@ -431,11 +432,62 @@ void ovn_northd_lb_vip_init(struct
ovn_northd_lb_vip *lb_vip_nb,
> > >   ovn_lb_get_health_check(nbrec_lb, vip_port_str, template);
> > >   }
> > >
> > > +static void
> > > +ovn_lb_vip_backends_health_check_init(const struct ovn_northd_lb *lb,
> > > +  const struct ovn_lb_vip
*lb_vip,
> > > +  struct ovn_northd_lb_vip
*lb_vip_nb)
> > > +{
> > > +struct ds key = DS_EMPTY_INITIALIZER;
> > > +
> > > +for (size_t j = 0; j < lb_vip->n_backends; j++) {
> > > +struct ovn_lb_backend *backend = &lb_vip->backends[j];
> > > +ds_clear(&key);
> > > +ds_put_format(&key, IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)
> > > +  ? "%s" : "[%s]", backend->ip_str);
> > > +
> > > +const char *s = smap_get(&lb->nlb->ip_port_mappings,
ds_cstr(&key));
> > > +if (!s) {
> > > +continue;
> > > +}
> > > +
> > > +char *svc_mon_src_ip = NULL;
> > > +char *port_name = xstrdup(s);
> > > +char *p = strstr(port_name, ":");
> > > +if (p) {
> > > +*p = 0;
> > > +p++;
> > > +struct sockaddr_storage svc_mon_src_addr;
> > > +if (!inet_parse_address(p, &svc_mon_src_addr)) {
> > > +static struct vlog_rate_limit rl =
> > > +VLOG_RATE_LIMIT_INIT(5, 1);
> > > +VLOG_WARN_RL(&rl, "Invalid svc mon src IP %s", p);
> > > +} else {
> > > +struct ds src_ip_s = DS_EMPTY_INITIALIZER;
> > > +ss_format_address_nobracks(&svc_mon_src_addr,
> > > +&src_ip_s);
> > > +svc_mon_src_ip = ds_steal_cstr(&src_ip_s);
> > > +}
> > > +}
> > > +
> > > +if (svc_mon_src_ip) {
> > > +struct ovn_northd_lb_backend *backend_nb =
> > > +&lb_vip_nb->backends_nb[j];
> > > +backend_nb->health_check = true;
> > > +backend_nb->logical_port = xstrdup(port_name);
> > > +backend_nb->svc_mon_src_ip = svc_mon_src_ip;
> > > +}
> > > +free(port_name);
> > > +}
> > > +
> > > +ds_destroy(&key);
> > > +}
> > > +
> > >   static
> > >   void ovn_northd_lb_vip_destroy(struct ovn_northd_lb_vip *vip)
> > >   {
> > >   free(vip->backend_ips);
> > >   for (size_t i = 0; i < vip->n_backends; i++) {
> > > +free(vip->backends_nb[i].logical_port);
> > >   free(vip->backends_nb[i].svc_mon_src_ip);
> > >   }
> > >   free(vip->backends_nb);
> > > @@ -555,8 +607,7 @@ ovn_lb_get_health_check(const struct
nbrec_load_balancer *nbrec_lb,
> > >   }
> > >
> > >   struct ovn_northd_lb *
> > > -ovn_northd_lb_create(const struct nbrec_load_balancer *nbrec_lb,
> > > - size_t n_ls_datapaths, siz

Re: [ovs-dev] [PATCH ovn v2 2/8] northd: Add a new engine node - northd_lb_data.

2023-07-14 Thread Numan Siddique 
On Thu, Jul 13, 2023 at 7:40 AM Han Zhou  wrote:
>
> On Wed, Jul 12, 2023 at 11:54 PM Han Zhou  wrote:
> >
> >
> >
> > On Sat, Jul 8, 2023 at 3:57 AM Mark Michelson  wrote:
> > >
> > > Hi Numan,
> > >
> > > I have one small nit below.
> >
> > +1
> > Acked-by: Han Zhou 
> >
>
> Sorry that I forgot one small comment regarding the naming of the new node
> (and the related files). I think it is better to be just lb_data instead of
> northd_lb_data.
> The node northd_data was named that way because it was initially the single
> node that contains almost everything in northd. For each I-P node that is
> extracted from the northd_data, the northd_ prefix becomes unnecessary.
> What do you think?

Sure.  Sounds good.  I'll remove the "northd_"  in v3.

Numan

>
> Regards,
> Han
> > >
> > > On 7/7/23 01:53, num...@ovn.org wrote:
> > > > From: Numan Siddique 
> > > >
> > > > This patch separates out the 'lbs' and 'lb_groups' from the 'northd'
> engine
> > > > node data into a new engine node  'northd_lb_data'. This new node
> becomes
> > > > an input to the 'northd' node.
> > > >
> > > > This makes handling the NB load balancer and load balancer group
> changes
> > > > easier.
> > > >
> > > > Signed-off-by: Numan Siddique 
> > > > ---
> > > >   lib/lb.c   | 201 +--
> > > >   lib/lb.h   |  86 +++--
> > > >   northd/automake.mk |   2 +
> > > >   northd/en-lflow.c  |   3 +-
> > > >   northd/en-northd-lb-data.c | 126 +++
> > > >   northd/en-northd-lb-data.h |  19 ++
> > > >   northd/en-northd.c |  11 +-
> > > >   northd/en-sync-sb.c|   2 +-
> > > >   northd/inc-proc-northd.c   |   8 +-
> > > >   northd/northd.c| 673
> +
> > > >   northd/northd.h|  15 +-
> > > >   11 files changed, 780 insertions(+), 366 deletions(-)
> > > >   create mode 100644 northd/en-northd-lb-data.c
> > > >   create mode 100644 northd/en-northd-lb-data.h
> > > >
> > > > diff --git a/lib/lb.c b/lib/lb.c
> > > > index 7afdaed65b..429dbf15af 100644
> > > > --- a/lib/lb.c
> > > > +++ b/lib/lb.c
> > > > @@ -26,6 +26,7 @@
> > > >   #include "openvswitch/vlog.h"
> > > >   #include "lib/bitmap.h"
> > > >   #include "lib/smap.h"
> > > > +#include "socket-util.h"
> > > >
> > > >   VLOG_DEFINE_THIS_MODULE(lb);
> > > >
> > > > @@ -431,11 +432,62 @@ void ovn_northd_lb_vip_init(struct
> ovn_northd_lb_vip *lb_vip_nb,
> > > >   ovn_lb_get_health_check(nbrec_lb, vip_port_str, template);
> > > >   }
> > > >
> > > > +static void
> > > > +ovn_lb_vip_backends_health_check_init(const struct ovn_northd_lb *lb,
> > > > +  const struct ovn_lb_vip
> *lb_vip,
> > > > +  struct ovn_northd_lb_vip
> *lb_vip_nb)
> > > > +{
> > > > +struct ds key = DS_EMPTY_INITIALIZER;
> > > > +
> > > > +for (size_t j = 0; j < lb_vip->n_backends; j++) {
> > > > +struct ovn_lb_backend *backend = &lb_vip->backends[j];
> > > > +ds_clear(&key);
> > > > +ds_put_format(&key, IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)
> > > > +  ? "%s" : "[%s]", backend->ip_str);
> > > > +
> > > > +const char *s = smap_get(&lb->nlb->ip_port_mappings,
> ds_cstr(&key));
> > > > +if (!s) {
> > > > +continue;
> > > > +}
> > > > +
> > > > +char *svc_mon_src_ip = NULL;
> > > > +char *port_name = xstrdup(s);
> > > > +char *p = strstr(port_name, ":");
> > > > +if (p) {
> > > > +*p = 0;
> > > > +p++;
> > > > +struct sockaddr_storage svc_mon_src_addr;
> > > > +if (!inet_parse_address(p, &svc_mon_src_addr)) {
> > > > +static struct vlog_rate_limit rl =
> > > > +VLOG_RATE_LIMIT_INIT(5, 1);
> > > > +VLOG_WARN_RL(&rl, "Invalid svc mon src IP %s", p);
> > > > +} else {
> > > > +struct ds src_ip_s = DS_EMPTY_INITIALIZER;
> > > > +ss_format_address_nobracks(&svc_mon_src_addr,
> > > > +&src_ip_s);
> > > > +svc_mon_src_ip = ds_steal_cstr(&src_ip_s);
> > > > +}
> > > > +}
> > > > +
> > > > +if (svc_mon_src_ip) {
> > > > +struct ovn_northd_lb_backend *backend_nb =
> > > > +&lb_vip_nb->backends_nb[j];
> > > > +backend_nb->health_check = true;
> > > > +backend_nb->logical_port = xstrdup(port_name);
> > > > +backend_nb->svc_mon_src_ip = svc_mon_src_ip;
> > > > +}
> > > > +free(port_name);
> > > > +}
> > > > +
> > > > +ds_destroy(&key);
> > > > +}
> > > > +
> > > >   static
> > > >   void ovn_northd_lb_vip_destroy(struct ovn_northd_lb_vip *vip)
> > > >   {
> > > >   free(vip->backend_ips);
> > > >   for (size_t i = 0; i < vip->n_backends; i++) {
> > > > +free(vip->backends_nb[i].logical_port)

Re: [ovs-dev] [PATCH ovn v2 2/8] northd: Add a new engine node - northd_lb_data.

2023-07-17 Thread Han Zhou 
On Fri, Jul 14, 2023 at 9:12 PM Numan Siddique  wrote:
>
> On Thu, Jul 13, 2023 at 7:40 AM Han Zhou  wrote:
> >
> > On Wed, Jul 12, 2023 at 11:54 PM Han Zhou  wrote:
> > >
> > >
> > >
> > > On Sat, Jul 8, 2023 at 3:57 AM Mark Michelson 
wrote:
> > > >
> > > > Hi Numan,
> > > >
> > > > I have one small nit below.
> > >
> > > +1
> > > Acked-by: Han Zhou 
> > >
> >
> > Sorry that I forgot one small comment regarding the naming of the new
node
> > (and the related files). I think it is better to be just lb_data
instead of
> > northd_lb_data.
> > The node northd_data was named that way because it was initially the
single
> > node that contains almost everything in northd. For each I-P node that
is
> > extracted from the northd_data, the northd_ prefix becomes unnecessary.
> > What do you think?
>
> Sure.  Sounds good.  I'll remove the "northd_"  in v3.
>
> Numan
>
> >
> > Regards,
> > Han
> > > >
> > > > On 7/7/23 01:53, num...@ovn.org wrote:
> > > > > From: Numan Siddique 
> > > > >
> > > > > This patch separates out the 'lbs' and 'lb_groups' from the
'northd'
> > engine
> > > > > node data into a new engine node  'northd_lb_data'. This new node
> > becomes
> > > > > an input to the 'northd' node.
> > > > >
> > > > > This makes handling the NB load balancer and load balancer group
> > changes
> > > > > easier.
> > > > >
> > > > > Signed-off-by: Numan Siddique 
> > > > > ---
> > > > >   lib/lb.c   | 201 +--
> > > > >   lib/lb.h   |  86 +++--
> > > > >   northd/automake.mk |   2 +
> > > > >   northd/en-lflow.c  |   3 +-
> > > > >   northd/en-northd-lb-data.c | 126 +++
> > > > >   northd/en-northd-lb-data.h |  19 ++
> > > > >   northd/en-northd.c |  11 +-
> > > > >   northd/en-sync-sb.c|   2 +-
> > > > >   northd/inc-proc-northd.c   |   8 +-
> > > > >   northd/northd.c| 673
> > +
> > > > >   northd/northd.h|  15 +-
> > > > >   11 files changed, 780 insertions(+), 366 deletions(-)
> > > > >   create mode 100644 northd/en-northd-lb-data.c
> > > > >   create mode 100644 northd/en-northd-lb-data.h
> > > > >
> > > > > diff --git a/lib/lb.c b/lib/lb.c
> > > > > index 7afdaed65b..429dbf15af 100644
> > > > > --- a/lib/lb.c
> > > > > +++ b/lib/lb.c
> > > > > @@ -26,6 +26,7 @@
> > > > >   #include "openvswitch/vlog.h"
> > > > >   #include "lib/bitmap.h"
> > > > >   #include "lib/smap.h"
> > > > > +#include "socket-util.h"
> > > > >
> > > > >   VLOG_DEFINE_THIS_MODULE(lb);
> > > > >
> > > > > @@ -431,11 +432,62 @@ void ovn_northd_lb_vip_init(struct
> > ovn_northd_lb_vip *lb_vip_nb,
> > > > >   ovn_lb_get_health_check(nbrec_lb, vip_port_str,
template);
> > > > >   }
> > > > >
> > > > > +static void
> > > > > +ovn_lb_vip_backends_health_check_init(const struct ovn_northd_lb
*lb,
> > > > > +  const struct ovn_lb_vip
> > *lb_vip,
> > > > > +  struct ovn_northd_lb_vip
> > *lb_vip_nb)
> > > > > +{
> > > > > +struct ds key = DS_EMPTY_INITIALIZER;
> > > > > +
> > > > > +for (size_t j = 0; j < lb_vip->n_backends; j++) {
> > > > > +struct ovn_lb_backend *backend = &lb_vip->backends[j];
> > > > > +ds_clear(&key);
> > > > > +ds_put_format(&key, IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)
> > > > > +  ? "%s" : "[%s]", backend->ip_str);
> > > > > +
> > > > > +const char *s = smap_get(&lb->nlb->ip_port_mappings,
> > ds_cstr(&key));
> > > > > +if (!s) {
> > > > > +continue;
> > > > > +}
> > > > > +
> > > > > +char *svc_mon_src_ip = NULL;
> > > > > +char *port_name = xstrdup(s);
> > > > > +char *p = strstr(port_name, ":");
> > > > > +if (p) {
> > > > > +*p = 0;
> > > > > +p++;
> > > > > +struct sockaddr_storage svc_mon_src_addr;
> > > > > +if (!inet_parse_address(p, &svc_mon_src_addr)) {
> > > > > +static struct vlog_rate_limit rl =
> > > > > +VLOG_RATE_LIMIT_INIT(5, 1);
> > > > > +VLOG_WARN_RL(&rl, "Invalid svc mon src IP %s",
p);
> > > > > +} else {
> > > > > +struct ds src_ip_s = DS_EMPTY_INITIALIZER;
> > > > > +ss_format_address_nobracks(&svc_mon_src_addr,
> > > > > +&src_ip_s);
> > > > > +svc_mon_src_ip = ds_steal_cstr(&src_ip_s);
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +if (svc_mon_src_ip) {
> > > > > +struct ovn_northd_lb_backend *backend_nb =
> > > > > +&lb_vip_nb->backends_nb[j];
> > > > > +backend_nb->health_check = true;
> > > > > +backend_nb->logical_port = xstrdup(port_name);
> > > > > +backend_nb->svc_mon_src_ip = svc_mon_src_ip;
> > > > > +}
> > > > > +free(port_name);
> > > > > +}
> > >