On Mon, Jan 8, 2024 at 3:52 PM Numan Siddique <num...@ovn.org> wrote:
>
> On Fri, Jan 5, 2024 at 11:36 AM Numan Siddique <num...@ovn.org> wrote:
> >
> > On Tue, Dec 19, 2023 at 2:37 AM Han Zhou <hz...@ovn.org> wrote:
> > >
> > > On Mon, Nov 27, 2023 at 6:39 PM <num...@ovn.org> wrote:
> > > >
> > > > From: Numan Siddique <num...@ovn.org>
> > > >
> > > > Any changes to northd engine node due to load balancers
> > > > are now handled in 'sync_to_sb_lb' node to sync the changed
> > > > load balancers to SB load balancers.
> > > >
> > > > The logic to sync the SB load balancers is changed a bit and it
> > > > now mimics the SB lflow sync.
> > > >
> > > > Below are the scale testing results done with all the patches
applied
> > > > in this series using ovn-heater. The test ran the scenario -
> > > > ocp-500-density-heavy.yml [1].
> > > >
> > > > The resuts are:
> > > >
> > > >
> > >
-------------------------------------------------------------------------------------------------------------------------------------------------------
> > > > Min (s) Median (s) 90%ile (s)
> > > 99%ile (s) Max (s) Mean (s) Total (s) Count
> > > Failed
> > > >
> > >
-------------------------------------------------------------------------------------------------------------------------------------------------------
> > > > Iteration Total 0.136883 1.129016 1.192001
> > > 1.204167 1.212728 0.665017 83.127099 125
0
> > > > Namespace.add_ports 0.005216 0.005736 0.007034
> > > 0.015486 0.018978 0.006211 0.776373 125
0
> > > > WorkerNode.bind_port 0.035030 0.046082 0.052469
> > > 0.058293 0.060311 0.045973 11.493259 250
0
> > > > WorkerNode.ping_port 0.005057 0.006727 1.047692
> > > 1.069253 1.071336 0.266896 66.724094 250
0
> > > >
> > >
-------------------------------------------------------------------------------------------------------------------------------------------------------
> > > >
> > > > The results with the present main [2] are:
> > > >
> > > >
> > >
-------------------------------------------------------------------------------------------------------------------------------------------------------
> > > > Min (s) Median (s) 90%ile (s)
> > > 99%ile (s) Max (s) Mean (s) Total (s) Count
> > > Failed
> > > >
> > >
-------------------------------------------------------------------------------------------------------------------------------------------------------
> > > > Iteration Total 0.135491 2.223805 3.311270
> > > 3.339078 3.345346 1.729172 216.146495 125
0
> > > > Namespace.add_ports 0.005380 0.005744 0.006819
> > > 0.018773 0.020800 0.006292 0.786532 125
0
> > > > WorkerNode.bind_port 0.034179 0.046055 0.053488
> > > 0.058801 0.071043 0.046117 11.529311 250
0
> > > > WorkerNode.ping_port 0.004956 0.006952 3.086952
> > > 3.191743 3.192807 0.791544 197.886026 250
0
> > > >
> > >
-------------------------------------------------------------------------------------------------------------------------------------------------------
> > > >
> > > > [1] -
> > >
https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml
> > > > [2] - 2a12cda890a7("controller, northd: Wait for cleanup before
replying
> > > to exit")
> > > >
> > > > Signed-off-by: Numan Siddique <num...@ovn.org>
> > > > ---
> > > > northd/en-sync-sb.c | 445
+++++++++++++++++++++++++++++++++++++--
> > > > northd/inc-proc-northd.c | 1 +
> > > > northd/lflow-mgr.c | 196 ++++++-----------
> > > > northd/lflow-mgr.h | 23 +-
> > > > northd/northd.c | 238 ---------------------
> > > > northd/northd.h | 6 -
> > > > tests/ovn-northd.at | 103 +++++----
> > > > 7 files changed, 585 insertions(+), 427 deletions(-)
> > > >
> > > > diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
> > > > index 73b30272c4..62c5dbd20f 100644
> > > > --- a/northd/en-sync-sb.c
> > > > +++ b/northd/en-sync-sb.c
> > > > @@ -30,6 +30,7 @@
> > > > #include "lib/ovn-nb-idl.h"
> > > > #include "lib/ovn-sb-idl.h"
> > > > #include "lib/ovn-util.h"
> > > > +#include "lflow-mgr.h"
> > > > #include "northd.h"
> > > >
> > > > #include "openvswitch/vlog.h"
> > > > @@ -53,6 +54,38 @@ static void build_port_group_address_set(const
struct
> > > nbrec_port_group *,
> > > > struct svec *ipv4_addrs,
> > > > struct svec *ipv6_addrs);
> > > >
> > > > +struct sb_lb_table;
> > > > +struct sb_lb_record;
> > > > +static void sb_lb_table_init(struct sb_lb_table *);
> > > > +static void sb_lb_table_clear(struct sb_lb_table *);
> > > > +static struct sb_lb_record *sb_lb_table_find(struct hmap *sb_lbs,
> > > > + const struct uuid *);
> > > > +static void sb_lb_table_build_and_sync(struct sb_lb_table *,
> > > > + struct ovsdb_idl_txn *ovnsb_txn,
> > > > + const struct
sbrec_load_balancer_table *,
> > > > + const struct
> > > sbrec_logical_dp_group_table *,
> > > > + struct hmap *lb_dps_map,
> > > > + struct ovn_datapaths *ls_datapaths,
> > > > + struct ovn_datapaths *lr_datapaths,
> > > > + struct chassis_features *);
> > > > +static void sync_sb_lb_record(struct sb_lb_record *,
> > > > + const struct sbrec_load_balancer *,
> > > > + const struct
sbrec_logical_dp_group_table
> > > *,
> > > > + struct sb_lb_table *,
> > > > + struct ovsdb_idl_txn *ovnsb_txn,
> > > > + struct ovn_datapaths *ls_datapaths,
> > > > + struct ovn_datapaths *lr_datapaths,
> > > > + struct chassis_features *);
> > > > +static void sync_changed_lbs(struct sb_lb_table *,
> > > > + struct ovsdb_idl_txn *ovnsb_txn,
> > > > + const struct
sbrec_load_balancer_table *,
> > > > + const struct
sbrec_logical_dp_group_table *,
> > > > + struct tracked_lbs *,
> > > > + struct ovn_datapaths *ls_datapaths,
> > > > + struct ovn_datapaths *lr_datapaths,
> > > > + struct chassis_features *);
> > > > +static bool check_sb_lb_duplicates(const struct
> > > sbrec_load_balancer_table *);
> > > > +
> > > > void *
> > > > en_sync_to_sb_init(struct engine_node *node OVS_UNUSED,
> > > > struct engine_arg *arg OVS_UNUSED)
> > > > @@ -211,51 +244,103 @@
sync_to_sb_addr_set_nb_port_group_handler(struct
> > > engine_node *node,
> > > > /* sync_to_sb_lb engine node functions.
> > > > * This engine node syncs the SB load balancers.
> > > > */
> > > > +struct sb_lb_record {
> > > > + struct hmap_node key_node; /* Index on 'nblb->header_.uuid'.
*/
> > > > +
> > > > + struct ovn_lb_datapaths *lb_dps;
> > > > + const struct sbrec_load_balancer *sbrec_lb;
> > > > + struct ovn_dp_group *ls_dpg;
> > > > + struct ovn_dp_group *lr_dpg;
> > > > + struct uuid sb_uuid;
> > > > +};
> > > > +
> > > > +struct sb_lb_table {
> > > > + struct hmap entries; /* Stores struct sb_lb_record. */
> > > > + struct hmap ls_dp_groups;
> > > > + struct hmap lr_dp_groups;
> > > > +};
> > > > +
> > > > +struct ed_type_sync_to_sb_lb_data {
> > > > + struct sb_lb_table sb_lbs;
> > > > +};
> > > > +
> > >
> > > This patch defines the data of the en_sync_to_sb_lb_data node, which
is
> > > one-to-one mapping with the ovn_lb_datapaths records of
> > > the lb_datapaths_map from the northd node. I wonder if we should
extract
> > > the lb_datapaths_map from northd node and combine with this node. The
node
> > > will be input of the lflow node for generating LB related flows. One
> > > obvious benefit is that it could avoid some replications (one node, so
> > > maintain just one table) and avoid creating and managing redundant
> > > dp_groups between the LB lflows and SB LB.
> > > Is this something you considered already? Any drawbacks?
> >
> > I think its a good idea. For each lb_datapaths we need to set the
> > datapath id bits
> > correctly. I didn't look into this approach in v4 as it would take
> > some effort to implement this.
> > I'll try to address this in v5 or perhaps as a follow up patch.
> >
> > Presently we have "lb_data" engine node and with this patch
> > "en_sync_sb_lb" node. Perhaps
> > we can have just one node "lb_data". But that would require that we
> > also allocate the datapath id
> > for each logical swithc/logical router in a separate engine node
> > (before northd engine node).
> > If we take this approach we could also handle logical switch and
> > logical router deletions incrementally
> > in the future more easily if required. But I'm not sure if we want to
> > go that way as it may complicate
> > the I-P handling further. Any thoughts on this ?
> >
> > I'll take a look at your suggested approach and see if its possible to
> > do in v5. But I don't want it to block
> > this entire series. If it is straightforward enough I'll try to
> > include in v5. Otherwise I'd prefer it to be a follow up patch.
> >
> >
> > >
> > > > void *
> > > > en_sync_to_sb_lb_init(struct engine_node *node OVS_UNUSED,
> > > > struct engine_arg *arg OVS_UNUSED)
> > > > {
> > > > - return NULL;
> > > > + struct ed_type_sync_to_sb_lb_data *data = xzalloc(sizeof
*data);
> > > > + sb_lb_table_init(&data->sb_lbs);
> > > > +
> > > > + return data;
> > > > }
> > > >
> > > > void
> > > > -en_sync_to_sb_lb_run(struct engine_node *node, void *data
OVS_UNUSED)
> > > > +en_sync_to_sb_lb_run(struct engine_node *node, void *data_)
> > > > {
> > > > + struct northd_data *northd_data =
engine_get_input_data("northd",
> > > node);
> > > > const struct sbrec_load_balancer_table *sb_load_balancer_table
=
> > > > EN_OVSDB_GET(engine_get_input("SB_load_balancer", node));
> > > > + const struct sbrec_logical_dp_group_table *sb_dpgrp_table =
> > > > + EN_OVSDB_GET(engine_get_input("SB_logical_dp_group",
node));
> > > > struct ed_type_global_config *global_config =
> > > > engine_get_input_data("global_config", node);
> > > > +
> > > > const struct engine_context *eng_ctx = engine_get_context();
> > > > - struct northd_data *northd_data =
engine_get_input_data("northd",
> > > node);
> > > > + struct ed_type_sync_to_sb_lb_data *data = data_;
> > > > +
> > > > + sb_lb_table_clear(&data->sb_lbs);
> > > > + sb_lb_table_init(&data->sb_lbs);
> > > > + sb_lb_table_build_and_sync(&data->sb_lbs,
eng_ctx->ovnsb_idl_txn,
> > > > + sb_load_balancer_table,
> > > > + sb_dpgrp_table,
> > > > + &northd_data->lb_datapaths_map,
> > > > + &northd_data->ls_datapaths,
> > > > + &northd_data->lr_datapaths,
> > > > + &global_config->features);
> > > >
> > > > - sync_lbs(eng_ctx->ovnsb_idl_txn, sb_load_balancer_table,
> > > > - &northd_data->ls_datapaths,
&northd_data->lr_datapaths,
> > > > - &northd_data->lb_datapaths_map,
&global_config->features);
> > > > engine_set_node_state(node, EN_UPDATED);
> > > > }
> > > >
> > > > void
> > > > -en_sync_to_sb_lb_cleanup(void *data OVS_UNUSED)
> > > > +en_sync_to_sb_lb_cleanup(void *data_)
> > > > {
> > > > -
> > > > + struct ed_type_sync_to_sb_lb_data *data = data_;
> > > > + sb_lb_table_clear(&data->sb_lbs);
> > > > }
> > > >
> > > > bool
> > > > -sync_to_sb_lb_northd_handler(struct engine_node *node, void *data
> > > OVS_UNUSED)
> > > > +sync_to_sb_lb_northd_handler(struct engine_node *node, void *data_)
> > > > {
> > > > struct northd_data *nd = engine_get_input_data("northd", node);
> > > >
> > > > - if (!northd_has_tracked_data(&nd->trk_data) ||
> > > > - northd_has_lbs_in_tracked_data(&nd->trk_data)) {
> > > > + if (!northd_has_tracked_data(&nd->trk_data)) {
> > > > /* Return false if no tracking data or if lbs changed. */
> > >
> > > This comment should be removed.
> >
> > I think I may have missed this comment in v4. I'll address it in v5.
> >
> >
> > >
> > > > return false;
> > > > }
> > > >
> > > > + if (!northd_has_lbs_in_tracked_data(&nd->trk_data)) {
> > > > + return true;
> > > > + }
> > > > +
> > > > + const struct engine_context *eng_ctx = engine_get_context();
> > > > + if (!eng_ctx->ovnsb_idl_txn) {
> > > > + return false;
> > > > + }
> > > > +
> > > > + const struct sbrec_logical_dp_group_table *sb_dpgrp_table =
> > > > + EN_OVSDB_GET(engine_get_input("SB_logical_dp_group",
node));
> > > > + const struct sbrec_load_balancer_table *sb_lb_table =
> > > > + EN_OVSDB_GET(engine_get_input("SB_load_balancer", node));
> > > > + struct ed_type_global_config *global_config =
> > > > + engine_get_input_data("global_config", node);
> > > > + struct ed_type_sync_to_sb_lb_data *data = data_;
> > > > +
> > > > + sync_changed_lbs(&data->sb_lbs, eng_ctx->ovnsb_idl_txn,
sb_lb_table,
> > > > + sb_dpgrp_table, &nd->trk_data.trk_lbs,
> > > > + &nd->ls_datapaths, &nd->lr_datapaths,
> > > > + &global_config->features);
> > > >
> > > > - /* There are only NB LSP related changes and these can be
safely
> > > > - * ignore and returned true. However in case the northd engine
> > > > - * tracking data includes other changes, we need to do
additional
> > > > - * checks before safely ignoring. */
> > > > + engine_set_node_state(node, EN_UPDATED);
> > > > return true;
> > > > }
> > > >
> > > > @@ -529,3 +614,333 @@ sb_address_set_lookup_by_name(struct
> > > ovsdb_idl_index *sbrec_addr_set_by_name,
> > > >
> > > > return retval;
> > > > }
> > > > +
> > > > +/* static functions related to sync_to_sb_lb */
> > > > +
> > > > +static void
> > > > +sb_lb_table_init(struct sb_lb_table *sb_lbs)
> > > > +{
> > > > + hmap_init(&sb_lbs->entries);
> > > > + ovn_dp_groups_init(&sb_lbs->ls_dp_groups);
> > > > + ovn_dp_groups_init(&sb_lbs->lr_dp_groups);
> > > > +}
> > > > +
> > > > +static void
> > > > +sb_lb_table_clear(struct sb_lb_table *sb_lbs)
> > >
> > > It would be better to rename to sb_lb_table_destroy, since "clear"
suggests
> > > it resets to the initiated state but not destroys the structure.
> >
> > Ack. Addressed in v4.
> >
> >
> > >
> > > > +{
> > > > + struct sb_lb_record *sb_lb;
> > > > + HMAP_FOR_EACH_POP (sb_lb, key_node, &sb_lbs->entries) {
> > > > + free(sb_lb);
> > > > + }
> > > > + hmap_destroy(&sb_lbs->entries);
> > > > +
> > > > + ovn_dp_groups_destroy(&sb_lbs->ls_dp_groups);
> > > > + ovn_dp_groups_destroy(&sb_lbs->lr_dp_groups);
> > > > +}
> > > > +
> > > > +static struct sb_lb_record *
> > > > +sb_lb_table_find(struct hmap *sb_lbs, const struct uuid *lb_uuid)
> > > > +{
> > > > + struct sb_lb_record *sb_lb;
> > > > + HMAP_FOR_EACH_WITH_HASH (sb_lb, key_node, uuid_hash(lb_uuid),
> > > > + sb_lbs) {
> > > > + if (uuid_equals(&sb_lb->lb_dps->lb->nlb->header_.uuid,
lb_uuid))
> > > {
> > > > + return sb_lb;
> > > > + }
> > > > + }
> > > > +
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +static void
> > > > +sb_lb_table_build_and_sync(struct sb_lb_table *sb_lbs,
> > > > + struct ovsdb_idl_txn *ovnsb_txn,
> > > > + const struct sbrec_load_balancer_table
*sb_lb_table,
> > > > + const struct sbrec_logical_dp_group_table
> > > *sb_dpgrp_table,
> > > > + struct hmap *lb_dps_map,
> > > > + struct ovn_datapaths *ls_datapaths,
> > > > + struct ovn_datapaths *lr_datapaths,
> > > > + struct chassis_features *chassis_features)
> > > > +{
> > > > + struct hmap tmp_sb_lbs = HMAP_INITIALIZER(&tmp_sb_lbs);
> > > > + struct ovn_lb_datapaths *lb_dps;
> > > > + struct sb_lb_record *sb_lb;
> > > > +
> > > > + HMAP_FOR_EACH (lb_dps, hmap_node, lb_dps_map) {
> > > > + if (!lb_dps->n_nb_ls && !lb_dps->n_nb_lr) {
> > > > + continue;
> > > > + }
> > > > +
> > > > + sb_lb = xzalloc(sizeof *sb_lb);
> > > > + sb_lb->lb_dps = lb_dps;
> > > > + hmap_insert(&tmp_sb_lbs, &sb_lb->key_node,
> > > > + uuid_hash(&lb_dps->lb->nlb->header_.uuid));
> > > > + }
> > > > +
> > > > + const struct sbrec_load_balancer *sbrec_lb;
> > > > + SBREC_LOAD_BALANCER_TABLE_FOR_EACH_SAFE (sbrec_lb,
> > > > + sb_lb_table) {
> > > > + const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids,
> > > "lb_id");
> > > > + struct uuid lb_uuid;
> > > > + if (!nb_lb_uuid || !uuid_from_string(&lb_uuid,
nb_lb_uuid)) {
> > > > + sbrec_load_balancer_delete(sbrec_lb);
> > > > + continue;
> > > > + }
> > > > +
> > > > + sb_lb = sb_lb_table_find(&tmp_sb_lbs, &lb_uuid);
> > > > + if (sb_lb) {
> > > > + sb_lb->sbrec_lb = sbrec_lb;
> > > > + sync_sb_lb_record(sb_lb, sbrec_lb, sb_dpgrp_table,
sb_lbs,
> > > > + ovnsb_txn, ls_datapaths,
lr_datapaths,
> > > > + chassis_features);
> > > > +
> > > > + hmap_remove(&tmp_sb_lbs, &sb_lb->key_node);
> > > > + hmap_insert(&sb_lbs->entries, &sb_lb->key_node,
> > > > +
> > > uuid_hash(&sb_lb->lb_dps->lb->nlb->header_.uuid));
> > > > + } else {
> > > > + sbrec_load_balancer_delete(sbrec_lb);
> > > > + }
> > > > + }
> > > > +
> > > > + HMAP_FOR_EACH_POP (sb_lb, key_node, &tmp_sb_lbs) {
> > > > + sync_sb_lb_record(sb_lb, NULL, sb_dpgrp_table, sb_lbs,
> > > > + ovnsb_txn, ls_datapaths, lr_datapaths,
> > > > + chassis_features);
> > > > + hmap_insert(&sb_lbs->entries, &sb_lb->key_node,
> > > > +
uuid_hash(&sb_lb->lb_dps->lb->nlb->header_.uuid));
> > > > + }
> > > > +
> > > > + hmap_destroy(&tmp_sb_lbs);
> > > > +}
> > > > +
> > > > +static void
> > > > +sync_sb_lb_record(struct sb_lb_record *sb_lb,
> > > > + const struct sbrec_load_balancer *sbrec_lb,
> > > > + const struct sbrec_logical_dp_group_table
> > > *sb_dpgrp_table,
> > > > + struct sb_lb_table *sb_lbs,
> > > > + struct ovsdb_idl_txn *ovnsb_txn,
> > > > + struct ovn_datapaths *ls_datapaths,
> > > > + struct ovn_datapaths *lr_datapaths,
> > > > + struct chassis_features *chassis_features)
> > > > +{
> > > > + struct sbrec_logical_dp_group *sbrec_ls_dp_group = NULL;
> > > > + struct sbrec_logical_dp_group *sbrec_lr_dp_group = NULL;
> > > > + const struct ovn_lb_datapaths *lb_dps;
> > > > + struct ovn_dp_group *pre_sync_ls_dpg;
> > > > + struct ovn_dp_group *pre_sync_lr_dpg;
> > > > +
> > > > + lb_dps = sb_lb->lb_dps;
> > > > + pre_sync_ls_dpg = sb_lb->ls_dpg;
> > > > + pre_sync_lr_dpg = sb_lb->lr_dpg;
> > > > +
> > > > + if (!sbrec_lb) {
> > > > + sb_lb->sb_uuid = uuid_random();
> > > > + sbrec_lb =
sbrec_load_balancer_insert_persist_uuid(ovnsb_txn,
> > > > +
> > > &sb_lb->sb_uuid);
> > > > + char *lb_id = xasprintf(
> > > > + UUID_FMT, UUID_ARGS(&lb_dps->lb->nlb->header_.uuid));
> > > > + const struct smap external_ids =
> > > > + SMAP_CONST1(&external_ids, "lb_id", lb_id);
> > > > + sbrec_load_balancer_set_external_ids(sbrec_lb,
&external_ids);
> > > > + free(lb_id);
> > > > + } else {
> > > > + sb_lb->sb_uuid = sbrec_lb->header_.uuid;
> > > > + sbrec_ls_dp_group =
> > > > + chassis_features->ls_dpg_column
> > > > + ? sbrec_lb->ls_datapath_group
> > > > + : sbrec_lb->datapath_group; /* deprecated */
> > > > +
> > > > + sbrec_lr_dp_group = sbrec_lb->lr_datapath_group;
> > > > + }
> > > > +
> > > > + if (lb_dps->n_nb_ls) {
> > > > + sb_lb->ls_dpg = ovn_dp_group_get(&sb_lbs->ls_dp_groups,
> > > > + lb_dps->n_nb_ls,
> > > > + lb_dps->nb_ls_map,
> > > > + ods_size(ls_datapaths));
> > > > + if (sb_lb->ls_dpg) {
> > > > + /* Update the dpg's sb dp_group. */
> > > > + sb_lb->ls_dpg->dp_group =
> > > > +
sbrec_logical_dp_group_table_get_for_uuid(sb_dpgrp_table,
> > > > +
> > > &sb_lb->ls_dpg->dpg_uuid);
> > > > + ovs_assert(sb_lb->ls_dpg->dp_group);
> > >
> > > This is unsafe. If a SB dp_group record is deleted from SB DB
directly,
> > > this would crash. The code should check and do error handling here.
> >
> > I missed to address this in v4. I'll address in v5.
>
> Hi Han,
>
> I thought about this scenario. And I think it's better to assert here
> rather than handle it.
> I don't think users or CMS should try to delete the SB logical_dp_groups
>
> SB logical_dp_groups table is a non root table and in order to delete
> it, user has to find the
> rows (of either SB Logical_flow or SB load_Balancer) referencing it
> and clear the
> column (Logical_flow.logical_dp_group or
> Load_Balancer.lr_datapath_group/ls_datapath_group)
>
> If a user deletes any SB logical flows or SB load_balancers (or even
> modifies these rows) ovn-northd
> doesn't do anything. It doesn't trigger a full recompute or handle
> the changes. I think we don't track
> these SB tables. So if a user does it now, the SB state is already
> out of sync. And later when ovn-northd
> handles NB load balancer changes incrementally, it is better to assert
> than fix it. Because there may be other
> logical flows not related to load balancers which are out of sync in the
SB DB.
>
> Assertion would help us identify this and a restart of ovn-northd will
> fix the SB DB state.
>
> FYI - The same above assert is also present in northd/lflow-mgr.c.
>
> Let me know what do you think.
I think we should avoid ovn-northd crash caused by any external changes,
even though the change is tricky to be applied. So if someone clears the
column (logical_flow.logical_dp_group or
Load_Balancer.lr_datapath_group/ls_datapath_group) to trigger a DP group
deletion in SB, would it result in ovn-northd crash? Assert is used to
ensure internal logic is correct and abort when there is obvious internal
logic error/corrupted state, which can help debugging code bugs at the
earliest point. It shouldn't be used at places where states can be affected
by user behavior (including mistakes/malicious behaviors) or external APIs.
In this case, I think we can simply print a warning and fallback to
recompute (to correct the incorrect state).
Thanks,
Han
>
> Thanks
> Numan
>
>
> > >
> > > > + } else {
> > > > + sb_lb->ls_dpg = ovn_dp_group_create(
> > > > + ovnsb_txn, &sb_lbs->ls_dp_groups,
> > > > + sbrec_ls_dp_group,
> > > > + lb_dps->n_nb_ls, lb_dps->nb_ls_map,
> > > > + ods_size(ls_datapaths), true,
> > > > + ls_datapaths,
> > > > + lr_datapaths);
> > > > + }
> > > > +
> > > > + if (chassis_features->ls_dpg_column) {
> > > > + sbrec_load_balancer_set_ls_datapath_group(sbrec_lb,
> > > > +
> > > sb_lb->ls_dpg->dp_group);
> > > > + sbrec_load_balancer_set_datapath_group(sbrec_lb, NULL);
> > > > + } else {
> > > > + /* datapath_group column is deprecated. */
> > > > + sbrec_load_balancer_set_ls_datapath_group(sbrec_lb,
NULL);
> > > > + sbrec_load_balancer_set_datapath_group(sbrec_lb,
> > > > +
> > > sb_lb->ls_dpg->dp_group);
> > > > +
> > > > + }
> > > > + } else {
> > > > + sbrec_load_balancer_set_ls_datapath_group(sbrec_lb, NULL);
> > > > + sbrec_load_balancer_set_datapath_group(sbrec_lb, NULL);
> > > > + }
> > > > +
> > > > +
> > > > + if (lb_dps->n_nb_lr) {
> > > > + sb_lb->lr_dpg = ovn_dp_group_get(&sb_lbs->lr_dp_groups,
> > > > + lb_dps->n_nb_lr,
> > > > + lb_dps->nb_lr_map,
> > > > + ods_size(lr_datapaths));
> > > > + if (sb_lb->lr_dpg) {
> > > > + /* Update the dpg's sb dp_group. */
> > > > + sb_lb->lr_dpg->dp_group =
> > > > +
sbrec_logical_dp_group_table_get_for_uuid(sb_dpgrp_table,
> > > > +
> > > &sb_lb->lr_dpg->dpg_uuid);
> > > > + ovs_assert(sb_lb->lr_dpg->dp_group);
> > >
> > > Same as above.
> > >
> >
> > Ack. I'll address in v5.
> >
> >
> > Thanks for the review.
> > Numan
> >
> >
> > > Thanks,
> > > Han
> > >
> > > > + } else {
> > > > + sb_lb->lr_dpg = ovn_dp_group_create(
> > > > + ovnsb_txn, &sb_lbs->lr_dp_groups,
> > > > + sbrec_lr_dp_group,
> > > > + lb_dps->n_nb_lr, lb_dps->nb_lr_map,
> > > > + ods_size(lr_datapaths), false,
> > > > + ls_datapaths,
> > > > + lr_datapaths);
> > > > + }
> > > > +
> > > > + sbrec_load_balancer_set_lr_datapath_group(sbrec_lb,
> > > > +
> > > sb_lb->lr_dpg->dp_group);
> > > > + } else {
> > > > + sbrec_load_balancer_set_lr_datapath_group(sbrec_lb, NULL);
> > > > + }
> > > > +
> > > > + if (pre_sync_ls_dpg != sb_lb->ls_dpg) {
> > > > + if (sb_lb->ls_dpg) {
> > > > + inc_ovn_dp_group_ref(sb_lb->ls_dpg);
> > > > + }
> > > > + if (pre_sync_ls_dpg) {
> > > > + dec_ovn_dp_group_ref(&sb_lbs->ls_dp_groups,
pre_sync_ls_dpg);
> > > > + }
> > > > + }
> > > > +
> > > > + if (pre_sync_lr_dpg != sb_lb->lr_dpg) {
> > > > + if (sb_lb->lr_dpg) {
> > > > + inc_ovn_dp_group_ref(sb_lb->lr_dpg);
> > > > + }
> > > > + if (pre_sync_lr_dpg) {
> > > > + dec_ovn_dp_group_ref(&sb_lbs->lr_dp_groups,
pre_sync_lr_dpg);
> > > > + }
> > > > + }
> > > > +
> > > > + /* Update columns. */
> > > > + sbrec_load_balancer_set_name(sbrec_lb, lb_dps->lb->nlb->name);
> > > > + sbrec_load_balancer_set_vips(sbrec_lb,
> > > > +
ovn_northd_lb_get_vips(lb_dps->lb));
> > > > + sbrec_load_balancer_set_protocol(sbrec_lb,
> > > lb_dps->lb->nlb->protocol);
> > > > +
> > > > + /* Store the fact that northd provides the original
(destination IP +
> > > > + * transport port) tuple.
> > > > + */
> > > > + struct smap options;
> > > > + smap_clone(&options, &lb_dps->lb->nlb->options);
> > > > + smap_replace(&options, "hairpin_orig_tuple", "true");
> > > > + sbrec_load_balancer_set_options(sbrec_lb, &options);
> > > > + /* Clearing 'datapaths' column, since 'dp_group' is in use. */
> > > > + sbrec_load_balancer_set_datapaths(sbrec_lb, NULL, 0);
> > > > + smap_destroy(&options);
> > > > +}
> > > > +
> > > > +static void
> > > > +sync_changed_lbs(struct sb_lb_table *sb_lbs,
> > > > + struct ovsdb_idl_txn *ovnsb_txn,
> > > > + const struct sbrec_load_balancer_table
*sb_lb_table,
> > > > + const struct sbrec_logical_dp_group_table
> > > *sb_dpgrp_table,
> > > > + struct tracked_lbs *trk_lbs,
> > > > + struct ovn_datapaths *ls_datapaths,
> > > > + struct ovn_datapaths *lr_datapaths,
> > > > + struct chassis_features *chassis_features)
> > > > +{
> > > > + struct ovn_lb_datapaths *lb_dps;
> > > > + struct hmapx_node *hmapx_node;
> > > > + struct sb_lb_record *sb_lb;
> > > > +
> > > > + HMAPX_FOR_EACH (hmapx_node, &trk_lbs->deleted) {
> > > > + lb_dps = hmapx_node->data;
> > > > +
> > > > + sb_lb = sb_lb_table_find(&sb_lbs->entries,
> > > > + &lb_dps->lb->nlb->header_.uuid);
> > > > + if (sb_lb) {
> > > > + const struct sbrec_load_balancer *sbrec_lb =
> > > > + sbrec_load_balancer_table_get_for_uuid(sb_lb_table,
> > > > +
&sb_lb->sb_uuid);
> > > > + if (sbrec_lb) {
> > > > + sbrec_load_balancer_delete(sbrec_lb);
> > > > + }
> > > > +
> > > > + hmap_remove(&sb_lbs->entries, &sb_lb->key_node);
> > > > + free(sb_lb);
> > > > + }
> > > > + }
> > > > +
> > > > + HMAPX_FOR_EACH (hmapx_node, &trk_lbs->crupdated) {
> > > > + lb_dps = hmapx_node->data;
> > > > +
> > > > + sb_lb = sb_lb_table_find(&sb_lbs->entries,
> > > > + &lb_dps->lb->nlb->header_.uuid);
> > > > +
> > > > + if (!sb_lb && !lb_dps->n_nb_ls && !lb_dps->n_nb_lr) {
> > > > + continue;
> > > > + }
> > > > +
> > > > + if (!sb_lb) {
> > > > + sb_lb = xzalloc(sizeof *sb_lb);
> > > > + sb_lb->lb_dps = lb_dps;
> > > > + hmap_insert(&sb_lbs->entries, &sb_lb->key_node,
> > > > + uuid_hash(&lb_dps->lb->nlb->header_.uuid));
> > > > + } else {
> > > > + sb_lb->sbrec_lb =
> > > > + sbrec_load_balancer_table_get_for_uuid(sb_lb_table,
> > > > +
&sb_lb->sb_uuid);
> > > > + }
> > > > +
> > > > + if (sb_lb && !lb_dps->n_nb_ls && !lb_dps->n_nb_lr) {
> > > > + const struct sbrec_load_balancer *sbrec_lb =
> > > > + sbrec_load_balancer_table_get_for_uuid(sb_lb_table,
> > > > +
&sb_lb->sb_uuid);
> > > > + if (sbrec_lb) {
> > > > + sbrec_load_balancer_delete(sbrec_lb);
> > > > + }
> > > > +
> > > > + hmap_remove(&sb_lbs->entries, &sb_lb->key_node);
> > > > + free(sb_lb);
> > > > + }
> > > > +
> > > > + sync_sb_lb_record(sb_lb, sb_lb->sbrec_lb, sb_dpgrp_table,
sb_lbs,
> > > > + ovnsb_txn, ls_datapaths, lr_datapaths,
> > > > + chassis_features);
> > > > + }
> > > > +}
> > > > +
> > > > +static bool
> > > > +check_sb_lb_duplicates(const struct sbrec_load_balancer_table
*table)
> > > > +{
> > > > + struct sset existing_nb_lb_uuids =
> > > > + SSET_INITIALIZER(&existing_nb_lb_uuids);
> > > > + const struct sbrec_load_balancer *sbrec_lb;
> > > > + bool duplicates = false;
> > > > +
> > > > + SBREC_LOAD_BALANCER_TABLE_FOR_EACH (sbrec_lb, table) {
> > > > + const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids,
> > > "lb_id");
> > > > + if (nb_lb_uuid && !sset_add(&existing_nb_lb_uuids,
nb_lb_uuid)) {
> > > > + duplicates = true;
> > > > + break;
> > > > + }
> > > > + }
> > > > +
> > > > + sset_destroy(&existing_nb_lb_uuids);
> > > > + return duplicates;
> > > > +}
> > > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> > > > index 28f397ff39..7688e7af93 100644
> > > > --- a/northd/inc-proc-northd.c
> > > > +++ b/northd/inc-proc-northd.c
> > > > @@ -277,6 +277,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop
*nb,
> > > > sync_to_sb_lb_northd_handler);
> > > > engine_add_input(&en_sync_to_sb_lb, &en_sb_load_balancer,
> > > > sync_to_sb_lb_sb_load_balancer);
> > > > + engine_add_input(&en_sync_to_sb_lb, &en_sb_logical_dp_group,
NULL);
> > > >
> > > > engine_add_input(&en_sync_to_sb_pb, &en_northd,
> > > > sync_to_sb_pb_northd_handler);
> > > > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
> > > > index d779e7e087..6f0e9ae9cb 100644
> > > > --- a/northd/lflow-mgr.c
> > > > +++ b/northd/lflow-mgr.c
> > > > @@ -72,21 +72,6 @@ static struct ovs_mutex *lflow_hash_lock(const
struct
> > > hmap *lflow_table,
> > > > uint32_t hash);
> > > > static void lflow_hash_unlock(struct ovs_mutex *hash_lock);
> > > >
> > > > -static struct ovn_dp_group *ovn_dp_group_get(
> > > > - struct hmap *dp_groups, size_t desired_n,
> > > > - const unsigned long *desired_bitmap,
> > > > - size_t bitmap_len);
> > > > -static struct ovn_dp_group *ovn_dp_group_create(
> > > > - struct ovsdb_idl_txn *ovnsb_txn, struct hmap *dp_groups,
> > > > - struct sbrec_logical_dp_group *, size_t desired_n,
> > > > - const unsigned long *desired_bitmap,
> > > > - size_t bitmap_len, bool is_switch,
> > > > - const struct ovn_datapaths *ls_datapaths,
> > > > - const struct ovn_datapaths *lr_datapaths);
> > > > -static struct ovn_dp_group *ovn_dp_group_get(
> > > > - struct hmap *dp_groups, size_t desired_n,
> > > > - const unsigned long *desired_bitmap,
> > > > - size_t bitmap_len);
> > > > static struct sbrec_logical_dp_group
> > > *ovn_sb_insert_or_update_logical_dp_group(
> > > > struct ovsdb_idl_txn *ovnsb_txn,
> > > > struct sbrec_logical_dp_group *,
> > > > @@ -501,31 +486,81 @@ lflow_table_add_lflow_default_drop(struct
> > > lflow_table *lflow_table,
> > > > where, lflow_ref);
> > > > }
> > > >
> > > > -/* Given a desired bitmap, finds a datapath group in 'dp_groups'.
If it
> > > > - * doesn't exist, creates a new one and adds it to 'dp_groups'.
> > > > +struct ovn_dp_group *
> > > > +ovn_dp_group_get(struct hmap *dp_groups, size_t desired_n,
> > > > + const unsigned long *desired_bitmap,
> > > > + size_t bitmap_len)
> > > > +{
> > > > + uint32_t hash;
> > > > +
> > > > + hash = hash_int(desired_n, 0);
> > > > + return ovn_dp_group_find(dp_groups, desired_bitmap, bitmap_len,
> > > hash);
> > > > +}
> > > > +
> > > > +/* Creates a new datapath group and adds it to 'dp_groups'.
> > > > * If 'sb_group' is provided, function will try to re-use this
group by
> > > > - * either taking it directly, or by modifying, if it's not already
in
> > > use. */
> > > > + * either taking it directly, or by modifying, if it's not already
in
> > > use.
> > > > + * Caller should first call ovn_dp_group_get() before calling this
> > > function. */
> > > > struct ovn_dp_group *
> > > > -ovn_dp_group_get_or_create(struct ovsdb_idl_txn *ovnsb_txn,
> > > > - struct hmap *dp_groups,
> > > > - struct sbrec_logical_dp_group *sb_group,
> > > > - size_t desired_n,
> > > > - const unsigned long *desired_bitmap,
> > > > - size_t bitmap_len,
> > > > - bool is_switch,
> > > > - const struct ovn_datapaths
*ls_datapaths,
> > > > - const struct ovn_datapaths
*lr_datapaths)
> > > > +ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn,
> > > > + struct hmap *dp_groups,
> > > > + struct sbrec_logical_dp_group *sb_group,
> > > > + size_t desired_n,
> > > > + const unsigned long *desired_bitmap,
> > > > + size_t bitmap_len,
> > > > + bool is_switch,
> > > > + const struct ovn_datapaths *ls_datapaths,
> > > > + const struct ovn_datapaths *lr_datapaths)
> > > > {
> > > > struct ovn_dp_group *dpg;
> > > >
> > > > - dpg = ovn_dp_group_get(dp_groups, desired_n, desired_bitmap,
> > > bitmap_len);
> > > > - if (dpg) {
> > > > - return dpg;
> > > > + bool update_dp_group = false, can_modify = false;
> > > > + unsigned long *dpg_bitmap;
> > > > + size_t i, n = 0;
> > > > +
> > > > + dpg_bitmap = sb_group ? bitmap_allocate(bitmap_len) : NULL;
> > > > + for (i = 0; sb_group && i < sb_group->n_datapaths; i++) {
> > > > + struct ovn_datapath *datapath_od;
> > > > +
> > > > + datapath_od = ovn_datapath_from_sbrec(
> > > > + ls_datapaths ? &ls_datapaths->datapaths :
NULL,
> > > > + lr_datapaths ? &lr_datapaths->datapaths :
NULL,
> > > > + sb_group->datapaths[i]);
> > > > + if (!datapath_od || ovn_datapath_is_stale(datapath_od)) {
> > > > + break;
> > > > + }
> > > > + bitmap_set1(dpg_bitmap, datapath_od->index);
> > > > + n++;
> > > > + }
> > > > + if (!sb_group || i != sb_group->n_datapaths) {
> > > > + /* No group or stale group. Not going to be used. */
> > > > + update_dp_group = true;
> > > > + can_modify = true;
> > > > + } else if (!bitmap_equal(dpg_bitmap, desired_bitmap,
bitmap_len)) {
> > > > + /* The group in Sb is different. */
> > > > + update_dp_group = true;
> > > > + /* We can modify existing group if it's not already in
use. */
> > > > + can_modify = !ovn_dp_group_find(dp_groups, dpg_bitmap,
> > > > + bitmap_len, hash_int(n,
0));
> > > > + }
> > > > +
> > > > + bitmap_free(dpg_bitmap);
> > > > +
> > > > + dpg = xzalloc(sizeof *dpg);
> > > > + dpg->bitmap = bitmap_clone(desired_bitmap, bitmap_len);
> > > > + if (!update_dp_group) {
> > > > + dpg->dp_group = sb_group;
> > > > + } else {
> > > > + dpg->dp_group = ovn_sb_insert_or_update_logical_dp_group(
> > > > + ovnsb_txn,
> > > > + can_modify ? sb_group : NULL,
> > > > + desired_bitmap,
> > > > + is_switch ? ls_datapaths :
lr_datapaths);
> > > > }
> > > > + dpg->dpg_uuid = dpg->dp_group->header_.uuid;
> > > > + hmap_insert(dp_groups, &dpg->node, hash_int(desired_n, 0));
> > > >
> > > > - return ovn_dp_group_create(ovnsb_txn, dp_groups, sb_group,
desired_n,
> > > > - desired_bitmap, bitmap_len,
is_switch,
> > > > - ls_datapaths, lr_datapaths);
> > > > + return dpg;
> > > > }
> > > >
> > > > void
> > > > @@ -908,24 +943,6 @@ ovn_dp_group_find(const struct hmap *dp_groups,
> > > > return NULL;
> > > > }
> > > >
> > > > -static void
> > > > -inc_ovn_dp_group_ref(struct ovn_dp_group *dpg)
> > > > -{
> > > > - dpg->refcnt++;
> > > > -}
> > > > -
> > > > -static void
> > > > -dec_ovn_dp_group_ref(struct hmap *dp_groups, struct ovn_dp_group
*dpg)
> > > > -{
> > > > - dpg->refcnt--;
> > > > -
> > > > - if (!dpg->refcnt) {
> > > > - hmap_remove(dp_groups, &dpg->node);
> > > > - free(dpg->bitmap);
> > > > - free(dpg);
> > > > - }
> > > > -}
> > > > -
> > > > static struct sbrec_logical_dp_group *
> > > > ovn_sb_insert_or_update_logical_dp_group(
> > > > struct ovsdb_idl_txn *ovnsb_txn,
> > > > @@ -952,83 +969,6 @@ ovn_sb_insert_or_update_logical_dp_group(
> > > > return dp_group;
> > > > }
> > > >
> > > > -static struct ovn_dp_group *
> > > > -ovn_dp_group_get(struct hmap *dp_groups, size_t desired_n,
> > > > - const unsigned long *desired_bitmap,
> > > > - size_t bitmap_len)
> > > > -{
> > > > - uint32_t hash;
> > > > -
> > > > - hash = hash_int(desired_n, 0);
> > > > - return ovn_dp_group_find(dp_groups, desired_bitmap, bitmap_len,
> > > hash);
> > > > -}
> > > > -
> > > > -/* Creates a new datapath group and adds it to 'dp_groups'.
> > > > - * If 'sb_group' is provided, function will try to re-use this
group by
> > > > - * either taking it directly, or by modifying, if it's not already
in
> > > use.
> > > > - * Caller should first call ovn_dp_group_get() before calling this
> > > function. */
> > > > -static struct ovn_dp_group *
> > > > -ovn_dp_group_create(struct ovsdb_idl_txn *ovnsb_txn,
> > > > - struct hmap *dp_groups,
> > > > - struct sbrec_logical_dp_group *sb_group,
> > > > - size_t desired_n,
> > > > - const unsigned long *desired_bitmap,
> > > > - size_t bitmap_len,
> > > > - bool is_switch,
> > > > - const struct ovn_datapaths *ls_datapaths,
> > > > - const struct ovn_datapaths *lr_datapaths)
> > > > -{
> > > > - struct ovn_dp_group *dpg;
> > > > -
> > > > - bool update_dp_group = false, can_modify = false;
> > > > - unsigned long *dpg_bitmap;
> > > > - size_t i, n = 0;
> > > > -
> > > > - dpg_bitmap = sb_group ? bitmap_allocate(bitmap_len) : NULL;
> > > > - for (i = 0; sb_group && i < sb_group->n_datapaths; i++) {
> > > > - struct ovn_datapath *datapath_od;
> > > > -
> > > > - datapath_od = ovn_datapath_from_sbrec(
> > > > - ls_datapaths ? &ls_datapaths->datapaths :
NULL,
> > > > - lr_datapaths ? &lr_datapaths->datapaths :
NULL,
> > > > - sb_group->datapaths[i]);
> > > > - if (!datapath_od || ovn_datapath_is_stale(datapath_od)) {
> > > > - break;
> > > > - }
> > > > - bitmap_set1(dpg_bitmap, datapath_od->index);
> > > > - n++;
> > > > - }
> > > > - if (!sb_group || i != sb_group->n_datapaths) {
> > > > - /* No group or stale group. Not going to be used. */
> > > > - update_dp_group = true;
> > > > - can_modify = true;
> > > > - } else if (!bitmap_equal(dpg_bitmap, desired_bitmap,
bitmap_len)) {
> > > > - /* The group in Sb is different. */
> > > > - update_dp_group = true;
> > > > - /* We can modify existing group if it's not already in
use. */
> > > > - can_modify = !ovn_dp_group_find(dp_groups, dpg_bitmap,
> > > > - bitmap_len, hash_int(n,
0));
> > > > - }
> > > > -
> > > > - bitmap_free(dpg_bitmap);
> > > > -
> > > > - dpg = xzalloc(sizeof *dpg);
> > > > - dpg->bitmap = bitmap_clone(desired_bitmap, bitmap_len);
> > > > - if (!update_dp_group) {
> > > > - dpg->dp_group = sb_group;
> > > > - } else {
> > > > - dpg->dp_group = ovn_sb_insert_or_update_logical_dp_group(
> > > > - ovnsb_txn,
> > > > - can_modify ? sb_group : NULL,
> > > > - desired_bitmap,
> > > > - is_switch ? ls_datapaths :
lr_datapaths);
> > > > - }
> > > > - dpg->dpg_uuid = dpg->dp_group->header_.uuid;
> > > > - hmap_insert(dp_groups, &dpg->node, hash_int(desired_n, 0));
> > > > -
> > > > - return dpg;
> > > > -}
> > > > -
> > > > /* Adds an OVN datapath to a datapath group of existing logical
flow.
> > > > * Version to use when hash bucket locking is NOT required or the
> > > corresponding
> > > > * hash lock is already taken. */
> > > > diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
> > > > index c65cd70e71..583afa6cb3 100644
> > > > --- a/northd/lflow-mgr.h
> > > > +++ b/northd/lflow-mgr.h
> > > > @@ -160,7 +160,10 @@ ovn_dp_groups_init(struct hmap *dp_groups)
> > > > }
> > > >
> > > > void ovn_dp_groups_destroy(struct hmap *dp_groups);
> > > > -struct ovn_dp_group *ovn_dp_group_get_or_create(
> > > > +struct ovn_dp_group *ovn_dp_group_get(struct hmap *dp_groups,
size_t
> > > desired_n,
> > > > + const unsigned long
> > > *desired_bitmap,
> > > > + size_t bitmap_len);
> > > > +struct ovn_dp_group *ovn_dp_group_create(
> > > > struct ovsdb_idl_txn *ovnsb_txn, struct hmap *dp_groups,
> > > > struct sbrec_logical_dp_group *sb_group,
> > > > size_t desired_n, const unsigned long *desired_bitmap,
> > > > @@ -168,4 +171,22 @@ struct ovn_dp_group
*ovn_dp_group_get_or_create(
> > > > const struct ovn_datapaths *ls_datapaths,
> > > > const struct ovn_datapaths *lr_datapaths);
> > > >
> > > > +static inline void
> > > > +inc_ovn_dp_group_ref(struct ovn_dp_group *dpg)
> > > > +{
> > > > + dpg->refcnt++;
> > > > +}
> > > > +
> > > > +static inline void
> > > > +dec_ovn_dp_group_ref(struct hmap *dp_groups, struct ovn_dp_group
*dpg)
> > > > +{
> > > > + dpg->refcnt--;
> > > > +
> > > > + if (!dpg->refcnt) {
> > > > + hmap_remove(dp_groups, &dpg->node);
> > > > + free(dpg->bitmap);
> > > > + free(dpg);
> > > > + }
> > > > +}
> > > > +
> > > > #endif /* LFLOW_MGR_H */
> > > > \ No newline at end of file
> > > > diff --git a/northd/northd.c b/northd/northd.c
> > > > index e1eda9fba5..b5a7f73fdb 100644
> > > > --- a/northd/northd.c
> > > > +++ b/northd/northd.c
> > > > @@ -3786,244 +3786,6 @@ build_lb_port_related_data(
> > > > build_lswitch_lbs_from_lrouter(lr_datapaths, lb_dps_map,
> > > lb_group_dps_map);
> > > > }
> > > >
> > > > -struct sb_lb {
> > > > - struct hmap_node hmap_node;
> > > > -
> > > > - const struct sbrec_load_balancer *slb;
> > > > - struct ovn_dp_group *dpg;
> > > > - struct ovn_dp_group *lr_dpg;
> > > > - struct uuid lb_uuid;
> > > > -};
> > > > -
> > > > -static struct sb_lb *
> > > > -find_slb_in_sb_lbs(struct hmap *sb_lbs, const struct uuid *lb_uuid)
> > > > -{
> > > > - struct sb_lb *sb_lb;
> > > > - HMAP_FOR_EACH_WITH_HASH (sb_lb, hmap_node, uuid_hash(lb_uuid),
> > > sb_lbs) {
> > > > - if (uuid_equals(&sb_lb->lb_uuid, lb_uuid)) {
> > > > - return sb_lb;
> > > > - }
> > > > - }
> > > > -
> > > > - return NULL;
> > > > -}
> > > > -
> > > > -/* Syncs relevant load balancers (applied to logical switches) to
the
> > > > - * Southbound database.
> > > > - */
> > > > -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 ovn_datapaths *lr_datapaths,
> > > > - struct hmap *lb_dps_map,
> > > > - struct chassis_features *chassis_features)
> > > > -{
> > > > - struct hmap ls_dp_groups = HMAP_INITIALIZER(&ls_dp_groups);
> > > > - struct hmap lr_dp_groups = HMAP_INITIALIZER(&lr_dp_groups);
> > > > - struct ovn_lb_datapaths *lb_dps;
> > > > - struct hmap sb_lbs = HMAP_INITIALIZER(&sb_lbs);
> > > > -
> > > > - /* Delete any stale SB load balancer rows and create datapath
> > > > - * groups for existing ones. */
> > > > - struct hmapx existing_lbs = HMAPX_INITIALIZER(&existing_lbs);
> > > > - const struct sbrec_load_balancer *sbrec_lb;
> > > > - SBREC_LOAD_BALANCER_TABLE_FOR_EACH_SAFE (sbrec_lb,
> > > > - sbrec_load_balancer_table) {
> > > > - const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids,
> > > "lb_id");
> > > > - struct uuid lb_uuid;
> > > > - if (!nb_lb_uuid || !uuid_from_string(&lb_uuid,
nb_lb_uuid)) {
> > > > - sbrec_load_balancer_delete(sbrec_lb);
> > > > - continue;
> > > > - }
> > > > -
> > > > - /* 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.
> > > > - *
> > > > - * There is also a special case in which duplicate LBs
might be
> > > created
> > > > - * in the SB, e.g., due to the fact that OVSDB only ensures
> > > > - * "at-least-once" consistency for clustered database
tables that
> > > > - * are not indexed in any way.
> > > > - */
> > > > - lb_dps = ovn_lb_datapaths_find(lb_dps_map, &lb_uuid);
> > > > - if (!lb_dps || (!lb_dps->n_nb_ls && !lb_dps->n_nb_lr) ||
> > > > - !hmapx_add(&existing_lbs, lb_dps)) {
> > > > - sbrec_load_balancer_delete(sbrec_lb);
> > > > - continue;
> > > > - }
> > > > -
> > > > - struct sb_lb *sb_lb = xzalloc(sizeof *sb_lb);
> > > > - sb_lb->lb_uuid = lb_uuid;
> > > > - sb_lb->slb = sbrec_lb;
> > > > - hmap_insert(&sb_lbs, &sb_lb->hmap_node,
uuid_hash(&lb_uuid));
> > > > -
> > > > - /* Find or create datapath group for this load balancer. */
> > > > - if (lb_dps->n_nb_ls) {
> > > > - struct sbrec_logical_dp_group *ls_datapath_group
> > > > - = chassis_features->ls_dpg_column
> > > > - ? sb_lb->slb->ls_datapath_group
> > > > - : sb_lb->slb->datapath_group; /* deprecated */
> > > > - sb_lb->dpg = ovn_dp_group_get_or_create(
> > > > - ovnsb_txn, &ls_dp_groups,
> > > > - ls_datapath_group,
> > > > - lb_dps->n_nb_ls, lb_dps->nb_ls_map,
> > > > - ods_size(ls_datapaths), true,
> > > > - ls_datapaths, NULL);
> > > > - }
> > > > - if (lb_dps->n_nb_lr) {
> > > > - sb_lb->lr_dpg = ovn_dp_group_get_or_create(
> > > > - ovnsb_txn, &lr_dp_groups,
> > > > - sb_lb->slb->lr_datapath_group,
> > > > - lb_dps->n_nb_lr, lb_dps->nb_lr_map,
> > > > - ods_size(lr_datapaths), false,
> > > > - NULL, lr_datapaths);
> > > > - }
> > > > - }
> > > > - hmapx_destroy(&existing_lbs);
> > > > -
> > > > - /* Create SB Load balancer records if not present and sync
> > > > - * the SB load balancer columns. */
> > > > - HMAP_FOR_EACH (lb_dps, hmap_node, lb_dps_map) {
> > > > -
> > > > - if (!lb_dps->n_nb_ls && !lb_dps->n_nb_lr) {
> > > > - continue;
> > > > - }
> > > > -
> > > > - /* Store the fact that northd provides the original
(destination
> > > IP +
> > > > - * transport port) tuple.
> > > > - */
> > > > - struct smap options;
> > > > - smap_clone(&options, &lb_dps->lb->nlb->options);
> > > > - smap_replace(&options, "hairpin_orig_tuple", "true");
> > > > -
> > > > - struct sb_lb *sb_lb = find_slb_in_sb_lbs(&sb_lbs,
> > > > -
> > > &lb_dps->lb->nlb->header_.uuid);
> > > > - ovs_assert(!sb_lb || (sb_lb->slb && (sb_lb->dpg ||
> > > sb_lb->lr_dpg)));
> > > > - struct ovn_dp_group *lb_dpg = NULL, *lb_lr_dpg = NULL;
> > > > - if (!sb_lb) {
> > > > - sbrec_lb = sbrec_load_balancer_insert(ovnsb_txn);
> > > > - char *lb_id = xasprintf(
> > > > - UUID_FMT,
UUID_ARGS(&lb_dps->lb->nlb->header_.uuid));
> > > > - const struct smap external_ids =
> > > > - SMAP_CONST1(&external_ids, "lb_id", lb_id);
> > > > - sbrec_load_balancer_set_external_ids(sbrec_lb,
> > > &external_ids);
> > > > - free(lb_id);
> > > > - } else {
> > > > - sbrec_lb = sb_lb->slb;
> > > > - lb_dpg = sb_lb->dpg;
> > > > - lb_lr_dpg = sb_lb->lr_dpg;
> > > > - }
> > > > -
> > > > - /* Find or create datapath group for this load balancer. */
> > > > - if (!lb_dpg && lb_dps->n_nb_ls) {
> > > > - struct sbrec_logical_dp_group *ls_datapath_group
> > > > - = chassis_features->ls_dpg_column
> > > > - ? sbrec_lb->ls_datapath_group
> > > > - : sbrec_lb->datapath_group; /* deprecated */
> > > > - lb_dpg = ovn_dp_group_get_or_create(
> > > > - ovnsb_txn, &ls_dp_groups,
> > > > - ls_datapath_group,
> > > > - lb_dps->n_nb_ls, lb_dps->nb_ls_map,
> > > > - ods_size(ls_datapaths), true,
> > > > - ls_datapaths, NULL);
> > > > - }
> > > > - if (!lb_lr_dpg && lb_dps->n_nb_lr) {
> > > > - lb_lr_dpg = ovn_dp_group_get_or_create(
> > > > - ovnsb_txn, &lr_dp_groups,
> > > > - sbrec_lb->lr_datapath_group,
> > > > - lb_dps->n_nb_lr, lb_dps->nb_lr_map,
> > > > - ods_size(lr_datapaths), false,
> > > > - NULL, lr_datapaths);
> > > > - }
> > > > -
> > > > - /* Update columns. */
> > > > - sbrec_load_balancer_set_name(sbrec_lb,
lb_dps->lb->nlb->name);
> > > > - sbrec_load_balancer_set_vips(sbrec_lb,
> > > > -
ovn_northd_lb_get_vips(lb_dps->lb));
> > > > - sbrec_load_balancer_set_protocol(sbrec_lb,
> > > lb_dps->lb->nlb->protocol);
> > > > -
> > > > - if (chassis_features->ls_dpg_column) {
> > > > - sbrec_load_balancer_set_ls_datapath_group(
> > > > - sbrec_lb, lb_dpg ? lb_dpg->dp_group : NULL
> > > > - );
> > > > - sbrec_load_balancer_set_datapath_group(sbrec_lb, NULL);
> > > > - } else {
> > > > - /* datapath_group column is deprecated. */
> > > > - sbrec_load_balancer_set_ls_datapath_group(sbrec_lb,
NULL);
> > > > - sbrec_load_balancer_set_datapath_group(
> > > > - sbrec_lb, lb_dpg ? lb_dpg->dp_group : NULL
> > > > - );
> > > > - }
> > > > -
> > > > - sbrec_load_balancer_set_lr_datapath_group(
> > > > - sbrec_lb, lb_lr_dpg ? lb_lr_dpg->dp_group : NULL
> > > > - );
> > > > - sbrec_load_balancer_set_options(sbrec_lb, &options);
> > > > - /* Clearing 'datapaths' column, since 'dp_group' is in
use. */
> > > > - sbrec_load_balancer_set_datapaths(sbrec_lb, NULL, 0);
> > > > - smap_destroy(&options);
> > > > - }
> > > > -
> > > > - struct ovn_dp_group *dpg;
> > > > - 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(&lr_dp_groups);
> > > > -
> > > > - struct sb_lb *sb_lb;
> > > > - HMAP_FOR_EACH_POP (sb_lb, hmap_node, &sb_lbs) {
> > > > - free(sb_lb);
> > > > - }
> > > > - hmap_destroy(&sb_lbs);
> > > > -
> > > > - /* Datapath_Binding.load_balancers is not used anymore, it's
still
> > > in the
> > > > - * schema for compatibility reasons. Reset it to empty, just
in
> > > case.
> > > > - */
> > > > - struct ovn_datapath *od;
> > > > - HMAP_FOR_EACH (od, key_node, &ls_datapaths->datapaths) {
> > > > - ovs_assert(od->nbs);
> > > > -
> > > > - if (od->sb->n_load_balancers) {
> > > > - sbrec_datapath_binding_set_load_balancers(od->sb,
NULL, 0);
> > > > - }
> > > > - }
> > > > - HMAP_FOR_EACH (od, key_node, &lr_datapaths->datapaths) {
> > > > - ovs_assert(od->nbr);
> > > > -
> > > > - if (od->sb->n_load_balancers) {
> > > > - sbrec_datapath_binding_set_load_balancers(od->sb,
NULL, 0);
> > > > - }
> > > > - }
> > > > -}
> > > > -
> > > > -bool
> > > > -check_sb_lb_duplicates(const struct sbrec_load_balancer_table
*table)
> > > > -{
> > > > - struct sset existing_nb_lb_uuids =
> > > > - SSET_INITIALIZER(&existing_nb_lb_uuids);
> > > > - const struct sbrec_load_balancer *sbrec_lb;
> > > > - bool duplicates = false;
> > > > -
> > > > - SBREC_LOAD_BALANCER_TABLE_FOR_EACH (sbrec_lb, table) {
> > > > - const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids,
> > > "lb_id");
> > > > - if (nb_lb_uuid && !sset_add(&existing_nb_lb_uuids,
nb_lb_uuid)) {
> > > > - duplicates = true;
> > > > - break;
> > > > - }
> > > > - }
> > > > -
> > > > - sset_destroy(&existing_nb_lb_uuids);
> > > > - return duplicates;
> > > > -}
> > > > -
> > > > /* Syncs the SB port binding for the ovn_port 'op' of a logical
switch
> > > port.
> > > > * Caller should make sure that the OVN SB IDL txn is not NULL.
> > > Presently it
> > > > * only syncs the nat column of port binding corresponding to the
> > > 'op->nbsp' */
> > > > diff --git a/northd/northd.h b/northd/northd.h
> > > > index 7e48bb966d..8dabc17eb7 100644
> > > > --- a/northd/northd.h
> > > > +++ b/northd/northd.h
> > > > @@ -762,12 +762,6 @@ void run_update_worker_pool(int n_threads);
> > > >
> > > > const struct ovn_datapath *northd_get_datapath_for_port(
> > > > const struct hmap *ls_ports, const char *port_name);
> > > > -void sync_lbs(struct ovsdb_idl_txn *, const struct
> > > sbrec_load_balancer_table *,
> > > > - struct ovn_datapaths *ls_datapaths,
> > > > - struct ovn_datapaths *lr_datapaths,
> > > > - struct hmap *lbs,
> > > > - struct chassis_features *chassis_features);
> > > > -bool check_sb_lb_duplicates(const struct sbrec_load_balancer_table
*);
> > > >
> > > > struct lr_stateful_table;
> > > > void sync_pbs(struct ovsdb_idl_txn *, struct hmap *ls_ports,
> > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > > > index 03d525bdc2..b0d3fde1da 100644
> > > > --- a/tests/ovn-northd.at
> > > > +++ b/tests/ovn-northd.at
> > > > @@ -2938,6 +2938,8 @@ sw1_sb_uuid=$(fetch_column datapath_binding
_uuid
> > > external_ids:name=sw1)
> > > > echo "$sw0_sb_uuid" > sw_sb_uuids
> > > > echo "$sw1_sb_uuid" >> sw_sb_uuids
> > > >
> > > > +lb0_dp_group=$(fetch_column sb:load_balancer ls_datapath_group
name=lb0)
> > > > +
> > > > echo
> > > > echo "__file__:__line__: Check that SB lb0 has sw0 and sw1 in
datapaths
> > > column."
> > > > AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find
> > > Logical_DP_Group dnl
> > > > @@ -2945,6 +2947,8 @@ AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns
> > > _uuid,datapaths find Logical_DP_Gr
> > > > $(cat sw_sb_uuids | sort)
> > > > ])
> > > >
> > > > +lbg0_dp_group=$(fetch_column sb:load_balancer ls_datapath_group
> > > name=lbg0)
> > > > +
> > > > echo
> > > > echo "__file__:__line__: Check that SB lbg0 has sw0 and sw1 in
datapaths
> > > column."
> > > > AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find
> > > Logical_DP_Group dnl
> > > > @@ -10502,9 +10506,9 @@ check ovn-nbctl --wait=sb lb-add lb1
10.0.0.10:80
> > > 10.0.0.3:80
> > > > check_engine_stats lb_data norecompute compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > -
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> > > > +
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > >
> > > > check ovn-nbctl --wait=sb set load_balancer .
> > > ip_port_mappings:10.0.0.3=sw0-p1:10.0.0.2
> > > > @@ -10512,7 +10516,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10522,7 +10526,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > >
> > > > @@ -10531,7 +10535,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > >
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10541,7 +10545,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > >
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10599,7 +10603,7 @@ check_engine_stats lr_stateful norecompute
compute
> > > > # A LB applied to a switch/router triggers:
> > > > # - a recompute in the first iteration (handling northd change)
> > > > # - a compute in the second iteration (handling SB update)
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE((1))
> > > > @@ -10612,7 +10616,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> > > >
> > > > # Cleanup the vip of lb1.
> > > > @@ -10623,7 +10627,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> > > >
> > > > # Set the vips of lb1 back
> > > > @@ -10634,7 +10638,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> > > >
> > > > # Add another vip to lb1
> > > > @@ -10645,7 +10649,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> > > >
> > > > # Disassociate lb1 from sw0. There should be a full recompute of
northd
> > > engine node.
> > > > @@ -10668,7 +10672,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > >
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10689,7 +10693,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Modify the backend of the lb1 vip
> > > > @@ -10699,7 +10703,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Cleanup the vip of lb1.
> > > > @@ -10709,7 +10713,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Set the vips of lb1 back
> > > > @@ -10719,7 +10723,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Add another vip to lb1
> > > > @@ -10729,7 +10733,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10761,7 +10765,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10778,7 +10782,7 @@ check ovn-nbctl --wait=sb add
load_balancer_group
> > > . load_Balancer $lb1_uuid
> > > > check_engine_stats lb_data norecompute compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10788,7 +10792,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > >
> > > > # Update lb and this should not result in northd recompute
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10798,7 +10802,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > >
> > > > # Modify the backend of the lb1 vip
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10808,7 +10812,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Cleanup the vip of lb1.
> > > > @@ -10819,7 +10823,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Set the vips of lb1 back
> > > > @@ -10830,7 +10834,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Add another vip to lb1
> > > > @@ -10841,7 +10845,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10857,7 +10861,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Modify the backend of the lb1 vip
> > > > @@ -10867,7 +10871,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Cleanup the vip of lb1.
> > > > @@ -10877,7 +10881,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Set the vips of lb1 back
> > > > @@ -10887,7 +10891,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Add another vip to lb1
> > > > @@ -10897,7 +10901,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10917,7 +10921,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10961,7 +10965,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10971,7 +10975,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10980,7 +10984,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -10990,7 +10994,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -11000,7 +11004,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -11010,7 +11014,7 @@ check_engine_stats lb_data norecompute
compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -11041,7 +11045,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Deleting lb2 should result in lflow recompute as it is
> > > > @@ -11053,7 +11057,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats ls_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > -check_engine_stats sync_to_sb_lb recompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -11087,6 +11091,7 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
> > > > check_engine_stats lb_data norecompute compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Clear the VIPs of lb1
> > > > @@ -11095,6 +11100,7 @@ check ovn-nbctl --wait=sb clear
load_balancer .
> > > vips
> > > > check_engine_stats lb_data norecompute compute
> > > > check_engine_stats northd norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> > > > @@ -11102,6 +11108,7 @@ check ovn-nbctl --wait=sb lb-del lb1
> > > > check_engine_stats lb_data norecompute compute
> > > > check_engine_stats northd recompute nocompute
> > > > check_engine_stats lflow recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb recompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > AT_CLEANUP
> > > > @@ -11129,6 +11136,7 @@ check_engine_stats northd recompute
nocompute
> > > > check_engine_stats lr_nat recompute nocompute
> > > > check_engine_stats lr_stateful recompute nocompute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb recompute nocompute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11142,6 +11150,7 @@ check_engine_stats northd recompute compute
> > > > check_engine_stats lr_nat recompute nocompute
> > > > check_engine_stats lr_stateful recompute nocompute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb recompute nocompute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11154,6 +11163,7 @@ check_engine_stats northd recompute compute
> > > > check_engine_stats lr_nat recompute nocompute
> > > > check_engine_stats lr_stateful recompute nocompute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb recompute nocompute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11180,6 +11190,7 @@ check_engine_stats northd recompute
nocompute
> > > > check_engine_stats lr_nat recompute nocompute
> > > > check_engine_stats lr_stateful recompute nocompute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb recompute nocompute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11192,6 +11203,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_nat norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Update the NAT options column
> > > > @@ -11201,6 +11213,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_nat norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Update the NAT external_ip column
> > > > @@ -11211,6 +11224,7 @@ check_engine_stats lr_nat norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Update the NAT logical_ip column
> > > > @@ -11221,6 +11235,7 @@ check_engine_stats lr_nat norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Update the NAT type
> > > > @@ -11231,6 +11246,7 @@ check_engine_stats lr_nat norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Create a dnat_and_snat NAT with external_mac and logical_port
> > > > @@ -11241,6 +11257,7 @@ check_engine_stats lr_nat norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > nat2_uuid=$(ovn-nbctl --bare --columns _uuid find nat
> > > logical_ip=10.0.0.4)
> > > > @@ -11252,6 +11269,7 @@ check_engine_stats lr_nat norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Create a load balancer and add the lb vip as NAT
> > > > @@ -11268,6 +11286,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_nat norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11279,6 +11298,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_nat norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11290,6 +11310,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_nat norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11301,6 +11322,7 @@ check_engine_stats northd norecompute
compute
> > > > check_engine_stats lr_nat norecompute compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11312,6 +11334,7 @@ check_engine_stats lr_nat norecompute
compute
> > > > check_engine_stats lr_stateful norecompute compute
> > > > check_engine_stats lflow norecompute compute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb norecompute compute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > # Create router Policy
> > > > @@ -11321,6 +11344,7 @@ check_engine_stats northd recompute
nocompute
> > > > check_engine_stats lr_nat recompute nocompute
> > > > check_engine_stats lr_stateful recompute nocompute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb recompute nocompute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > @@ -11330,6 +11354,7 @@ check_engine_stats northd recompute
nocompute
> > > > check_engine_stats lr_nat recompute nocompute
> > > > check_engine_stats lr_stateful recompute nocompute
> > > > check_engine_stats sync_to_sb_pb recompute nocompute
> > > > +check_engine_stats sync_to_sb_lb recompute nocompute
> > > > check_engine_stats lflow recompute nocompute
> > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > >
> > > > --
> > > > 2.41.0
> > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > d...@openvswitch.org
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > _______________________________________________
> > > dev mailing list
> > > d...@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
