[ovs-dev] [PATCH ovn v2 2/7] northd: Refactor load balancer vip parsing.
Numan Siddique
numans at ovn.org
Tue Nov 3 09:44:23 UTC 2020
On Fri, Oct 30, 2020 at 1:52 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> On 10/27/20 6:16 PM, numans at ovn.org wrote:
> > From: Numan Siddique <numans at 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 at 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 at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list