Hi Jacob Thanks for your review. Em qua., 12 de ago. de 2026 às 11:53, Jacob Tanenbaum <[email protected]> escreveu:
> > > On Fri, Jul 24, 2026 at 12:42 PM Lucas Vargas Dias > <[email protected]> wrote: > >> When a logical switch is created or updated, >> lflow_handle_northd_ls_changes() >> already builds and syncs that switch's ls_stateful flows together with its >> by_ls flows (so the shared datapath groups stay stable). The ls_stateful >> record for the same switch also shows up in the ls_stateful node's tracked >> "crupdated" set, so lflow_ls_stateful_handler() would rebuild and resync >> those exact flows a second time. >> >> Pass northd's tracked switches down to lflow_handle_ls_stateful_changes() >> and skip any crupdated ls_stateful record whose switch datapath was >> already >> handled by lflow_handle_northd_ls_changes(). Records for pre-existing >> switches (e.g. an ACL or port-group change) are untouched and still >> processed here. >> >> Assisted-bt: Claude Opus 4.8, Claude Code >> > > nit: Assisted-by: > I agree > > >> Signed-off-by: Lucas Vargas Dias <[email protected]> >> --- >> northd/en-lflow.c | 9 +++++++++ >> northd/northd.c | 24 ++++++++++++++++++++++++ >> northd/northd.h | 1 + >> 3 files changed, 34 insertions(+) >> >> diff --git a/northd/en-lflow.c b/northd/en-lflow.c >> index 9a517ae48..052e22c9d 100644 >> --- a/northd/en-lflow.c >> +++ b/northd/en-lflow.c >> @@ -236,6 +236,14 @@ lflow_ls_stateful_handler(struct engine_node *node, >> void *data) >> return EN_UNHANDLED; >> } >> >> + /* Switch datapaths created/updated in this run had their ls_stateful >> + * flows already handled by lflow_handle_northd_ls_changes(); pass >> northd's >> + * tracked switches so we don't reprocess them here. */ >> + struct northd_data *northd_data = engine_get_input_data("northd", >> node); >> + const struct tracked_dps *trk_switches = >> + northd_has_lswitches_in_tracked_data(&northd_data->trk_data) >> + ? &northd_data->trk_data.trk_switches : NULL; >> + >> const struct engine_context *eng_ctx = engine_get_context(); >> struct lflow_data *lflow_data = data; >> struct lflow_input lflow_input; >> @@ -243,6 +251,7 @@ lflow_ls_stateful_handler(struct engine_node *node, >> void *data) >> lflow_get_input_data(node, &lflow_input); >> if (!lflow_handle_ls_stateful_changes(eng_ctx->ovnsb_idl_txn, >> &ls_sful_data->trk_data, >> + trk_switches, >> &lflow_input, >> lflow_data->lflow_table)) { >> return EN_UNHANDLED; >> diff --git a/northd/northd.c b/northd/northd.c >> index 9ecc072f3..d15ad8395 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -22126,6 +22126,7 @@ exit: >> bool >> lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn, >> struct ls_stateful_tracked_data >> *trk_data, >> + const struct tracked_dps *trk_switches, >> struct lflow_input *lflow_input, >> struct lflow_table *lflows) >> { >> @@ -22139,6 +22140,17 @@ lflow_handle_ls_stateful_changes(struct >> ovsdb_idl_txn *ovnsb_txn, >> ovs_assert(od->nbs && uuid_equals(&od->nbs->header_.uuid, >> &ls_stateful_rec->nbs_uuid)); >> >> + /* Newly created/updated switch datapaths already had their >> + * ls_stateful flows built and synced by >> + * lflow_handle_northd_ls_changes() (which processes both the >> by_ls >> + * and ls_stateful refs together to keep shared datapath groups >> + * stable). Skip them here to avoid rebuilding the same flows. >> */ >> + if (trk_switches && hmapx_contains(&trk_switches->crupdated, >> + CONST_CAST(struct >> ovn_datapath *, >> + od))) { >> > > Why are you using CONST_CAST here and other places you call > hmapx_contains? > I'll adjust, it's possible to add the od directly. > > >> + continue; >> + } >> + >> lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref); >> >> /* Generate new lflows. */ >> @@ -22156,6 +22168,18 @@ lflow_handle_ls_stateful_changes(struct >> ovsdb_idl_txn *ovnsb_txn, >> * those datapath groups within those flows over and over again. */ >> HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) { >> struct ls_stateful_record *ls_stateful_rec = hmapx_node->data; >> + >> + /* Already synced by lflow_handle_northd_ls_changes() (see >> above). */ >> + if (trk_switches) { >> + const struct ovn_datapath *od = >> + ovn_datapaths_find_by_index(lflow_input->ls_datapaths, >> + ls_stateful_rec->ls_index); >> + if (hmapx_contains(&trk_switches->crupdated, >> + CONST_CAST(struct ovn_datapath *, od))) { >> + continue; >> + } >> + } >> + >> > > Why only for created/updated switches should previously processed > ls_stateful be skipped why not also on delete? > > deleted switches are unlinked in lflow_handle_northd_ls_changes and after, sync, where flows are deleted.It will be redundant process deleted switches here. Regards, Lucas /* Sync the new flows to SB. */ >> bool handled = lflow_ref_sync_lflows( >> ls_stateful_rec->lflow_ref, lflows, ovnsb_txn, >> diff --git a/northd/northd.h b/northd/northd.h >> index 217f3c6fb..20e03e1a5 100644 >> --- a/northd/northd.h >> +++ b/northd/northd.h >> @@ -1024,6 +1024,7 @@ bool lflow_handle_lr_stateful_changes(struct >> ovsdb_idl_txn *, >> struct lflow_table *lflows); >> bool lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *, >> struct ls_stateful_tracked_data *, >> + const struct tracked_dps >> *trk_switches, >> struct lflow_input *, >> struct lflow_table *lflows); >> bool northd_handle_sb_port_binding_changes( >> -- >> 2.43.0 >> >> >> -- >> >> >> >> >> _'Esta mensagem é direcionada apenas para os endereços constantes no >> cabeçalho inicial. Se você não está listado nos endereços constantes no >> cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa >> mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas >> estão >> imediatamente anuladas e proibidas'._ >> >> >> * **'Apesar do Magazine Luiza tomar >> todas as precauções razoáveis para assegurar que nenhum vírus esteja >> presente nesse e-mail, a empresa não poderá aceitar a responsabilidade >> por >> quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* >> >> >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
