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
