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
> ---
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, >n_allocated_dps, sizeof *lb->dps);
+if (lb->n_allocated_nb_ls == lb->n_nb_ls) {
+lb->nb_ls = x2nrealloc(lb->nb_ls, >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(>ips_v4);
sset_destroy(>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,
>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, _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(, >nlb->options);
smap_replace(, "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 @@
