Hi Ales, Aside from the tiny nit below the change looks OK to me. I know Mark reviewed previous versions so I'll let him confirm he's OK with this version too.
On 6/11/25 9:37 AM, Ales Musil via dev wrote: > Any change to ACL would result in full lflow recompute. Instead of > that reprocess only the relevant logical switch. In order to get > track the relation between ACL and LS from the ACL side we need to > store all related ACL uuids in the ls_stateful_record. > I think it would be nice to get an estimate from a real(istic) OVN deployment of how much the memory usage of northd would increase due to this change. Assuming that's not a significant increase, and assuming the tiny comment I had below is addresed feel free to add my ack: Acked-by: Dumitru Ceara <dce...@redhat.com> > Reported-at: https://issues.redhat.com/browse/FDP-755 > Signed-off-by: Ales Musil <amu...@redhat.com> > --- > v4: Rebase on top of latest main. > Remove check for logs/meter column changes. > v3: Rebase on top of latest main. > v2: Rebase on top of latest main. > Optimize the related ACLs and flags computation further. > --- > northd/en-ls-stateful.c | 172 ++++++++++++++------------------------- > northd/en-ls-stateful.h | 6 ++ > northd/inc-proc-northd.c | 2 +- > tests/ovn-northd.at | 24 +++--- > 4 files changed, 79 insertions(+), 125 deletions(-) > > +enum engine_input_handler_result > +ls_stateful_acl_handler(struct engine_node *node, void *data_) > +{ > + struct ed_type_ls_stateful *data = data_; > + const struct nbrec_acl_table *nbrec_acl_table = > + EN_OVSDB_GET(engine_get_input("NB_acl", node)); > + > + const struct nbrec_acl *acl; > + NBREC_ACL_TABLE_FOR_EACH_TRACKED (acl, nbrec_acl_table) { > + /* The creation and deletion is handled in relation to LS/PG rather > + * than the ACL itself. */ > + if (nbrec_acl_is_new(acl) || nbrec_acl_is_deleted(acl)) { > + continue; > } > > - if (modified) { > - /* Add the ls_stateful_rec to the tracking data. */ > - hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); > + struct ls_stateful_record *ls_stateful_rec; > + LS_STATEFUL_TABLE_FOR_EACH (ls_stateful_rec, &data->table) { > + if (uuidset_contains(&ls_stateful_rec->related_acls, > + &acl->header_.uuid)) { > + hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); > + } > } > } > > if (ls_stateful_has_tracked_data(&data->trk_data)) { > return EN_HANDLED_UPDATED; > } > + Nit: unrelated. > return EN_HANDLED_UNCHANGED; > } > Thanks, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
