A port can be added to the postponed set from two places:
claim_lport() and the OIF_MARK_UP/OIF_INSTALL_FLOWS handling in
if-status.c, both via lport_maybe_postpone(). In the if-status case,
once the claim threshold has passed, if-status sets pb->chassis itself
through local_binding_set_pb() but never removes the port from the
postponed set.
>From then on the postponed_ports engine node is non-empty forever, so
every engine run processes the port again through
runtime_data_postponed_ports_handler() -> handle_updated_port() ->
claim_lport(). Since pb->chassis is already this chassis,
claim_lport() takes the "already claimed" branch, which does not remove
the port from the set either. The port is added to the tracked
bindings each time, so all port groups containing it are marked
updated and every logical flow referencing those port groups is
re-processed on every iteration.
The sequence that leaves a port in the set is:
1. This chassis claims a port, and its interface is still in the
OIF_INSTALL_FLOWS or OIF_MARK_UP state.
2. Within the claim threshold (500ms), another chassis sets
pb->chassis to itself.
3. Both binding (claim_lport()) and if-status postpone claiming the
port back, and it is added to the postponed set.
4. Once the threshold has passed, if-status sets pb->chassis back to
this chassis before binding handles the postponed claim. Binding
normally runs first, as engine_run() is called before
if_status_mgr_update() in the main loop, but if the engine run is
cancelled (e.g. a recompute is pending while recomputes are not
allowed because of an OpenFlow backlog), if-status claims the port
on its own.
5. From then on claim_lport() only sees a port that is already bound
to this chassis, so the port is never removed from the set.
Step 2 typically happens when two chassis briefly claim the same port,
e.g. around a VM live migration, a VM re-created on another server
with the same port, or a stale interface with the same iface-id left
behind on another server. The postponed state survives until
ovn-controller is restarted.
On servers with large ACL port groups this kept ovn-controller at
100% CPU indefinitely (observed for weeks, until restart), with each
iteration spending ~200ms re-processing ACL flows. Combined with the
conjunction ordering issue fixed separately in ofctrl, every iteration
also removed and re-added thousands of unchanged OpenFlow flows.
Fix it by removing the port from the postponed set in the "already
claimed by this chassis" branch of claim_lport(), as the claim is
complete at that point.
The new test reproduces the stuck state deterministically: it makes
ovn-controller postpone a claim by binding the port to another chassis
in the SB DB, then binds it back to this chassis while ovn-controller
is paused, as if-status would, and checks that the postponed_ports node
no longer reports changes on subsequent engine runs.
Fixes: f64818d43933 ("controller: throttle port claim attempts from if-status")
Assisted-by: Claude Opus 5.5, Claude Code
Signed-off-by: Ruairi Carroll <[email protected]>
---
controller/binding.c | 5 ++++
tests/ovn.at | 58 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+)
diff --git a/controller/binding.c b/controller/binding.c
index 3e8114e7ec..c14f9ee2f6 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1473,6 +1473,11 @@ claim_lport(const struct sbrec_port_binding *pb,
register_claim_timestamp(pb->logical_port, now);
sset_find_and_delete(postponed_ports, pb->logical_port);
} else {
+ /* The port may have been postponed by if-status, which then set
+ * pb->chassis itself without going through this function. The
+ * claim is done, so the port must not stay postponed, otherwise
+ * it is re-processed on every engine run. */
+ sset_find_and_delete(postponed_ports, pb->logical_port);
update_tracked = true;
if ((pb->n_up && !pb->up[0]) ||
(is_vif && !smap_get_bool(&iface_rec->external_ids,
diff --git a/tests/ovn.at b/tests/ovn.at
index 7801494674..8621766730 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -43462,6 +43462,64 @@ ignored_dp=ls0])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Port claimed outside of binding does not stay postponed])
+AT_KEYWORDS([claim postpone])
+ovn_start
+
+check ovn-nbctl ls-add ls0
+check ovn-nbctl lsp-add ls0 lsp0
+
+net_add n1
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+# A chassis that is not running ovn-controller, so that nothing releases or
+# claims the ports bound to it.
+check ovn-sbctl chassis-add fake geneve 192.168.0.100
+hv1_uuid=$(fetch_column Chassis _uuid name=hv1)
+fake_uuid=$(fetch_column Chassis _uuid name=fake)
+
+as hv1 check ovn-appctl -t ovn-controller vlog/set binding:file:dbg
+as hv1 check ovs-vsctl add-port br-int vif -- \
+ set Interface vif external_ids:iface-id=lsp0
+wait_for_ports_up lsp0
+pb_uuid=$(fetch_column Port_Binding _uuid logical_port=lsp0)
+
+# Bind lsp0 to the other chassis shortly after hv1 claimed it, so that hv1
+# postpones claiming it back. If hv1 was too late to postpone, it claims
+# lsp0 back right away, so just try again.
+OVS_WAIT_UNTIL([
+ check ovn-sbctl set Port_Binding $pb_uuid chassis=$fake_uuid
+ sleep 0.1
+ grep -q "Postponed claim on logical port lsp0" hv1/ovn-controller.log
+])
+
+# Bind lsp0 back to hv1 without going through binding, as if-status does
+# once the claim threshold has passed. hv1 sees lsp0 bound to itself, so
+# the claim is complete.
+sleep_controller hv1
+check ovn-sbctl set Port_Binding $pb_uuid chassis=$hv1_uuid
+wake_up_controller hv1
+wait_column "$hv1_uuid" Port_Binding chassis logical_port=lsp0
+check ovn-nbctl --wait=hv sync
+
+# lsp0 must not stay postponed, otherwise it is processed again on every
+# engine run.
+as hv1 check ovn-appctl -t ovn-controller vlog/set inc_proc_eng:file:dbg
+check ovn-nbctl --wait=hv sync
+check ovn-nbctl --wait=hv sync
+as hv1 check ovn-appctl -t ovn-controller vlog/set inc_proc_eng:file:info
+AT_CHECK([grep -c "node: postponed_ports, old_state Stale, new_state Updated" \
+ hv1/ovn-controller.log], [1], [0
+])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
dnl This test checks that the megaflows translated by ovs-vswitchd
dnl don't match on IPv6 source and destination addresses for
dnl simple switching.
--
2.56.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
