Message ID | 20201027171653.1180789-1-numans@ovn.org |
---|---|
State | Changes Requested |
Headers | show |
Series | Optimize load balancer hairpin logical flows. | expand |
On 10/27/20 6:16 PM, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > Parsing of the load balancer VIPs is moved to a separate file - lib/lb.c. > ovn-northd makes use of these functions. Upcoming patch will make use of these > util functions for parsing SB Load_Balancers. > > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > lib/automake.mk | 4 +- > lib/lb.c | 236 ++++++++++++++++++++++++++++++++++++ > lib/lb.h | 77 ++++++++++++ > lib/ovn-util.c | 28 +++++ > lib/ovn-util.h | 2 + > northd/ovn-northd.c | 286 +++++--------------------------------------- > 6 files changed, 378 insertions(+), 255 deletions(-) > create mode 100644 lib/lb.c > create mode 100644 lib/lb.h > > diff --git a/lib/automake.mk b/lib/automake.mk > index f3e9c8818b..430cd11fc6 100644 > --- a/lib/automake.mk > +++ b/lib/automake.mk > @@ -23,7 +23,9 @@ lib_libovn_la_SOURCES = \ > lib/ovn-util.h \ > lib/logical-fields.c \ > lib/inc-proc-eng.c \ > - lib/inc-proc-eng.h > + lib/inc-proc-eng.h \ > + lib/lb.c \ > + lib/lb.h > nodist_lib_libovn_la_SOURCES = \ > lib/ovn-dirs.c \ > lib/ovn-nb-idl.c \ > diff --git a/lib/lb.c b/lib/lb.c > new file mode 100644 > index 0000000000..db2d3d552f > --- /dev/null > +++ b/lib/lb.c > @@ -0,0 +1,236 @@ > +/* Copyright (c) 2020, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > + > +#include "lb.h" > +#include "lib/ovn-nb-idl.h" > +#include "lib/ovn-sb-idl.h" > +#include "lib/ovn-util.h" > + > +/* OpenvSwitch lib includes. */ > +#include "openvswitch/vlog.h" > +#include "lib/smap.h" > + > +VLOG_DEFINE_THIS_MODULE(lb); > + > +static struct ovn_lb * > +ovn_lb_create(const struct smap *vips) > +{ > + struct ovn_lb *lb = xzalloc(sizeof *lb); > + > + lb->n_vips = smap_count(vips); > + lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); > + struct smap_node *node; > + size_t n_vips = 0; > + > + SMAP_FOR_EACH (node, vips) { > + char *vip; > + uint16_t port; > + int addr_family; > + > + if (!ip_address_and_port_from_key(node->key, &vip, &port, > + &addr_family)) { > + continue; > + } > + > + lb->vips[n_vips].vip = vip; > + lb->vips[n_vips].vip_port = port; > + lb->vips[n_vips].addr_family = addr_family; > + lb->vips[n_vips].vip_port_str = xstrdup(node->key); > + lb->vips[n_vips].backend_ips = xstrdup(node->value); > + > + char *tokstr = xstrdup(node->value); > + char *save_ptr = NULL; > + char *token; > + size_t n_backends = 0; > + /* Format for a backend ips : IP1:port1,IP2:port2,...". */ > + for (token = strtok_r(tokstr, ",", &save_ptr); > + token != NULL; > + token = strtok_r(NULL, ",", &save_ptr)) { > + n_backends++; > + } > + > + free(tokstr); > + tokstr = xstrdup(node->value); > + save_ptr = NULL; > + > + lb->vips[n_vips].n_backends = n_backends; > + lb->vips[n_vips].backends = xcalloc(n_backends, > + sizeof (struct lb_vip_backend)); This should be 'sizeof *lb->vips[n_vips].backends'. > + Nit: It's probably fine to remove an empty line here, the one above is already indented enough to the right. > + size_t i = 0; > + for (token = strtok_r(tokstr, ",", &save_ptr); > + token != NULL; > + token = strtok_r(NULL, ",", &save_ptr)) { > + char *backend_ip; > + uint16_t backend_port; > + > + if (!ip_address_and_port_from_key(token, &backend_ip, > + &backend_port, > + &addr_family)) { > + continue; > + } > + > + lb->vips[n_vips].backends[i].ip = backend_ip; > + lb->vips[n_vips].backends[i].port = backend_port; > + lb->vips[n_vips].backends[i].addr_family = addr_family; > + i++; > + } > + > + free(tokstr); > + n_vips++; > + } > + > + return lb; > +} > + > +struct ovn_lb * > +ovn_nb_lb_create(const struct nbrec_load_balancer *nbrec_lb, > + struct hmap *ports, struct hmap *lbs, > + void * (*ovn_port_find)(const struct hmap *ports, > + const char *name)) > +{ > + struct ovn_lb *lb = ovn_lb_create(&nbrec_lb->vips); > + hmap_insert(lbs, &lb->hmap_node, uuid_hash(&nbrec_lb->header_.uuid)); > + lb->nlb = nbrec_lb; > + lb->nb_lb = true; > + > + for (size_t i = 0; i < lb->n_vips; i++) { > + struct lb_vip *lb_vip = &lb->vips[i]; > + > + struct nbrec_load_balancer_health_check *lb_health_check = NULL; > + if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { > + if (nbrec_lb->n_health_check > 0) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, > + "SCTP load balancers do not currently support " > + "health checks. Not creating health checks for " > + "load balancer " UUID_FMT, > + UUID_ARGS(&nbrec_lb->header_.uuid)); > + } > + } else { > + for (size_t j = 0; j < nbrec_lb->n_health_check; j++) { > + if (!strcmp(nbrec_lb->health_check[j]->vip, > + lb_vip->vip_port_str)) { > + lb_health_check = nbrec_lb->health_check[i]; > + break; > + } > + } > + } > + > + lb_vip->lb_health_check = lb_health_check; > + > + for (size_t j = 0; j < lb_vip->n_backends; j++) { > + struct lb_vip_backend *backend = &lb_vip->backends[j]; > + > + struct ovn_port *op = NULL; > + char *svc_mon_src_ip = NULL; > + const char *s = smap_get(&nbrec_lb->ip_port_mappings, > + backend->ip); > + if (s) { > + char *port_name = xstrdup(s); > + char *p = strstr(port_name, ":"); > + if (p) { > + *p = 0; > + p++; > + op = ovn_port_find(ports, port_name); > + svc_mon_src_ip = xstrdup(p); > + } > + free(port_name); > + } > + > + backend->op = op; > + backend->svc_mon_src_ip = svc_mon_src_ip; > + } > + } > + > + if (nbrec_lb->n_selection_fields) { > + char *proto = NULL; > + if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { > + proto = nbrec_lb->protocol; > + } > + > + struct ds sel_fields = DS_EMPTY_INITIALIZER; > + for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { > + char *field = lb->nlb->selection_fields[i]; > + if (!strcmp(field, "tp_src") && proto) { > + ds_put_format(&sel_fields, "%s_src,", proto); > + } else if (!strcmp(field, "tp_dst") && proto) { > + ds_put_format(&sel_fields, "%s_dst,", proto); > + } else { > + ds_put_format(&sel_fields, "%s,", field); > + } > + } > + ds_chomp(&sel_fields, ','); > + lb->selection_fields = ds_steal_cstr(&sel_fields); > + } > + > + return lb; > +} > + > +struct ovn_lb * > +ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb) > +{ > + struct ovn_lb *lb = ovn_lb_create(&sbrec_lb->vips); > + lb->slb = sbrec_lb; > + lb->nb_lb = false; > + > + return lb; > +} > + > +struct ovn_lb * > +ovn_lb_find(struct hmap *lbs, struct uuid *uuid) > +{ > + struct ovn_lb *lb; > + size_t hash = uuid_hash(uuid); > + HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { > + if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { > + return lb; > + } > + } > + > + return NULL; > +} > + > +void > +ovn_lb_destroy(struct ovn_lb *lb) > +{ > + for (size_t i = 0; i < lb->n_vips; i++) { > + free(lb->vips[i].vip); > + free(lb->vips[i].backend_ips); > + free(lb->vips[i].vip_port_str); > + > + for (size_t j = 0; j < lb->vips[i].n_backends; j++) { > + free(lb->vips[i].backends[j].ip); > + free(lb->vips[i].backends[j].svc_mon_src_ip); > + } > + > + free(lb->vips[i].backends); > + } > + free(lb->vips); > + free(lb->selection_fields); This is not exactly correct if the record was created with ovn_sb_lb_create(). It happens to work but it's actually freeing padding that happens to have been initialized to 0. > +} > + > +void > +ovn_lbs_destroy(struct hmap *lbs) > +{ > + struct ovn_lb *lb; > + HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { > + ovn_lb_destroy(lb); > + free(lb); > + } > + hmap_destroy(lbs); > +} > diff --git a/lib/lb.h b/lib/lb.h > new file mode 100644 > index 0000000000..ffb3ba1fd1 > --- /dev/null > +++ b/lib/lb.h > @@ -0,0 +1,77 @@ > +/* Copyright (c) 2020, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > + > +#ifndef OVN_LIB_LB_H > +#define OVN_LIB_LB_H 1 > + > +#include "openvswitch/hmap.h" > + > +struct nbrec_load_balancer; > +struct sbrec_load_balancer; > +struct ovn_port; > +struct uuid; > + > +struct ovn_lb { > + struct hmap_node hmap_node; > + > + bool nb_lb; /* NB load balancer or SB load balancer. */ > + union { > + struct { > + const struct nbrec_load_balancer *nlb; /* May be NULL. */ > + char *selection_fields; > + }; > + const struct sbrec_load_balancer *slb; /* May be NULL. */ > + }; > + > + struct lb_vip *vips; > + size_t n_vips; > +}; I think it's better to have two different structures for NB/SB load balancers. We never store them together anyway. What about something like: struct ovn_lb { struct hmap_node hmap_node; struct lb_vip *vips; size_t n_vips; }; struct ovn_nb_lb { struct ovn_lb common; const struct nbrec_load_balancer *nlb; char *selection_fields; /* Some more fields that are NB specific. */ }; struct ovn_sb_lb { struct ovn_lb common; const struct sbrec_load_balancer *slb; /* Some more fields that are SB specific. */ }; struct ovn_nb_lb * ovn_nb_lb_from_lb(struct ovn_lb *lb) { return CONTAINER_OF(lb, struct ovn_nb_lb *, common); } struct ovn_sb_lb * ovn_sb_lb_from_lb(struct ovn_lb *lb) { return CONTAINER_OF(lb, struct ovn_sb_lb *, common); } > + > +struct lb_vip { > + char *vip; > + uint16_t vip_port; > + int addr_family; > + char *backend_ips; > + char *vip_port_str; > + > + /* Valid only for NB load balancer. */ This is not really true. In patch 3/7 we use some of the struct lb_vip_backend fields from SB load balancers 'backends'. > + struct nbrec_load_balancer_health_check *lb_health_check; > + struct lb_vip_backend *backends; > + size_t n_backends; If we'd have separate NB/SB structures this would go in the NB-specific part. > +}; > + > +struct lb_vip_backend { > + char *ip; > + uint16_t port; > + int addr_family; > + > + /* Valid only for NB load balancer. */ > + struct ovn_port *op; /* Logical port to which the ip belong to. */ > + bool health_check; > + char *svc_mon_src_ip; /* Source IP to use for monitoring. */ > + const struct sbrec_service_monitor *sbrec_monitor; If we'd have separate NB/SB structures this would go in the NB-specific part. Thanks, Dumitru > +}; > + > +struct ovn_lb *ovn_nb_lb_create( > + const struct nbrec_load_balancer *nbrec_lb, > + struct hmap *ports, struct hmap *lbs, > + void * (*ovn_port_find)(const struct hmap *ports, const char *name)); > +struct ovn_lb *ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb); > +struct ovn_lb * ovn_lb_find(struct hmap *lbs, struct uuid *uuid); > +void ovn_lb_destroy(struct ovn_lb *lb); > +void ovn_lbs_destroy(struct hmap *lbs); > + > +#endif /* OVN_LIB_LB_H 1 */ > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index 1daf665037..02770bd55a 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -24,6 +24,7 @@ > #include "ovn-sb-idl.h" > #include "unixctl.h" > #include <ctype.h> > +#include "socket-util.h" > > VLOG_DEFINE_THIS_MODULE(ovn_util); > > @@ -667,3 +668,30 @@ ovn_smap_get_uint(const struct smap *smap, const char *key, unsigned int def) > > return u_value; > } > + > +/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and > + * 'ip_address'. The caller must free() the memory allocated for > + * 'ip_address'. > + * Returns true if parsing of 'key' was successful, false otherwise. > + */ > +bool > +ip_address_and_port_from_key(const char *key, char **ip_address, > + uint16_t *port, int *addr_family) > +{ > + struct sockaddr_storage ss; > + if (!inet_parse_active(key, 0, &ss, false)) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad ip address or port for key %s", key); > + *ip_address = NULL; > + *port = 0; > + *addr_family = 0; > + return false; > + } > + > + struct ds s = DS_EMPTY_INITIALIZER; > + ss_format_address_nobracks(&ss, &s); > + *ip_address = ds_steal_cstr(&s); > + *port = ss_get_port(&ss); > + *addr_family = ss.ss_family; > + return true; > +} > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index 1d22a691f5..7fe53fd736 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -157,6 +157,8 @@ char *normalize_v46_prefix(const struct v46_ip *prefix, unsigned int plen); > unsigned int ovn_smap_get_uint(const struct smap *smap, const char *key, > unsigned int def); > > +bool ip_address_and_port_from_key(const char *key, char **ip_address, > + uint16_t *port, int *addr_family); > /* Returns a lowercase copy of orig. > * Caller must free the returned string. > */ > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index d11888d203..1da31caf3d 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -35,6 +35,7 @@ > #include "lib/ovn-nb-idl.h" > #include "lib/ovn-sb-idl.h" > #include "lib/ovn-util.h" > +#include "lib/lb.h" > #include "ovn/actions.h" > #include "ovn/logical-fields.h" > #include "packets.h" > @@ -2525,10 +2526,6 @@ join_logical_ports(struct northd_context *ctx, > } > } > > -static bool > -ip_address_and_port_from_lb_key(const char *key, char **ip_address, > - uint16_t *port, int *addr_family); > - > static void > get_router_load_balancer_ips(const struct ovn_datapath *od, > struct sset *all_ips_v4, struct sset *all_ips_v6) > @@ -2548,8 +2545,8 @@ get_router_load_balancer_ips(const struct ovn_datapath *od, > uint16_t port; > int addr_family; > > - if (!ip_address_and_port_from_lb_key(node->key, &ip_address, &port, > - &addr_family)) { > + if (!ip_address_and_port_from_key(node->key, &ip_address, &port, > + &addr_family)) { > continue; > } > > @@ -3309,53 +3306,6 @@ cleanup_sb_ha_chassis_groups(struct northd_context *ctx, > } > } > > -struct ovn_lb { > - struct hmap_node hmap_node; > - > - const struct nbrec_load_balancer *nlb; /* May be NULL. */ > - char *selection_fields; > - struct lb_vip *vips; > - size_t n_vips; > -}; > - > -struct lb_vip { > - char *vip; > - uint16_t vip_port; > - int addr_family; > - char *backend_ips; > - > - bool health_check; > - struct lb_vip_backend *backends; > - size_t n_backends; > -}; > - > -struct lb_vip_backend { > - char *ip; > - uint16_t port; > - int addr_family; > - > - struct ovn_port *op; /* Logical port to which the ip belong to. */ > - bool health_check; > - char *svc_mon_src_ip; /* Source IP to use for monitoring. */ > - const struct sbrec_service_monitor *sbrec_monitor; > -}; > - > - > -static inline struct ovn_lb * > -ovn_lb_find(struct hmap *lbs, struct uuid *uuid) > -{ > - struct ovn_lb *lb; > - size_t hash = uuid_hash(uuid); > - HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { > - if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { > - return lb; > - } > - } > - > - return NULL; > -} > - > - > struct service_monitor_info { > struct hmap_node hmap_node; > const struct sbrec_service_monitor *sbrec_mon; > @@ -3395,126 +3345,36 @@ create_or_get_service_mon(struct northd_context *ctx, > return mon_info; > } > > -static struct ovn_lb * > -ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, > - const struct nbrec_load_balancer *nbrec_lb, > - struct hmap *ports, struct hmap *monitor_map) > +static void > +ovn_lb_svc_create(struct northd_context *ctx, struct ovn_lb *lb, > + struct hmap *monitor_map) > { > - struct ovn_lb *lb = xzalloc(sizeof *lb); > - > - size_t hash = uuid_hash(&nbrec_lb->header_.uuid); > - lb->nlb = nbrec_lb; > - hmap_insert(lbs, &lb->hmap_node, hash); > - > - lb->n_vips = smap_count(&nbrec_lb->vips); > - lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); > - struct smap_node *node; > - size_t n_vips = 0; > - > - SMAP_FOR_EACH (node, &nbrec_lb->vips) { > - char *vip; > - uint16_t port; > - int addr_family; > + for (size_t i = 0; i < lb->n_vips; i++) { > + struct lb_vip *lb_vip = &lb->vips[i]; > > - if (!ip_address_and_port_from_lb_key(node->key, &vip, &port, > - &addr_family)) { > + if (!lb_vip->lb_health_check) { > continue; > } > > - lb->vips[n_vips].vip = vip; > - lb->vips[n_vips].vip_port = port; > - lb->vips[n_vips].addr_family = addr_family; > - lb->vips[n_vips].backend_ips = xstrdup(node->value); > - > - struct nbrec_load_balancer_health_check *lb_health_check = NULL; > - if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { > - if (nbrec_lb->n_health_check > 0) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, > - "SCTP load balancers do not currently support " > - "health checks. Not creating health checks for " > - "load balancer " UUID_FMT, > - UUID_ARGS(&nbrec_lb->header_.uuid)); > - } > - } else { > - for (size_t i = 0; i < nbrec_lb->n_health_check; i++) { > - if (!strcmp(nbrec_lb->health_check[i]->vip, node->key)) { > - lb_health_check = nbrec_lb->health_check[i]; > - break; > - } > - } > - } > - > - char *tokstr = xstrdup(node->value); > - char *save_ptr = NULL; > - char *token; > - size_t n_backends = 0; > - /* Format for a backend ips : IP1:port1,IP2:port2,...". */ > - for (token = strtok_r(tokstr, ",", &save_ptr); > - token != NULL; > - token = strtok_r(NULL, ",", &save_ptr)) { > - n_backends++; > - } > - > - free(tokstr); > - tokstr = xstrdup(node->value); > - save_ptr = NULL; > - > - lb->vips[n_vips].n_backends = n_backends; > - lb->vips[n_vips].backends = xcalloc(n_backends, > - sizeof (struct lb_vip_backend)); > - lb->vips[n_vips].health_check = lb_health_check ? true: false; > - > - size_t i = 0; > - for (token = strtok_r(tokstr, ",", &save_ptr); > - token != NULL; > - token = strtok_r(NULL, ",", &save_ptr)) { > - char *backend_ip; > - uint16_t backend_port; > - > - if (!ip_address_and_port_from_lb_key(token, &backend_ip, > - &backend_port, > - &addr_family)) { > - continue; > - } > + for (size_t j = 0; j < lb_vip->n_backends; j++) { > + struct lb_vip_backend *backend = &lb_vip->backends[j]; > > - /* Get the logical port to which this ip belongs to. */ > - struct ovn_port *op = NULL; > - char *svc_mon_src_ip = NULL; > - const char *s = smap_get(&nbrec_lb->ip_port_mappings, > - backend_ip); > - if (s) { > - char *port_name = xstrdup(s); > - char *p = strstr(port_name, ":"); > - if (p) { > - *p = 0; > - p++; > - op = ovn_port_find(ports, port_name); > - svc_mon_src_ip = xstrdup(p); > - } > - free(port_name); > - } > - > - lb->vips[n_vips].backends[i].ip = backend_ip; > - lb->vips[n_vips].backends[i].port = backend_port; > - lb->vips[n_vips].backends[i].addr_family = addr_family; > - lb->vips[n_vips].backends[i].op = op; > - lb->vips[n_vips].backends[i].svc_mon_src_ip = svc_mon_src_ip; > - > - if (lb_health_check && op && svc_mon_src_ip) { > - const char *protocol = nbrec_lb->protocol; > + if (backend->op && backend->svc_mon_src_ip) { > + backend->health_check = true; > + const char *protocol = lb->nlb->protocol; > if (!protocol || !protocol[0]) { > protocol = "tcp"; > } > - lb->vips[n_vips].backends[i].health_check = true; > + backend->health_check = true; > struct service_monitor_info *mon_info = > - create_or_get_service_mon(ctx, monitor_map, backend_ip, > - op->nbsp->name, backend_port, > + create_or_get_service_mon(ctx, monitor_map, backend->ip, > + backend->op->nbsp->name, > + backend->port, > protocol); > > ovs_assert(mon_info); > sbrec_service_monitor_set_options( > - mon_info->sbrec_mon, &lb_health_check->options); > + mon_info->sbrec_mon, &lb_vip->lb_health_check->options); > struct eth_addr ea; > if (!mon_info->sbrec_mon->src_mac || > !eth_addr_from_string(mon_info->sbrec_mon->src_mac, &ea) || > @@ -3524,72 +3384,24 @@ ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, > } > > if (!mon_info->sbrec_mon->src_ip || > - strcmp(mon_info->sbrec_mon->src_ip, svc_mon_src_ip)) { > + strcmp(mon_info->sbrec_mon->src_ip, > + backend->svc_mon_src_ip)) { > sbrec_service_monitor_set_src_ip(mon_info->sbrec_mon, > - svc_mon_src_ip); > + backend->svc_mon_src_ip); > } > > - lb->vips[n_vips].backends[i].sbrec_monitor = > - mon_info->sbrec_mon; > + lb_vip->backends[j].sbrec_monitor = mon_info->sbrec_mon; > mon_info->required = true; > - } else { > - lb->vips[n_vips].backends[i].health_check = false; > } > - > - i++; > } > - > - free(tokstr); > - n_vips++; > - } > - > - char *proto = NULL; > - if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { > - proto = nbrec_lb->protocol; > } > - > - if (lb->nlb->n_selection_fields) { > - struct ds sel_fields = DS_EMPTY_INITIALIZER; > - for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { > - char *field = lb->nlb->selection_fields[i]; > - if (!strcmp(field, "tp_src") && proto) { > - ds_put_format(&sel_fields, "%s_src,", proto); > - } else if (!strcmp(field, "tp_dst") && proto) { > - ds_put_format(&sel_fields, "%s_dst,", proto); > - } else { > - ds_put_format(&sel_fields, "%s,", field); > - } > - } > - ds_chomp(&sel_fields, ','); > - lb->selection_fields = ds_steal_cstr(&sel_fields); > - } > - > - return lb; > -} > - > -static void > -ovn_lb_destroy(struct ovn_lb *lb) > -{ > - for (size_t i = 0; i < lb->n_vips; i++) { > - free(lb->vips[i].vip); > - free(lb->vips[i].backend_ips); > - > - for (size_t j = 0; j < lb->vips[i].n_backends; j++) { > - free(lb->vips[i].backends[j].ip); > - free(lb->vips[i].backends[j].svc_mon_src_ip); > - } > - > - free(lb->vips[i].backends); > - } > - free(lb->vips); > - free(lb->selection_fields); > } > > static void build_lb_vip_ct_lb_actions(struct lb_vip *lb_vip, > struct ds *action, > char *selection_fields) > { > - if (lb_vip->health_check) { > + if (lb_vip->lb_health_check) { > ds_put_cstr(action, "ct_lb(backends="); > > size_t n_active_backends = 0; > @@ -3644,7 +3456,12 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, > > const struct nbrec_load_balancer *nbrec_lb; > NBREC_LOAD_BALANCER_FOR_EACH (nbrec_lb, ctx->ovnnb_idl) { > - ovn_lb_create(ctx, lbs, nbrec_lb, ports, &monitor_map); > + ovn_nb_lb_create(nbrec_lb, ports, lbs, (void *)ovn_port_find); > + } > + > + struct ovn_lb *lb; > + HMAP_FOR_EACH (lb, hmap_node, lbs) { > + ovn_lb_svc_create(ctx, lb, &monitor_map); > } > > struct service_monitor_info *mon_info; > @@ -3658,16 +3475,6 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, > hmap_destroy(&monitor_map); > } > > -static void > -destroy_ovn_lbs(struct hmap *lbs) > -{ > - struct ovn_lb *lb; > - HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { > - ovn_lb_destroy(lb); > - free(lb); > - } > -} > - > /* Updates the southbound Port_Binding table so that it contains the logical > * switch ports specified by the northbound database. > * > @@ -5030,34 +4837,6 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) > } > } > > -/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and > - * 'ip_address'. The caller must free() the memory allocated for > - * 'ip_address'. > - * Returns true if parsing of 'key' was successful, false otherwise. > - */ > -static bool > -ip_address_and_port_from_lb_key(const char *key, char **ip_address, > - uint16_t *port, int *addr_family) > -{ > - struct sockaddr_storage ss; > - if (!inet_parse_active(key, 0, &ss, false)) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s", > - key); > - *ip_address = NULL; > - *port = 0; > - *addr_family = 0; > - return false; > - } > - > - struct ds s = DS_EMPTY_INITIALIZER; > - ss_format_address_nobracks(&ss, &s); > - *ip_address = ds_steal_cstr(&s); > - *port = ss_get_port(&ss); > - *addr_family = ss.ss_family; > - return true; > -} > - > /* > * Returns true if logical switch is configured with DNS records, false > * otherwise. > @@ -7004,7 +6783,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, > struct ovn_lb *lb; > HMAP_FOR_EACH (lb, hmap_node, lbs) { > for (size_t i = 0; i < lb->n_vips; i++) { > - if (!lb->vips[i].health_check) { > + if (!lb->vips[i].lb_health_check) { > continue; > } > > @@ -12358,8 +12137,7 @@ ovnnb_db_run(struct northd_context *ctx, > sync_meters(ctx); > sync_dns_entries(ctx, datapaths); > sync_lb_entries(ctx, datapaths); > - destroy_ovn_lbs(&lbs); > - hmap_destroy(&lbs); > + ovn_lbs_destroy(&lbs); > > struct ovn_igmp_group *igmp_group, *next_igmp_group; > >
On Fri, Oct 30, 2020 at 1:52 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 10/27/20 6:16 PM, numans@ovn.org wrote: > > From: Numan Siddique <numans@ovn.org> > > > > Parsing of the load balancer VIPs is moved to a separate file - lib/lb.c. > > ovn-northd makes use of these functions. Upcoming patch will make use of these > > util functions for parsing SB Load_Balancers. > > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > lib/automake.mk | 4 +- > > lib/lb.c | 236 ++++++++++++++++++++++++++++++++++++ > > lib/lb.h | 77 ++++++++++++ > > lib/ovn-util.c | 28 +++++ > > lib/ovn-util.h | 2 + > > northd/ovn-northd.c | 286 +++++--------------------------------------- > > 6 files changed, 378 insertions(+), 255 deletions(-) > > create mode 100644 lib/lb.c > > create mode 100644 lib/lb.h > > > > diff --git a/lib/automake.mk b/lib/automake.mk > > index f3e9c8818b..430cd11fc6 100644 > > --- a/lib/automake.mk > > +++ b/lib/automake.mk > > @@ -23,7 +23,9 @@ lib_libovn_la_SOURCES = \ > > lib/ovn-util.h \ > > lib/logical-fields.c \ > > lib/inc-proc-eng.c \ > > - lib/inc-proc-eng.h > > + lib/inc-proc-eng.h \ > > + lib/lb.c \ > > + lib/lb.h > > nodist_lib_libovn_la_SOURCES = \ > > lib/ovn-dirs.c \ > > lib/ovn-nb-idl.c \ > > diff --git a/lib/lb.c b/lib/lb.c > > new file mode 100644 > > index 0000000000..db2d3d552f > > --- /dev/null > > +++ b/lib/lb.c > > @@ -0,0 +1,236 @@ > > +/* Copyright (c) 2020, Red Hat, Inc. > > + * > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > + * you may not use this file except in compliance with the License. > > + * You may obtain a copy of the License at: > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, software > > + * distributed under the License is distributed on an "AS IS" BASIS, > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > > + * See the License for the specific language governing permissions and > > + * limitations under the License. > > + */ > > + > > +#include <config.h> > > + > > +#include "lb.h" > > +#include "lib/ovn-nb-idl.h" > > +#include "lib/ovn-sb-idl.h" > > +#include "lib/ovn-util.h" > > + > > +/* OpenvSwitch lib includes. */ > > +#include "openvswitch/vlog.h" > > +#include "lib/smap.h" > > + > > +VLOG_DEFINE_THIS_MODULE(lb); > > + > > +static struct ovn_lb * > > +ovn_lb_create(const struct smap *vips) > > +{ > > + struct ovn_lb *lb = xzalloc(sizeof *lb); > > + > > + lb->n_vips = smap_count(vips); > > + lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); > > + struct smap_node *node; > > + size_t n_vips = 0; > > + > > + SMAP_FOR_EACH (node, vips) { > > + char *vip; > > + uint16_t port; > > + int addr_family; > > + > > + if (!ip_address_and_port_from_key(node->key, &vip, &port, > > + &addr_family)) { > > + continue; > > + } > > + > > + lb->vips[n_vips].vip = vip; > > + lb->vips[n_vips].vip_port = port; > > + lb->vips[n_vips].addr_family = addr_family; > > + lb->vips[n_vips].vip_port_str = xstrdup(node->key); > > + lb->vips[n_vips].backend_ips = xstrdup(node->value); > > + > > + char *tokstr = xstrdup(node->value); > > + char *save_ptr = NULL; > > + char *token; > > + size_t n_backends = 0; > > + /* Format for a backend ips : IP1:port1,IP2:port2,...". */ > > + for (token = strtok_r(tokstr, ",", &save_ptr); > > + token != NULL; > > + token = strtok_r(NULL, ",", &save_ptr)) { > > + n_backends++; > > + } > > + > > + free(tokstr); > > + tokstr = xstrdup(node->value); > > + save_ptr = NULL; > > + > > + lb->vips[n_vips].n_backends = n_backends; > > + lb->vips[n_vips].backends = xcalloc(n_backends, > > + sizeof (struct lb_vip_backend)); > > This should be 'sizeof *lb->vips[n_vips].backends'. This part of the code is moved from ovn-northd.c. We could use both - an expression or a structure in sizeof. The coding guidelines give preference to the former. > > > + > > Nit: It's probably fine to remove an empty line here, the one above is already > indented enough to the right. > > > + size_t i = 0; > > + for (token = strtok_r(tokstr, ",", &save_ptr); > > + token != NULL; > > + token = strtok_r(NULL, ",", &save_ptr)) { > > + char *backend_ip; > > + uint16_t backend_port; > > + > > + if (!ip_address_and_port_from_key(token, &backend_ip, > > + &backend_port, > > + &addr_family)) { > > + continue; > > + } > > + > > + lb->vips[n_vips].backends[i].ip = backend_ip; > > + lb->vips[n_vips].backends[i].port = backend_port; > > + lb->vips[n_vips].backends[i].addr_family = addr_family; > > + i++; > > + } > > + > > + free(tokstr); > > + n_vips++; > > + } > > + > > + return lb; > > +} > > + > > +struct ovn_lb * > > +ovn_nb_lb_create(const struct nbrec_load_balancer *nbrec_lb, > > + struct hmap *ports, struct hmap *lbs, > > + void * (*ovn_port_find)(const struct hmap *ports, > > + const char *name)) > > +{ > > + struct ovn_lb *lb = ovn_lb_create(&nbrec_lb->vips); > > + hmap_insert(lbs, &lb->hmap_node, uuid_hash(&nbrec_lb->header_.uuid)); > > + lb->nlb = nbrec_lb; > > + lb->nb_lb = true; > > + > > + for (size_t i = 0; i < lb->n_vips; i++) { > > + struct lb_vip *lb_vip = &lb->vips[i]; > > + > > + struct nbrec_load_balancer_health_check *lb_health_check = NULL; > > + if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { > > + if (nbrec_lb->n_health_check > 0) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > + VLOG_WARN_RL(&rl, > > + "SCTP load balancers do not currently support " > > + "health checks. Not creating health checks for " > > + "load balancer " UUID_FMT, > > + UUID_ARGS(&nbrec_lb->header_.uuid)); > > + } > > + } else { > > + for (size_t j = 0; j < nbrec_lb->n_health_check; j++) { > > + if (!strcmp(nbrec_lb->health_check[j]->vip, > > + lb_vip->vip_port_str)) { > > + lb_health_check = nbrec_lb->health_check[i]; > > + break; > > + } > > + } > > + } > > + > > + lb_vip->lb_health_check = lb_health_check; > > + > > + for (size_t j = 0; j < lb_vip->n_backends; j++) { > > + struct lb_vip_backend *backend = &lb_vip->backends[j]; > > + > > + struct ovn_port *op = NULL; > > + char *svc_mon_src_ip = NULL; > > + const char *s = smap_get(&nbrec_lb->ip_port_mappings, > > + backend->ip); > > + if (s) { > > + char *port_name = xstrdup(s); > > + char *p = strstr(port_name, ":"); > > + if (p) { > > + *p = 0; > > + p++; > > + op = ovn_port_find(ports, port_name); > > + svc_mon_src_ip = xstrdup(p); > > + } > > + free(port_name); > > + } > > + > > + backend->op = op; > > + backend->svc_mon_src_ip = svc_mon_src_ip; > > + } > > + } > > + > > + if (nbrec_lb->n_selection_fields) { > > + char *proto = NULL; > > + if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { > > + proto = nbrec_lb->protocol; > > + } > > + > > + struct ds sel_fields = DS_EMPTY_INITIALIZER; > > + for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { > > + char *field = lb->nlb->selection_fields[i]; > > + if (!strcmp(field, "tp_src") && proto) { > > + ds_put_format(&sel_fields, "%s_src,", proto); > > + } else if (!strcmp(field, "tp_dst") && proto) { > > + ds_put_format(&sel_fields, "%s_dst,", proto); > > + } else { > > + ds_put_format(&sel_fields, "%s,", field); > > + } > > + } > > + ds_chomp(&sel_fields, ','); > > + lb->selection_fields = ds_steal_cstr(&sel_fields); > > + } > > + > > + return lb; > > +} > > + > > +struct ovn_lb * > > +ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb) > > +{ > > + struct ovn_lb *lb = ovn_lb_create(&sbrec_lb->vips); > > + lb->slb = sbrec_lb; > > + lb->nb_lb = false; > > + > > + return lb; > > +} > > + > > +struct ovn_lb * > > +ovn_lb_find(struct hmap *lbs, struct uuid *uuid) > > +{ > > + struct ovn_lb *lb; > > + size_t hash = uuid_hash(uuid); > > + HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { > > + if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { > > + return lb; > > + } > > + } > > + > > + return NULL; > > +} > > + > > +void > > +ovn_lb_destroy(struct ovn_lb *lb) > > +{ > > + for (size_t i = 0; i < lb->n_vips; i++) { > > + free(lb->vips[i].vip); > > + free(lb->vips[i].backend_ips); > > + free(lb->vips[i].vip_port_str); > > + > > + for (size_t j = 0; j < lb->vips[i].n_backends; j++) { > > + free(lb->vips[i].backends[j].ip); > > + free(lb->vips[i].backends[j].svc_mon_src_ip); > > + } > > + > > + free(lb->vips[i].backends); > > + } > > + free(lb->vips); > > + free(lb->selection_fields); > > This is not exactly correct if the record was created with ovn_sb_lb_create(). > It happens to work but it's actually freeing padding that happens to have been > initialized to 0. selection_fields is of type char *, which is set to 0 during allocation. And calling free with a NULL pointer is fine right ? > > > +} > > + > > +void > > +ovn_lbs_destroy(struct hmap *lbs) > > +{ > > + struct ovn_lb *lb; > > + HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { > > + ovn_lb_destroy(lb); > > + free(lb); > > + } > > + hmap_destroy(lbs); > > +} > > diff --git a/lib/lb.h b/lib/lb.h > > new file mode 100644 > > index 0000000000..ffb3ba1fd1 > > --- /dev/null > > +++ b/lib/lb.h > > @@ -0,0 +1,77 @@ > > +/* Copyright (c) 2020, Red Hat, Inc. > > + * > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > + * you may not use this file except in compliance with the License. > > + * You may obtain a copy of the License at: > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, software > > + * distributed under the License is distributed on an "AS IS" BASIS, > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > > + * See the License for the specific language governing permissions and > > + * limitations under the License. > > + */ > > + > > + > > +#ifndef OVN_LIB_LB_H > > +#define OVN_LIB_LB_H 1 > > + > > +#include "openvswitch/hmap.h" > > + > > +struct nbrec_load_balancer; > > +struct sbrec_load_balancer; > > +struct ovn_port; > > +struct uuid; > > + > > +struct ovn_lb { > > + struct hmap_node hmap_node; > > + > > + bool nb_lb; /* NB load balancer or SB load balancer. */ > > + union { > > + struct { > > + const struct nbrec_load_balancer *nlb; /* May be NULL. */ > > + char *selection_fields; > > + }; > > + const struct sbrec_load_balancer *slb; /* May be NULL. */ > > + }; > > + > > + struct lb_vip *vips; > > + size_t n_vips; > > +}; > > I think it's better to have two different structures for NB/SB load balancers. > We never store them together anyway. What about something like: I thought about this, but I think it gets complicated. Like every backend in NB DB can have a health check related params. Now how do we represent that in struct ovn_nb_lb ? If we want to have separate structures, we should have separate lb_vip and lb_backend structures for NB and SB and a separate parser function too. I'm in favor of having proposed structures in this patch. > > struct ovn_lb { > struct hmap_node hmap_node; > struct lb_vip *vips; > size_t n_vips; > }; > > struct ovn_nb_lb { > struct ovn_lb common; > const struct nbrec_load_balancer *nlb; > char *selection_fields; > /* Some more fields that are NB specific. */ > }; > > struct ovn_sb_lb { > struct ovn_lb common; > const struct sbrec_load_balancer *slb; > /* Some more fields that are SB specific. */ > }; > > struct ovn_nb_lb * > ovn_nb_lb_from_lb(struct ovn_lb *lb) > { > return CONTAINER_OF(lb, struct ovn_nb_lb *, common); > } > > struct ovn_sb_lb * > ovn_sb_lb_from_lb(struct ovn_lb *lb) > { > return CONTAINER_OF(lb, struct ovn_sb_lb *, common); > } > > > + > > +struct lb_vip { > > + char *vip; > > + uint16_t vip_port; > > + int addr_family; > > + char *backend_ips; > > + char *vip_port_str; > > + > > + /* Valid only for NB load balancer. */ > > This is not really true. In patch 3/7 we use some of the struct lb_vip_backend > fields from SB load balancers 'backends'. > > > + struct nbrec_load_balancer_health_check *lb_health_check; > > + struct lb_vip_backend *backends; > > + size_t n_backends; > > If we'd have separate NB/SB structures this would go in the NB-specific part. Do you have suggestions on how to define the NB/SB structure for this ? > > > +}; > > + > > +struct lb_vip_backend { > > + char *ip; > > + uint16_t port; > > + int addr_family; > > + > > + /* Valid only for NB load balancer. */ > > + struct ovn_port *op; /* Logical port to which the ip belong to. */ > > + bool health_check; > > + char *svc_mon_src_ip; /* Source IP to use for monitoring. */ > > + const struct sbrec_service_monitor *sbrec_monitor; > > If we'd have separate NB/SB structures this would go in the NB-specific part. Do you have suggestions on how to define the NB/SB structure for this ? Thanks Numan > > Thanks, > Dumitru > > > +}; > > + > > +struct ovn_lb *ovn_nb_lb_create( > > + const struct nbrec_load_balancer *nbrec_lb, > > + struct hmap *ports, struct hmap *lbs, > > + void * (*ovn_port_find)(const struct hmap *ports, const char *name)); > > +struct ovn_lb *ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb); > > +struct ovn_lb * ovn_lb_find(struct hmap *lbs, struct uuid *uuid); > > +void ovn_lb_destroy(struct ovn_lb *lb); > > +void ovn_lbs_destroy(struct hmap *lbs); > > + > > +#endif /* OVN_LIB_LB_H 1 */ > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > index 1daf665037..02770bd55a 100644 > > --- a/lib/ovn-util.c > > +++ b/lib/ovn-util.c > > @@ -24,6 +24,7 @@ > > #include "ovn-sb-idl.h" > > #include "unixctl.h" > > #include <ctype.h> > > +#include "socket-util.h" > > > > VLOG_DEFINE_THIS_MODULE(ovn_util); > > > > @@ -667,3 +668,30 @@ ovn_smap_get_uint(const struct smap *smap, const char *key, unsigned int def) > > > > return u_value; > > } > > + > > +/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and > > + * 'ip_address'. The caller must free() the memory allocated for > > + * 'ip_address'. > > + * Returns true if parsing of 'key' was successful, false otherwise. > > + */ > > +bool > > +ip_address_and_port_from_key(const char *key, char **ip_address, > > + uint16_t *port, int *addr_family) > > +{ > > + struct sockaddr_storage ss; > > + if (!inet_parse_active(key, 0, &ss, false)) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > + VLOG_WARN_RL(&rl, "bad ip address or port for key %s", key); > > + *ip_address = NULL; > > + *port = 0; > > + *addr_family = 0; > > + return false; > > + } > > + > > + struct ds s = DS_EMPTY_INITIALIZER; > > + ss_format_address_nobracks(&ss, &s); > > + *ip_address = ds_steal_cstr(&s); > > + *port = ss_get_port(&ss); > > + *addr_family = ss.ss_family; > > + return true; > > +} > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > > index 1d22a691f5..7fe53fd736 100644 > > --- a/lib/ovn-util.h > > +++ b/lib/ovn-util.h > > @@ -157,6 +157,8 @@ char *normalize_v46_prefix(const struct v46_ip *prefix, unsigned int plen); > > unsigned int ovn_smap_get_uint(const struct smap *smap, const char *key, > > unsigned int def); > > > > +bool ip_address_and_port_from_key(const char *key, char **ip_address, > > + uint16_t *port, int *addr_family); > > /* Returns a lowercase copy of orig. > > * Caller must free the returned string. > > */ > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index d11888d203..1da31caf3d 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -35,6 +35,7 @@ > > #include "lib/ovn-nb-idl.h" > > #include "lib/ovn-sb-idl.h" > > #include "lib/ovn-util.h" > > +#include "lib/lb.h" > > #include "ovn/actions.h" > > #include "ovn/logical-fields.h" > > #include "packets.h" > > @@ -2525,10 +2526,6 @@ join_logical_ports(struct northd_context *ctx, > > } > > } > > > > -static bool > > -ip_address_and_port_from_lb_key(const char *key, char **ip_address, > > - uint16_t *port, int *addr_family); > > - > > static void > > get_router_load_balancer_ips(const struct ovn_datapath *od, > > struct sset *all_ips_v4, struct sset *all_ips_v6) > > @@ -2548,8 +2545,8 @@ get_router_load_balancer_ips(const struct ovn_datapath *od, > > uint16_t port; > > int addr_family; > > > > - if (!ip_address_and_port_from_lb_key(node->key, &ip_address, &port, > > - &addr_family)) { > > + if (!ip_address_and_port_from_key(node->key, &ip_address, &port, > > + &addr_family)) { > > continue; > > } > > > > @@ -3309,53 +3306,6 @@ cleanup_sb_ha_chassis_groups(struct northd_context *ctx, > > } > > } > > > > -struct ovn_lb { > > - struct hmap_node hmap_node; > > - > > - const struct nbrec_load_balancer *nlb; /* May be NULL. */ > > - char *selection_fields; > > - struct lb_vip *vips; > > - size_t n_vips; > > -}; > > - > > -struct lb_vip { > > - char *vip; > > - uint16_t vip_port; > > - int addr_family; > > - char *backend_ips; > > - > > - bool health_check; > > - struct lb_vip_backend *backends; > > - size_t n_backends; > > -}; > > - > > -struct lb_vip_backend { > > - char *ip; > > - uint16_t port; > > - int addr_family; > > - > > - struct ovn_port *op; /* Logical port to which the ip belong to. */ > > - bool health_check; > > - char *svc_mon_src_ip; /* Source IP to use for monitoring. */ > > - const struct sbrec_service_monitor *sbrec_monitor; > > -}; > > - > > - > > -static inline struct ovn_lb * > > -ovn_lb_find(struct hmap *lbs, struct uuid *uuid) > > -{ > > - struct ovn_lb *lb; > > - size_t hash = uuid_hash(uuid); > > - HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { > > - if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { > > - return lb; > > - } > > - } > > - > > - return NULL; > > -} > > - > > - > > struct service_monitor_info { > > struct hmap_node hmap_node; > > const struct sbrec_service_monitor *sbrec_mon; > > @@ -3395,126 +3345,36 @@ create_or_get_service_mon(struct northd_context *ctx, > > return mon_info; > > } > > > > -static struct ovn_lb * > > -ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, > > - const struct nbrec_load_balancer *nbrec_lb, > > - struct hmap *ports, struct hmap *monitor_map) > > +static void > > +ovn_lb_svc_create(struct northd_context *ctx, struct ovn_lb *lb, > > + struct hmap *monitor_map) > > { > > - struct ovn_lb *lb = xzalloc(sizeof *lb); > > - > > - size_t hash = uuid_hash(&nbrec_lb->header_.uuid); > > - lb->nlb = nbrec_lb; > > - hmap_insert(lbs, &lb->hmap_node, hash); > > - > > - lb->n_vips = smap_count(&nbrec_lb->vips); > > - lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); > > - struct smap_node *node; > > - size_t n_vips = 0; > > - > > - SMAP_FOR_EACH (node, &nbrec_lb->vips) { > > - char *vip; > > - uint16_t port; > > - int addr_family; > > + for (size_t i = 0; i < lb->n_vips; i++) { > > + struct lb_vip *lb_vip = &lb->vips[i]; > > > > - if (!ip_address_and_port_from_lb_key(node->key, &vip, &port, > > - &addr_family)) { > > + if (!lb_vip->lb_health_check) { > > continue; > > } > > > > - lb->vips[n_vips].vip = vip; > > - lb->vips[n_vips].vip_port = port; > > - lb->vips[n_vips].addr_family = addr_family; > > - lb->vips[n_vips].backend_ips = xstrdup(node->value); > > - > > - struct nbrec_load_balancer_health_check *lb_health_check = NULL; > > - if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { > > - if (nbrec_lb->n_health_check > 0) { > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > - VLOG_WARN_RL(&rl, > > - "SCTP load balancers do not currently support " > > - "health checks. Not creating health checks for " > > - "load balancer " UUID_FMT, > > - UUID_ARGS(&nbrec_lb->header_.uuid)); > > - } > > - } else { > > - for (size_t i = 0; i < nbrec_lb->n_health_check; i++) { > > - if (!strcmp(nbrec_lb->health_check[i]->vip, node->key)) { > > - lb_health_check = nbrec_lb->health_check[i]; > > - break; > > - } > > - } > > - } > > - > > - char *tokstr = xstrdup(node->value); > > - char *save_ptr = NULL; > > - char *token; > > - size_t n_backends = 0; > > - /* Format for a backend ips : IP1:port1,IP2:port2,...". */ > > - for (token = strtok_r(tokstr, ",", &save_ptr); > > - token != NULL; > > - token = strtok_r(NULL, ",", &save_ptr)) { > > - n_backends++; > > - } > > - > > - free(tokstr); > > - tokstr = xstrdup(node->value); > > - save_ptr = NULL; > > - > > - lb->vips[n_vips].n_backends = n_backends; > > - lb->vips[n_vips].backends = xcalloc(n_backends, > > - sizeof (struct lb_vip_backend)); > > - lb->vips[n_vips].health_check = lb_health_check ? true: false; > > - > > - size_t i = 0; > > - for (token = strtok_r(tokstr, ",", &save_ptr); > > - token != NULL; > > - token = strtok_r(NULL, ",", &save_ptr)) { > > - char *backend_ip; > > - uint16_t backend_port; > > - > > - if (!ip_address_and_port_from_lb_key(token, &backend_ip, > > - &backend_port, > > - &addr_family)) { > > - continue; > > - } > > + for (size_t j = 0; j < lb_vip->n_backends; j++) { > > + struct lb_vip_backend *backend = &lb_vip->backends[j]; > > > > - /* Get the logical port to which this ip belongs to. */ > > - struct ovn_port *op = NULL; > > - char *svc_mon_src_ip = NULL; > > - const char *s = smap_get(&nbrec_lb->ip_port_mappings, > > - backend_ip); > > - if (s) { > > - char *port_name = xstrdup(s); > > - char *p = strstr(port_name, ":"); > > - if (p) { > > - *p = 0; > > - p++; > > - op = ovn_port_find(ports, port_name); > > - svc_mon_src_ip = xstrdup(p); > > - } > > - free(port_name); > > - } > > - > > - lb->vips[n_vips].backends[i].ip = backend_ip; > > - lb->vips[n_vips].backends[i].port = backend_port; > > - lb->vips[n_vips].backends[i].addr_family = addr_family; > > - lb->vips[n_vips].backends[i].op = op; > > - lb->vips[n_vips].backends[i].svc_mon_src_ip = svc_mon_src_ip; > > - > > - if (lb_health_check && op && svc_mon_src_ip) { > > - const char *protocol = nbrec_lb->protocol; > > + if (backend->op && backend->svc_mon_src_ip) { > > + backend->health_check = true; > > + const char *protocol = lb->nlb->protocol; > > if (!protocol || !protocol[0]) { > > protocol = "tcp"; > > } > > - lb->vips[n_vips].backends[i].health_check = true; > > + backend->health_check = true; > > struct service_monitor_info *mon_info = > > - create_or_get_service_mon(ctx, monitor_map, backend_ip, > > - op->nbsp->name, backend_port, > > + create_or_get_service_mon(ctx, monitor_map, backend->ip, > > + backend->op->nbsp->name, > > + backend->port, > > protocol); > > > > ovs_assert(mon_info); > > sbrec_service_monitor_set_options( > > - mon_info->sbrec_mon, &lb_health_check->options); > > + mon_info->sbrec_mon, &lb_vip->lb_health_check->options); > > struct eth_addr ea; > > if (!mon_info->sbrec_mon->src_mac || > > !eth_addr_from_string(mon_info->sbrec_mon->src_mac, &ea) || > > @@ -3524,72 +3384,24 @@ ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, > > } > > > > if (!mon_info->sbrec_mon->src_ip || > > - strcmp(mon_info->sbrec_mon->src_ip, svc_mon_src_ip)) { > > + strcmp(mon_info->sbrec_mon->src_ip, > > + backend->svc_mon_src_ip)) { > > sbrec_service_monitor_set_src_ip(mon_info->sbrec_mon, > > - svc_mon_src_ip); > > + backend->svc_mon_src_ip); > > } > > > > - lb->vips[n_vips].backends[i].sbrec_monitor = > > - mon_info->sbrec_mon; > > + lb_vip->backends[j].sbrec_monitor = mon_info->sbrec_mon; > > mon_info->required = true; > > - } else { > > - lb->vips[n_vips].backends[i].health_check = false; > > } > > - > > - i++; > > } > > - > > - free(tokstr); > > - n_vips++; > > - } > > - > > - char *proto = NULL; > > - if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { > > - proto = nbrec_lb->protocol; > > } > > - > > - if (lb->nlb->n_selection_fields) { > > - struct ds sel_fields = DS_EMPTY_INITIALIZER; > > - for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { > > - char *field = lb->nlb->selection_fields[i]; > > - if (!strcmp(field, "tp_src") && proto) { > > - ds_put_format(&sel_fields, "%s_src,", proto); > > - } else if (!strcmp(field, "tp_dst") && proto) { > > - ds_put_format(&sel_fields, "%s_dst,", proto); > > - } else { > > - ds_put_format(&sel_fields, "%s,", field); > > - } > > - } > > - ds_chomp(&sel_fields, ','); > > - lb->selection_fields = ds_steal_cstr(&sel_fields); > > - } > > - > > - return lb; > > -} > > - > > -static void > > -ovn_lb_destroy(struct ovn_lb *lb) > > -{ > > - for (size_t i = 0; i < lb->n_vips; i++) { > > - free(lb->vips[i].vip); > > - free(lb->vips[i].backend_ips); > > - > > - for (size_t j = 0; j < lb->vips[i].n_backends; j++) { > > - free(lb->vips[i].backends[j].ip); > > - free(lb->vips[i].backends[j].svc_mon_src_ip); > > - } > > - > > - free(lb->vips[i].backends); > > - } > > - free(lb->vips); > > - free(lb->selection_fields); > > } > > > > static void build_lb_vip_ct_lb_actions(struct lb_vip *lb_vip, > > struct ds *action, > > char *selection_fields) > > { > > - if (lb_vip->health_check) { > > + if (lb_vip->lb_health_check) { > > ds_put_cstr(action, "ct_lb(backends="); > > > > size_t n_active_backends = 0; > > @@ -3644,7 +3456,12 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, > > > > const struct nbrec_load_balancer *nbrec_lb; > > NBREC_LOAD_BALANCER_FOR_EACH (nbrec_lb, ctx->ovnnb_idl) { > > - ovn_lb_create(ctx, lbs, nbrec_lb, ports, &monitor_map); > > + ovn_nb_lb_create(nbrec_lb, ports, lbs, (void *)ovn_port_find); > > + } > > + > > + struct ovn_lb *lb; > > + HMAP_FOR_EACH (lb, hmap_node, lbs) { > > + ovn_lb_svc_create(ctx, lb, &monitor_map); > > } > > > > struct service_monitor_info *mon_info; > > @@ -3658,16 +3475,6 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, > > hmap_destroy(&monitor_map); > > } > > > > -static void > > -destroy_ovn_lbs(struct hmap *lbs) > > -{ > > - struct ovn_lb *lb; > > - HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { > > - ovn_lb_destroy(lb); > > - free(lb); > > - } > > -} > > - > > /* Updates the southbound Port_Binding table so that it contains the logical > > * switch ports specified by the northbound database. > > * > > @@ -5030,34 +4837,6 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) > > } > > } > > > > -/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and > > - * 'ip_address'. The caller must free() the memory allocated for > > - * 'ip_address'. > > - * Returns true if parsing of 'key' was successful, false otherwise. > > - */ > > -static bool > > -ip_address_and_port_from_lb_key(const char *key, char **ip_address, > > - uint16_t *port, int *addr_family) > > -{ > > - struct sockaddr_storage ss; > > - if (!inet_parse_active(key, 0, &ss, false)) { > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > - VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s", > > - key); > > - *ip_address = NULL; > > - *port = 0; > > - *addr_family = 0; > > - return false; > > - } > > - > > - struct ds s = DS_EMPTY_INITIALIZER; > > - ss_format_address_nobracks(&ss, &s); > > - *ip_address = ds_steal_cstr(&s); > > - *port = ss_get_port(&ss); > > - *addr_family = ss.ss_family; > > - return true; > > -} > > - > > /* > > * Returns true if logical switch is configured with DNS records, false > > * otherwise. > > @@ -7004,7 +6783,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, > > struct ovn_lb *lb; > > HMAP_FOR_EACH (lb, hmap_node, lbs) { > > for (size_t i = 0; i < lb->n_vips; i++) { > > - if (!lb->vips[i].health_check) { > > + if (!lb->vips[i].lb_health_check) { > > continue; > > } > > > > @@ -12358,8 +12137,7 @@ ovnnb_db_run(struct northd_context *ctx, > > sync_meters(ctx); > > sync_dns_entries(ctx, datapaths); > > sync_lb_entries(ctx, datapaths); > > - destroy_ovn_lbs(&lbs); > > - hmap_destroy(&lbs); > > + ovn_lbs_destroy(&lbs); > > > > struct ovn_igmp_group *igmp_group, *next_igmp_group; > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, Nov 3, 2020 at 3:14 PM Numan Siddique <numans@ovn.org> wrote: > > On Fri, Oct 30, 2020 at 1:52 AM Dumitru Ceara <dceara@redhat.com> wrote: > > > > On 10/27/20 6:16 PM, numans@ovn.org wrote: > > > From: Numan Siddique <numans@ovn.org> > > > > > > Parsing of the load balancer VIPs is moved to a separate file - lib/lb.c. > > > ovn-northd makes use of these functions. Upcoming patch will make use of these > > > util functions for parsing SB Load_Balancers. > > > > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > > --- > > > lib/automake.mk | 4 +- > > > lib/lb.c | 236 ++++++++++++++++++++++++++++++++++++ > > > lib/lb.h | 77 ++++++++++++ > > > lib/ovn-util.c | 28 +++++ > > > lib/ovn-util.h | 2 + > > > northd/ovn-northd.c | 286 +++++--------------------------------------- > > > 6 files changed, 378 insertions(+), 255 deletions(-) > > > create mode 100644 lib/lb.c > > > create mode 100644 lib/lb.h > > > > > > diff --git a/lib/automake.mk b/lib/automake.mk > > > index f3e9c8818b..430cd11fc6 100644 > > > --- a/lib/automake.mk > > > +++ b/lib/automake.mk > > > @@ -23,7 +23,9 @@ lib_libovn_la_SOURCES = \ > > > lib/ovn-util.h \ > > > lib/logical-fields.c \ > > > lib/inc-proc-eng.c \ > > > - lib/inc-proc-eng.h > > > + lib/inc-proc-eng.h \ > > > + lib/lb.c \ > > > + lib/lb.h > > > nodist_lib_libovn_la_SOURCES = \ > > > lib/ovn-dirs.c \ > > > lib/ovn-nb-idl.c \ > > > diff --git a/lib/lb.c b/lib/lb.c > > > new file mode 100644 > > > index 0000000000..db2d3d552f > > > --- /dev/null > > > +++ b/lib/lb.c > > > @@ -0,0 +1,236 @@ > > > +/* Copyright (c) 2020, Red Hat, Inc. > > > + * > > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > > + * you may not use this file except in compliance with the License. > > > + * You may obtain a copy of the License at: > > > + * > > > + * http://www.apache.org/licenses/LICENSE-2.0 > > > + * > > > + * Unless required by applicable law or agreed to in writing, software > > > + * distributed under the License is distributed on an "AS IS" BASIS, > > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > > > + * See the License for the specific language governing permissions and > > > + * limitations under the License. > > > + */ > > > + > > > +#include <config.h> > > > + > > > +#include "lb.h" > > > +#include "lib/ovn-nb-idl.h" > > > +#include "lib/ovn-sb-idl.h" > > > +#include "lib/ovn-util.h" > > > + > > > +/* OpenvSwitch lib includes. */ > > > +#include "openvswitch/vlog.h" > > > +#include "lib/smap.h" > > > + > > > +VLOG_DEFINE_THIS_MODULE(lb); > > > + > > > +static struct ovn_lb * > > > +ovn_lb_create(const struct smap *vips) > > > +{ > > > + struct ovn_lb *lb = xzalloc(sizeof *lb); > > > + > > > + lb->n_vips = smap_count(vips); > > > + lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); > > > + struct smap_node *node; > > > + size_t n_vips = 0; > > > + > > > + SMAP_FOR_EACH (node, vips) { > > > + char *vip; > > > + uint16_t port; > > > + int addr_family; > > > + > > > + if (!ip_address_and_port_from_key(node->key, &vip, &port, > > > + &addr_family)) { > > > + continue; > > > + } > > > + > > > + lb->vips[n_vips].vip = vip; > > > + lb->vips[n_vips].vip_port = port; > > > + lb->vips[n_vips].addr_family = addr_family; > > > + lb->vips[n_vips].vip_port_str = xstrdup(node->key); > > > + lb->vips[n_vips].backend_ips = xstrdup(node->value); > > > + > > > + char *tokstr = xstrdup(node->value); > > > + char *save_ptr = NULL; > > > + char *token; > > > + size_t n_backends = 0; > > > + /* Format for a backend ips : IP1:port1,IP2:port2,...". */ > > > + for (token = strtok_r(tokstr, ",", &save_ptr); > > > + token != NULL; > > > + token = strtok_r(NULL, ",", &save_ptr)) { > > > + n_backends++; > > > + } > > > + > > > + free(tokstr); > > > + tokstr = xstrdup(node->value); > > > + save_ptr = NULL; > > > + > > > + lb->vips[n_vips].n_backends = n_backends; > > > + lb->vips[n_vips].backends = xcalloc(n_backends, > > > + sizeof (struct lb_vip_backend)); > > > > This should be 'sizeof *lb->vips[n_vips].backends'. > > This part of the code is moved from ovn-northd.c. We could use both - > an expression > or a structure in sizeof. The coding guidelines give preference to the former. > > > > > > + > > > > Nit: It's probably fine to remove an empty line here, the one above is already > > indented enough to the right. > > > > > + size_t i = 0; > > > + for (token = strtok_r(tokstr, ",", &save_ptr); > > > + token != NULL; > > > + token = strtok_r(NULL, ",", &save_ptr)) { > > > + char *backend_ip; > > > + uint16_t backend_port; > > > + > > > + if (!ip_address_and_port_from_key(token, &backend_ip, > > > + &backend_port, > > > + &addr_family)) { > > > + continue; > > > + } > > > + > > > + lb->vips[n_vips].backends[i].ip = backend_ip; > > > + lb->vips[n_vips].backends[i].port = backend_port; > > > + lb->vips[n_vips].backends[i].addr_family = addr_family; > > > + i++; > > > + } > > > + > > > + free(tokstr); > > > + n_vips++; > > > + } > > > + > > > + return lb; > > > +} > > > + > > > +struct ovn_lb * > > > +ovn_nb_lb_create(const struct nbrec_load_balancer *nbrec_lb, > > > + struct hmap *ports, struct hmap *lbs, > > > + void * (*ovn_port_find)(const struct hmap *ports, > > > + const char *name)) > > > +{ > > > + struct ovn_lb *lb = ovn_lb_create(&nbrec_lb->vips); > > > + hmap_insert(lbs, &lb->hmap_node, uuid_hash(&nbrec_lb->header_.uuid)); > > > + lb->nlb = nbrec_lb; > > > + lb->nb_lb = true; > > > + > > > + for (size_t i = 0; i < lb->n_vips; i++) { > > > + struct lb_vip *lb_vip = &lb->vips[i]; > > > + > > > + struct nbrec_load_balancer_health_check *lb_health_check = NULL; > > > + if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { > > > + if (nbrec_lb->n_health_check > 0) { > > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > > + VLOG_WARN_RL(&rl, > > > + "SCTP load balancers do not currently support " > > > + "health checks. Not creating health checks for " > > > + "load balancer " UUID_FMT, > > > + UUID_ARGS(&nbrec_lb->header_.uuid)); > > > + } > > > + } else { > > > + for (size_t j = 0; j < nbrec_lb->n_health_check; j++) { > > > + if (!strcmp(nbrec_lb->health_check[j]->vip, > > > + lb_vip->vip_port_str)) { > > > + lb_health_check = nbrec_lb->health_check[i]; > > > + break; > > > + } > > > + } > > > + } > > > + > > > + lb_vip->lb_health_check = lb_health_check; > > > + > > > + for (size_t j = 0; j < lb_vip->n_backends; j++) { > > > + struct lb_vip_backend *backend = &lb_vip->backends[j]; > > > + > > > + struct ovn_port *op = NULL; > > > + char *svc_mon_src_ip = NULL; > > > + const char *s = smap_get(&nbrec_lb->ip_port_mappings, > > > + backend->ip); > > > + if (s) { > > > + char *port_name = xstrdup(s); > > > + char *p = strstr(port_name, ":"); > > > + if (p) { > > > + *p = 0; > > > + p++; > > > + op = ovn_port_find(ports, port_name); > > > + svc_mon_src_ip = xstrdup(p); > > > + } > > > + free(port_name); > > > + } > > > + > > > + backend->op = op; > > > + backend->svc_mon_src_ip = svc_mon_src_ip; > > > + } > > > + } > > > + > > > + if (nbrec_lb->n_selection_fields) { > > > + char *proto = NULL; > > > + if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { > > > + proto = nbrec_lb->protocol; > > > + } > > > + > > > + struct ds sel_fields = DS_EMPTY_INITIALIZER; > > > + for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { > > > + char *field = lb->nlb->selection_fields[i]; > > > + if (!strcmp(field, "tp_src") && proto) { > > > + ds_put_format(&sel_fields, "%s_src,", proto); > > > + } else if (!strcmp(field, "tp_dst") && proto) { > > > + ds_put_format(&sel_fields, "%s_dst,", proto); > > > + } else { > > > + ds_put_format(&sel_fields, "%s,", field); > > > + } > > > + } > > > + ds_chomp(&sel_fields, ','); > > > + lb->selection_fields = ds_steal_cstr(&sel_fields); > > > + } > > > + > > > + return lb; > > > +} > > > + > > > +struct ovn_lb * > > > +ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb) > > > +{ > > > + struct ovn_lb *lb = ovn_lb_create(&sbrec_lb->vips); > > > + lb->slb = sbrec_lb; > > > + lb->nb_lb = false; > > > + > > > + return lb; > > > +} > > > + > > > +struct ovn_lb * > > > +ovn_lb_find(struct hmap *lbs, struct uuid *uuid) > > > +{ > > > + struct ovn_lb *lb; > > > + size_t hash = uuid_hash(uuid); > > > + HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { > > > + if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { > > > + return lb; > > > + } > > > + } > > > + > > > + return NULL; > > > +} > > > + > > > +void > > > +ovn_lb_destroy(struct ovn_lb *lb) > > > +{ > > > + for (size_t i = 0; i < lb->n_vips; i++) { > > > + free(lb->vips[i].vip); > > > + free(lb->vips[i].backend_ips); > > > + free(lb->vips[i].vip_port_str); > > > + > > > + for (size_t j = 0; j < lb->vips[i].n_backends; j++) { > > > + free(lb->vips[i].backends[j].ip); > > > + free(lb->vips[i].backends[j].svc_mon_src_ip); > > > + } > > > + > > > + free(lb->vips[i].backends); > > > + } > > > + free(lb->vips); > > > + free(lb->selection_fields); > > > > This is not exactly correct if the record was created with ovn_sb_lb_create(). > > It happens to work but it's actually freeing padding that happens to have been > > initialized to 0. > > selection_fields is of type char *, which is set to 0 during > allocation. And calling free with a NULL > pointer is fine right ? You're right. Please ignore my comment. Thanks Numan > > > > > > > +} > > > + > > > +void > > > +ovn_lbs_destroy(struct hmap *lbs) > > > +{ > > > + struct ovn_lb *lb; > > > + HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { > > > + ovn_lb_destroy(lb); > > > + free(lb); > > > + } > > > + hmap_destroy(lbs); > > > +} > > > diff --git a/lib/lb.h b/lib/lb.h > > > new file mode 100644 > > > index 0000000000..ffb3ba1fd1 > > > --- /dev/null > > > +++ b/lib/lb.h > > > @@ -0,0 +1,77 @@ > > > +/* Copyright (c) 2020, Red Hat, Inc. > > > + * > > > + * Licensed under the Apache License, Version 2.0 (the "License"); > > > + * you may not use this file except in compliance with the License. > > > + * You may obtain a copy of the License at: > > > + * > > > + * http://www.apache.org/licenses/LICENSE-2.0 > > > + * > > > + * Unless required by applicable law or agreed to in writing, software > > > + * distributed under the License is distributed on an "AS IS" BASIS, > > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > > > + * See the License for the specific language governing permissions and > > > + * limitations under the License. > > > + */ > > > + > > > + > > > +#ifndef OVN_LIB_LB_H > > > +#define OVN_LIB_LB_H 1 > > > + > > > +#include "openvswitch/hmap.h" > > > + > > > +struct nbrec_load_balancer; > > > +struct sbrec_load_balancer; > > > +struct ovn_port; > > > +struct uuid; > > > + > > > +struct ovn_lb { > > > + struct hmap_node hmap_node; > > > + > > > + bool nb_lb; /* NB load balancer or SB load balancer. */ > > > + union { > > > + struct { > > > + const struct nbrec_load_balancer *nlb; /* May be NULL. */ > > > + char *selection_fields; > > > + }; > > > + const struct sbrec_load_balancer *slb; /* May be NULL. */ > > > + }; > > > + > > > + struct lb_vip *vips; > > > + size_t n_vips; > > > +}; > > > > I think it's better to have two different structures for NB/SB load balancers. > > We never store them together anyway. What about something like: > > I thought about this, but I think it gets complicated. Like every > backend in NB DB can have > a health check related params. Now how do we represent that in struct > ovn_nb_lb ? > > If we want to have separate structures, we should have separate lb_vip > and lb_backend structures > for NB and SB and a separate parser function too. > > I'm in favor of having proposed structures in this patch. > > > > > struct ovn_lb { > > struct hmap_node hmap_node; > > struct lb_vip *vips; > > size_t n_vips; > > }; > > > > struct ovn_nb_lb { > > struct ovn_lb common; > > const struct nbrec_load_balancer *nlb; > > char *selection_fields; > > /* Some more fields that are NB specific. */ > > }; > > > > struct ovn_sb_lb { > > struct ovn_lb common; > > const struct sbrec_load_balancer *slb; > > /* Some more fields that are SB specific. */ > > }; > > > > struct ovn_nb_lb * > > ovn_nb_lb_from_lb(struct ovn_lb *lb) > > { > > return CONTAINER_OF(lb, struct ovn_nb_lb *, common); > > } > > > > struct ovn_sb_lb * > > ovn_sb_lb_from_lb(struct ovn_lb *lb) > > { > > return CONTAINER_OF(lb, struct ovn_sb_lb *, common); > > } > > > > > + > > > +struct lb_vip { > > > + char *vip; > > > + uint16_t vip_port; > > > + int addr_family; > > > + char *backend_ips; > > > + char *vip_port_str; > > > + > > > + /* Valid only for NB load balancer. */ > > > > This is not really true. In patch 3/7 we use some of the struct lb_vip_backend > > fields from SB load balancers 'backends'. > > > > > + struct nbrec_load_balancer_health_check *lb_health_check; > > > + struct lb_vip_backend *backends; > > > + size_t n_backends; > > > > If we'd have separate NB/SB structures this would go in the NB-specific part. > > Do you have suggestions on how to define the NB/SB structure for this ? > > > > > > > +}; > > > + > > > +struct lb_vip_backend { > > > + char *ip; > > > + uint16_t port; > > > + int addr_family; > > > + > > > + /* Valid only for NB load balancer. */ > > > + struct ovn_port *op; /* Logical port to which the ip belong to. */ > > > + bool health_check; > > > + char *svc_mon_src_ip; /* Source IP to use for monitoring. */ > > > + const struct sbrec_service_monitor *sbrec_monitor; > > > > If we'd have separate NB/SB structures this would go in the NB-specific part. > > Do you have suggestions on how to define the NB/SB structure for this ? > > Thanks > Numan > > > > > Thanks, > > Dumitru > > > > > +}; > > > + > > > +struct ovn_lb *ovn_nb_lb_create( > > > + const struct nbrec_load_balancer *nbrec_lb, > > > + struct hmap *ports, struct hmap *lbs, > > > + void * (*ovn_port_find)(const struct hmap *ports, const char *name)); > > > +struct ovn_lb *ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb); > > > +struct ovn_lb * ovn_lb_find(struct hmap *lbs, struct uuid *uuid); > > > +void ovn_lb_destroy(struct ovn_lb *lb); > > > +void ovn_lbs_destroy(struct hmap *lbs); > > > + > > > +#endif /* OVN_LIB_LB_H 1 */ > > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > > index 1daf665037..02770bd55a 100644 > > > --- a/lib/ovn-util.c > > > +++ b/lib/ovn-util.c > > > @@ -24,6 +24,7 @@ > > > #include "ovn-sb-idl.h" > > > #include "unixctl.h" > > > #include <ctype.h> > > > +#include "socket-util.h" > > > > > > VLOG_DEFINE_THIS_MODULE(ovn_util); > > > > > > @@ -667,3 +668,30 @@ ovn_smap_get_uint(const struct smap *smap, const char *key, unsigned int def) > > > > > > return u_value; > > > } > > > + > > > +/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and > > > + * 'ip_address'. The caller must free() the memory allocated for > > > + * 'ip_address'. > > > + * Returns true if parsing of 'key' was successful, false otherwise. > > > + */ > > > +bool > > > +ip_address_and_port_from_key(const char *key, char **ip_address, > > > + uint16_t *port, int *addr_family) > > > +{ > > > + struct sockaddr_storage ss; > > > + if (!inet_parse_active(key, 0, &ss, false)) { > > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > > + VLOG_WARN_RL(&rl, "bad ip address or port for key %s", key); > > > + *ip_address = NULL; > > > + *port = 0; > > > + *addr_family = 0; > > > + return false; > > > + } > > > + > > > + struct ds s = DS_EMPTY_INITIALIZER; > > > + ss_format_address_nobracks(&ss, &s); > > > + *ip_address = ds_steal_cstr(&s); > > > + *port = ss_get_port(&ss); > > > + *addr_family = ss.ss_family; > > > + return true; > > > +} > > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > > > index 1d22a691f5..7fe53fd736 100644 > > > --- a/lib/ovn-util.h > > > +++ b/lib/ovn-util.h > > > @@ -157,6 +157,8 @@ char *normalize_v46_prefix(const struct v46_ip *prefix, unsigned int plen); > > > unsigned int ovn_smap_get_uint(const struct smap *smap, const char *key, > > > unsigned int def); > > > > > > +bool ip_address_and_port_from_key(const char *key, char **ip_address, > > > + uint16_t *port, int *addr_family); > > > /* Returns a lowercase copy of orig. > > > * Caller must free the returned string. > > > */ > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > > index d11888d203..1da31caf3d 100644 > > > --- a/northd/ovn-northd.c > > > +++ b/northd/ovn-northd.c > > > @@ -35,6 +35,7 @@ > > > #include "lib/ovn-nb-idl.h" > > > #include "lib/ovn-sb-idl.h" > > > #include "lib/ovn-util.h" > > > +#include "lib/lb.h" > > > #include "ovn/actions.h" > > > #include "ovn/logical-fields.h" > > > #include "packets.h" > > > @@ -2525,10 +2526,6 @@ join_logical_ports(struct northd_context *ctx, > > > } > > > } > > > > > > -static bool > > > -ip_address_and_port_from_lb_key(const char *key, char **ip_address, > > > - uint16_t *port, int *addr_family); > > > - > > > static void > > > get_router_load_balancer_ips(const struct ovn_datapath *od, > > > struct sset *all_ips_v4, struct sset *all_ips_v6) > > > @@ -2548,8 +2545,8 @@ get_router_load_balancer_ips(const struct ovn_datapath *od, > > > uint16_t port; > > > int addr_family; > > > > > > - if (!ip_address_and_port_from_lb_key(node->key, &ip_address, &port, > > > - &addr_family)) { > > > + if (!ip_address_and_port_from_key(node->key, &ip_address, &port, > > > + &addr_family)) { > > > continue; > > > } > > > > > > @@ -3309,53 +3306,6 @@ cleanup_sb_ha_chassis_groups(struct northd_context *ctx, > > > } > > > } > > > > > > -struct ovn_lb { > > > - struct hmap_node hmap_node; > > > - > > > - const struct nbrec_load_balancer *nlb; /* May be NULL. */ > > > - char *selection_fields; > > > - struct lb_vip *vips; > > > - size_t n_vips; > > > -}; > > > - > > > -struct lb_vip { > > > - char *vip; > > > - uint16_t vip_port; > > > - int addr_family; > > > - char *backend_ips; > > > - > > > - bool health_check; > > > - struct lb_vip_backend *backends; > > > - size_t n_backends; > > > -}; > > > - > > > -struct lb_vip_backend { > > > - char *ip; > > > - uint16_t port; > > > - int addr_family; > > > - > > > - struct ovn_port *op; /* Logical port to which the ip belong to. */ > > > - bool health_check; > > > - char *svc_mon_src_ip; /* Source IP to use for monitoring. */ > > > - const struct sbrec_service_monitor *sbrec_monitor; > > > -}; > > > - > > > - > > > -static inline struct ovn_lb * > > > -ovn_lb_find(struct hmap *lbs, struct uuid *uuid) > > > -{ > > > - struct ovn_lb *lb; > > > - size_t hash = uuid_hash(uuid); > > > - HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { > > > - if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { > > > - return lb; > > > - } > > > - } > > > - > > > - return NULL; > > > -} > > > - > > > - > > > struct service_monitor_info { > > > struct hmap_node hmap_node; > > > const struct sbrec_service_monitor *sbrec_mon; > > > @@ -3395,126 +3345,36 @@ create_or_get_service_mon(struct northd_context *ctx, > > > return mon_info; > > > } > > > > > > -static struct ovn_lb * > > > -ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, > > > - const struct nbrec_load_balancer *nbrec_lb, > > > - struct hmap *ports, struct hmap *monitor_map) > > > +static void > > > +ovn_lb_svc_create(struct northd_context *ctx, struct ovn_lb *lb, > > > + struct hmap *monitor_map) > > > { > > > - struct ovn_lb *lb = xzalloc(sizeof *lb); > > > - > > > - size_t hash = uuid_hash(&nbrec_lb->header_.uuid); > > > - lb->nlb = nbrec_lb; > > > - hmap_insert(lbs, &lb->hmap_node, hash); > > > - > > > - lb->n_vips = smap_count(&nbrec_lb->vips); > > > - lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); > > > - struct smap_node *node; > > > - size_t n_vips = 0; > > > - > > > - SMAP_FOR_EACH (node, &nbrec_lb->vips) { > > > - char *vip; > > > - uint16_t port; > > > - int addr_family; > > > + for (size_t i = 0; i < lb->n_vips; i++) { > > > + struct lb_vip *lb_vip = &lb->vips[i]; > > > > > > - if (!ip_address_and_port_from_lb_key(node->key, &vip, &port, > > > - &addr_family)) { > > > + if (!lb_vip->lb_health_check) { > > > continue; > > > } > > > > > > - lb->vips[n_vips].vip = vip; > > > - lb->vips[n_vips].vip_port = port; > > > - lb->vips[n_vips].addr_family = addr_family; > > > - lb->vips[n_vips].backend_ips = xstrdup(node->value); > > > - > > > - struct nbrec_load_balancer_health_check *lb_health_check = NULL; > > > - if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { > > > - if (nbrec_lb->n_health_check > 0) { > > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > > > - VLOG_WARN_RL(&rl, > > > - "SCTP load balancers do not currently support " > > > - "health checks. Not creating health checks for " > > > - "load balancer " UUID_FMT, > > > - UUID_ARGS(&nbrec_lb->header_.uuid)); > > > - } > > > - } else { > > > - for (size_t i = 0; i < nbrec_lb->n_health_check; i++) { > > > - if (!strcmp(nbrec_lb->health_check[i]->vip, node->key)) { > > > - lb_health_check = nbrec_lb->health_check[i]; > > > - break; > > > - } > > > - } > > > - } > > > - > > > - char *tokstr = xstrdup(node->value); > > > - char *save_ptr = NULL; > > > - char *token; > > > - size_t n_backends = 0; > > > - /* Format for a backend ips : IP1:port1,IP2:port2,...". */ > > > - for (token = strtok_r(tokstr, ",", &save_ptr); > > > - token != NULL; > > > - token = strtok_r(NULL, ",", &save_ptr)) { > > > - n_backends++; > > > - } > > > - > > > - free(tokstr); > > > - tokstr = xstrdup(node->value); > > > - save_ptr = NULL; > > > - > > > - lb->vips[n_vips].n_backends = n_backends; > > > - lb->vips[n_vips].backends = xcalloc(n_backends, > > > - sizeof (struct lb_vip_backend)); > > > - lb->vips[n_vips].health_check = lb_health_check ? true: false; > > > - > > > - size_t i = 0; > > > - for (token = strtok_r(tokstr, ",", &save_ptr); > > > - token != NULL; > > > - token = strtok_r(NULL, ",", &save_ptr)) { > > > - char *backend_ip; > > > - uint16_t backend_port; > > > - > > > - if (!ip_address_and_port_from_lb_key(token, &backend_ip, > > > - &backend_port, > > > - &addr_family)) { > > > - continue; > > > - } > > > + for (size_t j = 0; j < lb_vip->n_backends; j++) { > > > + struct lb_vip_backend *backend = &lb_vip->backends[j]; > > > > > > - /* Get the logical port to which this ip belongs to. */ > > > - struct ovn_port *op = NULL; > > > - char *svc_mon_src_ip = NULL; > > > - const char *s = smap_get(&nbrec_lb->ip_port_mappings, > > > - backend_ip); > > > - if (s) { > > > - char *port_name = xstrdup(s); > > > - char *p = strstr(port_name, ":"); > > > - if (p) { > > > - *p = 0; > > > - p++; > > > - op = ovn_port_find(ports, port_name); > > > - svc_mon_src_ip = xstrdup(p); > > > - } > > > - free(port_name); > > > - } > > > - > > > - lb->vips[n_vips].backends[i].ip = backend_ip; > > > - lb->vips[n_vips].backends[i].port = backend_port; > > > - lb->vips[n_vips].backends[i].addr_family = addr_family; > > > - lb->vips[n_vips].backends[i].op = op; > > > - lb->vips[n_vips].backends[i].svc_mon_src_ip = svc_mon_src_ip; > > > - > > > - if (lb_health_check && op && svc_mon_src_ip) { > > > - const char *protocol = nbrec_lb->protocol; > > > + if (backend->op && backend->svc_mon_src_ip) { > > > + backend->health_check = true; > > > + const char *protocol = lb->nlb->protocol; > > > if (!protocol || !protocol[0]) { > > > protocol = "tcp"; > > > } > > > - lb->vips[n_vips].backends[i].health_check = true; > > > + backend->health_check = true; > > > struct service_monitor_info *mon_info = > > > - create_or_get_service_mon(ctx, monitor_map, backend_ip, > > > - op->nbsp->name, backend_port, > > > + create_or_get_service_mon(ctx, monitor_map, backend->ip, > > > + backend->op->nbsp->name, > > > + backend->port, > > > protocol); > > > > > > ovs_assert(mon_info); > > > sbrec_service_monitor_set_options( > > > - mon_info->sbrec_mon, &lb_health_check->options); > > > + mon_info->sbrec_mon, &lb_vip->lb_health_check->options); > > > struct eth_addr ea; > > > if (!mon_info->sbrec_mon->src_mac || > > > !eth_addr_from_string(mon_info->sbrec_mon->src_mac, &ea) || > > > @@ -3524,72 +3384,24 @@ ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, > > > } > > > > > > if (!mon_info->sbrec_mon->src_ip || > > > - strcmp(mon_info->sbrec_mon->src_ip, svc_mon_src_ip)) { > > > + strcmp(mon_info->sbrec_mon->src_ip, > > > + backend->svc_mon_src_ip)) { > > > sbrec_service_monitor_set_src_ip(mon_info->sbrec_mon, > > > - svc_mon_src_ip); > > > + backend->svc_mon_src_ip); > > > } > > > > > > - lb->vips[n_vips].backends[i].sbrec_monitor = > > > - mon_info->sbrec_mon; > > > + lb_vip->backends[j].sbrec_monitor = mon_info->sbrec_mon; > > > mon_info->required = true; > > > - } else { > > > - lb->vips[n_vips].backends[i].health_check = false; > > > } > > > - > > > - i++; > > > } > > > - > > > - free(tokstr); > > > - n_vips++; > > > - } > > > - > > > - char *proto = NULL; > > > - if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { > > > - proto = nbrec_lb->protocol; > > > } > > > - > > > - if (lb->nlb->n_selection_fields) { > > > - struct ds sel_fields = DS_EMPTY_INITIALIZER; > > > - for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { > > > - char *field = lb->nlb->selection_fields[i]; > > > - if (!strcmp(field, "tp_src") && proto) { > > > - ds_put_format(&sel_fields, "%s_src,", proto); > > > - } else if (!strcmp(field, "tp_dst") && proto) { > > > - ds_put_format(&sel_fields, "%s_dst,", proto); > > > - } else { > > > - ds_put_format(&sel_fields, "%s,", field); > > > - } > > > - } > > > - ds_chomp(&sel_fields, ','); > > > - lb->selection_fields = ds_steal_cstr(&sel_fields); > > > - } > > > - > > > - return lb; > > > -} > > > - > > > -static void > > > -ovn_lb_destroy(struct ovn_lb *lb) > > > -{ > > > - for (size_t i = 0; i < lb->n_vips; i++) { > > > - free(lb->vips[i].vip); > > > - free(lb->vips[i].backend_ips); > > > - > > > - for (size_t j = 0; j < lb->vips[i].n_backends; j++) { > > > - free(lb->vips[i].backends[j].ip); > > > - free(lb->vips[i].backends[j].svc_mon_src_ip); > > > - } > > > - > > > - free(lb->vips[i].backends); > > > - } > > > - free(lb->vips); > > > - free(lb->selection_fields); > > > } > > > > > > static void build_lb_vip_ct_lb_actions(struct lb_vip *lb_vip, > > > struct ds *action, > > > char *selection_fields) > > > { > > > - if (lb_vip->health_check) { > > > + if (lb_vip->lb_health_check) { > > > ds_put_cstr(action, "ct_lb(backends="); > > > > > > size_t n_active_backends = 0; > > > @@ -3644,7 +3456,12 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, > > > > > > const struct nbrec_load_balancer *nbrec_lb; > > > NBREC_LOAD_BALANCER_FOR_EACH (nbrec_lb, ctx->ovnnb_idl) { > > > - ovn_lb_create(ctx, lbs, nbrec_lb, ports, &monitor_map); > > > + ovn_nb_lb_create(nbrec_lb, ports, lbs, (void *)ovn_port_find); > > > + } > > > + > > > + struct ovn_lb *lb; > > > + HMAP_FOR_EACH (lb, hmap_node, lbs) { > > > + ovn_lb_svc_create(ctx, lb, &monitor_map); > > > } > > > > > > struct service_monitor_info *mon_info; > > > @@ -3658,16 +3475,6 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, > > > hmap_destroy(&monitor_map); > > > } > > > > > > -static void > > > -destroy_ovn_lbs(struct hmap *lbs) > > > -{ > > > - struct ovn_lb *lb; > > > - HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { > > > - ovn_lb_destroy(lb); > > > - free(lb); > > > - } > > > -} > > > - > > > /* Updates the southbound Port_Binding table so that it contains the logical > > > * switch ports specified by the northbound database. > > > * > > > @@ -5030,34 +4837,6 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) > > > } > > > } > > > > > > -/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and > > > - * 'ip_address'. The caller must free() the memory allocated for > > > - * 'ip_address'. > > > - * Returns true if parsing of 'key' was successful, false otherwise. > > > - */ > > > -static bool > > > -ip_address_and_port_from_lb_key(const char *key, char **ip_address, > > > - uint16_t *port, int *addr_family) > > > -{ > > > - struct sockaddr_storage ss; > > > - if (!inet_parse_active(key, 0, &ss, false)) { > > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > > - VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s", > > > - key); > > > - *ip_address = NULL; > > > - *port = 0; > > > - *addr_family = 0; > > > - return false; > > > - } > > > - > > > - struct ds s = DS_EMPTY_INITIALIZER; > > > - ss_format_address_nobracks(&ss, &s); > > > - *ip_address = ds_steal_cstr(&s); > > > - *port = ss_get_port(&ss); > > > - *addr_family = ss.ss_family; > > > - return true; > > > -} > > > - > > > /* > > > * Returns true if logical switch is configured with DNS records, false > > > * otherwise. > > > @@ -7004,7 +6783,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, > > > struct ovn_lb *lb; > > > HMAP_FOR_EACH (lb, hmap_node, lbs) { > > > for (size_t i = 0; i < lb->n_vips; i++) { > > > - if (!lb->vips[i].health_check) { > > > + if (!lb->vips[i].lb_health_check) { > > > continue; > > > } > > > > > > @@ -12358,8 +12137,7 @@ ovnnb_db_run(struct northd_context *ctx, > > > sync_meters(ctx); > > > sync_dns_entries(ctx, datapaths); > > > sync_lb_entries(ctx, datapaths); > > > - destroy_ovn_lbs(&lbs); > > > - hmap_destroy(&lbs); > > > + ovn_lbs_destroy(&lbs); > > > > > > struct ovn_igmp_group *igmp_group, *next_igmp_group; > > > > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/lib/automake.mk b/lib/automake.mk index f3e9c8818b..430cd11fc6 100644 --- a/lib/automake.mk +++ b/lib/automake.mk @@ -23,7 +23,9 @@ lib_libovn_la_SOURCES = \ lib/ovn-util.h \ lib/logical-fields.c \ lib/inc-proc-eng.c \ - lib/inc-proc-eng.h + lib/inc-proc-eng.h \ + lib/lb.c \ + lib/lb.h nodist_lib_libovn_la_SOURCES = \ lib/ovn-dirs.c \ lib/ovn-nb-idl.c \ diff --git a/lib/lb.c b/lib/lb.c new file mode 100644 index 0000000000..db2d3d552f --- /dev/null +++ b/lib/lb.c @@ -0,0 +1,236 @@ +/* Copyright (c) 2020, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <config.h> + +#include "lb.h" +#include "lib/ovn-nb-idl.h" +#include "lib/ovn-sb-idl.h" +#include "lib/ovn-util.h" + +/* OpenvSwitch lib includes. */ +#include "openvswitch/vlog.h" +#include "lib/smap.h" + +VLOG_DEFINE_THIS_MODULE(lb); + +static struct ovn_lb * +ovn_lb_create(const struct smap *vips) +{ + struct ovn_lb *lb = xzalloc(sizeof *lb); + + lb->n_vips = smap_count(vips); + lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); + struct smap_node *node; + size_t n_vips = 0; + + SMAP_FOR_EACH (node, vips) { + char *vip; + uint16_t port; + int addr_family; + + if (!ip_address_and_port_from_key(node->key, &vip, &port, + &addr_family)) { + continue; + } + + lb->vips[n_vips].vip = vip; + lb->vips[n_vips].vip_port = port; + lb->vips[n_vips].addr_family = addr_family; + lb->vips[n_vips].vip_port_str = xstrdup(node->key); + lb->vips[n_vips].backend_ips = xstrdup(node->value); + + char *tokstr = xstrdup(node->value); + char *save_ptr = NULL; + char *token; + size_t n_backends = 0; + /* Format for a backend ips : IP1:port1,IP2:port2,...". */ + for (token = strtok_r(tokstr, ",", &save_ptr); + token != NULL; + token = strtok_r(NULL, ",", &save_ptr)) { + n_backends++; + } + + free(tokstr); + tokstr = xstrdup(node->value); + save_ptr = NULL; + + lb->vips[n_vips].n_backends = n_backends; + lb->vips[n_vips].backends = xcalloc(n_backends, + sizeof (struct lb_vip_backend)); + + size_t i = 0; + for (token = strtok_r(tokstr, ",", &save_ptr); + token != NULL; + token = strtok_r(NULL, ",", &save_ptr)) { + char *backend_ip; + uint16_t backend_port; + + if (!ip_address_and_port_from_key(token, &backend_ip, + &backend_port, + &addr_family)) { + continue; + } + + lb->vips[n_vips].backends[i].ip = backend_ip; + lb->vips[n_vips].backends[i].port = backend_port; + lb->vips[n_vips].backends[i].addr_family = addr_family; + i++; + } + + free(tokstr); + n_vips++; + } + + return lb; +} + +struct ovn_lb * +ovn_nb_lb_create(const struct nbrec_load_balancer *nbrec_lb, + struct hmap *ports, struct hmap *lbs, + void * (*ovn_port_find)(const struct hmap *ports, + const char *name)) +{ + struct ovn_lb *lb = ovn_lb_create(&nbrec_lb->vips); + hmap_insert(lbs, &lb->hmap_node, uuid_hash(&nbrec_lb->header_.uuid)); + lb->nlb = nbrec_lb; + lb->nb_lb = true; + + for (size_t i = 0; i < lb->n_vips; i++) { + struct lb_vip *lb_vip = &lb->vips[i]; + + struct nbrec_load_balancer_health_check *lb_health_check = NULL; + if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { + if (nbrec_lb->n_health_check > 0) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, + "SCTP load balancers do not currently support " + "health checks. Not creating health checks for " + "load balancer " UUID_FMT, + UUID_ARGS(&nbrec_lb->header_.uuid)); + } + } else { + for (size_t j = 0; j < nbrec_lb->n_health_check; j++) { + if (!strcmp(nbrec_lb->health_check[j]->vip, + lb_vip->vip_port_str)) { + lb_health_check = nbrec_lb->health_check[i]; + break; + } + } + } + + lb_vip->lb_health_check = lb_health_check; + + for (size_t j = 0; j < lb_vip->n_backends; j++) { + struct lb_vip_backend *backend = &lb_vip->backends[j]; + + struct ovn_port *op = NULL; + char *svc_mon_src_ip = NULL; + const char *s = smap_get(&nbrec_lb->ip_port_mappings, + backend->ip); + if (s) { + char *port_name = xstrdup(s); + char *p = strstr(port_name, ":"); + if (p) { + *p = 0; + p++; + op = ovn_port_find(ports, port_name); + svc_mon_src_ip = xstrdup(p); + } + free(port_name); + } + + backend->op = op; + backend->svc_mon_src_ip = svc_mon_src_ip; + } + } + + if (nbrec_lb->n_selection_fields) { + char *proto = NULL; + if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { + proto = nbrec_lb->protocol; + } + + struct ds sel_fields = DS_EMPTY_INITIALIZER; + for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { + char *field = lb->nlb->selection_fields[i]; + if (!strcmp(field, "tp_src") && proto) { + ds_put_format(&sel_fields, "%s_src,", proto); + } else if (!strcmp(field, "tp_dst") && proto) { + ds_put_format(&sel_fields, "%s_dst,", proto); + } else { + ds_put_format(&sel_fields, "%s,", field); + } + } + ds_chomp(&sel_fields, ','); + lb->selection_fields = ds_steal_cstr(&sel_fields); + } + + return lb; +} + +struct ovn_lb * +ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb) +{ + struct ovn_lb *lb = ovn_lb_create(&sbrec_lb->vips); + lb->slb = sbrec_lb; + lb->nb_lb = false; + + return lb; +} + +struct ovn_lb * +ovn_lb_find(struct hmap *lbs, struct uuid *uuid) +{ + struct ovn_lb *lb; + size_t hash = uuid_hash(uuid); + HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { + if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { + return lb; + } + } + + return NULL; +} + +void +ovn_lb_destroy(struct ovn_lb *lb) +{ + for (size_t i = 0; i < lb->n_vips; i++) { + free(lb->vips[i].vip); + free(lb->vips[i].backend_ips); + free(lb->vips[i].vip_port_str); + + for (size_t j = 0; j < lb->vips[i].n_backends; j++) { + free(lb->vips[i].backends[j].ip); + free(lb->vips[i].backends[j].svc_mon_src_ip); + } + + free(lb->vips[i].backends); + } + free(lb->vips); + free(lb->selection_fields); +} + +void +ovn_lbs_destroy(struct hmap *lbs) +{ + struct ovn_lb *lb; + HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { + ovn_lb_destroy(lb); + free(lb); + } + hmap_destroy(lbs); +} diff --git a/lib/lb.h b/lib/lb.h new file mode 100644 index 0000000000..ffb3ba1fd1 --- /dev/null +++ b/lib/lb.h @@ -0,0 +1,77 @@ +/* Copyright (c) 2020, Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +#ifndef OVN_LIB_LB_H +#define OVN_LIB_LB_H 1 + +#include "openvswitch/hmap.h" + +struct nbrec_load_balancer; +struct sbrec_load_balancer; +struct ovn_port; +struct uuid; + +struct ovn_lb { + struct hmap_node hmap_node; + + bool nb_lb; /* NB load balancer or SB load balancer. */ + union { + struct { + const struct nbrec_load_balancer *nlb; /* May be NULL. */ + char *selection_fields; + }; + const struct sbrec_load_balancer *slb; /* May be NULL. */ + }; + + struct lb_vip *vips; + size_t n_vips; +}; + +struct lb_vip { + char *vip; + uint16_t vip_port; + int addr_family; + char *backend_ips; + char *vip_port_str; + + /* Valid only for NB load balancer. */ + struct nbrec_load_balancer_health_check *lb_health_check; + struct lb_vip_backend *backends; + size_t n_backends; +}; + +struct lb_vip_backend { + char *ip; + uint16_t port; + int addr_family; + + /* Valid only for NB load balancer. */ + struct ovn_port *op; /* Logical port to which the ip belong to. */ + bool health_check; + char *svc_mon_src_ip; /* Source IP to use for monitoring. */ + const struct sbrec_service_monitor *sbrec_monitor; +}; + +struct ovn_lb *ovn_nb_lb_create( + const struct nbrec_load_balancer *nbrec_lb, + struct hmap *ports, struct hmap *lbs, + void * (*ovn_port_find)(const struct hmap *ports, const char *name)); +struct ovn_lb *ovn_sb_lb_create(const struct sbrec_load_balancer *sbrec_lb); +struct ovn_lb * ovn_lb_find(struct hmap *lbs, struct uuid *uuid); +void ovn_lb_destroy(struct ovn_lb *lb); +void ovn_lbs_destroy(struct hmap *lbs); + +#endif /* OVN_LIB_LB_H 1 */ diff --git a/lib/ovn-util.c b/lib/ovn-util.c index 1daf665037..02770bd55a 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -24,6 +24,7 @@ #include "ovn-sb-idl.h" #include "unixctl.h" #include <ctype.h> +#include "socket-util.h" VLOG_DEFINE_THIS_MODULE(ovn_util); @@ -667,3 +668,30 @@ ovn_smap_get_uint(const struct smap *smap, const char *key, unsigned int def) return u_value; } + +/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and + * 'ip_address'. The caller must free() the memory allocated for + * 'ip_address'. + * Returns true if parsing of 'key' was successful, false otherwise. + */ +bool +ip_address_and_port_from_key(const char *key, char **ip_address, + uint16_t *port, int *addr_family) +{ + struct sockaddr_storage ss; + if (!inet_parse_active(key, 0, &ss, false)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_WARN_RL(&rl, "bad ip address or port for key %s", key); + *ip_address = NULL; + *port = 0; + *addr_family = 0; + return false; + } + + struct ds s = DS_EMPTY_INITIALIZER; + ss_format_address_nobracks(&ss, &s); + *ip_address = ds_steal_cstr(&s); + *port = ss_get_port(&ss); + *addr_family = ss.ss_family; + return true; +} diff --git a/lib/ovn-util.h b/lib/ovn-util.h index 1d22a691f5..7fe53fd736 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -157,6 +157,8 @@ char *normalize_v46_prefix(const struct v46_ip *prefix, unsigned int plen); unsigned int ovn_smap_get_uint(const struct smap *smap, const char *key, unsigned int def); +bool ip_address_and_port_from_key(const char *key, char **ip_address, + uint16_t *port, int *addr_family); /* Returns a lowercase copy of orig. * Caller must free the returned string. */ diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index d11888d203..1da31caf3d 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -35,6 +35,7 @@ #include "lib/ovn-nb-idl.h" #include "lib/ovn-sb-idl.h" #include "lib/ovn-util.h" +#include "lib/lb.h" #include "ovn/actions.h" #include "ovn/logical-fields.h" #include "packets.h" @@ -2525,10 +2526,6 @@ join_logical_ports(struct northd_context *ctx, } } -static bool -ip_address_and_port_from_lb_key(const char *key, char **ip_address, - uint16_t *port, int *addr_family); - static void get_router_load_balancer_ips(const struct ovn_datapath *od, struct sset *all_ips_v4, struct sset *all_ips_v6) @@ -2548,8 +2545,8 @@ get_router_load_balancer_ips(const struct ovn_datapath *od, uint16_t port; int addr_family; - if (!ip_address_and_port_from_lb_key(node->key, &ip_address, &port, - &addr_family)) { + if (!ip_address_and_port_from_key(node->key, &ip_address, &port, + &addr_family)) { continue; } @@ -3309,53 +3306,6 @@ cleanup_sb_ha_chassis_groups(struct northd_context *ctx, } } -struct ovn_lb { - struct hmap_node hmap_node; - - const struct nbrec_load_balancer *nlb; /* May be NULL. */ - char *selection_fields; - struct lb_vip *vips; - size_t n_vips; -}; - -struct lb_vip { - char *vip; - uint16_t vip_port; - int addr_family; - char *backend_ips; - - bool health_check; - struct lb_vip_backend *backends; - size_t n_backends; -}; - -struct lb_vip_backend { - char *ip; - uint16_t port; - int addr_family; - - struct ovn_port *op; /* Logical port to which the ip belong to. */ - bool health_check; - char *svc_mon_src_ip; /* Source IP to use for monitoring. */ - const struct sbrec_service_monitor *sbrec_monitor; -}; - - -static inline struct ovn_lb * -ovn_lb_find(struct hmap *lbs, struct uuid *uuid) -{ - struct ovn_lb *lb; - size_t hash = uuid_hash(uuid); - HMAP_FOR_EACH_WITH_HASH (lb, hmap_node, hash, lbs) { - if (uuid_equals(&lb->nlb->header_.uuid, uuid)) { - return lb; - } - } - - return NULL; -} - - struct service_monitor_info { struct hmap_node hmap_node; const struct sbrec_service_monitor *sbrec_mon; @@ -3395,126 +3345,36 @@ create_or_get_service_mon(struct northd_context *ctx, return mon_info; } -static struct ovn_lb * -ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, - const struct nbrec_load_balancer *nbrec_lb, - struct hmap *ports, struct hmap *monitor_map) +static void +ovn_lb_svc_create(struct northd_context *ctx, struct ovn_lb *lb, + struct hmap *monitor_map) { - struct ovn_lb *lb = xzalloc(sizeof *lb); - - size_t hash = uuid_hash(&nbrec_lb->header_.uuid); - lb->nlb = nbrec_lb; - hmap_insert(lbs, &lb->hmap_node, hash); - - lb->n_vips = smap_count(&nbrec_lb->vips); - lb->vips = xcalloc(lb->n_vips, sizeof (struct lb_vip)); - struct smap_node *node; - size_t n_vips = 0; - - SMAP_FOR_EACH (node, &nbrec_lb->vips) { - char *vip; - uint16_t port; - int addr_family; + for (size_t i = 0; i < lb->n_vips; i++) { + struct lb_vip *lb_vip = &lb->vips[i]; - if (!ip_address_and_port_from_lb_key(node->key, &vip, &port, - &addr_family)) { + if (!lb_vip->lb_health_check) { continue; } - lb->vips[n_vips].vip = vip; - lb->vips[n_vips].vip_port = port; - lb->vips[n_vips].addr_family = addr_family; - lb->vips[n_vips].backend_ips = xstrdup(node->value); - - struct nbrec_load_balancer_health_check *lb_health_check = NULL; - if (nbrec_lb->protocol && !strcmp(nbrec_lb->protocol, "sctp")) { - if (nbrec_lb->n_health_check > 0) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, - "SCTP load balancers do not currently support " - "health checks. Not creating health checks for " - "load balancer " UUID_FMT, - UUID_ARGS(&nbrec_lb->header_.uuid)); - } - } else { - for (size_t i = 0; i < nbrec_lb->n_health_check; i++) { - if (!strcmp(nbrec_lb->health_check[i]->vip, node->key)) { - lb_health_check = nbrec_lb->health_check[i]; - break; - } - } - } - - char *tokstr = xstrdup(node->value); - char *save_ptr = NULL; - char *token; - size_t n_backends = 0; - /* Format for a backend ips : IP1:port1,IP2:port2,...". */ - for (token = strtok_r(tokstr, ",", &save_ptr); - token != NULL; - token = strtok_r(NULL, ",", &save_ptr)) { - n_backends++; - } - - free(tokstr); - tokstr = xstrdup(node->value); - save_ptr = NULL; - - lb->vips[n_vips].n_backends = n_backends; - lb->vips[n_vips].backends = xcalloc(n_backends, - sizeof (struct lb_vip_backend)); - lb->vips[n_vips].health_check = lb_health_check ? true: false; - - size_t i = 0; - for (token = strtok_r(tokstr, ",", &save_ptr); - token != NULL; - token = strtok_r(NULL, ",", &save_ptr)) { - char *backend_ip; - uint16_t backend_port; - - if (!ip_address_and_port_from_lb_key(token, &backend_ip, - &backend_port, - &addr_family)) { - continue; - } + for (size_t j = 0; j < lb_vip->n_backends; j++) { + struct lb_vip_backend *backend = &lb_vip->backends[j]; - /* Get the logical port to which this ip belongs to. */ - struct ovn_port *op = NULL; - char *svc_mon_src_ip = NULL; - const char *s = smap_get(&nbrec_lb->ip_port_mappings, - backend_ip); - if (s) { - char *port_name = xstrdup(s); - char *p = strstr(port_name, ":"); - if (p) { - *p = 0; - p++; - op = ovn_port_find(ports, port_name); - svc_mon_src_ip = xstrdup(p); - } - free(port_name); - } - - lb->vips[n_vips].backends[i].ip = backend_ip; - lb->vips[n_vips].backends[i].port = backend_port; - lb->vips[n_vips].backends[i].addr_family = addr_family; - lb->vips[n_vips].backends[i].op = op; - lb->vips[n_vips].backends[i].svc_mon_src_ip = svc_mon_src_ip; - - if (lb_health_check && op && svc_mon_src_ip) { - const char *protocol = nbrec_lb->protocol; + if (backend->op && backend->svc_mon_src_ip) { + backend->health_check = true; + const char *protocol = lb->nlb->protocol; if (!protocol || !protocol[0]) { protocol = "tcp"; } - lb->vips[n_vips].backends[i].health_check = true; + backend->health_check = true; struct service_monitor_info *mon_info = - create_or_get_service_mon(ctx, monitor_map, backend_ip, - op->nbsp->name, backend_port, + create_or_get_service_mon(ctx, monitor_map, backend->ip, + backend->op->nbsp->name, + backend->port, protocol); ovs_assert(mon_info); sbrec_service_monitor_set_options( - mon_info->sbrec_mon, &lb_health_check->options); + mon_info->sbrec_mon, &lb_vip->lb_health_check->options); struct eth_addr ea; if (!mon_info->sbrec_mon->src_mac || !eth_addr_from_string(mon_info->sbrec_mon->src_mac, &ea) || @@ -3524,72 +3384,24 @@ ovn_lb_create(struct northd_context *ctx, struct hmap *lbs, } if (!mon_info->sbrec_mon->src_ip || - strcmp(mon_info->sbrec_mon->src_ip, svc_mon_src_ip)) { + strcmp(mon_info->sbrec_mon->src_ip, + backend->svc_mon_src_ip)) { sbrec_service_monitor_set_src_ip(mon_info->sbrec_mon, - svc_mon_src_ip); + backend->svc_mon_src_ip); } - lb->vips[n_vips].backends[i].sbrec_monitor = - mon_info->sbrec_mon; + lb_vip->backends[j].sbrec_monitor = mon_info->sbrec_mon; mon_info->required = true; - } else { - lb->vips[n_vips].backends[i].health_check = false; } - - i++; } - - free(tokstr); - n_vips++; - } - - char *proto = NULL; - if (nbrec_lb->protocol && nbrec_lb->protocol[0]) { - proto = nbrec_lb->protocol; } - - if (lb->nlb->n_selection_fields) { - struct ds sel_fields = DS_EMPTY_INITIALIZER; - for (size_t i = 0; i < lb->nlb->n_selection_fields; i++) { - char *field = lb->nlb->selection_fields[i]; - if (!strcmp(field, "tp_src") && proto) { - ds_put_format(&sel_fields, "%s_src,", proto); - } else if (!strcmp(field, "tp_dst") && proto) { - ds_put_format(&sel_fields, "%s_dst,", proto); - } else { - ds_put_format(&sel_fields, "%s,", field); - } - } - ds_chomp(&sel_fields, ','); - lb->selection_fields = ds_steal_cstr(&sel_fields); - } - - return lb; -} - -static void -ovn_lb_destroy(struct ovn_lb *lb) -{ - for (size_t i = 0; i < lb->n_vips; i++) { - free(lb->vips[i].vip); - free(lb->vips[i].backend_ips); - - for (size_t j = 0; j < lb->vips[i].n_backends; j++) { - free(lb->vips[i].backends[j].ip); - free(lb->vips[i].backends[j].svc_mon_src_ip); - } - - free(lb->vips[i].backends); - } - free(lb->vips); - free(lb->selection_fields); } static void build_lb_vip_ct_lb_actions(struct lb_vip *lb_vip, struct ds *action, char *selection_fields) { - if (lb_vip->health_check) { + if (lb_vip->lb_health_check) { ds_put_cstr(action, "ct_lb(backends="); size_t n_active_backends = 0; @@ -3644,7 +3456,12 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, const struct nbrec_load_balancer *nbrec_lb; NBREC_LOAD_BALANCER_FOR_EACH (nbrec_lb, ctx->ovnnb_idl) { - ovn_lb_create(ctx, lbs, nbrec_lb, ports, &monitor_map); + ovn_nb_lb_create(nbrec_lb, ports, lbs, (void *)ovn_port_find); + } + + struct ovn_lb *lb; + HMAP_FOR_EACH (lb, hmap_node, lbs) { + ovn_lb_svc_create(ctx, lb, &monitor_map); } struct service_monitor_info *mon_info; @@ -3658,16 +3475,6 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap *ports, hmap_destroy(&monitor_map); } -static void -destroy_ovn_lbs(struct hmap *lbs) -{ - struct ovn_lb *lb; - HMAP_FOR_EACH_POP (lb, hmap_node, lbs) { - ovn_lb_destroy(lb); - free(lb); - } -} - /* Updates the southbound Port_Binding table so that it contains the logical * switch ports specified by the northbound database. * @@ -5030,34 +4837,6 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) } } -/* For a 'key' of the form "IP:port" or just "IP", sets 'port' and - * 'ip_address'. The caller must free() the memory allocated for - * 'ip_address'. - * Returns true if parsing of 'key' was successful, false otherwise. - */ -static bool -ip_address_and_port_from_lb_key(const char *key, char **ip_address, - uint16_t *port, int *addr_family) -{ - struct sockaddr_storage ss; - if (!inet_parse_active(key, 0, &ss, false)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s", - key); - *ip_address = NULL; - *port = 0; - *addr_family = 0; - return false; - } - - struct ds s = DS_EMPTY_INITIALIZER; - ss_format_address_nobracks(&ss, &s); - *ip_address = ds_steal_cstr(&s); - *port = ss_get_port(&ss); - *addr_family = ss.ss_family; - return true; -} - /* * Returns true if logical switch is configured with DNS records, false * otherwise. @@ -7004,7 +6783,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, struct ovn_lb *lb; HMAP_FOR_EACH (lb, hmap_node, lbs) { for (size_t i = 0; i < lb->n_vips; i++) { - if (!lb->vips[i].health_check) { + if (!lb->vips[i].lb_health_check) { continue; } @@ -12358,8 +12137,7 @@ ovnnb_db_run(struct northd_context *ctx, sync_meters(ctx); sync_dns_entries(ctx, datapaths); sync_lb_entries(ctx, datapaths); - destroy_ovn_lbs(&lbs); - hmap_destroy(&lbs); + ovn_lbs_destroy(&lbs); struct ovn_igmp_group *igmp_group, *next_igmp_group;