On 6/30/21 11:33 AM, Lorenzo Bianconi wrote:
> add logical routers datapath references in ovn_northd_lb data structure.
> This is a preliminary patch to invert the logic used during the lb flow
> creation in order to visit lb first and then related datapath.
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
Hi Lorenzo,
This change looks OK but I think we can unify a bit the logic between
router and switch load balancers already (I know you do some of that
in patch 7/9).
> lib/lb.c | 11 +++++++++++
> lib/lb.h | 6 ++++++
> northd/ovn-northd.c | 17 +++++++++++++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/lib/lb.c b/lib/lb.c
> index 4cb46b346..d24672b82 100644
> --- a/lib/lb.c
> +++ b/lib/lb.c
> @@ -245,6 +245,16 @@ ovn_northd_lb_add_datapath(struct ovn_northd_lb *lb,
> lb->dps[lb->n_dps++] = sb;
> }
>
> +void
> +ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od)
ovn_northd_lb_add_datapath() above does almost the same thing but for
logical switch datapaths.
I think you can rename ovn_northd_lb_add_datapath() to
ovn_northd_lb_add_ls() in this patch, adjust it a bit and drop patch
7/9. What do you think about the following incremental?
Regards,
Dumitru
diff --git a/lib/lb.c b/lib/lb.c
index d24672b82..578eff26b 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -236,13 +236,13 @@ ovn_northd_lb_find(struct hmap *lbs, const struct uuid
*uuid)
}
void
-ovn_northd_lb_add_datapath(struct ovn_northd_lb *lb,
- const struct sbrec_datapath_binding *sb)
+ovn_northd_lb_add_ls(struct ovn_northd_lb *lb, struct ovn_datapath *od)
{
- if (lb->n_allocated_dps == lb->n_dps) {
- lb->dps = x2nrealloc(lb->dps, &lb->n_allocated_dps, sizeof *lb->dps);
+ if (lb->n_allocated_nb_ls == lb->n_nb_ls) {
+ lb->nb_ls = x2nrealloc(lb->nb_ls, &lb->n_allocated_nb_ls,
+ sizeof *lb->nb_ls);
}
- lb->dps[lb->n_dps++] = sb;
+ lb->nb_ls[lb->n_nb_ls++] = od;
}
void
@@ -267,7 +267,7 @@ ovn_northd_lb_destroy(struct ovn_northd_lb *lb)
sset_destroy(&lb->ips_v4);
sset_destroy(&lb->ips_v6);
free(lb->selection_fields);
- free(lb->dps);
+ free(lb->nb_ls);
free(lb->nb_lr);
free(lb);
}
diff --git a/lib/lb.h b/lib/lb.h
index 4e8fd6604..c06114ea4 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -42,13 +42,13 @@ struct ovn_northd_lb {
struct sset ips_v4;
struct sset ips_v6;
- size_t n_dps;
- size_t n_allocated_dps;
- const struct sbrec_datapath_binding **dps;
+ size_t n_nb_ls;
+ size_t n_allocated_nb_ls;
+ const struct ovn_datapath **nb_ls;
size_t n_nb_lr;
size_t n_allocated_nb_lr;
- struct ovn_datapath **nb_lr;
+ const struct ovn_datapath **nb_lr;
};
struct ovn_lb_vip {
@@ -87,8 +87,7 @@ 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_datapath(struct ovn_northd_lb *,
- const struct sbrec_datapath_binding *);
+void ovn_northd_lb_add_ls(struct ovn_northd_lb *, struct ovn_datapath *od);
void
ovn_northd_lb_add_lr(struct ovn_northd_lb *lb, struct ovn_datapath *od);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 95cef812e..b7ca841c4 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3410,7 +3410,7 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap
*datapaths,
&od->nbs->load_balancer[i]->header_.uuid;
lb = ovn_northd_lb_find(lbs, lb_uuid);
- ovn_northd_lb_add_datapath(lb, od->sb);
+ ovn_northd_lb_add_ls(lb, od);
}
}
@@ -3442,7 +3442,7 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap
*datapaths,
}
lb = ovn_northd_lb_find(lbs, &lb_uuid);
- if (lb && lb->n_dps) {
+ if (lb && lb->n_nb_ls) {
lb->slb = sbrec_lb;
} else {
sbrec_load_balancer_delete(sbrec_lb);
@@ -3453,7 +3453,7 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap
*datapaths,
* the SB load balancer columns. */
HMAP_FOR_EACH (lb, hmap_node, lbs) {
- if (!lb->n_dps) {
+ if (!lb->n_nb_ls) {
continue;
}
@@ -3464,6 +3464,13 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap
*datapaths,
smap_clone(&options, &lb->nlb->options);
smap_replace(&options, "hairpin_orig_tuple", "true");
+ struct sbrec_datapath_binding **lb_dps =
+ xmalloc(lb->n_nb_ls * sizeof *lb_dps);
+ for (size_t i = 0; i < lb->n_nb_ls; i++) {
+ lb_dps[i] = CONST_CAST(struct sbrec_datapath_binding *,
+ lb->nb_ls[i]->sb);
+ }
+
if (!lb->slb) {
sbrec_lb = sbrec_load_balancer_insert(ctx->ovnsb_txn);
lb->slb = sbrec_lb;
@@ -3477,11 +3484,10 @@ build_ovn_lbs(struct northd_context *ctx, struct hmap
*datapaths,
sbrec_load_balancer_set_name(lb->slb, lb->nlb->name);
sbrec_load_balancer_set_vips(lb->slb, &lb->nlb->vips);
sbrec_load_balancer_set_protocol(lb->slb, lb->nlb->protocol);
+ sbrec_load_balancer_set_datapaths(lb->slb, lb_dps, lb->n_nb_ls);
sbrec_load_balancer_set_options(lb->slb, &options);
- sbrec_load_balancer_set_datapaths(
- lb->slb, (struct sbrec_datapath_binding **)lb->dps,
- lb->n_dps);
smap_destroy(&options);
+ free(lb_dps);
}
/* Set the list of associated load balanacers to a logical switch
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
