On Mon, Sep 11, 2023 at 5:58 PM Han Zhou <[email protected]> wrote:
>
> On Mon, Sep 11, 2023 at 2:15 PM Numan Siddique <[email protected]> wrote:
> >
> > On Mon, Sep 11, 2023 at 2:18 PM Han Zhou <[email protected]> wrote:
> > >
> > > On Mon, Sep 11, 2023 at 9:48 AM Numan Siddique <[email protected]> wrote:
> > > >
> > > > On Fri, Sep 1, 2023 at 12:52 AM Han Zhou <[email protected]> wrote:
> > > > >
> > > > > On Thu, Aug 31, 2023 at 7:14 PM Han Zhou <[email protected]> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Fri, Aug 18, 2023 at 1:58 AM <[email protected]> wrote:
> > > > > > >
> > > > > > > From: Numan Siddique <[email protected]>
> > > > > > >
> > > > > > > 'lb_data' engine node now also handles logical switch changes.
> > > > > > > Its data maintains ls to lb related information. i.e if a
> > > > > > > logical switch sw0 has lb1, lb2 and lb3 associated then
> > > > > > > it stores this info in its data. And when a new load balancer
> > > > > > > lb4 is associated to it, it stores this information in its
> > > > > > > tracked data so that 'northd' engine node can handle it
> > > > > > > accordingly. Tracked data will have information like:
> > > > > > > changed ls -> {sw0 : {associated_lbs: [lb4]}
> > > > > > >
> > > > > > > In the 'northd' engne node, an additional handler for 'lb_data'
> > > > > > > is added after the 'nbrec_logical_switch' changes. With this
> patch
> > > > > > > we now have 2 handlers for 'lb_data' in 'northd' engine node.
> > > > > > >
> > > > > > > ----
> > > > > > > engine_add_input(&en_northd, &en_lb_data,
> > > > > northd_lb_data_handler_pre_od);
> > > > > > > engine_add_input(&en_northd, &en_nb_logical_switch,
> > > > > > > northd_nb_logical_switch_handler);
> > > > > > > engine_add_input(&en_northd, &en_lb_data,
> > > > > northd_lb_data_handler_post_od);
> > > > > > > ----
> > > > > > >
> > > > > > > The first handler 'northd_lb_data_handler_pre_od' is called
> before
> > > the
> > > > > > > 'northd_nb_logical_switch_handler' handler and it just creates
> or
> > > > > > > deletes the lb_datapaths hmap for the tracked lbs.
> > > > > > >
> > > > > > > The second handler 'northd_lb_data_handler_post_od' is called
> after
> > > > > > > the 'northd_nb_logical_switch_handler' handler and it updates
> the
> > > > > > > ovn_lb_datapaths's 'nb_ls_map' bitmap.
> > > > > > >
> > > > > > > Eg. If the lb_data has the below tracked data:
> > > > > > >
> > > > > > > tracked_data = {'crupdated_lbs': [lb1, lb2],
> > > > > > > 'deleted_lbs': [lb3],
> > > > > > > 'crupdated_lb_groups': [lbg1, lbg2],
> > > > > > > 'crupdated_ls_lbs': [{ls: sw0, assoc_lbs: [lb1],
> > > > > > > {ls: sw1, assoc_lbs: [lb1,
> > > lb2]}
> > > > > > >
> > > > > > > then the first handler northd_lb_data_handler_pre_od(), creates
> the
> > > > > > > ovn_lb_datapaths object for lb1 and lb2 and deletes lb3 from
> > > > > > > the ovn_lb_datapaths hmap. Similarly for the created or
> updated lb
> > > > > > > groups lbg1 and lbg2.
> > > > > > >
> > > > > > > The second handler northd_lb_data_handler_post_od() updates the
> > > > > > > nb_ls_bitmap of lb1 for sw0 and sw1 and nb_ls_bitmap of lb2 for
> sw1.
> > > > > > >
> > > > > > > This second handler is added mainly so that the actual logical
> > > switch
> > > > > > > handler northd_nb_logical_switch_handler() has done modifying
> the
> > > > > > > 'od' struct.
> > > > > > >
> > > > > >
> > > >
> > > > Hi Han and others,
> > > >
> > > > Thanks for the reviews.
> > > >
> > > >
> > > > > > Thanks for explaining the pre_od and post_od approach here! If we
> do
> > > want
> > > > > to go with this approach I'd suggest adding comments also in the
> code so
> > > > > that people can understand the code easily without having to look
> at the
> > > > > commit history.
> > > > > > However, I am concerned with the this approach. Here are the
> reasons:
> > > > > >
> > > > > > 1. The I-P engine implementation didn't consider such usage that
> add
> > > > > dependency between two nodes twice, although it didn't explicitly
> call
> > > this
> > > > > out in the documentation. If this is considered valid and necessary
> use
> > > > > case, we should call this out in the I-P engine documentation so
> that
> > > > > people modifying I-P engine implementation is aware of this and
> won't
> > > break
> > > > > it, and of course we should carefully evaluate the side-effects that
> > > can be
> > > > > caused by this.
> > > > > >
> > > > > > 2. For the current use case I think it will lead to dangling
> pointer
> > > in
> > > > > this _pre_od handling, because a LS may have been deleted from IDL,
> > > while
> > > > > this function still tries to access od->nbs, e.g. in
> > > > > init_lb_for_datapaths(). Because of this, we should handle LB data
> only
> > > > > after handling LS changes.
> > > > > >
> > > > > > 3. From the explanation above I can see what is done by pre_od and
> > > > > post_od but still don't understand why can't we do both steps after
> > > > > handling LS changes. Could you explain why? And is it possible to
> move
> > > both
> > > > > steps to post_od?
> > > >
> > > >
> > > > In the earlier versions , lb_data engine node was not handling logical
> > > > switch or logical router changes.
> > > > It was only handling the load balancer changes.
> > > > northd engine node handler for lb_data had to determine the
> > > > associated or disassociated load balancers/lb groups
> > > > for a logical switch or router. And I could that properly using 2
> > > > handlers (pre_od and post_od).
> > > > In later versions, I moved that logic to the lb_data engine node
> > > > itself. But I thought I'll keep 2 handlers, the pre_od() to
> > > > just handle the load balancer/group changes and the post_od for
> > > > updating the datapath bitmap of the lb_dps.
> > > > IMO that seemed cleaner.
> > > >
> > > > But as you mentioned we can do all these in one handler. So in v7 I
> > > > just moved to one handler.
> > > >
> > > >
> > > >
> > > > > >
> > > > > > Please find some more comments below.
> > > > > >
> > > > > > > Signed-off-by: Numan Siddique <[email protected]>
> > > > > > > ---
> > > > > > > lib/lb.c | 5 +-
> > > > > > > northd/en-lb-data.c | 176
> > > +++++++++++++++++++++++++++++++++++++++
> > > > > > > northd/en-lb-data.h | 17 ++++
> > > > > > > northd/en-lflow.c | 6 ++
> > > > > > > northd/en-northd.c | 40 +++++++--
> > > > > > > northd/en-northd.h | 3 +-
> > > > > > > northd/inc-proc-northd.c | 5 +-
> > > > > > > northd/northd.c | 104 +++++++++++++++++++----
> > > > > > > northd/northd.h | 15 ++--
> > > > > > > tests/ovn-northd.at | 56 +++++++++----
> > > > > > > 10 files changed, 380 insertions(+), 47 deletions(-)
> > > > > > >
> > > > > > > diff --git a/lib/lb.c b/lib/lb.c
> > > > > > > index 6fd67e2218..e6c9dc2be2 100644
> > > > > > > --- a/lib/lb.c
> > > > > > > +++ b/lib/lb.c
> > > > > > > @@ -1088,7 +1088,10 @@ ovn_lb_datapaths_add_ls(struct
> > > ovn_lb_datapaths
> > > > > *lb_dps, size_t n,
> > > > > > > struct ovn_datapath **ods)
> > > > > > > {
> > > > > > > for (size_t i = 0; i < n; i++) {
> > > > > > > - bitmap_set1(lb_dps->nb_ls_map, ods[i]->index);
> > > > > > > + if (!bitmap_is_set(lb_dps->nb_ls_map, ods[i]->index)) {
> > > > > > > + bitmap_set1(lb_dps->nb_ls_map, ods[i]->index);
> > > > > > > + lb_dps->n_nb_ls++;
> > > > > > > + }
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c
> > > > > > > index 23f2cb1021..e0c4db1422 100644
> > > > > > > --- a/northd/en-lb-data.c
> > > > > > > +++ b/northd/en-lb-data.c
> > > > > > > @@ -39,6 +39,14 @@ static void lb_data_destroy(struct
> > > ed_type_lb_data
> > > > > *);
> > > > > > > static void build_lbs(const struct nbrec_load_balancer_table *,
> > > > > > > const struct
> nbrec_load_balancer_group_table
> > > *,
> > > > > > > struct hmap *lbs, struct hmap
> *lb_groups);
> > > > > > > +static void build_od_lb_map(const struct
> > > nbrec_logical_switch_table *,
> > > > > > > + struct hmap *od_lb_map);
> > > > > > > +static struct od_lb_data *find_od_lb_data(struct hmap
> *od_lb_map,
> > > > > > > + const struct uuid
> > > *od_uuid);
> > > > > > > +static void destroy_od_lb_data(struct od_lb_data *od_lb_data);
> > > > > > > +static struct od_lb_data *create_od_lb_data(struct hmap
> *od_lb_map,
> > > > > > > + const struct uuid
> > > > > *od_uuid);
> > > > > > > +
> > > > > > > static struct ovn_lb_group *create_lb_group(
> > > > > > > const struct nbrec_load_balancer_group *, struct hmap *lbs,
> > > > > > > struct hmap *lb_groups);
> > > > > > > @@ -54,6 +62,7 @@ static struct crupdated_lb_group *
> > > > > > > struct
> tracked_lb_data
> > > *);
> > > > > > > static void add_deleted_lb_group_to_tracked_data(
> > > > > > > struct ovn_lb_group *, struct tracked_lb_data *);
> > > > > > > +static bool is_ls_lbs_changed(const struct nbrec_logical_switch
> > > *nbs);
> > > > > > >
> > > > > > > /* 'lb_data' engine node manages the NB load balancers and load
> > > > > balancer
> > > > > > > * groups. For each NB LB, it creates 'struct ovn_northd_lb'
> and
> > > > > > > @@ -80,9 +89,13 @@ en_lb_data_run(struct engine_node *node, void
> > > *data)
> > > > > > > EN_OVSDB_GET(engine_get_input("NB_load_balancer",
> node));
> > > > > > > const struct nbrec_load_balancer_group_table *nb_lbg_table
> =
> > > > > > > EN_OVSDB_GET(engine_get_input("NB_load_balancer_group",
> > > node));
> > > > > > > + const struct nbrec_logical_switch_table *nb_ls_table =
> > > > > > > + EN_OVSDB_GET(engine_get_input("NB_logical_switch",
> node));
> > > > > > >
> > > > > > > lb_data->tracked = false;
> > > > > > > build_lbs(nb_lb_table, nb_lbg_table, &lb_data->lbs,
> > > > > &lb_data->lb_groups);
> > > > > > > + build_od_lb_map(nb_ls_table, &lb_data->ls_lb_map);
> > > > > > > +
> > > > > > > engine_set_node_state(node, EN_UPDATED);
> > > > > > > }
> > > > > > >
> > > > > > > @@ -230,18 +243,104 @@
> lb_data_load_balancer_group_handler(struct
> > > > > engine_node *node, void *data)
> > > > > > > return true;
> > > > > > > }
> > > > > > >
> > > > > > > +struct od_lb_data {
> > > > > > > + struct hmap_node hmap_node;
> > > > > > > + struct uuid od_uuid;
> > > > > > > + struct uuidset *lbs;
> > > > > > > + struct uuidset *lbgrps;
> > > > > > > +};
> > > > > > > +
> > > > > > > +bool
> > > > > > > +lb_data_logical_switch_handler(struct engine_node *node, void
> > > *data)
> > > > > > > +{
> > > > > > > + struct ed_type_lb_data *lb_data = (struct ed_type_lb_data
> *)
> > > data;
> > > > > > > + const struct nbrec_logical_switch_table *nb_ls_table =
> > > > > > > + EN_OVSDB_GET(engine_get_input("NB_logical_switch",
> node));
> > > > > > > +
> > > > > > > + struct tracked_lb_data *trk_lb_data =
> > > &lb_data->tracked_lb_data;
> > > > > > > + lb_data->tracked = true;
> > > > > > > +
> > > > > > > + bool changed = false;
> > > > > > > + const struct nbrec_logical_switch *nbs;
> > > > > > > + NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (nbs,
> nb_ls_table)
> > > {
> > > > > > > + if (nbrec_logical_switch_is_deleted(nbs)) {
> > > > > > > + struct od_lb_data *od_lb_data =
> > > > > > > + find_od_lb_data(&lb_data->ls_lb_map,
> > > > > &nbs->header_.uuid);
> > > > > > > + if (od_lb_data) {
> > > > > > > + hmap_remove(&lb_data->ls_lb_map,
> > > > > &od_lb_data->hmap_node);
> > > > > > > + destroy_od_lb_data(od_lb_data);
> > > > > > > + }
> > > > > >
> > > > > > Why not tracking deleted ls_lb mapping?
> > > >
> > > > Presently lb_data engine node only provides the associated or
> > > > disassociated load balancers / lb groups in its tracking data
> > > > for a created or updated logical switch.
> > > >
> > > > If any logical switch (or router) is deleted, northd engine node
> > > > results in full recompute. So there is not much value in providing
> > > > that
> > > > information in the lb_data's tracked data.
> > > >
> > > Thanks for explaining. I think it is better to avoid implicitly relying
> on
> > > recompute triggered by other node/handler to ensure the correctness,
> > > because it is hard to follow the logic of the code and hard to maintain.
> >
> > lb_data engine node
> > - maintains the data related to load balancers and their
> > association with logical switches and routers.
> > - And for any changes related to load balancers/lb groups, it
> > provides tracking information to the dependent nodes about
> > - load balancer addition/deletion/updation
> > - and load balancer (lb group) associating or
> > disassociating to a logical switch or router
> >
> > The northd engine node maintains the hmap of "lb_datapaths" and
> > "lb_group_datapaths" related to load balancers.
> > It can build this information from all the relevant input nodes -
> > which would be nb_logical_switch and lb_data.
> > When a logical switch gets deleted and when we add I-P for it, northd
> > engine node needs to clear the deleted logical switch's index id from
> > its
> > associated load balancers in the "lb_datapaths" and
> > "lb_group_datapaths" hmap. And it can get the list of associated lbs
> > using nbs->n_load_balancer and nbs->load_balancer columns
> > and clear the index in the "lb_datapaths" and "lb_group_datapaths" hmap.
> >
> OK, but the same would apply for newly created LS, right? When a LS is
> created and handled I-P in northd node, it can also establish the mappings
> from its input, right? So what the lb_data_logical_switch_handler really
> cares about is the LS-DP association change when LS is NOT created/deleted,
> while the function skips deleted LS handling but tracks for newly created
> LS. I think this might have also caused the confusion. If such tracking is
> not useful at all (even when LS create/delete I-P is handled in northd
> node) we shouldn't track them. I'd skip the LS creation as well in this
> function. Is my understanding correct?
After reading your comments, I think it makes sense to also include
the deleted ls/lr in the tracked data of lb_data.
Otherwise northd engine has to call ovn_lb_datapaths_add_ls() in 2
places. One when a logical switch is created in the
ls_handler and the other in lb_data_handler when the logical switch
gets updated with the association of a
load balancer/lb group.
The code snipped I shared below tracks the deleted logical switch with
lb/lb grp associations.
>
> > That is why I thought it is not necessary for the "lb_data" to also
> > provide this information.
> >
> > But I don't think it's hard to include that.
> >
> > In the next version, I'll be including the changes below.
> >
> >
> > --------------------------------
> > diff --git a/northd/en-lb-data.c b/northd/en-lb-data.c
> > index 02b1bfd7a4..f51d61e0f1 100644
> > --- a/northd/en-lb-data.c
> > +++ b/northd/en-lb-data.c
> > @@ -243,13 +243,6 @@ lb_data_load_balancer_group_handler(struct
> > engine_node *node, void *data)
> > return true;
> > }
> >
> > -struct od_lb_data {
> > - struct hmap_node hmap_node;
> > - struct uuid od_uuid;
> > - struct uuidset *lbs;
> > - struct uuidset *lbgrps;
> > -};
> > -
> > bool
> > lb_data_logical_switch_handler(struct engine_node *node, void *data)
> > {
> > @@ -268,7 +261,7 @@ lb_data_logical_switch_handler(struct engine_node
> > *node, void *data)
> > find_od_lb_data(&lb_data->ls_lb_map, &nbs->header_.uuid);
> > if (od_lb_data) {
> > hmap_remove(&lb_data->ls_lb_map, &od_lb_data->hmap_node);
> > - destroy_od_lb_data(od_lb_data);
> > + hmapx_add(&trk_lb_data->deleted_od_lb_data, od_lb_data);
> > }
> > } else {
> > if (!is_ls_lbs_changed(nbs)) {
> > @@ -341,6 +334,7 @@ lb_data_init(struct ed_type_lb_data *lb_data)
> > hmap_init(&trk_lb_data->crupdated_lbgrps);
> > hmapx_init(&trk_lb_data->deleted_lbgrps);
> > ovs_list_init(&trk_lb_data->crupdated_ls_lbs);
> > + hmapx_destroy(&trk_lb_data->deleted_od_lb_data);
> > }
> >
> > static void
> > @@ -369,6 +363,7 @@ lb_data_destroy(struct ed_type_lb_data *lb_data)
> > hmapx_destroy(&lb_data->tracked_lb_data.deleted_lbs);
> > hmapx_destroy(&lb_data->tracked_lb_data.deleted_lbgrps);
> > hmap_destroy(&lb_data->tracked_lb_data.crupdated_lbgrps);
> > + hmapx_destroy(&lb_data->tracked_lb_data.deleted_od_lb_data);
> > }
> >
> > static void
> > @@ -500,6 +495,11 @@ destroy_tracked_data(struct ed_type_lb_data *lb_data)
> >
> > free(codlb);
> > }
> > +
> > + HMAPX_FOR_EACH_SAFE(node,
> &lb_data->tracked_lb_data.deleted_od_lb_data) {
> > + destroy_od_lb_data(node->data);
> > + hmapx_delete(&lb_data->tracked_lb_data.deleted_od_lb_data, node);
> > + }
> > }
> >
> > static void
> > diff --git a/northd/en-lb-data.h b/northd/en-lb-data.h
> > index 022b38523b..6b2ba11cb8 100644
> > --- a/northd/en-lb-data.h
> > +++ b/northd/en-lb-data.h
> > @@ -53,6 +53,10 @@ struct tracked_lb_data {
> > * 'struct crupdated_od_lb_data' */
> > struct ovs_list crupdated_ls_lbs;
> >
> > + /* hmapx of deleted logical switches which have load balancer or lb
> groups
> > + * associated with it. hmapx_node is 'struct od_lb_data'. */
> > + struct hmapx deleted_od_lb_data;
> > +
> > /* Indicates if any of the tracked lb has health checks enabled. */
> > bool has_health_checks;
> >
> > @@ -64,6 +68,14 @@ struct tracked_lb_data {
> > bool has_dissassoc_lbs_from_od;
> > };
> >
> > +/* Datapath (logical switch) to lb/lbgrp association data. */
> > +struct od_lb_data {
> > + struct hmap_node hmap_node;
> > + struct uuid od_uuid;
> > + struct uuidset *lbs;
> > + struct uuidset *lbgrps;
> > +};
> > +
> > /* struct which maintains the data of the engine node lb_data. */
> > struct ed_type_lb_data {
> > /* hmap of load balancers. hmap node is 'struct ovn_northd_lb *' */
> > @@ -71,6 +83,8 @@ struct ed_type_lb_data {
> >
> > /* hmap of load balancer groups. hmap node is 'struct ovn_lb_group
> *' */
> > struct hmap lbgrps;
> > +
> > + /* hmap of ls to lb map. hmap node is 'struct od_lb_data'. */
> > struct hmap ls_lb_map;
> >
> > /* tracked data*/
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 31904bd808..f2aedb213e 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -5473,6 +5473,12 @@ northd_handle_lb_data_changes(struct
> > tracked_lb_data *trk_lb_data,
> > return false;
> > }
> >
> > + /* Fall back to recompute if any logical switch or router was
> deleted which
> > + * had load balancer or lb group association. */
> > + if (!hmapx_is_empty(&trk_lb_data->deleted_od_lb_data)) {
> > + return false;
> > + }
> > +
> > struct ovn_lb_datapaths *lb_dps;
> > struct ovn_northd_lb *lb;
> > struct ovn_datapath *od;
> >
> > ----------------------------------
> >
> >
> > > For example, if someone implements I-P for logical_switch deletion the
> > > northd node, there is no direct hint for him/her to update the logic
> here.
> > > I suggest adding the tracking here if it is not complex. Otherwise, at
> > > least add TODO/XXX comments to call out the implicite dependency for
> such
> > > considerations.
> > >
> > > > > >
> > > > > > > + } else {
> > > > > > > + if (!is_ls_lbs_changed(nbs)) {
> > > > > > > + continue;
> > > > > > > + }
> > > > > > > + struct crupdated_od_lb_data *codlb = xzalloc(sizeof
> > > > > *codlb);
> > > > > > > + codlb->od_uuid = nbs->header_.uuid;
> > > > > > > + uuidset_init(&codlb->assoc_lbs);
> > > > > > > +
> > > > > > > + struct od_lb_data *od_lb_data =
> > > > > > > + find_od_lb_data(&lb_data->ls_lb_map,
> > > > > &nbs->header_.uuid);
> > > > > > > + if (!od_lb_data) {
> > > > > > > + od_lb_data =
> create_od_lb_data(&lb_data->ls_lb_map,
> > > > > > > +
> > > &nbs->header_.uuid);
> > > > > > > + }
> > > > > > > +
> > > > > > > + struct uuidset *pre_lb_uuids = od_lb_data->lbs;
> > > > > > > + od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs);
> > > > > > > + uuidset_init(od_lb_data->lbs);
> > > > > > > +
> > > > > > > + for (size_t i = 0; i < nbs->n_load_balancer; i++) {
> > > > > > > + const struct uuid *lb_uuid =
> > > > > > > + &nbs->load_balancer[i]->header_.uuid;
> > > > > > > + uuidset_insert(od_lb_data->lbs, lb_uuid);
> > > > > > > +
> > > > > > > + struct uuidset_node *unode =
> > > uuidset_find(pre_lb_uuids,
> > > > > > > +
> lb_uuid);
> > > > > > > +
> > > > > > > + if (!unode ||
> (nbrec_load_balancer_row_get_seqno(
> > > > > > > + nbs->load_balancer[i],
> > > > > OVSDB_IDL_CHANGE_MODIFY) > 0)) {
> > > > > > > + /* Add this lb to the tracked data. */
> > > > > > > + uuidset_insert(&codlb->assoc_lbs, lb_uuid);
> > > > > > > + changed = true;
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (unode) {
> > > > > > > + uuidset_delete(pre_lb_uuids, unode);
> > > > > > > + }
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (!uuidset_is_empty(pre_lb_uuids)) {
> > > > > > > + trk_lb_data->has_dissassoc_lbs_from_od = true;
> > > > > > > + changed = true;
> > > > > > > + }
> > > > > > > +
> > > > > > > + uuidset_destroy(pre_lb_uuids);
> > > > > > > + free(pre_lb_uuids);
> > > > > > > +
> > > > > > > + ovs_list_insert(&trk_lb_data->crupdated_ls_lbs,
> > > > > &codlb->list_node);
> > > > > > > + }
> > > > > > > + }
> > > > > > > +
> > > > > > > + if (changed) {
> > > > > > > + engine_set_node_state(node, EN_UPDATED);
> > > > > > > + }
> > > > > > > + return true;
> > > > > > > +}
> > > > > > > +
> > > > > > > /* static functions. */
> > > > > > > static void
> > > > > > > lb_data_init(struct ed_type_lb_data *lb_data)
> > > > > > > {
> > > > > > > hmap_init(&lb_data->lbs);
> > > > > > > hmap_init(&lb_data->lb_groups);
> > > > > > > + hmap_init(&lb_data->ls_lb_map);
> > > > > > >
> > > > > > > struct tracked_lb_data *trk_lb_data =
> > > &lb_data->tracked_lb_data;
> > > > > > > hmap_init(&trk_lb_data->crupdated_lbs);
> > > > > > > hmapx_init(&trk_lb_data->deleted_lbs);
> > > > > > > hmap_init(&trk_lb_data->crupdated_lb_groups);
> > > > > > > hmapx_init(&trk_lb_data->deleted_lb_groups);
> > > > > > > + ovs_list_init(&trk_lb_data->crupdated_ls_lbs);
> > > > > > > }
> > > > > > >
> > > > > > > static void
> > > > > > > @@ -259,6 +358,12 @@ lb_data_destroy(struct ed_type_lb_data
> > > *lb_data)
> > > > > > > }
> > > > > > > hmap_destroy(&lb_data->lb_groups);
> > > > > > >
> > > > > > > + struct od_lb_data *od_lb_data;
> > > > > > > + HMAP_FOR_EACH_POP (od_lb_data, hmap_node,
> &lb_data->ls_lb_map)
> > > {
> > > > > > > + destroy_od_lb_data(od_lb_data);
> > > > > > > + }
> > > > > > > + hmap_destroy(&lb_data->ls_lb_map);
> > > > > > > +
> > > > > > > destroy_tracked_data(lb_data);
> > > > > > > hmap_destroy(&lb_data->tracked_lb_data.crupdated_lbs);
> > > > > > > hmapx_destroy(&lb_data->tracked_lb_data.deleted_lbs);
> > > > > > > @@ -301,12 +406,67 @@ create_lb_group(const struct
> > > > > nbrec_load_balancer_group *nbrec_lb_group,
> > > > > > > return lb_group;
> > > > > > > }
> > > > > > >
> > > > > > > +static void
> > > > > > > +build_od_lb_map(const struct nbrec_logical_switch_table
> > > > > *nbrec_ls_table,
> > > > > > > + struct hmap *od_lb_map)
> > > > > > > +{
> > > > > > > + const struct nbrec_logical_switch *nbrec_ls;
> > > > > > > + NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbrec_ls,
> nbrec_ls_table)
> > > {
> > > > > > > + if (!nbrec_ls->n_load_balancer) {
> > > > > > > + continue;
> > > > > > > + }
> > > > > > > +
> > > > > > > + struct od_lb_data *od_lb_data =
> > > > > > > + create_od_lb_data(od_lb_map,
> &nbrec_ls->header_.uuid);
> > > > > > > + for (size_t i = 0; i < nbrec_ls->n_load_balancer; i++)
> {
> > > > > > > + uuidset_insert(od_lb_data->lbs,
> > > > > > > +
> > > &nbrec_ls->load_balancer[i]->header_.uuid);
> > > > > > > + }
> > > > > > > + }
> > > > > > > +}
> > > > > > > +
> > > > > > > +static struct od_lb_data *
> > > > > > > +create_od_lb_data(struct hmap *od_lb_map, const struct uuid
> > > *od_uuid)
> > > > > > > +{
> > > > > > > + struct od_lb_data *od_lb_data = xzalloc(sizeof
> *od_lb_data);
> > > > > > > + od_lb_data->od_uuid = *od_uuid;
> > > > > > > + od_lb_data->lbs = xzalloc(sizeof *od_lb_data->lbs);
> > > > > > > + uuidset_init(od_lb_data->lbs);
> > > > > > > +
> > > > > > > + hmap_insert(od_lb_map, &od_lb_data->hmap_node,
> > > > > > > + uuid_hash(&od_lb_data->od_uuid));
> > > > > > > + return od_lb_data;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static struct od_lb_data *
> > > > > > > +find_od_lb_data(struct hmap *od_lb_map, const struct uuid
> *od_uuid)
> > > > > > > +{
> > > > > > > + struct od_lb_data *od_lb_data;
> > > > > > > + HMAP_FOR_EACH_WITH_HASH (od_lb_data, hmap_node,
> > > uuid_hash(od_uuid),
> > > > > > > + od_lb_map) {
> > > > > > > + if (uuid_equals(&od_lb_data->od_uuid, od_uuid)) {
> > > > > > > + return od_lb_data;
> > > > > > > + }
> > > > > > > + }
> > > > > > > +
> > > > > > > + return NULL;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void
> > > > > > > +destroy_od_lb_data(struct od_lb_data *od_lb_data)
> > > > > > > +{
> > > > > > > + uuidset_destroy(od_lb_data->lbs);
> > > > > > > + free(od_lb_data->lbs);
> > > > > > > + free(od_lb_data);
> > > > > > > +}
> > > > > > > +
> > > > > > > static void
> > > > > > > destroy_tracked_data(struct ed_type_lb_data *lb_data)
> > > > > > > {
> > > > > > > lb_data->tracked = false;
> > > > > > > lb_data->tracked_lb_data.has_health_checks = false;
> > > > > > > lb_data->tracked_lb_data.has_dissassoc_lbs_from_lb_grops =
> > > false;
> > > > > > > + lb_data->tracked_lb_data.has_dissassoc_lbs_from_od = false;
> > > > > > >
> > > > > > > struct hmapx_node *node;
> > > > > > > HMAPX_FOR_EACH_SAFE (node,
> > > &lb_data->tracked_lb_data.deleted_lbs) {
> > > > > > > @@ -331,6 +491,15 @@ destroy_tracked_data(struct ed_type_lb_data
> > > > > *lb_data)
> > > > > > > hmapx_destroy(&crupdated_lbg->assoc_lbs);
> > > > > > > free(crupdated_lbg);
> > > > > > > }
> > > > > > > +
> > > > > > > + struct crupdated_od_lb_data *codlb;
> > > > > > > + LIST_FOR_EACH_SAFE (codlb, list_node,
> > > > > > > +
> > > &lb_data->tracked_lb_data.crupdated_ls_lbs) {
> > > > > > > + ovs_list_remove(&codlb->list_node);
> > > > > > > + uuidset_destroy(&codlb->assoc_lbs);
> > > > > > > +
> > > > > > > + free(codlb);
> > > > > > > + }
> > > > > > > }
> > > > > > >
> > > > > > > static void
> > > > > > > @@ -376,3 +545,10 @@ add_deleted_lb_group_to_tracked_data(struct
> > > > > ovn_lb_group *lbg,
> > > > > > > {
> > > > > > > hmapx_add(&tracked_lb_data->deleted_lb_groups, lbg);
> > > > > > > }
> > > > > > > +
> > > > > > > +static bool
> > > > > > > +is_ls_lbs_changed(const struct nbrec_logical_switch *nbs) {
> > > > > > > + return ((nbrec_logical_switch_is_new(nbs) &&
> > > nbs->n_load_balancer)
> > > > > > > + || nbrec_logical_switch_is_updated(nbs,
> > > > > > > +
> NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER));
> > > > > > > +}
> > > > > > > diff --git a/northd/en-lb-data.h b/northd/en-lb-data.h
> > > > > > > index 2e9a024620..b2f86322a2 100644
> > > > > > > --- a/northd/en-lb-data.h
> > > > > > > +++ b/northd/en-lb-data.h
> > > > > > > @@ -6,6 +6,7 @@
> > > > > > > #include "openvswitch/hmap.h"
> > > > > > > #include "include/openvswitch/list.h"
> > > > > > > #include "lib/hmapx.h"
> > > > > > > +#include "lib/uuidset.h"
> > > > > > >
> > > > > > > #include "lib/inc-proc-eng.h"
> > > > > > >
> > > > > > > @@ -27,6 +28,13 @@ struct crupdated_lb_group {
> > > > > > > struct hmapx assoc_lbs;
> > > > > > > };
> > > > > > >
> > > > > > > +struct crupdated_od_lb_data {
> > > > > > > + struct ovs_list list_node;
> > > > > > > +
> > > > > > > + struct uuid od_uuid;
> > > > > > > + struct uuidset assoc_lbs;
> > > > > > > +};
> > > > > > > +
> > > > > > > struct tracked_lb_data {
> > > > > > > /* Both created and updated lbs. hmapx node is 'struct
> > > > > crupdated_lb *'. */
> > > > > > > struct hmap crupdated_lbs;
> > > > > > > @@ -41,12 +49,19 @@ struct tracked_lb_data {
> > > > > > > /* Deleted lb_groups. hmapx node is 'struct ovn_lb_group
> *'.
> > > */
> > > > > > > struct hmapx deleted_lb_groups;
> > > > > > >
> > > > > > > + /* List of logical switch <-> lb changes. List node is
> > > > > > > + * 'struct crupdated_od_lb_data' */
> > > > > > > + struct ovs_list crupdated_ls_lbs;
> > > > > > > +
> > > > > > > /* Indicates if any of the tracked lb has health checks
> > > enabled. */
> > > > > > > bool has_health_checks;
> > > > > > >
> > > > > > > /* Indicates if any lb got disassociated from a lb group
> > > > > > > * but not deleted. */
> > > > > > > bool has_dissassoc_lbs_from_lb_grops;
> > > > > > > +
> > > > > > > + /* Indicates if a lb was disassociated from a logical
> switch.
> > > */
> > > > > > > + bool has_dissassoc_lbs_from_od;
> > > > > >
> > > > > > When a LS is deleted, all the LBs attached to the LS are
> dissasociated
> > > > > from it, but this is not tracked in
> lb_data_logical_switch_handler(). Is
> > > > > this missing?
> > > >
> > > > No. It's intentional. Please see the above comment.
> > > >
> > > >
> > > > > >
> > > > > > > };
> > > > > > >
> > > > > > > /* struct which maintains the data of the engine node lb_data.
> */
> > > > > > > @@ -56,6 +71,7 @@ struct ed_type_lb_data {
> > > > > > >
> > > > > > > /* hmap of load balancer groups. hmap node is 'struct
> > > > > ovn_lb_group *' */
> > > > > > > struct hmap lb_groups;
> > > > > > > + struct hmap ls_lb_map;
> > > > > > >
> > > > > > > /* tracked data*/
> > > > > > > bool tracked;
> > > > > > > @@ -69,5 +85,6 @@ void en_lb_data_clear_tracked_data(void
> *data);
> > > > > > >
> > > > > > > bool lb_data_load_balancer_handler(struct engine_node *, void
> > > *data);
> > > > > > > bool lb_data_load_balancer_group_handler(struct engine_node *,
> void
> > > > > *data);
> > > > > > > +bool lb_data_logical_switch_handler(struct engine_node *, void
> > > *data);
> > > > > > >
> > > > > > > #endif /* end of EN_NORTHD_LB_DATA_H */
> > > > > > > diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> > > > > > > index db1bcbccd6..77e2eff056 100644
> > > > > > > --- a/northd/en-lflow.c
> > > > > > > +++ b/northd/en-lflow.c
> > > > > > > @@ -102,6 +102,12 @@ lflow_northd_handler(struct engine_node
> *node,
> > > > > > > if (!northd_data->change_tracked) {
> > > > > > > return false;
> > > > > > > }
> > > > > > > +
> > > > > > > + /* Fall back to recompute if lb related data has changed.
> */
> > > > > > > + if (northd_data->lb_changed) {
> > > > > > > + return false;
> > > > > > > + }
> > > > > > > +
> > > > > > > const struct engine_context *eng_ctx =
> engine_get_context();
> > > > > > > struct lflow_data *lflow_data = data;
> > > > > > >
> > > > > > > diff --git a/northd/en-northd.c b/northd/en-northd.c
> > > > > > > index d59ef062df..9d1838a1a4 100644
> > > > > > > --- a/northd/en-northd.c
> > > > > > > +++ b/northd/en-northd.c
> > > > > > > @@ -207,7 +207,7 @@ northd_sb_port_binding_handler(struct
> > > engine_node
> > > > > *node,
> > > > > > > }
> > > > > > >
> > > > > > > bool
> > > > > > > -northd_lb_data_handler(struct engine_node *node, void *data)
> > > > > > > +northd_lb_data_handler_pre_od(struct engine_node *node, void
> *data)
> > > > > > > {
> > > > > > > struct ed_type_lb_data *lb_data =
> > > engine_get_input_data("lb_data",
> > > > > node);
> > > > > > >
> > > > > > > @@ -230,19 +230,47 @@ northd_lb_data_handler(struct engine_node
> > > *node,
> > > > > void *data)
> > > > > > >
> > > > > > > /* Fall back to recompute if load balancer groups are
> deleted.
> > > */
> > > > > > > if
> > > (!hmapx_is_empty(&lb_data->tracked_lb_data.deleted_lb_groups)) {
> > > > > > > + }
> > > > >
> > > > > The above if branch is empty, but according to the comment it is
> > > supposed
> > > > > to return false.
> > > > > (Sorry that I forgot to mention this in my previous reply)
> > > >
> > > > Ack. Fixed it.
> > > >
> > > > >
> > > > > Regards,
> > > > > Han
> > > > >
> > > > > > > +
> > > > > > > + if (lb_data->tracked_lb_data.has_dissassoc_lbs_from_od) {
> > > > > > > return false;
> > > > > > > }
> > > > > > >
> > > > > > > struct northd_data *nd = data;
> > > > > > >
> > > > > > > - if
> (!northd_handle_lb_data_changes(&lb_data->tracked_lb_data,
> > > > > > > - &nd->ls_datapaths,
> > > > > > > - &nd->lr_datapaths,
> > > > > > > - &nd->lb_datapaths_map,
> > > > > > > -
> > > &nd->lb_group_datapaths_map)) {
> > > > > > > + if
> > > > > (!northd_handle_lb_data_changes_pre_od(&lb_data->tracked_lb_data,
> > > > > > > +
> &nd->ls_datapaths,
> > > > > > > +
> &nd->lr_datapaths,
> > > > > > > +
> > > &nd->lb_datapaths_map,
> > > > > > > +
> > > > > &nd->lb_group_datapaths_map)) {
> > > > > > > + return false;
> > > > > > > + }
> > > > > > > +
> > > > > > > + engine_set_node_state(node, EN_UPDATED);
> > > > > > > + return true;
> > > > > > > +}
> > > > > > > +
> > > > > > > +bool
> > > > > > > +northd_lb_data_handler_post_od(struct engine_node *node, void
> > > *data)
> > > > > > > +{
> > > > > > > + struct ed_type_lb_data *lb_data =
> > > > > > > + engine_get_input_data("lb_data", node);
> > > > > > > +
> > > > > > > + ovs_assert(lb_data->tracked);
> > > > > > > +
> > > ovs_assert(!lb_data->tracked_lb_data.has_dissassoc_lbs_from_od);
> > > > > > > +
> > > > > > > + struct northd_data *nd = data;
> > > > > > > +
> > > > > > > + if
> > > > > (!northd_handle_lb_data_changes_post_od(&lb_data->tracked_lb_data,
> > > > > > > +
> &nd->ls_datapaths,
> > > > > > > +
> > > &nd->lb_datapaths_map))
> > > > > {
> > > > > > > return false;
> > > > > > > }
> > > > > > >
> > > > > > > + /* Indicate the depedendant engine nodes that load
> > > balancer/group
> > > > > > > + * related data has changed (including association to
> logical
> > > > > > > + * switch/router). */
> > > > > > > + nd->lb_changed = true;
> > > > > > > engine_set_node_state(node, EN_UPDATED);
> > > > > > > return true;
> > > > > > > }
> > > > > > > diff --git a/northd/en-northd.h b/northd/en-northd.h
> > > > > > > index 3c77b64bb2..5926e7a9d3 100644
> > > > > > > --- a/northd/en-northd.h
> > > > > > > +++ b/northd/en-northd.h
> > > > > > > @@ -17,6 +17,7 @@ void en_northd_clear_tracked_data(void *data);
> > > > > > > bool northd_nb_nb_global_handler(struct engine_node *, void
> *data
> > > > > OVS_UNUSED);
> > > > > > > bool northd_nb_logical_switch_handler(struct engine_node *,
> void
> > > > > *data);
> > > > > > > bool northd_sb_port_binding_handler(struct engine_node *, void
> > > *data);
> > > > > > > -bool northd_lb_data_handler(struct engine_node *, void *data);
> > > > > > > +bool northd_lb_data_handler_pre_od(struct engine_node *, void
> > > *data);
> > > > > > > +bool northd_lb_data_handler_post_od(struct engine_node *, void
> > > *data);
> > > > > > >
> > > > > > > #endif /* EN_NORTHD_H */
> > > > > > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> > > > > > > index 75d059645f..402c94e88c 100644
> > > > > > > --- a/northd/inc-proc-northd.c
> > > > > > > +++ b/northd/inc-proc-northd.c
> > > > > > > @@ -152,6 +152,8 @@ void inc_proc_northd_init(struct
> ovsdb_idl_loop
> > > *nb,
> > > > > > > lb_data_load_balancer_handler);
> > > > > > > engine_add_input(&en_lb_data, &en_nb_load_balancer_group,
> > > > > > > lb_data_load_balancer_group_handler);
> > > > > > > + engine_add_input(&en_lb_data, &en_nb_logical_switch,
> > > > > > > + lb_data_logical_switch_handler);
> > > > > > >
> > > > > > > engine_add_input(&en_northd, &en_nb_port_group, NULL);
> > > > > > > engine_add_input(&en_northd, &en_nb_acl, NULL);
> > > > > > > @@ -180,9 +182,10 @@ void inc_proc_northd_init(struct
> ovsdb_idl_loop
> > > > > *nb,
> > > > > > > northd_sb_port_binding_handler);
> > > > > > > engine_add_input(&en_northd, &en_nb_nb_global,
> > > > > > > northd_nb_nb_global_handler);
> > > > > > > - engine_add_input(&en_northd, &en_lb_data,
> > > northd_lb_data_handler);
> > > > > > > + engine_add_input(&en_northd, &en_lb_data,
> > > > > northd_lb_data_handler_pre_od);
> > > > > > > engine_add_input(&en_northd, &en_nb_logical_switch,
> > > > > > > northd_nb_logical_switch_handler);
> > > > > > > + engine_add_input(&en_northd, &en_lb_data,
> > > > > northd_lb_data_handler_post_od);
> > > > > > >
> > > > > > > engine_add_input(&en_mac_binding_aging, &en_nb_nb_global,
> > > NULL);
> > > > > > > engine_add_input(&en_mac_binding_aging, &en_sb_mac_binding,
> > > NULL);
> > > > > > > diff --git a/northd/northd.c b/northd/northd.c
> > > > > > > index 4cc9ef8c8d..6e8efbd496 100644
> > > > > > > --- a/northd/northd.c
> > > > > > > +++ b/northd/northd.c
> > > > > > > @@ -853,7 +853,6 @@ ovn_datapath_destroy(struct hmap *datapaths,
> > > struct
> > > > > ovn_datapath *od)
> > > > > > > ovn_ls_port_group_destroy(&od->nb_pgs);
> > > > > > > destroy_mcast_info_for_datapath(od);
> > > > > > > destroy_ports_for_datapath(od);
> > > > > > > -
> > > > > > > free(od);
> > > > > > > }
> > > > > > > }
> > > > > > > @@ -4919,6 +4918,7 @@ destroy_northd_data_tracked_changes(struct
> > > > > northd_data *nd)
> > > > > > > }
> > > > > > >
> > > > > > > nd->change_tracked = false;
> > > > > > > + nd->lb_changed = false;
> > > > > > > }
> > > > > > >
> > > > > > > /* Check if a changed LSP can be handled incrementally within
> the
> > > I-P
> > > > > engine
> > > > > > > @@ -5034,6 +5034,7 @@ ls_port_create(struct ovsdb_idl_txn
> > > *ovnsb_txn,
> > > > > struct hmap *ls_ports,
> > > > > > > * incrementally handled.
> > > > > > > * Presently supports i-p for the below changes:
> > > > > > > * - logical switch ports.
> > > > > > > + * - load balancers.
> > > > > > > */
> > > > > > > static bool
> > > > > > > ls_changes_can_be_handled(
> > > > > > > @@ -5042,8 +5043,11 @@ ls_changes_can_be_handled(
> > > > > > > /* Check if the columns are changed in this row. */
> > > > > > > enum nbrec_logical_switch_column_id col;
> > > > > > > for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++)
> {
> > > > > > > - if (nbrec_logical_switch_is_updated(ls, col) &&
> > > > > > > - col != NBREC_LOGICAL_SWITCH_COL_PORTS) {
> > > > > > > + if (nbrec_logical_switch_is_updated(ls, col)) {
> > > > > > > + if (col == NBREC_LOGICAL_SWITCH_COL_PORTS ||
> > > > > > > + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER)
> {
> > > > > > > + continue;
> > > > > > > + }
> > > > > > > return false;
> > > > > > > }
> > > > > > > }
> > > > > > > @@ -5072,12 +5076,6 @@ ls_changes_can_be_handled(
> > > > > > > return false;
> > > > > > > }
> > > > > > > }
> > > > > > > - for (size_t i = 0; i < ls->n_load_balancer; i++) {
> > > > > > > - if
> (nbrec_load_balancer_row_get_seqno(ls->load_balancer[i],
> > > > > > > - OVSDB_IDL_CHANGE_MODIFY) > 0) {
> > > > > > > - return false;
> > > > > > > - }
> > > > > > > - }
> > > > > > > for (size_t i = 0; i < ls->n_load_balancer_group; i++) {
> > > > > > > if
> > > > > (nbrec_load_balancer_group_row_get_seqno(ls->load_balancer_group[i],
> > > > > > > OVSDB_IDL_CHANGE_MODIFY) > 0) {
> > > > > > > @@ -5357,6 +5355,7 @@ northd_handle_ls_changes(struct
> ovsdb_idl_txn
> > > > > *ovnsb_idl_txn,
> > > > > > > if (!ovs_list_is_empty(&nd->tracked_ls_changes.updated)) {
> > > > > > > nd->change_tracked = true;
> > > > > > > }
> > > > > > > +
> > > > > > > return true;
> > > > > > >
> > > > > > > fail:
> > > > > > > @@ -5425,16 +5424,21 @@ northd_handle_sb_port_binding_changes(
> > > > > > >
> > > > > > > /* Handler for lb_data engine changes. For every tracked
> lb_data
> > > > > > > * it creates or deletes the
> > > ovn_lb_datapaths/ovn_lb_group_datapaths
> > > > > > > - * from the lb_datapaths hmap and lb_group_datapaths hmap. */
> > > > > > > + * from the lb_datapaths hmap and lb_group_datapaths hmap.
> > > > > > > + *
> > > > > > > + * This handler is called if there are any lb_data changes but
> > > before
> > > > > > > + * processing the logical switch changes.
> > > > > > > + * */
> > > > > > > bool
> > > > > > > -northd_handle_lb_data_changes(struct tracked_lb_data
> *trk_lb_data,
> > > > > > > - struct ovn_datapaths
> *ls_datapaths,
> > > > > > > - struct ovn_datapaths
> *lr_datapaths,
> > > > > > > - struct hmap *lb_datapaths_map,
> > > > > > > - struct hmap
> *lb_group_datapaths_map)
> > > > > > > +northd_handle_lb_data_changes_pre_od(struct tracked_lb_data
> > > > > *trk_lb_data,
> > > > > > > + struct ovn_datapaths
> > > > > *ls_datapaths,
> > > > > > > + struct ovn_datapaths
> > > > > *lr_datapaths,
> > > > > > > + struct hmap
> *lb_datapaths_map,
> > > > > > > + struct hmap
> > > > > *lb_group_datapaths_map)
> > > > > > > {
> > > > > > > struct ovn_lb_datapaths *lb_dps;
> > > > > > > struct ovn_northd_lb *lb;
> > > > > > > + struct ovn_datapath *od;
> > > > > > > struct hmapx_node *hmapx_node;
> > > > > > > HMAPX_FOR_EACH (hmapx_node, &trk_lb_data->deleted_lbs) {
> > > > > > > lb = hmapx_node->data;
> > > > > > > @@ -5442,6 +5446,16 @@ northd_handle_lb_data_changes(struct
> > > > > tracked_lb_data *trk_lb_data,
> > > > > > >
> > > > > > > lb_dps = ovn_lb_datapaths_find(lb_datapaths_map,
> lb_uuid);
> > > > > > > ovs_assert(lb_dps);
> > > > > > > +
> > > > > > > + /* Re-evaluate 'od->has_lb_vip for od's associated
> with the
> > > > > > > + * deleted lb. */
> > > > > > > + size_t index;
> > > > > > > + BITMAP_FOR_EACH_1 (index, ods_size(ls_datapaths),
> > > > > > > + lb_dps->nb_ls_map) {
> > > > > > > + od = ls_datapaths->array[index];
> > > > > > > + init_lb_for_datapath(od);
> > > > > > > + }
> > > > > > > +
> > > > > > > hmap_remove(lb_datapaths_map, &lb_dps->hmap_node);
> > > > > > > ovn_lb_datapaths_destroy(lb_dps);
> > > > > > > }
> > > > > > > @@ -5481,6 +5495,65 @@ northd_handle_lb_data_changes(struct
> > > > > tracked_lb_data *trk_lb_data,
> > > > > > > return true;
> > > > > > > }
> > > > > > >
> > > > > > > +/* This handler is called if there are any lb_data changes but
> afer
> > > > > processing
> > > > > > > + * the logical switch changes.
> > > > > > > + *
> > > > > > > + * For every tracked data it does the following:
> > > > > > > + * For any changes to a created or updated logical switch due
> to
> > > > > > > + * association of a load balancer (eg. ovn-nbctl ls-lb-add sw0
> > > lb1),
> > > > > > > + * the logical switch datapath is added to the load balancer
> > > > > (represented
> > > > > > > + * by 'struct ovn_lb_datapaths') by calling
> > > ovn_lb_datapaths_add_ls().
> > > > > > > + *
> > > > > > > + * For every created or updated load balancer in the tracked
> data,
> > > > > > > + * it gets the associated logical switches and for each switch
> it
> > > > > > > + * re-evaluates 'od->has_lb_vip'. Since this handler is called
> > > after
> > > > > > > + * handling any logical switch changes. */
> > > > > >
> > > > > > Is the last sentence incomplete?
> > > >
> > > > It is. In fact it is continuation of previous sentence.
> > > > > >
> > > > > > And I really didn't understand the last part. If there is any
> change
> > > of
> > > > > the relationship between a LS and a LB, it should be reflected in
> > > > > crupdated_ls_lbs, and handled in the first loop. So what's the
> purpose
> > > of
> > > > > the second loop?
> > > >
> > > > I'm not very sure If I understood your comment. Please see the v7 and
> > > > let me know if you still have some comments.
> > > >
> > >
> > > I will comment and explain in v7.
> > > >
> > > > > >
> > > > > > > +bool
> > > > > > > +northd_handle_lb_data_changes_post_od(struct tracked_lb_data
> > > > > *trk_lb_data,
> > > > > > > + struct ovn_datapaths
> > > > > *ls_datapaths,
> > > > > > > + struct hmap
> > > *lb_datapaths_map)
> > > > > > > +{
> > > > > > > + ovs_assert(!trk_lb_data->has_health_checks);
> > > > > > > +
> > > > > > > + struct ovn_northd_lb *lb;
> > > > > > > + struct ovn_lb_datapaths *lb_dps;
> > > > > > > + struct ovn_datapath *od;
> > > > > > > + struct crupdated_od_lb_data *codlb;
> > > > > > > +
> > > > > > > + LIST_FOR_EACH (codlb, list_node,
> > > &trk_lb_data->crupdated_ls_lbs) {
> > > > > > > + od = ovn_datapath_find(&ls_datapaths->datapaths,
> > > > > &codlb->od_uuid);
> > > > > > > + ovs_assert(od);
> > > > > > > +
> > > > > > > + struct uuidset_node *uuidnode;
> > > > > > > + UUIDSET_FOR_EACH (uuidnode, &codlb->assoc_lbs) {
> > > > > > > + lb_dps = ovn_lb_datapaths_find(lb_datapaths_map,
> > > > > &uuidnode->uuid);
> > > > > > > + ovs_assert(lb_dps);
> > > > > > > + ovn_lb_datapaths_add_ls(lb_dps, 1, &od);
> > > > > > > + }
> > > > > > > +
> > > > > > > + /* Re-evaluate 'od->has_lb_vip' */
> > > > > > > + init_lb_for_datapath(od);
> > > > > > > + }
> > > > > > > +
> > > > > > > + struct crupdated_lb *clb;
> > > > > > > + HMAP_FOR_EACH (clb, hmap_node,
> &trk_lb_data->crupdated_lbs) {
> > > > > > > + lb = clb->lb;
> > > > > > > + const struct uuid *lb_uuid = &lb->nlb->header_.uuid;
> > > > > > > +
> > > > > > > + lb_dps = ovn_lb_datapaths_find(lb_datapaths_map,
> lb_uuid);
> > > > > > > + ovs_assert(lb_dps);
> > > > > > > + size_t index;
> > > > > > > + BITMAP_FOR_EACH_1 (index, ods_size(ls_datapaths),
> > > > > > > + lb_dps->nb_ls_map) {
> > > > > > > + od = ls_datapaths->array[index];
> > > > > > > + /* Re-evaluate 'od->has_lb_vip' */
> > > > > > > + init_lb_for_datapath(od);
> > > > > > > + }
> > > > > > > + }
> > > > > > > +
> > > > > > > + return true;
> > > > > > > +}
> > > > > > > +
> > > > > > > struct multicast_group {
> > > > > > > const char *name;
> > > > > > > uint16_t key; /*
> > > > > OVN_MIN_MULTICAST...OVN_MAX_MULTICAST. */
> > > > > > > @@ -16667,6 +16740,7 @@ bool
> lflow_handle_northd_ls_changes(struct
> > > > > ovsdb_idl_txn *ovnsb_txn,
> > > > > > > struct hmap *lflows)
> > > > > > > {
> > > > > > > struct ls_change *ls_change;
> > > > > > > +
> > > > > > > LIST_FOR_EACH (ls_change, list_node, &ls_changes->updated)
> {
> > > > > > > const struct sbrec_multicast_group *sbmc_flood =
> > > > > > >
> > > > > mcast_group_lookup(lflow_input->sbrec_mcast_group_by_name_dp,
> > > > > > > diff --git a/northd/northd.h b/northd/northd.h
> > > > > > > index 7d17921fa2..0ed7215356 100644
> > > > > > > --- a/northd/northd.h
> > > > > > > +++ b/northd/northd.h
> > > > > > > @@ -120,6 +120,8 @@ struct northd_data {
> > > > > > > struct hmap svc_monitor_map;
> > > > > > > bool change_tracked;
> > > > > > > struct tracked_ls_changes tracked_ls_changes;
> > > > > > > + bool lb_changed; /* Indicates if load balancers changed or
> > > > > association of
> > > > > > > + * load balancer to logical switch/router
> > > > > changed. */
> > > > > >
> > > > > > I think this var is unnecessary. We already have "change_tracked"
> > > which
> > > > > indicates if all the changes of the northd_data is tracked. Now if
> there
> > > > > are LB changes, and apparently we don't track it (at least not in
> this
> > > > > patch), we can simply set change_tracked to false, meaning some
> changes
> > > are
> > > > > not tracked (and so the dependent nodes would need to recompute
> > > > > accordingly).
> > > >
> > > > If we take the below ovn-nbctl command as an example
> > > > -----
> > > > ovn-nbctl --wait=sb ls-lb-add sw0 lb1 -- lsp-add sw0 sw0p1
> > > > -----
> > > >
> > > > 2 northd handlers will be called. northd_nb_logical_switch_handler()
> > > > will be called first and then
> > > > northd_lb_data_handler() will be called.
> > > >
> > > > northd_handle_ls_changes() will handle the changes to the logical
> > > > switch and sets nd->change_tracked = true.
> > > > northd_lb_data_handler() will handle the lb related changes. IMO
> > > > northd_lb_data_handler() should not set
> > > > "nd->change_tracked = false" because it is not aware of what changes
> > > > the northd_handle_ls_changes() has done.
> > > >
> > > nd->change_tracked is for the whole northd node, as the var name
> suggests,
> > > so partially untracked is still untracked.
> > > So, depending on what we want to achieve here:
> > >
> > > 1. If the only thing we care about LB related data in northd-data is
> > > whether LB has changes but not care about the details of the change, we
> can
> > > say the nd->lb_changed is everything we want to track for LB data, and
> we
> > > can still keep nd->change_tracked as true because that's all that we
> want
> > > to track for the northd, and we are sure that anything else in northd is
> > > NOT changed, such as nd->lr_ports.
> >
> > This is my intention of adding the "lb_changes". If you see the patch
> > 14 of v6, it includes
> > the actual changes related to "lb" and lflow engine node handles the
> > changes accordingly.
> >
> > My point is until we add this support, we only care if lb changed or
> > not and that's included in the tracked data.
> >
> Great, so we are on the same page that nd->change_tracked still means the
> whole northd node is tracked (instead of just for LS changes).
>
> > >
> > > 2. However, if we want to just tell that now we have untracked changes
> in
> > > northd-data other than the LS changes, meaning we don't know what else
> is
> > > changed and not sure about the impact (e.g. we don't know if the related
> > > changes would impact nd->lr_ports), then we should just set
> > > nd->change_tracked to false, regardless of change tracking status for
> any
> > > other change handler.
> > >
> > > > I think it is better for the dependent engine nodes to make a decision
> > > > to fall back to recompute or ignore "nd->lb_changed".
> > > >
> > > > If northd_lb_data_handler() sets "nd->change_tracked = false", then
> > > > the engine node "sync_from_sb" will unnecessarily
> > > > fall back to full recompute for the above example since the handle
> > > > sync_from_sb_northd_handler() returns false if nd->change_tracked is
> > > > false.
> > > >
> > > The original check in the sync_from_sb_northd_handler() was meant to
> ensure
> > > there is no change in the northd node other than LSP related changes.
> The
> > > original nd->change_tracked could ensure that. The assumption was that,
> > > anything other than LSP change happened in the northd data, the
> > > nd->change_tracked would be set to false. For example, if anything
> changed
> > > in nd->lr_ports, the sync_from_sb needs to recompute. If
> nd->change_tracked
> > > cannot ensure there is no other changes in the northd any more, the
> > > original check is not safe.
> >
> > But in this case the handler sync_from_sb_northd_handler() doesn't
> > need to do anything if lb_changed is also set to true.
> > But instead if northd_lb_data_handler() sets nd->change_tracked to
> > false we unnecessarily fall back to recompute for the
> > "sync_from_sb" node.
>
> At least the comment in the sync_from_sb_northd_handler() needs to be
> updated, because nd->change_tracked is not only about LS changes but also
> about LB related changes. Something like below:
>
> @@ -70,10 +70,12 @@ sync_from_sb_northd_handler(struct engine_node *node,
> {
> struct northd_data *nd = engine_get_input_data("northd", node);
> if (nd->change_tracked) {
> - /* There are only NB LSP related changes and the only field this
> node
> - * cares about is the "up" column, which is considered write-only
> to
> - * this node, so it is safe to ignore the change. (The real change
> - * matters to this node is always from the SB DB.) */
> + /* So far the changes tracked in northd don't impact this node.
> + *
> + * In particular, for the LS related changes, the only field this
> node
> + * cares about is the "up" column of LSP, which is considered
> + * write-only to this node, so it is safe to ignore the change.
> (The
> + * real change matters to this node is always from the SB DB.) */
> return true;
> }
> return false;
>
> What do you think?
> In theory we need to re-evaluate all the places the nd->change_tracked is
> used and see if the logic is still correct and if comments need to be
> updated, because the northd data changes indicated by this var is different
> now.
I think the comments needs to be updated in 2 places alteast. I can
update the comments in
v8.
Thanks
Numan
>
> Thanks,
> Han
>
> >
> > Thanks
> > Numan
> >
> > >
> > > >
> > > >
> > > > Request to please take a look at v7.
> > >
> > > Thanks Numan, will do.
> > >
> > > Han
> > >
> > > >
> > > > Thanks
> > > > Numan
> > > >
> > > >
> > > >
> > > > > >
> > > > > > In the future when we have the capability of tracking changes for
> LB,
> > > we
> > > > > will just have "tracked_lb_changes". In any case, we don't need this
> > > > > lb_changed.
> > > > > >
> > > > > > Thanks,
> > > > > > Han
> > > > > >
> > > > > > > };
> > > > > > >
> > > > > > > struct lflow_data {
> > > > > > > @@ -348,11 +350,14 @@ bool
> northd_handle_sb_port_binding_changes(
> > > > > > > const struct sbrec_port_binding_table *, struct hmap
> > > *ls_ports);
> > > > > > >
> > > > > > > struct tracked_lb_data;
> > > > > > > -bool northd_handle_lb_data_changes(struct tracked_lb_data *,
> > > > > > > - struct ovn_datapaths
> > > *ls_datapaths,
> > > > > > > - struct ovn_datapaths
> > > *lr_datapaths,
> > > > > > > - struct hmap
> *lb_datapaths_map,
> > > > > > > - struct hmap
> > > > > *lb_group_datapaths_map);
> > > > > > > +bool northd_handle_lb_data_changes_pre_od(struct
> tracked_lb_data *,
> > > > > > > + struct ovn_datapaths
> > > > > *ls_datapaths,
> > > > > > > + struct ovn_datapaths
> > > > > *lr_datapaths,
> > > > > > > + struct hmap
> > > > > *lb_datapaths_map,
> > > > > > > + struct hmap
> > > > > *lb_group_datapaths_map);
> > > > > > > +bool northd_handle_lb_data_changes_post_od(struct
> tracked_lb_data
> > > *,
> > > > > > > + struct ovn_datapaths
> > > > > *ls_datapaths,
> > > > > > > + struct hmap
> > > > > *lb_datapaths_map);
> > > > > > >
> > > > > > > void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn,
> > > > > > > const struct nbrec_bfd_table *,
> > > > > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > > > > > > index 213fd51d77..9e9b26ce09 100644
> > > > > > > --- a/tests/ovn-northd.at
> > > > > > > +++ b/tests/ovn-northd.at
> > > > > > > @@ -9861,22 +9861,23 @@ ovn-nbctl lsp-set-type sw0-lr0 router
> > > > > > > ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > ovn-nbctl --wait=sb lsp-set-options sw0-lr0 router-port=lr0-sw0
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > check_engine_stats northd recompute nocompute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > >
> > > > > > > -# Associate lb1 to sw0. There should be a full recompute of
> northd
> > > > > engine node
> > > > > > > +# Associate lb1 to sw0. There should be no recompute of northd
> > > engine
> > > > > node
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > -check_engine_stats northd recompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > +check_engine_stats northd norecompute compute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > # Modify the backend of the lb1 vip
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"
> 10.0.0.10:80
> > > > > "'='"10.0.0.100:80"'
> > > > > > > check_engine_stats lb_data norecompute compute
> > > > > > > -check_engine_stats northd recompute nocompute
> > > > > > > +check_engine_stats northd norecompute compute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > @@ -9884,7 +9885,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
> > > > > > > check_engine_stats lb_data norecompute compute
> > > > > > > -check_engine_stats northd recompute nocompute
> > > > > > > +check_engine_stats northd norecompute compute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > @@ -9892,7 +9893,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > 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 recompute nocompute
> > > > > > > +check_engine_stats northd norecompute compute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > @@ -9900,14 +9901,33 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80
> 10.0.0.30:8080
> > > > > > > check_engine_stats lb_data norecompute compute
> > > > > > > -check_engine_stats northd recompute nocompute
> > > > > > > +check_engine_stats northd norecompute compute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > # Disassociate lb1 from sw0. There should be a full recompute
> of
> > > > > northd engine node.
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb ls-lb-del sw0 lb1
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > +check_engine_stats northd recompute nocompute
> > > > > > > +check_engine_stats lflow recompute nocompute
> > > > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > +
> > > > > > > +# Associate lb1 to sw0 and also create a port sw0p1. This
> should
> > > > > result in
> > > > > > > +# full recompute of northd and lflow engine node.
> > > > > > > +check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > +check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 -- lsp-add sw0
> sw0p1
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > +check_engine_stats northd recompute compute
> > > > > > > +check_engine_stats lflow recompute nocompute
> > > > > > > +
> > > > > > > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > +check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > +
> > > > > > > +# Disassociate lb1 from sw0. There should be a recompute of
> northd
> > > > > engine node.
> > > > > > > +check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > +check ovn-nbctl --wait=sb ls-lb-del sw0 lb1
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > check_engine_stats northd recompute nocompute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > @@ -9995,7 +10015,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl add logical_switch sw0 load_balancer_group
> > > $lbg1_uuid
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > check_engine_stats northd recompute nocompute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > >
> > > > > > > @@ -10040,7 +10060,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl clear logical_switch sw0 load_balancer_group
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > check_engine_stats northd recompute nocompute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > >
> > > > > > > @@ -10092,7 +10112,7 @@ check_engine_stats lflow recompute
> nocompute
> > > > > > > # Add back lb group to logical switch and then delete it.
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl add logical_switch sw0 load_balancer_group
> > > $lbg1_uuid
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > check_engine_stats northd recompute nocompute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > >
> > > > > > > @@ -10133,7 +10153,7 @@ check_engine_stats lflow recompute
> nocompute
> > > > > > >
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl set logical_switch sw0
> > > load_balancer_group=$lbg1_uuid
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > check_engine_stats northd recompute nocompute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > @@ -10147,15 +10167,15 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb ls-lb-add sw0 lb2
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > -check_engine_stats northd recompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > +check_engine_stats northd norecompute compute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb ls-lb-add sw0 lb3
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > -check_engine_stats northd recompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > +check_engine_stats northd norecompute compute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > @@ -10169,7 +10189,7 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > >
> > > > > > > check as northd ovn-appctl -t NORTHD_TYPE
> inc-engine/clear-stats
> > > > > > > check ovn-nbctl --wait=sb ls-lb-del sw0 lb2
> > > > > > > -check_engine_stats lb_data norecompute nocompute
> > > > > > > +check_engine_stats lb_data norecompute compute
> > > > > > > check_engine_stats northd recompute nocompute
> > > > > > > check_engine_stats lflow recompute nocompute
> > > > > > > CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > > > > > > --
> > > > > > > 2.40.1
> > > > > > >
> > > > > > > _______________________________________________
> > > > > > > dev mailing list
> > > > > > > [email protected]
> > > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > > > _______________________________________________
> > > > > dev mailing list
> > > > > [email protected]
> > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > _______________________________________________
> > > dev mailing list
> > > [email protected]
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
