| Message ID | 20220907202616.624528-3-i.maximets@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | northd: Optimize preparation of load balancers. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 9/7/22 22:26, Ilya Maximets wrote: > Filling arrays of switches and routers for load balancers > one-by-one is not very efficient. Copying them in bulk > allows to save a noticeable amount of time in setups with > large load balancer groups. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > lib/lb.c | 16 ++++++++++------ > lib/lb.h | 15 +++++++++++---- > northd/northd.c | 30 +++++++++++++++++++++++------- > 3 files changed, 44 insertions(+), 17 deletions(-) > > diff --git a/lib/lb.c b/lib/lb.c > index 6fb06bf87..f5c342c06 100644 > --- a/lib/lb.c > +++ b/lib/lb.c > @@ -239,23 +239,27 @@ ovn_northd_lb_find(struct hmap *lbs, const struct uuid *uuid) > } > > void > -ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od) > +ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, size_t n, > + struct ovn_datapath **ods) > { > - if (lb->n_allocated_nb_lr == lb->n_nb_lr) { > + while (lb->n_allocated_nb_lr <= lb->n_nb_lr + n) { > lb->nb_lr = x2nrealloc(lb->nb_lr, &lb->n_allocated_nb_lr, > sizeof *lb->nb_lr); > } > - lb->nb_lr[lb->n_nb_lr++] = od; > + memcpy(&lb->nb_lr[lb->n_nb_lr], ods, n * sizeof *ods); > + lb->n_nb_lr += n; > } > > void > -ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, struct ovn_datapath *od) > +ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, size_t n, > + struct ovn_datapath **ods) > { > - if (lb->n_allocated_nb_ls == lb->n_nb_ls) { > + while (lb->n_allocated_nb_ls <= lb->n_nb_ls + n) { > lb->nb_ls = x2nrealloc(lb->nb_ls, &lb->n_allocated_nb_ls, > sizeof *lb->nb_ls); > } > - lb->nb_ls[lb->n_nb_ls++] = od; > + memcpy(&lb->nb_ls[lb->n_nb_ls], ods, n * sizeof *ods); > + lb->n_nb_ls += n; > } > > void > diff --git a/lib/lb.h b/lib/lb.h > index e7b2fc61e..9f4fbdfa9 100644 > --- a/lib/lb.h > +++ b/lib/lb.h > @@ -27,6 +27,7 @@ > struct nbrec_load_balancer; > struct sbrec_load_balancer; > struct sbrec_datapath_binding; > +struct ovn_datapath; > struct ovn_port; > struct uuid; > > @@ -89,16 +90,22 @@ struct ovn_northd_lb_backend { > struct ovn_northd_lb *ovn_northd_lb_create(const struct nbrec_load_balancer *); > struct ovn_northd_lb * ovn_northd_lb_find(struct hmap *, const struct uuid *); > void ovn_northd_lb_destroy(struct ovn_northd_lb *); > -void > -ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od); > -void > -ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, struct ovn_datapath *od); > +void ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, size_t n, > + struct ovn_datapath **ods); > +void ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, size_t n, > + struct ovn_datapath **ods); > > struct ovn_lb_group { > struct hmap_node hmap_node; > struct uuid uuid; > size_t n_lbs; > struct ovn_northd_lb **lbs; > + > + /* Datapaths to which this LB group is applied. */ > + size_t n_ls; > + struct ovn_datapath **ls; > + size_t n_lr; > + struct ovn_datapath **lr; > }; > > struct ovn_lb_group *ovn_lb_group_find(struct hmap *lb_groups, > diff --git a/northd/northd.c b/northd/northd.c > index fbf4c8d6c..2cb3316cd 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -3866,6 +3866,8 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, > lb_group->uuid = nbrec_lb_group->header_.uuid; > lb_group->n_lbs = nbrec_lb_group->n_load_balancer; > lb_group->lbs = xmalloc(lb_group->n_lbs * sizeof *lb_group->lbs); > + lb_group->ls = xmalloc(hmap_count(datapaths) * sizeof *lb_group->ls); > + lb_group->lr = xmalloc(hmap_count(datapaths) * sizeof *lb_group->lr); > > for (size_t i = 0; i < nbrec_lb_group->n_load_balancer; i++) { > const struct uuid *lb_uuid = > @@ -3887,16 +3889,21 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, > const struct uuid *lb_uuid = > &od->nbs->load_balancer[i]->header_.uuid; > lb = ovn_northd_lb_find(lbs, lb_uuid); > - ovn_northd_lb_add_ls(lb, od); > + ovn_northd_lb_add_ls(lb, 1, &od); > } > > for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) { > nbrec_lb_group = od->nbs->load_balancer_group[i]; > lb_group = ovn_lb_group_find(lb_groups, > &nbrec_lb_group->header_.uuid); > - for (size_t j = 0; j < lb_group->n_lbs; j++) { > - ovn_northd_lb_add_ls(lb_group->lbs[j], od); > - } > + lb_group->ls[lb_group->n_ls++] = od; Nit: similar to the comments from patch 1/4, I'd add a small (inline?) api to lib/lb.h to add a datapath to the set of switches/routers a lb_group is applied to. E.g., ovn_lb_group_add_ls() and ovn_lb_group_add_lr(). > + } > + } > + > + HMAP_FOR_EACH (lb_group, hmap_node, lb_groups) { > + for (size_t j = 0; j < lb_group->n_lbs; j++) { > + ovn_northd_lb_add_ls(lb_group->lbs[j], lb_group->n_ls, > + lb_group->ls); > } > } > > @@ -3909,7 +3916,7 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, > const struct uuid *lb_uuid = > &od->nbr->load_balancer[i]->header_.uuid; > lb = ovn_northd_lb_find(lbs, lb_uuid); > - ovn_northd_lb_add_lr(lb, od); > + ovn_northd_lb_add_lr(lb, 1, &od); > build_lrouter_lb_ips(od, lb); > } > > @@ -3917,12 +3924,19 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, > nbrec_lb_group = od->nbr->load_balancer_group[i]; > lb_group = ovn_lb_group_find(lb_groups, > &nbrec_lb_group->header_.uuid); > + lb_group->lr[lb_group->n_lr++] = od; Here too. > for (size_t j = 0; j < lb_group->n_lbs; j++) { > - ovn_northd_lb_add_lr(lb_group->lbs[j], od); > build_lrouter_lb_ips(od, lb_group->lbs[j]); > } > } > } > + > + HMAP_FOR_EACH (lb_group, hmap_node, lb_groups) { > + for (size_t j = 0; j < lb_group->n_lbs; j++) { > + ovn_northd_lb_add_lr(lb_group->lbs[j], lb_group->n_lr, > + lb_group->lr); > + } > + } > } > > static void > @@ -4107,7 +4121,7 @@ build_lswitch_lbs_from_lrouter(struct hmap *datapaths, struct hmap *lbs) > } > } > if (!installed) { > - ovn_northd_lb_add_ls(lb, od); > + ovn_northd_lb_add_ls(lb, 1, &od); > } > if (lb->nlb) { > od->has_lb_vip |= lb_has_vip(lb->nlb); > @@ -15414,6 +15428,8 @@ northd_destroy(struct northd_data *data) > struct ovn_lb_group *lb_group; > HMAP_FOR_EACH_POP (lb_group, hmap_node, &data->lb_groups) { > free(lb_group->lbs); > + free(lb_group->ls); > + free(lb_group->lr); If you add a ovn_lb_group_destroy() in patch 1/4 then this part should be added to that function instead. > free(lb_group); > } > hmap_destroy(&data->lb_groups); The comments above are minor, and if you address them feel free to include my ack in the next revision: Acked-by: Dumitru Ceara <dceara@redhat.com>
diff --git a/lib/lb.c b/lib/lb.c index 6fb06bf87..f5c342c06 100644 --- a/lib/lb.c +++ b/lib/lb.c @@ -239,23 +239,27 @@ ovn_northd_lb_find(struct hmap *lbs, const struct uuid *uuid) } void -ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od) +ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, size_t n, + struct ovn_datapath **ods) { - if (lb->n_allocated_nb_lr == lb->n_nb_lr) { + while (lb->n_allocated_nb_lr <= lb->n_nb_lr + n) { lb->nb_lr = x2nrealloc(lb->nb_lr, &lb->n_allocated_nb_lr, sizeof *lb->nb_lr); } - lb->nb_lr[lb->n_nb_lr++] = od; + memcpy(&lb->nb_lr[lb->n_nb_lr], ods, n * sizeof *ods); + lb->n_nb_lr += n; } void -ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, struct ovn_datapath *od) +ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, size_t n, + struct ovn_datapath **ods) { - if (lb->n_allocated_nb_ls == lb->n_nb_ls) { + while (lb->n_allocated_nb_ls <= lb->n_nb_ls + n) { lb->nb_ls = x2nrealloc(lb->nb_ls, &lb->n_allocated_nb_ls, sizeof *lb->nb_ls); } - lb->nb_ls[lb->n_nb_ls++] = od; + memcpy(&lb->nb_ls[lb->n_nb_ls], ods, n * sizeof *ods); + lb->n_nb_ls += n; } void diff --git a/lib/lb.h b/lib/lb.h index e7b2fc61e..9f4fbdfa9 100644 --- a/lib/lb.h +++ b/lib/lb.h @@ -27,6 +27,7 @@ struct nbrec_load_balancer; struct sbrec_load_balancer; struct sbrec_datapath_binding; +struct ovn_datapath; struct ovn_port; struct uuid; @@ -89,16 +90,22 @@ struct ovn_northd_lb_backend { struct ovn_northd_lb *ovn_northd_lb_create(const struct nbrec_load_balancer *); struct ovn_northd_lb * ovn_northd_lb_find(struct hmap *, const struct uuid *); void ovn_northd_lb_destroy(struct ovn_northd_lb *); -void -ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od); -void -ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, struct ovn_datapath *od); +void ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, size_t n, + struct ovn_datapath **ods); +void ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, size_t n, + struct ovn_datapath **ods); struct ovn_lb_group { struct hmap_node hmap_node; struct uuid uuid; size_t n_lbs; struct ovn_northd_lb **lbs; + + /* Datapaths to which this LB group is applied. */ + size_t n_ls; + struct ovn_datapath **ls; + size_t n_lr; + struct ovn_datapath **lr; }; struct ovn_lb_group *ovn_lb_group_find(struct hmap *lb_groups, diff --git a/northd/northd.c b/northd/northd.c index fbf4c8d6c..2cb3316cd 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3866,6 +3866,8 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, lb_group->uuid = nbrec_lb_group->header_.uuid; lb_group->n_lbs = nbrec_lb_group->n_load_balancer; lb_group->lbs = xmalloc(lb_group->n_lbs * sizeof *lb_group->lbs); + lb_group->ls = xmalloc(hmap_count(datapaths) * sizeof *lb_group->ls); + lb_group->lr = xmalloc(hmap_count(datapaths) * sizeof *lb_group->lr); for (size_t i = 0; i < nbrec_lb_group->n_load_balancer; i++) { const struct uuid *lb_uuid = @@ -3887,16 +3889,21 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, const struct uuid *lb_uuid = &od->nbs->load_balancer[i]->header_.uuid; lb = ovn_northd_lb_find(lbs, lb_uuid); - ovn_northd_lb_add_ls(lb, od); + ovn_northd_lb_add_ls(lb, 1, &od); } for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) { nbrec_lb_group = od->nbs->load_balancer_group[i]; lb_group = ovn_lb_group_find(lb_groups, &nbrec_lb_group->header_.uuid); - for (size_t j = 0; j < lb_group->n_lbs; j++) { - ovn_northd_lb_add_ls(lb_group->lbs[j], od); - } + lb_group->ls[lb_group->n_ls++] = od; + } + } + + HMAP_FOR_EACH (lb_group, hmap_node, lb_groups) { + for (size_t j = 0; j < lb_group->n_lbs; j++) { + ovn_northd_lb_add_ls(lb_group->lbs[j], lb_group->n_ls, + lb_group->ls); } } @@ -3909,7 +3916,7 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, const struct uuid *lb_uuid = &od->nbr->load_balancer[i]->header_.uuid; lb = ovn_northd_lb_find(lbs, lb_uuid); - ovn_northd_lb_add_lr(lb, od); + ovn_northd_lb_add_lr(lb, 1, &od); build_lrouter_lb_ips(od, lb); } @@ -3917,12 +3924,19 @@ build_lbs(struct northd_input *input_data, struct hmap *datapaths, nbrec_lb_group = od->nbr->load_balancer_group[i]; lb_group = ovn_lb_group_find(lb_groups, &nbrec_lb_group->header_.uuid); + lb_group->lr[lb_group->n_lr++] = od; for (size_t j = 0; j < lb_group->n_lbs; j++) { - ovn_northd_lb_add_lr(lb_group->lbs[j], od); build_lrouter_lb_ips(od, lb_group->lbs[j]); } } } + + HMAP_FOR_EACH (lb_group, hmap_node, lb_groups) { + for (size_t j = 0; j < lb_group->n_lbs; j++) { + ovn_northd_lb_add_lr(lb_group->lbs[j], lb_group->n_lr, + lb_group->lr); + } + } } static void @@ -4107,7 +4121,7 @@ build_lswitch_lbs_from_lrouter(struct hmap *datapaths, struct hmap *lbs) } } if (!installed) { - ovn_northd_lb_add_ls(lb, od); + ovn_northd_lb_add_ls(lb, 1, &od); } if (lb->nlb) { od->has_lb_vip |= lb_has_vip(lb->nlb); @@ -15414,6 +15428,8 @@ northd_destroy(struct northd_data *data) struct ovn_lb_group *lb_group; HMAP_FOR_EACH_POP (lb_group, hmap_node, &data->lb_groups) { free(lb_group->lbs); + free(lb_group->ls); + free(lb_group->lr); free(lb_group); } hmap_destroy(&data->lb_groups);
Filling arrays of switches and routers for load balancers one-by-one is not very efficient. Copying them in bulk allows to save a noticeable amount of time in setups with large load balancer groups. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- lib/lb.c | 16 ++++++++++------ lib/lb.h | 15 +++++++++++---- northd/northd.c | 30 +++++++++++++++++++++++------- 3 files changed, 44 insertions(+), 17 deletions(-)