Hi, I would like to know your option about this patch, since you created/acked patch [0]. Or if is there another way to resolve the Warn log message?
[0] https://github.com/ovn-org/ovn/commit/0e2bcf70ac4f769704c62de715b4905720d8ada3 Regards, Lucas Em ter., 18 de ago. de 2026 às 15:52, Lucas Vargas Dias <[email protected]> escreveu: > Commit 0e2bcf70ac4f moved the release of the dp group referenced by an > lflow from sync_lflow_to_sb() to do_ovn_lflow_add(), so that the group > is already free when a new one is looked up and the SB row can be > reused. However, do_ovn_lflow_add() is only called when the lflow is > generated again. An lflow that is merely unlinked from one of its > lflow_refs, and that survives because other lflow_refs still reference > it, never goes through do_ovn_lflow_add(): only its dp group bitmap > shrinks. When such an lflow is synced, it points to a different dp > group (or to a single datapath), and the reference to the previous > group is never dropped. > > The leaked group stays in the 'dp_groups' map with a non zero refcount > while no lflow uses it anymore, so its SB Logical_DP_Group row is > garbage collected. Any lflow that later needs that same set of > datapaths finds the leaked group, fails to look its row up and northd > logs: > > SB Logical flow [...]'s logical_dp_group column is not set (which is > unexpected). It should have been referencing the dp group [...] > > and falls back to a full recompute. > > Release the previous dp group after the new one has been taken. The > release done by do_ovn_lflow_add() clears 'lflow->dpg', so lflows that > were regenerated are not released twice. > > Fixes: 0e2bcf70ac4f ("northd: Change ovn_dp_groups to decrement refcount > in do_ovn_lflow_add.") > Signed-off-by: Lucas Vargas Dias <[email protected]> > --- > northd/lflow-mgr.c | 7 +++++++ > tests/ovn-northd.at | 51 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 58 insertions(+) > > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c > index ce9c4f854..2367156ad 100644 > --- a/northd/lflow-mgr.c > +++ b/northd/lflow-mgr.c > @@ -1264,6 +1264,13 @@ sync_lflow_to_sb(struct ovn_lflow *lflow, > > if (pre_sync_dpg != lflow->dpg) { > ovn_dp_group_use(lflow->dpg); > + /* The lflow's dp group bitmap may have changed without the lflow > + * being re-added (e.g. when it was only unlinked from one of its > + * lflow_refs), in which case do_ovn_lflow_add() didn't get the > + * chance to drop the reference to the previous dp group. Drop it > + * here, otherwise the dp group is leaked in 'dp_groups' with a > + * dangling reference to an SB row that gets garbage collected. */ > + ovn_dp_group_release(dp_groups, pre_sync_dpg); > } > > lflow->sync_state = LFLOW_SYNCED; > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 8c8d7852e..19887e6d7 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -23647,3 +23647,54 @@ AT_CHECK([as northd ovn-appctl -t ovn-northd > inc-engine/enable-stopwatch nonexis > OVN_CLEANUP_NORTHD > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([Datapath group reuse after an lflow loses a datapath]) > +ovn_start > + > +# All four switches have ACLs, so the logical flows that only depend on > +# "the switch has ACLs" are shared by the four of them. ls1 and ls2 use > +# tier 0 while ls3 and ls4 use tier 1, which changes the actions of the > +# egress "acl action" flows. Hence three datapath groups are expected: > +# {ls1, ls2}, {ls3, ls4} and {ls1, ls2, ls3, ls4}. > +check ovn-nbctl ls-add ls1 > +check ovn-nbctl ls-add ls2 > +check ovn-nbctl ls-add ls3 > +check ovn-nbctl ls-add ls4 > +check ovn-nbctl acl-add ls1 to-lport 1000 ip4 allow > +check ovn-nbctl acl-add ls2 to-lport 1000 ip6 allow > +check ovn-nbctl --tier=1 acl-add ls3 to-lport 1000 tcp allow > +check ovn-nbctl --wait=sb --tier=1 acl-add ls4 to-lport 1000 udp allow > + > +acl1=$(fetch_column nb:ACL _uuid match=ip4) > +check_row_count Logical_DP_Group 3 > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > +# Move ls1's ACL to tier 1. ls1 stops generating the egress "acl action" > +# flows it shared with ls2, so those flows are left with a single datapath > +# and the {ls1, ls2} datapath group becomes unused and is deleted. northd > +# has to drop the reference it holds to that group, otherwise the group is > +# leaked in the in-memory dp group table, still pointing to the SB row > that > +# has just been deleted. > +check ovn-nbctl --wait=sb set ACL $acl1 tier=1 > +check_engine_stats lflow norecompute compute > +check_row_count Logical_DP_Group 2 > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > + > +# Move ls1's ACL back to tier 0. The {ls1, ls2} datapath group is needed > +# again: a leaked group would be picked up here and northd would complain > +# about its dangling SB reference. > +check ovn-nbctl --wait=sb set ACL $acl1 tier=0 > +check_engine_stats lflow norecompute compute > +check_row_count Logical_DP_Group 3 > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +AT_CHECK([grep -q "logical_dp_group column is not set" \ > + northd/ovn-northd.log], [1]) > + > +OVN_CLEANUP_NORTHD > +AT_CLEANUP > +]) > -- > 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
