| Message ID | 20230911160041.179438-1-numans@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | northd: I-P for load balancer and lb groups | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Sep 11, 2023 at 9:01 AM <numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org> > > '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]} > > 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 northd handler 'northd_lb_data_handler' updates the > ovn_lb_datapaths's 'nb_ls_map' bitmap accordingly. > > 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]} > > The handler northd_lb_data_handler(), creates the > ovn_lb_datapaths object for lb1 and lb2 and deletes lb3 from > the ovn_lb_datapaths hmap. It does the same for the created or updated lb > groups lbg1 and lbg2 in the ovn_lbgrp_datapaths map. It also updates the > nb_ls_bitmap of lb1 for sw0 and sw1 and nb_ls_bitmap of lb2 for sw1. > > Reviewed-by: Ales Musil <amusil@redhat.com> > Acked-by: Mark Michelson <mmichels@redhat.com> > Signed-off-by: Numan Siddique <numans@ovn.org> > --- > 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 | 6 +- > northd/inc-proc-northd.c | 2 + > northd/northd.c | 83 +++++++++++++++--- > northd/northd.h | 4 +- > tests/ovn-northd.at | 56 +++++++++---- > 9 files changed, 322 insertions(+), 33 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 8acd9c8cb2..02b1bfd7a4 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_lbgrp * > struct tracked_lb_data *); > static void add_deleted_lbgrp_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->lbgrps); > + 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); > + } > + } 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->lbgrps); > + 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_lbgrps); > hmapx_init(&trk_lb_data->deleted_lbgrps); > + 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->lbgrps); > > + 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_lbgrps = 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_lbgrp_to_tracked_data(struct ovn_lb_group *lbg, > { > hmapx_add(&tracked_lb_data->deleted_lbgrps, 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 afb163dd9f..022b38523b 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_lbgrp { > 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_lbgrps; > > + /* 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_lbgrps; > + > + /* Indicates if a lb was disassociated from a logical switch. */ > + bool has_dissassoc_lbs_from_od; > }; > > /* 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 lbgrps; > + 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 ad8b62c735..2b84fef0ef 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -107,6 +107,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 1704a18834..8487b003f7 100644 > --- a/northd/en-northd.c > +++ b/northd/en-northd.c > @@ -1,4 +1,4 @@ > -/* > + /* > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > * You may obtain a copy of the License at: > @@ -214,6 +214,10 @@ northd_lb_data_handler(struct engine_node *node, void *data) > 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/inc-proc-northd.c b/northd/inc-proc-northd.c > index ccc6687207..303b58d43f 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -155,6 +155,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_logical_router, NULL); > engine_add_input(&en_northd, &en_nb_mirror, NULL); > diff --git a/northd/northd.c b/northd/northd.c > index e9cb906e2a..f767e0b39f 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -73,7 +73,7 @@ static bool install_ls_lb_from_router; > /* Use common zone for SNAT and DNAT if this option is set to "true". */ > static bool use_common_zone = false; > > -/* MAC allocated for service monitor usage. Just one mac is allocated > +/* MAC allocated for service monitor usage. Just one mac is allocatedg5534 > * for this purpose and ovn-controller's on each chassis will make use > * of this mac when sending out the packets to monitor the services > * defined in Service_Monitor Southbound table. Since these packets > @@ -852,7 +852,6 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) > free(od->l3dgw_ports); > destroy_mcast_info_for_datapath(od); > destroy_ports_for_datapath(od); > - > free(od); > } > } > @@ -4959,6 +4958,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 > @@ -5074,6 +5074,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( > @@ -5084,7 +5085,8 @@ ls_changes_can_be_handled( > for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) { > if (nbrec_logical_switch_is_updated(ls, col)) { > if (col == NBREC_LOGICAL_SWITCH_COL_ACLS || > - col == NBREC_LOGICAL_SWITCH_COL_PORTS) { > + col == NBREC_LOGICAL_SWITCH_COL_PORTS || > + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) { > continue; > } > return false; > @@ -5109,12 +5111,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) { > @@ -5365,6 +5361,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: > @@ -5431,9 +5428,20 @@ northd_handle_sb_port_binding_changes( > return true; > } > > -/* 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. */ > +/* Handler for lb_data engine changes. It does the following > + * For every tracked 'lb' and 'lb_group' > + * - it creates or deletes the ovn_lb_datapaths/ovn_lb_group_datapaths > + * from the lb_datapaths hmap and lb_group_datapaths hmap. > + * > + * - For any changes to a logical switch (in 'trk_lb_data->crupdated_ls_lbs') > + * 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 'lb' in the tracked data (trk_lb_data->crupdated_lbs) , > + * it gets the associated logical switches and for each switch it > + * re-evaluates 'od->has_lb_vip' to reflect any changes to the lb vips. > + * */ > bool > northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, > struct ovn_datapaths *ls_datapaths, > @@ -5459,8 +5467,15 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, > return false; > } > > + /* Fall back to recompute if any load balancer has been disassociated from > + * a logical switch or router. */ > + if (trk_lb_data->has_dissassoc_lbs_from_od) { > + return false; > + } > + > 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; > @@ -5468,10 +5483,22 @@ 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); > } > > + /* Create the 'lb_dps' if not already created for each > + * 'lb' in the trk_lb_data->crupdated_lbs. */ > struct crupdated_lb *clb; > HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) { > lb = clb->lb; > @@ -5504,6 +5531,37 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, > } > } > > + 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); > + } > + > + 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); To continue the discussion of one of my comments in v6: > > > 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. So here the loop on trk_lb_data->crupdated_lbs is to re-evaluate 'od->has_lb_vip' for LS that have any LB association change, right? But for my understanding the re-evaluation performed by the previous loop on trk_lb_data->crupdated_ls_lbs should have covered all such LS, and the current loop is unnecessary. Is there a case that the init_lb_for_datapath(od) would not be performed to a datapath by the previous loop but will be covered in this loop? Thanks, Han > + } > + } > + > return true; > } > > @@ -16529,6 +16587,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 2bb6910945..0d206a4e52 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -115,6 +115,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. */ > }; > > struct lflow_data { > @@ -345,7 +347,7 @@ 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); > + struct hmap *lbgrp_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 5c9da811fa..f0f8d1f503 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -10398,22 +10398,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 > > @@ -10421,7 +10422,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 > > @@ -10429,7 +10430,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 > > @@ -10437,14 +10438,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 not result in > +# full recompute of northd, but should rsult in full recompute of lflow 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 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 > + > +# 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 > @@ -10532,7 +10552,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 > > @@ -10577,7 +10597,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 > > @@ -10629,7 +10649,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 > > @@ -10670,7 +10690,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 > @@ -10684,15 +10704,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 > > @@ -10706,7 +10726,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.41.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Mon, Sep 11, 2023 at 9:21 PM Han Zhou <zhouhan@gmail.com> wrote: > > On Mon, Sep 11, 2023 at 9:01 AM <numans@ovn.org> wrote: > > > > From: Numan Siddique <numans@ovn.org> > > > > '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]} > > > > 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 northd handler 'northd_lb_data_handler' updates the > > ovn_lb_datapaths's 'nb_ls_map' bitmap accordingly. > > > > 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]} > > > > The handler northd_lb_data_handler(), creates the > > ovn_lb_datapaths object for lb1 and lb2 and deletes lb3 from > > the ovn_lb_datapaths hmap. It does the same for the created or updated lb > > groups lbg1 and lbg2 in the ovn_lbgrp_datapaths map. It also updates the > > nb_ls_bitmap of lb1 for sw0 and sw1 and nb_ls_bitmap of lb2 for sw1. > > > > Reviewed-by: Ales Musil <amusil@redhat.com> > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > --- > > 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 | 6 +- > > northd/inc-proc-northd.c | 2 + > > northd/northd.c | 83 +++++++++++++++--- > > northd/northd.h | 4 +- > > tests/ovn-northd.at | 56 +++++++++---- > > 9 files changed, 322 insertions(+), 33 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 8acd9c8cb2..02b1bfd7a4 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_lbgrp * > > struct tracked_lb_data *); > > static void add_deleted_lbgrp_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->lbgrps); > > + 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); > > + } > > + } 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->lbgrps); > > + 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_lbgrps); > > hmapx_init(&trk_lb_data->deleted_lbgrps); > > + 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->lbgrps); > > > > + 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_lbgrps = 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_lbgrp_to_tracked_data(struct > ovn_lb_group *lbg, > > { > > hmapx_add(&tracked_lb_data->deleted_lbgrps, 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 afb163dd9f..022b38523b 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_lbgrp { > > 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_lbgrps; > > > > + /* 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_lbgrps; > > + > > + /* Indicates if a lb was disassociated from a logical switch. */ > > + bool has_dissassoc_lbs_from_od; > > }; > > > > /* 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 lbgrps; > > + 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 ad8b62c735..2b84fef0ef 100644 > > --- a/northd/en-lflow.c > > +++ b/northd/en-lflow.c > > @@ -107,6 +107,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 1704a18834..8487b003f7 100644 > > --- a/northd/en-northd.c > > +++ b/northd/en-northd.c > > @@ -1,4 +1,4 @@ > > -/* > > + /* > > * Licensed under the Apache License, Version 2.0 (the "License"); > > * you may not use this file except in compliance with the License. > > * You may obtain a copy of the License at: > > @@ -214,6 +214,10 @@ northd_lb_data_handler(struct engine_node *node, > void *data) > > 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/inc-proc-northd.c b/northd/inc-proc-northd.c > > index ccc6687207..303b58d43f 100644 > > --- a/northd/inc-proc-northd.c > > +++ b/northd/inc-proc-northd.c > > @@ -155,6 +155,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_logical_router, NULL); > > engine_add_input(&en_northd, &en_nb_mirror, NULL); > > diff --git a/northd/northd.c b/northd/northd.c > > index e9cb906e2a..f767e0b39f 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -73,7 +73,7 @@ static bool install_ls_lb_from_router; > > /* Use common zone for SNAT and DNAT if this option is set to "true". */ > > static bool use_common_zone = false; > > > > -/* MAC allocated for service monitor usage. Just one mac is allocated > > +/* MAC allocated for service monitor usage. Just one mac is > allocatedg5534 > > * for this purpose and ovn-controller's on each chassis will make use > > * of this mac when sending out the packets to monitor the services > > * defined in Service_Monitor Southbound table. Since these packets > > @@ -852,7 +852,6 @@ ovn_datapath_destroy(struct hmap *datapaths, struct > ovn_datapath *od) > > free(od->l3dgw_ports); > > destroy_mcast_info_for_datapath(od); > > destroy_ports_for_datapath(od); > > - > > free(od); > > } > > } > > @@ -4959,6 +4958,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 > > @@ -5074,6 +5074,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( > > @@ -5084,7 +5085,8 @@ ls_changes_can_be_handled( > > for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) { > > if (nbrec_logical_switch_is_updated(ls, col)) { > > if (col == NBREC_LOGICAL_SWITCH_COL_ACLS || > > - col == NBREC_LOGICAL_SWITCH_COL_PORTS) { > > + col == NBREC_LOGICAL_SWITCH_COL_PORTS || > > + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) { > > continue; > > } > > return false; > > @@ -5109,12 +5111,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) { > > @@ -5365,6 +5361,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: > > @@ -5431,9 +5428,20 @@ northd_handle_sb_port_binding_changes( > > return true; > > } > > > > -/* 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. */ > > +/* Handler for lb_data engine changes. It does the following > > + * For every tracked 'lb' and 'lb_group' > > + * - it creates or deletes the ovn_lb_datapaths/ovn_lb_group_datapaths > > + * from the lb_datapaths hmap and lb_group_datapaths hmap. > > + * > > + * - For any changes to a logical switch (in > 'trk_lb_data->crupdated_ls_lbs') > > + * 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 'lb' in the tracked data (trk_lb_data->crupdated_lbs) , > > + * it gets the associated logical switches and for each switch it > > + * re-evaluates 'od->has_lb_vip' to reflect any changes to the lb > vips. > > + * */ > > bool > > northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, > > struct ovn_datapaths *ls_datapaths, > > @@ -5459,8 +5467,15 @@ northd_handle_lb_data_changes(struct > tracked_lb_data *trk_lb_data, > > return false; > > } > > > > + /* Fall back to recompute if any load balancer has been > disassociated from > > + * a logical switch or router. */ > > + if (trk_lb_data->has_dissassoc_lbs_from_od) { > > + return false; > > + } > > + > > 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; > > @@ -5468,10 +5483,22 @@ 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); > > } > > > > + /* Create the 'lb_dps' if not already created for each > > + * 'lb' in the trk_lb_data->crupdated_lbs. */ > > struct crupdated_lb *clb; > > HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) { > > lb = clb->lb; > > @@ -5504,6 +5531,37 @@ northd_handle_lb_data_changes(struct > tracked_lb_data *trk_lb_data, > > } > > } > > > > + 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); > > + } > > + > > + 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); > > To continue the discussion of one of my comments in v6: > > > > 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. > > So here the loop on trk_lb_data->crupdated_lbs is to re-evaluate > 'od->has_lb_vip' for LS that have any LB association change, right? Correct. > But for my understanding the re-evaluation performed by the previous loop on > trk_lb_data->crupdated_ls_lbs should have covered all such LS, and the > current loop is unnecessary. Is there a case that the > init_lb_for_datapath(od) would not be performed to a datapath by the > previous loop but will be covered in this loop? > It is required. Lets say there are 2 lbs - lb1 and lb2 ovn-nbctl lb-add lb1 10.0.0.10:80 20.0.0.20:8080' will ovn-nbctl lb-add lb2 10.0.0.20:80 20.0.0.30:8080 And lets say user runs the below command ovn-nbctl ls-lb-add sw0 lb1 In this case the lb_data's tracked data will have tracked_data = {'crupdated_lbs': [], 'deleted_lbs': [], 'crupdated_lb_groups': [], 'crupdated_ls_lbs': [{ls: sw0, assoc_lbs: [lb1]} The handling in northd_handle_lb_data_changes() is straightforward and od->has_lb_vip for sw0 is re-evaluated in the 'crupdated_ls_lbs' list iteration. Now lets say the user runs the below command ---- ovn-nbctl ls-lb-add sw1 lb2 -- ovn-nbctl clear load_balancer lb1 vips ---- In this case the lb_data's tracked data will have tracked_data = {'crupdated_lbs': ['lb1'], 'deleted_lbs': [], 'crupdated_lb_groups': [], 'crupdated_ls_lbs': [{ls: sw1, assoc_lbs: [lb2]} For the command 'ovn-nbctl clear load_balancer lb1 vips', ovsdl idl will only track the lb1 load balancer but not the logical switches it references and hence 'crupdated_ls_lbs' will not have sw0 in its entry. But we do need to evaluate the od->has_lb_vip for all the logical switches which references 'lb1' since the vip of lb1 is cleared now. I hope this clarifies your doubt. Thanks Numan > Thanks, > Han > > > + } > > + } > > + > > return true; > > } > > > > @@ -16529,6 +16587,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 2bb6910945..0d206a4e52 100644 > > --- a/northd/northd.h > > +++ b/northd/northd.h > > @@ -115,6 +115,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. > */ > > }; > > > > struct lflow_data { > > @@ -345,7 +347,7 @@ 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); > > + struct hmap *lbgrp_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 5c9da811fa..f0f8d1f503 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -10398,22 +10398,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 > > > > @@ -10421,7 +10422,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 > > > > @@ -10429,7 +10430,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 > > > > @@ -10437,14 +10438,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 not > result in > > +# full recompute of northd, but should rsult in full recompute of lflow > 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 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 > > + > > +# 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 > > @@ -10532,7 +10552,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 > > > > @@ -10577,7 +10597,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 > > > > @@ -10629,7 +10649,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 > > > > @@ -10670,7 +10690,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 > > @@ -10684,15 +10704,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 > > > > @@ -10706,7 +10726,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.41.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Sep 12, 2023 at 8:38 AM Numan Siddique <numans@ovn.org> wrote: > > On Mon, Sep 11, 2023 at 9:21 PM Han Zhou <zhouhan@gmail.com> wrote: > > > > On Mon, Sep 11, 2023 at 9:01 AM <numans@ovn.org> wrote: > > > > > > From: Numan Siddique <numans@ovn.org> > > > > > > '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]} > > > > > > 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 northd handler 'northd_lb_data_handler' updates the > > > ovn_lb_datapaths's 'nb_ls_map' bitmap accordingly. > > > > > > 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]} > > > > > > The handler northd_lb_data_handler(), creates the > > > ovn_lb_datapaths object for lb1 and lb2 and deletes lb3 from > > > the ovn_lb_datapaths hmap. It does the same for the created or updated lb > > > groups lbg1 and lbg2 in the ovn_lbgrp_datapaths map. It also updates the > > > nb_ls_bitmap of lb1 for sw0 and sw1 and nb_ls_bitmap of lb2 for sw1. > > > > > > Reviewed-by: Ales Musil <amusil@redhat.com> > > > Acked-by: Mark Michelson <mmichels@redhat.com> > > > Signed-off-by: Numan Siddique <numans@ovn.org> > > > --- > > > 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 | 6 +- > > > northd/inc-proc-northd.c | 2 + > > > northd/northd.c | 83 +++++++++++++++--- > > > northd/northd.h | 4 +- > > > tests/ovn-northd.at | 56 +++++++++---- > > > 9 files changed, 322 insertions(+), 33 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 8acd9c8cb2..02b1bfd7a4 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_lbgrp * > > > struct tracked_lb_data *); > > > static void add_deleted_lbgrp_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->lbgrps); > > > + 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); > > > + } > > > + } 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->lbgrps); > > > + 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_lbgrps); > > > hmapx_init(&trk_lb_data->deleted_lbgrps); > > > + 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->lbgrps); > > > > > > + 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_lbgrps = 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_lbgrp_to_tracked_data(struct > > ovn_lb_group *lbg, > > > { > > > hmapx_add(&tracked_lb_data->deleted_lbgrps, 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 afb163dd9f..022b38523b 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_lbgrp { > > > 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_lbgrps; > > > > > > + /* 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_lbgrps; > > > + > > > + /* Indicates if a lb was disassociated from a logical switch. */ > > > + bool has_dissassoc_lbs_from_od; > > > }; > > > > > > /* 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 lbgrps; > > > + 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 ad8b62c735..2b84fef0ef 100644 > > > --- a/northd/en-lflow.c > > > +++ b/northd/en-lflow.c > > > @@ -107,6 +107,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 1704a18834..8487b003f7 100644 > > > --- a/northd/en-northd.c > > > +++ b/northd/en-northd.c > > > @@ -1,4 +1,4 @@ > > > -/* > > > + /* > > > * Licensed under the Apache License, Version 2.0 (the "License"); > > > * you may not use this file except in compliance with the License. > > > * You may obtain a copy of the License at: > > > @@ -214,6 +214,10 @@ northd_lb_data_handler(struct engine_node *node, > > void *data) > > > 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/inc-proc-northd.c b/northd/inc-proc-northd.c > > > index ccc6687207..303b58d43f 100644 > > > --- a/northd/inc-proc-northd.c > > > +++ b/northd/inc-proc-northd.c > > > @@ -155,6 +155,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_logical_router, NULL); > > > engine_add_input(&en_northd, &en_nb_mirror, NULL); > > > diff --git a/northd/northd.c b/northd/northd.c > > > index e9cb906e2a..f767e0b39f 100644 > > > --- a/northd/northd.c > > > +++ b/northd/northd.c > > > @@ -73,7 +73,7 @@ static bool install_ls_lb_from_router; > > > /* Use common zone for SNAT and DNAT if this option is set to "true". */ > > > static bool use_common_zone = false; > > > > > > -/* MAC allocated for service monitor usage. Just one mac is allocated > > > +/* MAC allocated for service monitor usage. Just one mac is > > allocatedg5534 > > > * for this purpose and ovn-controller's on each chassis will make use > > > * of this mac when sending out the packets to monitor the services > > > * defined in Service_Monitor Southbound table. Since these packets > > > @@ -852,7 +852,6 @@ ovn_datapath_destroy(struct hmap *datapaths, struct > > ovn_datapath *od) > > > free(od->l3dgw_ports); > > > destroy_mcast_info_for_datapath(od); > > > destroy_ports_for_datapath(od); > > > - > > > free(od); > > > } > > > } > > > @@ -4959,6 +4958,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 > > > @@ -5074,6 +5074,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( > > > @@ -5084,7 +5085,8 @@ ls_changes_can_be_handled( > > > for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) { > > > if (nbrec_logical_switch_is_updated(ls, col)) { > > > if (col == NBREC_LOGICAL_SWITCH_COL_ACLS || > > > - col == NBREC_LOGICAL_SWITCH_COL_PORTS) { > > > + col == NBREC_LOGICAL_SWITCH_COL_PORTS || > > > + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) { > > > continue; > > > } > > > return false; > > > @@ -5109,12 +5111,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) { > > > @@ -5365,6 +5361,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: > > > @@ -5431,9 +5428,20 @@ northd_handle_sb_port_binding_changes( > > > return true; > > > } > > > > > > -/* 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. */ > > > +/* Handler for lb_data engine changes. It does the following > > > + * For every tracked 'lb' and 'lb_group' > > > + * - it creates or deletes the ovn_lb_datapaths/ovn_lb_group_datapaths > > > + * from the lb_datapaths hmap and lb_group_datapaths hmap. > > > + * > > > + * - For any changes to a logical switch (in > > 'trk_lb_data->crupdated_ls_lbs') > > > + * 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 'lb' in the tracked data (trk_lb_data->crupdated_lbs) , > > > + * it gets the associated logical switches and for each switch it > > > + * re-evaluates 'od->has_lb_vip' to reflect any changes to the lb > > vips. > > > + * */ > > > bool > > > northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, > > > struct ovn_datapaths *ls_datapaths, > > > @@ -5459,8 +5467,15 @@ northd_handle_lb_data_changes(struct > > tracked_lb_data *trk_lb_data, > > > return false; > > > } > > > > > > + /* Fall back to recompute if any load balancer has been > > disassociated from > > > + * a logical switch or router. */ > > > + if (trk_lb_data->has_dissassoc_lbs_from_od) { > > > + return false; > > > + } > > > + > > > 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; > > > @@ -5468,10 +5483,22 @@ 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); > > > } > > > > > > + /* Create the 'lb_dps' if not already created for each > > > + * 'lb' in the trk_lb_data->crupdated_lbs. */ > > > struct crupdated_lb *clb; > > > HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) { > > > lb = clb->lb; > > > @@ -5504,6 +5531,37 @@ northd_handle_lb_data_changes(struct > > tracked_lb_data *trk_lb_data, > > > } > > > } > > > > > > + 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); > > > + } > > > + > > > + 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); > > > > To continue the discussion of one of my comments in v6: > > > > > 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. > > > > So here the loop on trk_lb_data->crupdated_lbs is to re-evaluate > > 'od->has_lb_vip' for LS that have any LB association change, right? > Correct. > > > But for my understanding the re-evaluation performed by the previous loop on > > trk_lb_data->crupdated_ls_lbs should have covered all such LS, and the > > current loop is unnecessary. Is there a case that the > > init_lb_for_datapath(od) would not be performed to a datapath by the > > previous loop but will be covered in this loop? > > > > It is required. Lets say there are 2 lbs - lb1 and lb2 > > ovn-nbctl lb-add lb1 10.0.0.10:80 20.0.0.20:8080' will > ovn-nbctl lb-add lb2 10.0.0.20:80 20.0.0.30:8080 > > And lets say user runs the below command > > ovn-nbctl ls-lb-add sw0 lb1 > > In this case the lb_data's tracked data will have > > tracked_data = {'crupdated_lbs': [], > 'deleted_lbs': [], > 'crupdated_lb_groups': [], > 'crupdated_ls_lbs': [{ls: sw0, assoc_lbs: [lb1]} > > > The handling in northd_handle_lb_data_changes() is straightforward and > od->has_lb_vip for sw0 is re-evaluated in the 'crupdated_ls_lbs' list iteration. > > Now lets say the user runs the below command > > ---- > ovn-nbctl ls-lb-add sw1 lb2 -- ovn-nbctl clear load_balancer lb1 vips > ---- > > In this case the lb_data's tracked data will have > > tracked_data = {'crupdated_lbs': ['lb1'], > 'deleted_lbs': [], > 'crupdated_lb_groups': [], > 'crupdated_ls_lbs': [{ls: sw1, assoc_lbs: [lb2]} > > For the command 'ovn-nbctl clear load_balancer lb1 vips', ovsdl idl > will only track the lb1 load balancer but not > the logical switches it references and hence 'crupdated_ls_lbs' will > not have sw0 in its entry. > > But we do need to evaluate the od->has_lb_vip for all the logical switches > which references 'lb1' since the vip of lb1 is cleared now. > > I hope this clarifies your doubt. > Yes, thanks for explaining. I didn't realize that even without LS-LB association changes there still can be an impact to od->has_lb_vip only because of the LB update - I didn't think about the case where a LB has no VIPs. Regards, Han > Thanks > Numan > > > Thanks, > > Han > > > > > + } > > > + } > > > + > > > return true; > > > } > > > > > > @@ -16529,6 +16587,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 2bb6910945..0d206a4e52 100644 > > > --- a/northd/northd.h > > > +++ b/northd/northd.h > > > @@ -115,6 +115,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. > > */ > > > }; > > > > > > struct lflow_data { > > > @@ -345,7 +347,7 @@ 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); > > > + struct hmap *lbgrp_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 5c9da811fa..f0f8d1f503 100644 > > > --- a/tests/ovn-northd.at > > > +++ b/tests/ovn-northd.at > > > @@ -10398,22 +10398,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 > > > > > > @@ -10421,7 +10422,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 > > > > > > @@ -10429,7 +10430,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 > > > > > > @@ -10437,14 +10438,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 not > > result in > > > +# full recompute of northd, but should rsult in full recompute of lflow > > 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 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 > > > + > > > +# 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 > > > @@ -10532,7 +10552,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 > > > > > > @@ -10577,7 +10597,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 > > > > > > @@ -10629,7 +10649,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 > > > > > > @@ -10670,7 +10690,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 > > > @@ -10684,15 +10704,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 > > > > > > @@ -10706,7 +10726,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.41.0 > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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 8acd9c8cb2..02b1bfd7a4 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_lbgrp * struct tracked_lb_data *); static void add_deleted_lbgrp_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->lbgrps); + 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); + } + } 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->lbgrps); + 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_lbgrps); hmapx_init(&trk_lb_data->deleted_lbgrps); + 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->lbgrps); + 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_lbgrps = 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_lbgrp_to_tracked_data(struct ovn_lb_group *lbg, { hmapx_add(&tracked_lb_data->deleted_lbgrps, 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 afb163dd9f..022b38523b 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_lbgrp { 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_lbgrps; + /* 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_lbgrps; + + /* Indicates if a lb was disassociated from a logical switch. */ + bool has_dissassoc_lbs_from_od; }; /* 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 lbgrps; + 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 ad8b62c735..2b84fef0ef 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -107,6 +107,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 1704a18834..8487b003f7 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -1,4 +1,4 @@ -/* + /* * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at: @@ -214,6 +214,10 @@ northd_lb_data_handler(struct engine_node *node, void *data) 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/inc-proc-northd.c b/northd/inc-proc-northd.c index ccc6687207..303b58d43f 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -155,6 +155,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_logical_router, NULL); engine_add_input(&en_northd, &en_nb_mirror, NULL); diff --git a/northd/northd.c b/northd/northd.c index e9cb906e2a..f767e0b39f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -73,7 +73,7 @@ static bool install_ls_lb_from_router; /* Use common zone for SNAT and DNAT if this option is set to "true". */ static bool use_common_zone = false; -/* MAC allocated for service monitor usage. Just one mac is allocated +/* MAC allocated for service monitor usage. Just one mac is allocatedg5534 * for this purpose and ovn-controller's on each chassis will make use * of this mac when sending out the packets to monitor the services * defined in Service_Monitor Southbound table. Since these packets @@ -852,7 +852,6 @@ ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od) free(od->l3dgw_ports); destroy_mcast_info_for_datapath(od); destroy_ports_for_datapath(od); - free(od); } } @@ -4959,6 +4958,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 @@ -5074,6 +5074,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( @@ -5084,7 +5085,8 @@ ls_changes_can_be_handled( for (col = 0; col < NBREC_LOGICAL_SWITCH_N_COLUMNS; col++) { if (nbrec_logical_switch_is_updated(ls, col)) { if (col == NBREC_LOGICAL_SWITCH_COL_ACLS || - col == NBREC_LOGICAL_SWITCH_COL_PORTS) { + col == NBREC_LOGICAL_SWITCH_COL_PORTS || + col == NBREC_LOGICAL_SWITCH_COL_LOAD_BALANCER) { continue; } return false; @@ -5109,12 +5111,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) { @@ -5365,6 +5361,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: @@ -5431,9 +5428,20 @@ northd_handle_sb_port_binding_changes( return true; } -/* 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. */ +/* Handler for lb_data engine changes. It does the following + * For every tracked 'lb' and 'lb_group' + * - it creates or deletes the ovn_lb_datapaths/ovn_lb_group_datapaths + * from the lb_datapaths hmap and lb_group_datapaths hmap. + * + * - For any changes to a logical switch (in 'trk_lb_data->crupdated_ls_lbs') + * 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 'lb' in the tracked data (trk_lb_data->crupdated_lbs) , + * it gets the associated logical switches and for each switch it + * re-evaluates 'od->has_lb_vip' to reflect any changes to the lb vips. + * */ bool northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, struct ovn_datapaths *ls_datapaths, @@ -5459,8 +5467,15 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, return false; } + /* Fall back to recompute if any load balancer has been disassociated from + * a logical switch or router. */ + if (trk_lb_data->has_dissassoc_lbs_from_od) { + return false; + } + 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; @@ -5468,10 +5483,22 @@ 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); } + /* Create the 'lb_dps' if not already created for each + * 'lb' in the trk_lb_data->crupdated_lbs. */ struct crupdated_lb *clb; HMAP_FOR_EACH (clb, hmap_node, &trk_lb_data->crupdated_lbs) { lb = clb->lb; @@ -5504,6 +5531,37 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, } } + 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); + } + + 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; } @@ -16529,6 +16587,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 2bb6910945..0d206a4e52 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -115,6 +115,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. */ }; struct lflow_data { @@ -345,7 +347,7 @@ 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); + struct hmap *lbgrp_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 5c9da811fa..f0f8d1f503 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -10398,22 +10398,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 @@ -10421,7 +10422,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 @@ -10429,7 +10430,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 @@ -10437,14 +10438,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 not result in +# full recompute of northd, but should rsult in full recompute of lflow 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 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 + +# 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 @@ -10532,7 +10552,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 @@ -10577,7 +10597,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 @@ -10629,7 +10649,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 @@ -10670,7 +10690,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 @@ -10684,15 +10704,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 @@ -10706,7 +10726,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