Re: [ovs-dev] [PATCH ovn v2 2/7] northd: Refactor load balancer vip parsing.
On Tue, Nov 3, 2020 at 3:14 PM Numan Siddique wrote: > > On Fri, Oct 30, 2020 at 1:52 AM Dumitru Ceara wrote: > > > > On 10/27/20 6:16 PM, num...@ovn.org wrote: > > > From: Numan Siddique > > > > > > 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 > > > --- > > > 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 00..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 > > > + > > > +#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, , , > > > + _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, ",", _ptr); > > > +token != NULL; > > > +token = strtok_r(NULL, ",", _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, ",", _ptr); > > > +token != NULL; > > > +token = strtok_r(NULL, ",", _ptr)) { > > > +char *backend_ip; > > > +uint16_t backend_port; > > > + > > > +if
Re: [ovs-dev] [PATCH ovn v2 2/7] northd: Refactor load balancer vip parsing.
On Fri, Oct 30, 2020 at 1:52 AM Dumitru Ceara wrote: > > On 10/27/20 6:16 PM, num...@ovn.org wrote: > > From: Numan Siddique > > > > 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 > > --- > > 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 00..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 > > + > > +#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, , , > > + _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, ",", _ptr); > > +token != NULL; > > +token = strtok_r(NULL, ",", _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, ",", _ptr); > > +token != NULL; > > +token = strtok_r(NULL, ",", _ptr)) { > > +char *backend_ip; > > +uint16_t backend_port; > > + > > +if (!ip_address_and_port_from_key(token, _ip, > > + _port, > > + _family)) { > > +continue; > > +} > > + > > +lb->vips[n_vips].backends[i].ip = backend_ip; > > +lb->vips[n_vips].backends[i].port = backend_port; > > +
Re: [ovs-dev] [PATCH ovn v2 2/7] northd: Refactor load balancer vip parsing.
On 10/27/20 6:16 PM, num...@ovn.org wrote: > From: Numan Siddique > > 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 > --- > 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 00..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 > + > +#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, , , > + _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, ",", _ptr); > +token != NULL; > +token = strtok_r(NULL, ",", _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, ",", _ptr); > +token != NULL; > +token = strtok_r(NULL, ",", _ptr)) { > +char *backend_ip; > +uint16_t backend_port; > + > +if (!ip_address_and_port_from_key(token, _ip, > + _port, > + _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 =
[ovs-dev] [PATCH ovn v2 2/7] northd: Refactor load balancer vip parsing.
From: Numan Siddique 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 --- 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 00..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 + +#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, , , + _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, ",", _ptr); +token != NULL; +token = strtok_r(NULL, ",", _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, ",", _ptr); +token != NULL; +token = strtok_r(NULL, ",", _ptr)) { +char *backend_ip; +uint16_t backend_port; + +if (!ip_address_and_port_from_key(token, _ip, + _port, + _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(_lb->vips); +hmap_insert(lbs, >hmap_node, uuid_hash(_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 = >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 =