On Mon, Jul 28, 2025 at 5:42 PM Jacob Tanenbaum via dev < ovs-dev@openvswitch.org> wrote:
> Several operations on Port Groups required a full database > recalculation. > > 1. Creating a new Port Group > 2. Deleting a Port Group > 3. adding a port from a switch with no other ports on a port group > 4. removing a port from a port group that is the last one from a > specific switch > > Those four operations required a full database recalculation to ensure > that that the lflows from the ACLs and the southbound databases were > correctly populated. Instead reprocess only the relevant switches and > ACLs to avoid the cost of a full recalculation. This is done by having > the port_group_handler update the southbound database and pass the names > of the relevant switches to the ls_stateful_acl node. > > Tested using the ovn sandbox 3000 switches and 15000 ports, Creating a > port group with one ACL and one port > Without I-P improvements: > > Time spent on processing nb_cfg 2: > ovn-northd delay before processing: 1ms > ovn-northd completion: 132ms > > With I-P improvements: > > Time spent on processing nb_cfg 5: > ovn-northd delay before processing: 0ms > ovn-northd completion: 26ms > > Reported-at: https://issues.redhat.com/browse/FDP-758 > Reported-by: Dumitru Ceara <dce...@redhat.com> > Signed-off-by: Jacob Tanenbaum <jtane...@redhat.com> > Hello Jacob, I have a couple of comments on top of what Lorenzo already wrote. > > diff --git a/northd/en-lflow.c b/northd/en-lflow.c > index 63565ef80..ac8ebba16 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -157,22 +157,6 @@ lflow_northd_handler(struct engine_node *node, > return EN_HANDLED_UPDATED; > } > > -enum engine_input_handler_result > -lflow_port_group_handler(struct engine_node *node, void *data OVS_UNUSED) > -{ > - struct port_group_data *pg_data = > - engine_get_input_data("port_group", node); > - > - /* If the set of switches per port group didn't change then there's no > - * need to reprocess lflows. Otherwise, there might be a need to > - * add/delete port-group ACLs to/from switches. */ > - if (pg_data->ls_port_groups_sets_changed) { > - return EN_UNHANDLED; > - } > - > - return EN_HANDLED_UPDATED; > -} > - > enum engine_input_handler_result > lflow_lr_stateful_handler(struct engine_node *node, void *data) > { > diff --git a/northd/en-lflow.h b/northd/en-lflow.h > index d3c96c027..326be83f9 100644 > --- a/northd/en-lflow.h > +++ b/northd/en-lflow.h > @@ -20,8 +20,6 @@ void *en_lflow_init(struct engine_node *node, struct > engine_arg *arg); > void en_lflow_cleanup(void *data); > enum engine_input_handler_result lflow_northd_handler(struct engine_node > *, > void *data); > -enum engine_input_handler_result lflow_port_group_handler(struct > engine_node *, > - void *data); > enum engine_input_handler_result > lflow_lr_stateful_handler(struct engine_node *, void *data); > enum engine_input_handler_result > diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c > index 6aec43415..7b2a3e065 100644 > --- a/northd/en-ls-stateful.c > +++ b/northd/en-ls-stateful.c > @@ -159,11 +159,15 @@ ls_stateful_northd_handler(struct engine_node *node, > void *data_) > struct ls_stateful_record *ls_stateful_rec = > ls_stateful_table_find_(&data->table, od->nbs); > ovs_assert(ls_stateful_rec); > - ls_stateful_record_set_acls(ls_stateful_rec, od->nbs, > - input_data.ls_port_groups); > - > - /* Add the ls_stateful_rec to the tracking data. */ > - hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); > + /* Ensure that only one handler per engine run calls > + * ls_stateful_record_set_acls on the same ls_stateful rec. */ > + if (!hmapx_contains(&data->trk_data.crupdated, ls_stateful_rec)) { > You can replace this with 'if (hmapx_add(..))', as hmapx_add will return a pointer if it's a new item, NULL otherwise. > + ls_stateful_record_set_acls(ls_stateful_rec, od->nbs, > + input_data.ls_port_groups); > + > + /* Add the ls_stateful_rec to the tracking data. */ > + hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); > + } > } > > if (ls_stateful_has_tracked_data(&data->trk_data)) { > @@ -179,11 +183,24 @@ ls_stateful_port_group_handler(struct engine_node > *node, void *data_) > struct port_group_data *pg_data = > engine_get_input_data("port_group", node); > > - if (pg_data->ls_port_groups_sets_changed) { > - return EN_UNHANDLED; > + struct ed_type_ls_stateful *data = data_; > + struct hmapx_node *hmap_node; > + HMAPX_FOR_EACH (hmap_node, &pg_data->ls_port_groups_sets_changed) { > + const struct nbrec_logical_switch *nbs = hmap_node->data; > + struct ls_stateful_record *ls_stateful_rec = > + ls_stateful_table_find_(&data->table, nbs); > + ovs_assert(ls_stateful_rec); > + /* Ensure only one handler per engine run calls > + * ls_stateful_record_set_acls on the same ls_stateful rec. */ > + if (!hmapx_contains(&data->trk_data.crupdated, ls_stateful_rec)) { > Same here with the hmapx_add. > + ls_stateful_record_set_acls(ls_stateful_rec, > + nbs, > + &pg_data->ls_port_groups); > + /* Add the ls_stateful_rec to the tracking data. */ > + hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); > + } > } > > - struct ed_type_ls_stateful *data = data_; > if (ls_stateful_has_tracked_data(&data->trk_data)) { > return EN_HANDLED_UPDATED; > } > diff --git a/northd/en-port-group.c b/northd/en-port-group.c > index 4fc1a4f24..fd573d9df 100644 > --- a/northd/en-port-group.c > +++ b/northd/en-port-group.c > @@ -33,18 +33,24 @@ static struct ls_port_group *ls_port_group_create( > static void ls_port_group_destroy(struct ls_port_group_table *, > struct ls_port_group *); > > -static bool ls_port_group_process( > +static void ls_port_group_process( > struct ls_port_group_table *, > struct port_group_ls_table *, > + struct hmapx *, > const struct hmap *ls_ports, > const struct nbrec_port_group *, > - struct hmapx *updated_ls_port_groups); > + struct hmapx *updated_ls_port_groups, > + struct sset *pruned_ls_port_group_recs); > > static void ls_port_group_record_clear( > struct ls_port_group_table *, > struct port_group_ls_record *, > struct hmapx *cleared_ls_port_groups); > -static bool ls_port_group_record_prune(struct ls_port_group *); > + > +static bool ls_port_group_record_prune( > + struct ls_port_group *, > + const struct nbrec_port_group *, > + struct sset *); > > static struct ls_port_group_record *ls_port_group_record_create( > struct ls_port_group *, > @@ -118,8 +124,8 @@ ls_port_group_table_build( > { > const struct nbrec_port_group *nb_pg; > NBREC_PORT_GROUP_TABLE_FOR_EACH (nb_pg, pg_table) { > - ls_port_group_process(ls_port_groups, port_group_lses, > - ls_ports, nb_pg, NULL); > + ls_port_group_process(ls_port_groups, port_group_lses, NULL, > + ls_ports, nb_pg, NULL, NULL); > } > } > > @@ -206,20 +212,27 @@ ls_port_group_destroy(struct ls_port_group_table > *ls_port_groups, > } > } > > -/* Process a NB.Port_Group record and stores any updated ls_port_groups > - * in updated_ls_port_groups. Returns true if a new ls_port_group had > - * to be created or destroyed. > +/* Process a NB.Port_Group record updated the ls_port_group_table and the > + * port_group_ls_table. Stores a few different updates > + * 1. Updated ls_port_groups are stored in updated_ls_port_groups so the > I-P > + * can update the Southbound database > + * 2. If a port_groups switch set changes the switch is stored in > + * ls_port_groups_sets_changed so that later I-P nodes can recalculate > + * lflows. > + * 3. If a port_group has a switch removed from it's switch set it is > stored > + * in pruned_ls_port_group_recs so that the SB entry can be deleted. > */ > -static bool > +static void > ls_port_group_process(struct ls_port_group_table *ls_port_groups, > struct port_group_ls_table *port_group_lses, > + struct hmapx *ls_port_groups_sets_changed, > const struct hmap *ls_ports, > const struct nbrec_port_group *nb_pg, > - struct hmapx *updated_ls_port_groups) > + struct hmapx *updated_ls_port_groups, > + struct sset *pruned_ls_port_group_recs) > { > struct hmapx cleared_ls_port_groups = > HMAPX_INITIALIZER(&cleared_ls_port_groups); > - bool ls_pg_rec_created = false; > > struct port_group_ls_record *pg_ls = > port_group_ls_table_find(port_group_lses, nb_pg); > @@ -233,6 +246,12 @@ ls_port_group_process(struct ls_port_group_table > *ls_port_groups, > } > > for (size_t i = 0; i < nb_pg->n_ports; i++) { > + if (nbrec_port_group_is_deleted(nb_pg)) { > + /* When a port group is deleted we don't need to > + * updated the port group as the entry will be pruned > + */ > + break; > + } > const char *port_name = nb_pg->ports[i]->name; > const struct ovn_datapath *od = > northd_get_datapath_for_port(ls_ports, port_name); > @@ -262,7 +281,10 @@ ls_port_group_process(struct ls_port_group_table > *ls_port_groups, > ls_port_group_record_find(ls_pg, nb_pg); > if (!ls_pg_rec) { > ls_pg_rec = ls_port_group_record_create(ls_pg, nb_pg); > - ls_pg_rec_created = true; > + if (ls_port_groups_sets_changed) { > + hmapx_add(ls_port_groups_sets_changed, > + CONST_CAST(struct nbrec_logical_switch *, > od->nbs)); > + } > } > sset_add(&ls_pg_rec->ports, port_name); > > @@ -273,13 +295,18 @@ ls_port_group_process(struct ls_port_group_table > *ls_port_groups, > } > } > > - bool ls_pg_rec_destroyed = false; > struct hmapx_node *node; > HMAPX_FOR_EACH (node, &cleared_ls_port_groups) { > struct ls_port_group *ls_pg = node->data; > > - if (ls_port_group_record_prune(ls_pg)) { > - ls_pg_rec_destroyed = true; > + if (ls_port_group_record_prune(ls_pg, > + nb_pg, > + pruned_ls_port_group_recs)) { > + if (ls_port_groups_sets_changed) { > + hmapx_add(ls_port_groups_sets_changed, > + CONST_CAST(struct nbrec_logical_switch *,ls_pg->nbs)); > + hmapx_find_and_delete(&pg_ls->switches, ls_pg->nbs); Shouldn't this be outside the second if? It feels like it should be part of the 'ls_port_group_record_prune()' TBH. > + } > } > > if (hmap_is_empty(&ls_pg->nb_pgs)) { > @@ -287,8 +314,6 @@ ls_port_group_process(struct ls_port_group_table > *ls_port_groups, > } > } > hmapx_destroy(&cleared_ls_port_groups); > - > - return ls_pg_rec_created || ls_pg_rec_destroyed; > } > > /* Destroys all the struct ls_port_group_record that might be associated > to > @@ -320,17 +345,27 @@ ls_port_group_record_clear(struct > ls_port_group_table *ls_to_port_groups, > } > > static bool > -ls_port_group_record_prune(struct ls_port_group *ls_pg) > +ls_port_group_record_prune(struct ls_port_group *ls_pg, > + const struct nbrec_port_group *nb_pg, > + struct sset *pruned_ls_pg_rec) > { > struct ls_port_group_record *ls_pg_rec; > bool records_pruned = false; > > - HMAP_FOR_EACH_SAFE (ls_pg_rec, key_node, &ls_pg->nb_pgs) { > - if (sset_is_empty(&ls_pg_rec->ports)) { > - ls_port_group_record_destroy(ls_pg, ls_pg_rec); > - records_pruned = true; > + struct ds sb_pg_name = DS_EMPTY_INITIALIZER; > + ls_pg_rec = ls_port_group_record_find(ls_pg, nb_pg); > + if (sset_is_empty(&ls_pg_rec->ports) || > + nbrec_port_group_is_deleted(ls_pg_rec->nb_pg)) { > + if (pruned_ls_pg_rec) { > + get_sb_port_group_name(ls_pg_rec->nb_pg->name, > + ls_pg->sb_datapath_key, > + &sb_pg_name); > + sset_add(pruned_ls_pg_rec, ds_cstr(&sb_pg_name)); > } > + ls_port_group_record_destroy(ls_pg, ls_pg_rec); > + records_pruned = true; > } > + ds_destroy(&sb_pg_name); > return records_pruned; > } > > @@ -460,6 +495,10 @@ en_port_group_init(struct engine_node *node > OVS_UNUSED, > struct engine_arg *arg OVS_UNUSED) > { > struct port_group_data *pg_data = xmalloc(sizeof *pg_data); > + *pg_data = (struct port_group_data) { > + .ls_port_groups_sets_changed = > + HMAPX_INITIALIZER(&pg_data->ls_port_groups_sets_changed), > + }; > > ls_port_group_table_init(&pg_data->ls_port_groups); > port_group_ls_table_init(&pg_data->port_groups_lses); > @@ -473,6 +512,7 @@ en_port_group_cleanup(void *data_) > > ls_port_group_table_destroy(&data->ls_port_groups); > port_group_ls_table_destroy(&data->port_groups_lses); > + hmapx_destroy(&data->ls_port_groups_sets_changed); > } > > void > @@ -480,7 +520,7 @@ en_port_group_clear_tracked_data(void *data_) > { > struct port_group_data *data = data_; > > - data->ls_port_groups_sets_changed = true; > + hmapx_clear(&data->ls_port_groups_sets_changed); > } > > enum engine_node_state > @@ -514,73 +554,71 @@ port_group_nb_port_group_handler(struct engine_node > *node, void *data_) > struct port_group_input input_data = port_group_get_input_data(node); > const struct engine_context *eng_ctx = engine_get_context(); > struct port_group_data *data = data_; > - bool success = true; > > const struct nbrec_port_group_table *nb_pg_table = > EN_OVSDB_GET(engine_get_input("NB_port_group", node)); > const struct nbrec_port_group *nb_pg; > > - /* Return false if a port group is created or deleted. > - * Handle I-P for only updated port groups. */ > - NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg, nb_pg_table) { > - if (nbrec_port_group_is_new(nb_pg) || > - nbrec_port_group_is_deleted(nb_pg)) { > - return EN_UNHANDLED; > - } > - } > - > struct hmapx updated_ls_port_groups = > HMAPX_INITIALIZER(&updated_ls_port_groups); > > + struct sset stale_sb_port_groups = > SSET_INITIALIZER(&stale_sb_port_groups); > NBREC_PORT_GROUP_TABLE_FOR_EACH_TRACKED (nb_pg, nb_pg_table) { > - if (ls_port_group_process(&data->ls_port_groups, > - &data->port_groups_lses, > - input_data.ls_ports, > - nb_pg, &updated_ls_port_groups)) { > - success = false; > - break; > + ls_port_group_process(&data->ls_port_groups, > + &data->port_groups_lses, > + &data->ls_port_groups_sets_changed, > + input_data.ls_ports, > + nb_pg, &updated_ls_port_groups, > + &stale_sb_port_groups); > + > } > - } > > - /* If changes have been successfully processed incrementally then > update > + /* Changes have been successfully processed incrementally now update > * the SB too. */ > - if (success) { > - struct ovsdb_idl_index *sbrec_port_group_by_name = > - engine_ovsdb_node_get_index( > - engine_get_input("SB_port_group", node), > - "sbrec_port_group_by_name"); > - struct ds sb_pg_name = DS_EMPTY_INITIALIZER; > - > - struct hmapx_node *updated_node; > - HMAPX_FOR_EACH (updated_node, &updated_ls_port_groups) { > - const struct ls_port_group *ls_pg = updated_node->data; > - struct ls_port_group_record *ls_pg_rec; > - > - HMAP_FOR_EACH (ls_pg_rec, key_node, &ls_pg->nb_pgs) { > - get_sb_port_group_name(ls_pg_rec->nb_pg->name, > - ls_pg->sb_datapath_key, > - &sb_pg_name); > - > - const char *sb_pg_name_cstr = ds_cstr(&sb_pg_name); > - const struct sbrec_port_group *sb_pg = > - sb_port_group_lookup_by_name(sbrec_port_group_by_name, > - sb_pg_name_cstr); > - if (!sb_pg) { > - sb_pg = create_sb_port_group(eng_ctx->ovnsb_idl_txn, > - sb_pg_name_cstr); > - } > - struct sorted_array nb_ports = > - sorted_array_from_sset(&ls_pg_rec->ports); > - update_sb_port_group(&nb_ports, sb_pg); > - sorted_array_destroy(&nb_ports); > + struct ovsdb_idl_index *sbrec_port_group_by_name = > + engine_ovsdb_node_get_index( > + engine_get_input("SB_port_group", node), > + "sbrec_port_group_by_name"); > + struct ds sb_pg_name = DS_EMPTY_INITIALIZER; > + > + struct hmapx_node *updated_node; > + HMAPX_FOR_EACH (updated_node, &updated_ls_port_groups) { > + const struct ls_port_group *ls_pg = updated_node->data; > + struct ls_port_group_record *ls_pg_rec; > + > + HMAP_FOR_EACH (ls_pg_rec, key_node, &ls_pg->nb_pgs) { > + get_sb_port_group_name(ls_pg_rec->nb_pg->name, > + ls_pg->sb_datapath_key, > + &sb_pg_name); > + > + const char *sb_pg_name_cstr = ds_cstr(&sb_pg_name); > + const struct sbrec_port_group *sb_pg = > + sb_port_group_lookup_by_name(sbrec_port_group_by_name, > + sb_pg_name_cstr); > + if (!sb_pg) { > + sb_pg = create_sb_port_group(eng_ctx->ovnsb_idl_txn, > + sb_pg_name_cstr); > } > + struct sorted_array nb_ports = > + sorted_array_from_sset(&ls_pg_rec->ports); > + update_sb_port_group(&nb_ports, sb_pg); > + sorted_array_destroy(&nb_ports); > } > - ds_destroy(&sb_pg_name); > } > + ds_destroy(&sb_pg_name); > + > + const char *stale_sb_port_group_name; > + SSET_FOR_EACH (stale_sb_port_group_name, &stale_sb_port_groups) { > + const struct sbrec_port_group *sb_pg = > + sb_port_group_lookup_by_name(sbrec_port_group_by_name, > + stale_sb_port_group_name); > + sbrec_port_group_delete(sb_pg); > + } > + sset_destroy(&stale_sb_port_groups); > + > > - data->ls_port_groups_sets_changed = !success; > hmapx_destroy(&updated_ls_port_groups); > - return success ? EN_HANDLED_UPDATED : EN_UNHANDLED; > + return EN_HANDLED_UPDATED; > } > > static void > diff --git a/northd/en-port-group.h b/northd/en-port-group.h > index 8dcc950da..f07f118aa 100644 > --- a/northd/en-port-group.h > +++ b/northd/en-port-group.h > @@ -104,7 +104,7 @@ struct port_group_input { > struct port_group_data { > struct ls_port_group_table ls_port_groups; > struct port_group_ls_table port_groups_lses; > - bool ls_port_groups_sets_changed; > + struct hmapx ls_port_groups_sets_changed; > }; > > void *en_port_group_init(struct engine_node *, struct engine_arg *); > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index d43bfc16c..fc4ae58f8 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -348,7 +348,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_lflow, &en_sampling_app, NULL); > > engine_add_input(&en_lflow, &en_northd, lflow_northd_handler); > - engine_add_input(&en_lflow, &en_port_group, lflow_port_group_handler); > + engine_add_input(&en_lflow, &en_port_group, engine_noop_handler); > Please add a comment explaining why lflow doesn't need to handle PG changes anymore. > engine_add_input(&en_lflow, &en_lr_stateful, > lflow_lr_stateful_handler); > engine_add_input(&en_lflow, &en_ls_stateful, > lflow_ls_stateful_handler); > engine_add_input(&en_lflow, &en_multicast_igmp, > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 5e0747fcc..61d00e576 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -10100,27 +10100,11 @@ AS_BOX([Create new PG1 and PG2]) > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > check ovn-nbctl --wait=sb -- pg-add pg1 -- pg-add pg2 > dnl The northd node should not recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl The port_group node recomputes every time a NB port group is > added/deleted. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl The port_group node is an input for the lflow node. Port_group > -dnl recompute/compute triggers lflow recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > +check_engine_stats northd norecompute compute > +dnl The port_group node does not recompute when a NB port group is > added/deleted. > +check_engine_stats port_group norecompute compute > +dnl The Port group recompute/compute should not trigger an lflow > recompute. > +check_engine_stats lflow norecompute compute > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > AS_BOX([Add ACLs on PG1 and PG2]) > @@ -10137,28 +10121,15 @@ check_column "sw1.1" sb:Port_Group ports > name="${sw1_key}_pg1" > check_column "sw2.1" sb:Port_Group ports name="${sw2_key}_pg1" > > dnl The northd node should not recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl The port_group node recomputes also every time a port from a new > switch > -dnl is added to the group. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl The port_group node is an input for the lflow node. Port_group > -dnl recompute/compute triggers lflow recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > +check_engine_stats northd norecompute compute > +dnl The port_group node does not recompute when a new switch is added to > the > +dnl group > +check_engine_stats port_group norecompute compute > +dnl The port_group node is an input for the ls_statful node. Port_group > +dnl should not trigger a recompute. > +check_engine_stats ls_stateful norecompute compute > +dnl Port_Group compute/recompute should not trigger an lflow recompute. > +check_engine_stats lflow norecompute compute > dnl Expect ACL1 on sw1 and sw2 > check_acl_lflows 1 0 1 0 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > @@ -10173,29 +10144,16 @@ check_column "sw1.2" sb:Port_Group ports > name="${sw1_key}_pg2" > check_column "sw2.2" sb:Port_Group ports name="${sw2_key}_pg2" > > dnl The northd node should not recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl The port_group node recomputes also every time a port from a new > switch > +check_engine_stats northd norecompute compute > +dnl The port_group node should not recompute when a port from a new > switch > nit: Extra space. > dnl is added to the group. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl The port_group node is an input for the lflow node. Port_group > -dnl recompute/compute triggers lflow recompute (for ACLs). > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl Expect both ACLs on sw1 and sw2 > +check_engine_stats port_group norecompute compute > +dnl The port_group node is an input for the ls_statful node. Port_group > +dnl should not trigger a recompute > +check_engine_stats ls_stateful norecompute compute > +dnl Port_Group compute/recompute should not trigger an lflow recompute. > +check_engine_stats lflow norecompute compute > +dnl Expect both ACLs on sw1 and sw2. > check_acl_lflows 1 1 1 1 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -10210,29 +10168,11 @@ check_column "sw1.2 sw1.3" sb:Port_Group ports > name="${sw1_key}_pg2" > check_column "sw2.2 sw2.3" sb:Port_Group ports name="${sw2_key}_pg2" > > dnl The northd node should not recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We did not change the set of switches a pg is applied to, there > should be > -dnl no recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We did not change the set of switches a pg is applied to, there > should be > -dnl no recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl Expect both ACLs on sw1 and sw2 > +check_engine_stats northd norecompute compute > +check_engine_stats port_group norecompute compute > +check_engine_stats ls_stateful norecompute compute > +check_engine_stats lflow norecompute compute > +dnl Expect both ACLs on sw1 and sw2. > check_acl_lflows 1 1 1 1 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > @@ -10248,28 +10188,10 @@ check_column "sw2.2" sb:Port_Group ports > name="${sw2_key}_pg2" > > dnl The northd node should not recompute, it should handle nb_global > update > dnl though, therefore "compute: 1". > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We did not change the set of switches a pg is applied to, there > should be > -dnl no recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We did not change the set of switches a pg is applied to, there > should be > -dnl no recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > +check_engine_stats northd norecompute compute > +check_engine_stats port_group norecompute compute > +check_engine_stats ls_stateful norecompute compute > +check_engine_stats lflow norecompute compute > dnl Expect both ACLs on sw1 and sw2 > check_acl_lflows 1 1 1 1 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > @@ -10280,32 +10202,19 @@ check ovn-nbctl --wait=sb pg-set-ports pg2 sw1.2 > check_column "sw1.1" sb:Port_Group ports name="${sw1_key}_pg1" > check_column "sw2.1" sb:Port_Group ports name="${sw2_key}_pg1" > check_column "sw1.2" sb:Port_Group ports name="${sw1_key}_pg2" > + > AT_CHECK([fetch_column sb:Port_Group ports name="${sw2_key}_pg2"], [0], [ > ]) > > dnl The northd node should not recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be > +check_engine_stats northd norecompute compute > +dnl We changed the set of switches a pg is applied to, this should no > longer require > dnl a recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be > -dnl a recompute (for ACLs). > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > +check_engine_stats port_group norecompute compute > +dnl There should be no recompute for ls_stateful > +check_engine_stats ls_stateful norecompute compute > +dnl No recompute should occur. > +check_engine_stats lflow norecompute compute > dnl Expect both ACLs on sw1 and only the first one on sw2. > check_acl_lflows 1 1 1 0 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > @@ -10320,29 +10229,11 @@ check_column "sw1.2" sb:Port_Group ports > name="${sw1_key}_pg2" > AT_CHECK([fetch_column sb:Port_Group ports name="${sw2_key}_pg2"], [0], [ > ]) > > -dnl The northd node should not recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be > -dnl a recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be > -dnl a recompute (for ACLs). > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > +check_engine_stats northd norecompute compute > +check_engine_stats port_group norecompute compute > +check_engine_stats ls_stateful norecompute compute > +check_engine_stats lflow norecompute compute > + > dnl Expect both ACLs on sw1 and not on sw2. > check_acl_lflows 1 1 0 0 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > @@ -10357,36 +10248,18 @@ check_column "sw2.1" sb:Port_Group ports > name="${sw2_key}_pg1" > check_column "sw1.2" sb:Port_Group ports name="${sw1_key}_pg2" > check_column "sw2.2" sb:Port_Group ports name="${sw2_key}_pg2" > > -dnl The northd node should not recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be a > -dnl recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be a > -dnl recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > +check_engine_stats northd norecompute compute > +check_engine_stats port_group norecompute compute > +check_engine_stats ls_stateful norecompute compute > +check_engine_stats lflow norecompute compute > + > dnl Expect both ACLs on sw1 and sw2 > check_acl_lflows 1 1 1 1 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > AS_BOX([Remove second port from both PGs]) > check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > -check ovn-nbctl --wait=sb \ > +check ovn-nbctl --wait=sb \ > -- pg-set-ports pg1 sw1.1 \ > -- pg-set-ports pg2 sw1.2 > check_column "sw1.1" sb:Port_Group ports name="${sw1_key}_pg1" > @@ -10396,33 +10269,53 @@ check_column "sw1.2" sb:Port_Group ports > name="${sw1_key}_pg2" > AT_CHECK([fetch_column sb:Port_Group ports name="${sw2_key}_pg2"], [0], [ > ]) > > -dnl The northd node should not recompute,. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > northd], [0], [dnl > -Node: northd > -- recompute: 0 > -- compute: 1 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be a > -dnl recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > port_group], [0], [dnl > -Node: port_group > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl We changed the set of switches a pg is applied to, there should be a > -dnl recompute. > -AT_CHECK([as northd ovn-appctl -t ovn-northd inc-engine/show-stats > lflow], [0], [dnl > -Node: lflow > -- recompute: 1 > -- compute: 0 > -- cancel: 0 > -]) > -dnl Expect both ACLs on sw1 and no ACLs on sw2 > +check_engine_stats northd norecompute compute > +check_engine_stats port_group norecompute compute > +check_engine_stats ls_stateful norecompute compute > +check_engine_stats lflow norecompute compute > + > +dnl Expect both ACLs sw1 > check_acl_lflows 1 1 0 0 > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > +AS_BOX([Delete port groups PG1 and PG2]) > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb \ > + -- pg-del pg1 \ > + -- pg-del pg2 > + > +check_engine_stats northd norecompute compute > +check_engine_stats port_group norecompute compute > +check_engine_stats ls_stateful norecompute compute > +check_engine_stats lflow norecompute compute > + > +dnl Expect no ACLs on sw1 and no ACLs on sw2 > +check_acl_lflows 0 0 0 0 > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +AS_BOX([Add PG1 and PG2 again setting ports and ACLs in one transaction]) > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb \ > + -- pg-add pg1 \ > + -- pg-add pg2 \ > + -- acl-add pg1 from-lport 1 eth.src==41:41:41:41:41:41 allow \ > + -- acl-add pg2 from-lport 1 eth.src==42:42:42:42:42:42 allow \ > + -- pg-set-ports pg1 sw1.1 sw2.1 \ > + -- pg-set-ports pg2 sw1.2 sw2.2 > +check_column "sw1.1" sb:Port_Group ports name="${sw1_key}_pg1" > +check_column "sw2.1" sb:Port_Group ports name="${sw2_key}_pg1" > +check_column "sw1.2" sb:Port_Group ports name="${sw1_key}_pg2" > +check_column "sw2.2" sb:Port_Group ports name="${sw2_key}_pg2" > + > +check_engine_stats northd norecompute compute > +check_engine_stats port_group norecompute compute > +check_engine_stats ls_stateful norecompute compute > +check_engine_stats lflow norecompute compute > + > +dnl Expect both ACLs on sw1 and sw2 > +check_acl_lflows 1 1 1 1 > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > AT_CLEANUP > ]) > > -- > 2.50.1 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
