Re: [ovs-dev] [PATCH ovn 1/2] northd: sync lb applied to logical routers in sb db lb table
[...]
>
> There is a comment above that should be adjusted.
> "Delete any SB load balancer entries that refer to NB load balancers
> that don't exist anymore or are not applied to switches anymore."
> ->
> "Delete any SB load balancer entries that refer to NB load balancers
> that don't exist anymore or are not applied to switches/routers anymore."
>
> sbrec_load_balancer_delete(sbrec_lb);
ack, I will fix it.
Regards,
Lorenzo
> > continue;
> > }
> > @@ -4533,11 +4535,20 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
> > lb->slb = sbrec_lb;
> >
> > /* Find or create datapath group for this load balancer. */
> > -lb->dpg = ovn_dp_group_get_or_create(ovnsb_txn, &dp_groups,
> > - lb->slb->datapath_group,
> > - lb->n_nb_ls, lb->nb_ls_map,
> > - bitmap_len, true,
> > - ls_datapaths, NULL);
> > +if (lb->n_nb_ls) {
> > +lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn,
> > &ls_dp_groups,
> > +
> > lb->slb->datapath_group,
> > +lb->n_nb_ls,
> > lb->nb_ls_map,
> > +
> > ods_size(ls_datapaths),
> > +true, ls_datapaths,
> > NULL);
> > +}
> > +if (lb->n_nb_lr) {
> > +lb->lr_dpg = ovn_dp_group_get_or_create(ovnsb_txn,
> > &lr_dp_groups,
> > +
> > lb->slb->lr_datapath_group,
> > +lb->n_nb_lr,
> > lb->nb_lr_map,
> > +
> > ods_size(lr_datapaths),
> > +false, NULL,
> > lr_datapaths);
> > +}
> > }
> > hmapx_destroy(&existing_lbs);
> >
> > @@ -4545,7 +4556,7 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
> > * the SB load balancer columns. */
> > HMAP_FOR_EACH (lb, hmap_node, lbs) {
> >
> > -if (!lb->n_nb_ls) {
> > +if (!lb->n_nb_ls && !lb->n_nb_lr) {
> > continue;
> > }
> >
> > @@ -4568,19 +4579,33 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
> > }
> >
> > /* Find or create datapath group for this load balancer. */
> > -if (!lb->dpg) {
> > -lb->dpg = ovn_dp_group_get_or_create(ovnsb_txn, &dp_groups,
> > - lb->slb->datapath_group,
> > - lb->n_nb_ls,
> > lb->nb_ls_map,
> > - bitmap_len, true,
> > - ls_datapaths, NULL);
> > +if (!lb->ls_dpg && lb->n_nb_ls) {
> > +lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn,
> > &ls_dp_groups,
> > +
> > lb->slb->datapath_group,
> > +lb->n_nb_ls,
> > lb->nb_ls_map,
> > +
> > ods_size(ls_datapaths),
> > +true, ls_datapaths,
> > NULL);
> > +}
> > +if (!lb->lr_dpg && lb->n_nb_lr) {
> > +lb->lr_dpg = ovn_dp_group_get_or_create(ovnsb_txn,
> > &lr_dp_groups,
> > +
> > lb->slb->lr_datapath_group,
> > +lb->n_nb_lr,
> > lb->nb_lr_map,
> > +
> > ods_size(lr_datapaths),
> > +false, NULL,
> > lr_datapaths);
> > }
> >
> > /* Update columns. */
> > sbrec_load_balancer_set_name(lb->slb, lb->nlb->name);
> > sbrec_load_balancer_set_vips(lb->slb, ovn_northd_lb_get_vips(lb));
> > sbrec_load_balancer_set_protocol(lb->slb, lb->nlb->protocol);
> > -sbrec_load_balancer_set_datapath_group(lb->slb,
> > lb->dpg->dp_group);
> > +if (lb->ls_dpg) {
> > +sbrec_load_balancer_set_datapath_group(lb->slb,
> > + lb->ls_dpg->dp_group);
> > +}
> > +if (lb->lr_dpg) {
> > +sbrec_load_balancer_set_lr_datapath_group(lb->slb,
> > +
> > lb->lr_dpg->dp_group);
> > +}
> > sbrec_load_balancer_set_options(lb->slb, &options);
> > /* Clearing 'datapaths' column, since 'dp_group' is in use. */
> > sbrec_load_balancer_set_datapaths(lb->slb, NULL, 0);
> > @@ -4588,11 +4613,17 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
> > }
> >
> > struct ovn_dp_group *dpg;
> > -HMAP_FOR_EACH_POP (dpg, node, &dp_groups) {
> > +HMAP_FOR_EACH_POP (dpg, node, &ls_dp_groups) {
> > +bitmap_free(dpg->bitmap);
> > +free(dpg);
> > +}
> > +hmap_destroy(&ls_dp_groups);
> > +
> > +HMAP_FOR_EACH_POP (dpg, node, &lr_dp_groups) {
> > bitmap_free(dpg->bitmap);
> > free(dpg);
> > }
> > -hmap_destroy(&dp_groups);
> > +hmap_destroy(&lr_dp_groups);
> >
> > /* Datapath_Binding.load_bal
Re: [ovs-dev] [PATCH ovn 1/2] northd: sync lb applied to logical routers in sb db lb table
On Mon, Jun 19, 2023 at 4:43 PM Lorenzo Bianconi <
lorenzo.bianc...@redhat.com> wrote:
> Introduce lr_datapath_group column in the load_balancer table of the SB
> db.
> Sync load_balancers applied to logical_routers to Load_Balancer table in
> the SouthBound database.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2193323
> Signed-off-by: Lorenzo Bianconi
>
Hi Lorenzo,
I have one small comment down below.
---
> controller/local_data.c | 8 +++
> controller/ovn-controller.c | 6 ++
> lib/lb.h| 3 +-
> northd/northd.c | 78 ++--
> ovn-sb.ovsschema| 6 +-
> ovn-sb.xml | 6 ++
> tests/ovn-northd.at | 17 +-
> tests/system-ovn.at | 117
> 8 files changed, 218 insertions(+), 23 deletions(-)
>
> diff --git a/controller/local_data.c b/controller/local_data.c
> index cf0b21bb1..01cfbdefe 100644
> --- a/controller/local_data.c
> +++ b/controller/local_data.c
> @@ -670,5 +670,13 @@ lb_is_local(const struct sbrec_load_balancer
> *sbrec_lb,
> }
> }
>
> +dp_group = sbrec_lb->lr_datapath_group;
> +for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
> +if (get_local_datapath(local_datapaths,
> + dp_group->datapaths[i]->tunnel_key)) {
> +return true;
> +}
> +}
> +
> return false;
> }
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index a47406979..80e836e44 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2705,6 +2705,12 @@ load_balancers_by_dp_init(const struct hmap
> *local_datapaths,
> lb->datapath_group->datapaths[i],
> lb, lbs);
> }
> +for (size_t i = 0; lb->lr_datapath_group
> + && i < lb->lr_datapath_group->n_datapaths;
> i++) {
> +load_balancers_by_dp_add_one(local_datapaths,
> +
> lb->lr_datapath_group->datapaths[i],
> + lb, lbs);
> +}
> }
> return lbs;
> }
> diff --git a/lib/lb.h b/lib/lb.h
> index 23d8fc9e9..2456423cc 100644
> --- a/lib/lb.h
> +++ b/lib/lb.h
> @@ -85,7 +85,8 @@ struct ovn_northd_lb {
> size_t n_nb_lr;
> unsigned long *nb_lr_map;
>
> -struct ovn_dp_group *dpg;
> +struct ovn_dp_group *ls_dpg;
> +struct ovn_dp_group *lr_dpg;
> };
>
> struct ovn_lb_vip {
> diff --git a/northd/northd.c b/northd/northd.c
> index a45c8b53a..2203d6c4a 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -4497,10 +4497,11 @@ ovn_dp_group_get_or_create(struct ovsdb_idl_txn
> *ovnsb_txn,
> static void
> sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
> const struct sbrec_load_balancer_table
> *sbrec_load_balancer_table,
> - struct ovn_datapaths *ls_datapaths, struct hmap *lbs)
> + struct ovn_datapaths *ls_datapaths,
> + struct ovn_datapaths *lr_datapaths, struct hmap *lbs)
> {
> -struct hmap dp_groups = HMAP_INITIALIZER(&dp_groups);
> -size_t bitmap_len = ods_size(ls_datapaths);
> +struct hmap ls_dp_groups = HMAP_INITIALIZER(&ls_dp_groups);
> +struct hmap lr_dp_groups = HMAP_INITIALIZER(&lr_dp_groups);
> struct ovn_northd_lb *lb;
>
> /* Delete any stale SB load balancer rows and create datapath
> @@ -4525,7 +4526,8 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
> * are not indexed in any way.
> */
> lb = ovn_northd_lb_find(lbs, &lb_uuid);
> -if (!lb || !lb->n_nb_ls || !hmapx_add(&existing_lbs, lb)) {
> +if (!lb || (!lb->n_nb_ls && !lb->n_nb_lr) ||
> +!hmapx_add(&existing_lbs, lb)) {
>
There is a comment above that should be adjusted.
"Delete any SB load balancer entries that refer to NB load balancers
that don't exist anymore or are not applied to switches anymore."
->
"Delete any SB load balancer entries that refer to NB load balancers
that don't exist anymore or are not applied to switches/routers anymore."
sbrec_load_balancer_delete(sbrec_lb);
> continue;
> }
> @@ -4533,11 +4535,20 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
> lb->slb = sbrec_lb;
>
> /* Find or create datapath group for this load balancer. */
> -lb->dpg = ovn_dp_group_get_or_create(ovnsb_txn, &dp_groups,
> - lb->slb->datapath_group,
> - lb->n_nb_ls, lb->nb_ls_map,
> - bitmap_len, true,
> - ls_datapaths, NULL);
> +if (lb->n_nb_ls) {
> +lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn,
> &ls_dp_groups,
> +
> lb->slb->datapath_group,
> +lb->n_nb_ls,
> lb->nb_ls_map,
> +
> ods_size
[ovs-dev] [PATCH ovn 1/2] northd: sync lb applied to logical routers in sb db lb table
Introduce lr_datapath_group column in the load_balancer table of the SB
db.
Sync load_balancers applied to logical_routers to Load_Balancer table in
the SouthBound database.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2193323
Signed-off-by: Lorenzo Bianconi
---
controller/local_data.c | 8 +++
controller/ovn-controller.c | 6 ++
lib/lb.h| 3 +-
northd/northd.c | 78 ++--
ovn-sb.ovsschema| 6 +-
ovn-sb.xml | 6 ++
tests/ovn-northd.at | 17 +-
tests/system-ovn.at | 117
8 files changed, 218 insertions(+), 23 deletions(-)
diff --git a/controller/local_data.c b/controller/local_data.c
index cf0b21bb1..01cfbdefe 100644
--- a/controller/local_data.c
+++ b/controller/local_data.c
@@ -670,5 +670,13 @@ lb_is_local(const struct sbrec_load_balancer *sbrec_lb,
}
}
+dp_group = sbrec_lb->lr_datapath_group;
+for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
+if (get_local_datapath(local_datapaths,
+ dp_group->datapaths[i]->tunnel_key)) {
+return true;
+}
+}
+
return false;
}
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index a47406979..80e836e44 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -2705,6 +2705,12 @@ load_balancers_by_dp_init(const struct hmap
*local_datapaths,
lb->datapath_group->datapaths[i],
lb, lbs);
}
+for (size_t i = 0; lb->lr_datapath_group
+ && i < lb->lr_datapath_group->n_datapaths; i++) {
+load_balancers_by_dp_add_one(local_datapaths,
+ lb->lr_datapath_group->datapaths[i],
+ lb, lbs);
+}
}
return lbs;
}
diff --git a/lib/lb.h b/lib/lb.h
index 23d8fc9e9..2456423cc 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -85,7 +85,8 @@ struct ovn_northd_lb {
size_t n_nb_lr;
unsigned long *nb_lr_map;
-struct ovn_dp_group *dpg;
+struct ovn_dp_group *ls_dpg;
+struct ovn_dp_group *lr_dpg;
};
struct ovn_lb_vip {
diff --git a/northd/northd.c b/northd/northd.c
index a45c8b53a..2203d6c4a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4497,10 +4497,11 @@ ovn_dp_group_get_or_create(struct ovsdb_idl_txn
*ovnsb_txn,
static void
sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
const struct sbrec_load_balancer_table *sbrec_load_balancer_table,
- struct ovn_datapaths *ls_datapaths, struct hmap *lbs)
+ struct ovn_datapaths *ls_datapaths,
+ struct ovn_datapaths *lr_datapaths, struct hmap *lbs)
{
-struct hmap dp_groups = HMAP_INITIALIZER(&dp_groups);
-size_t bitmap_len = ods_size(ls_datapaths);
+struct hmap ls_dp_groups = HMAP_INITIALIZER(&ls_dp_groups);
+struct hmap lr_dp_groups = HMAP_INITIALIZER(&lr_dp_groups);
struct ovn_northd_lb *lb;
/* Delete any stale SB load balancer rows and create datapath
@@ -4525,7 +4526,8 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
* are not indexed in any way.
*/
lb = ovn_northd_lb_find(lbs, &lb_uuid);
-if (!lb || !lb->n_nb_ls || !hmapx_add(&existing_lbs, lb)) {
+if (!lb || (!lb->n_nb_ls && !lb->n_nb_lr) ||
+!hmapx_add(&existing_lbs, lb)) {
sbrec_load_balancer_delete(sbrec_lb);
continue;
}
@@ -4533,11 +4535,20 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
lb->slb = sbrec_lb;
/* Find or create datapath group for this load balancer. */
-lb->dpg = ovn_dp_group_get_or_create(ovnsb_txn, &dp_groups,
- lb->slb->datapath_group,
- lb->n_nb_ls, lb->nb_ls_map,
- bitmap_len, true,
- ls_datapaths, NULL);
+if (lb->n_nb_ls) {
+lb->ls_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &ls_dp_groups,
+lb->slb->datapath_group,
+lb->n_nb_ls, lb->nb_ls_map,
+ods_size(ls_datapaths),
+true, ls_datapaths, NULL);
+}
+if (lb->n_nb_lr) {
+lb->lr_dpg = ovn_dp_group_get_or_create(ovnsb_txn, &lr_dp_groups,
+lb->slb->lr_datapath_group,
+lb->n_nb_lr, lb->nb_lr_map,
+ods_size(lr_datapaths),
+false, NULL, lr_datapaths);
+}
}
h
