On 10/2/26 1:55 PM, Ales Musil via dev wrote:
> On Thu, Oct 1, 2026 at 10:01 PM Jacob Tanenbaum <[email protected]> wrote:
>
>> Split ovn-northd's en_lflow engine node into a compute-only
>> en_lflow node and a new en_dp_group node that
>> handles datapath-group resolution and all SB database
>> writes. This separation makes en_lflow independent of the
>> SB database, preparing for future incremental processing
>> improvements.
>>
>> en_lflow's handlers now track dirty lflow_refs in a hmapx
>> instead of calling lflow_ref_sync_lflows directly.
>> en_dp_group drains the dirty set on its lflow
>> handler, falling back to a full lflow_table_sync_to_sb when
>> en_lflow did a full recompute or when the IGMP/MLD handler
>> set the needs_full_sync flag.
>>
>> A new lflow_ref_unlink_and_prune() function replaces
>> lflow_ref_resync_flows() for the IGMP and IC-learned
>> service monitor handlers. These handlers use a single
>> shared lflow_ref whose flows are also built by the
>> per-datapath pipeline (with lflow_ref = NULL). The
>> per-datapath build's dp bits are not tracked by any
>> lflow_ref, so lflow_ref_unlink_lflows alone cannot clear
>> them via dp_refcnt. lflow_ref_unlink_and_prune destroys
>> all lrns and orphaned lflows in-memory without SB writes,
>> equivalent to lflow_ref_resync_flows' cleanup.
>>
>> The lflow_table_add_lflow__ upgrade-to-bitmap logic (for
>> single lflow_refs that contribute multiple datapaths to the
>> same lflow) is extended with per-datapath dp_refcnt
>> accounting during mid-cycle bitmap upgrades, preventing
>> premature dp bit release when multiple lflow_refs share a
>> flow across overlapping datapaths.
>>
>> Two test cases need to have sections removed. These tests relied upon
>> encountering a stale dp_group would cause all the lflows to be
>> recalculated. After this patch that is no longer the case and only the
>> lflows directly related to the transaction get modified. Encountering a
>> stale dp_group should no longer rebuild all the lflows.
>>
>> Reported-at: https://redhat.atlassian.net/browse/FDP-2747
>> Assisted-by: Claude Opus 4.8, Claude Code
>> Signed-off-by: Jacob Tanenbaum <[email protected]>
>>
>> ---
Hi, Jacob and Ales.
It seems like we see a significant performance regression with this
change in ovn-heater 500-node cluster-density scenario. The average
iteration time went up from 4.0 to 5.7 seconds, which is 42% increase.
I didn't dig too deep into the change myself, but I asked an LLM to
do some analysis and the output seems plausible. Could you please
take a look and confirm or dismiss?
commit b9809453f5466390f4a71e85c3fdd19a53ee1ea1
("northd: Split en_lflow into compute and sync nodes.")
Author: Jacob Tanenbaum <[email protected]>
This patch separates logical-flow computation from synchronization and
defers dirty-reference synchronization to the datapath-group engine node.
> lflow_data->trk_data.needs_full_sync = true;
> [...]
> if (hmapx_is_empty(&lflow_data->trk_data.dirty_lflow_refs) ||
> lflow_data->trk_data.needs_full_sync) {
>
> dp_group_sync_to_sb(node, lflow_data);
> return EN_HANDLED_UPDATED;
> }
Could setting needs_full_sync in lflow_multicast_igmp_handler() cause
ordinary logical switch port changes to synchronize the entire logical
flow table?
multicast_igmp_northd_handler() returns EN_UNHANDLED whenever a logical
switch port is created, updated, or deleted. The resulting multicast
recompute returns EN_UPDATED, so lflow_multicast_igmp_handler() sets
needs_full_sync even when lflow otherwise handles the port changes
incrementally. This path does not require learned IGMP groups.
The multicast_igmp node also manages non-IGMP multicast groups, including
ordinary flood, unknown-destination, and static groups. Port changes
therefore trigger this path even when IGMP/MLD snooping is disabled and
there are no learned membership groups. build_igmp_lflows() also rebuilds
multicast flood flows for all logical switches.
Before this patch, the multicast handler synchronized its lflow_ref and
the port handlers synchronized the affected port references. With this
change, dp_group_lflow_handler() instead calls lflow_table_sync_to_sb(),
visiting every in-memory flow and every SB Logical_Flow row, including
flows unrelated to the port changes.
In ovn-heater's 500-node cluster-density tests, we see:
> node: dp_group, handler for input lflow took 797ms
That message can include full-table synchronization inside the handler,
rather than only synchronization of dirty references. Although the log
alone does not identify which branch ran, the port-change path above
appears to introduce cluster-wide synchronization for each incremental
port-change batch.
Could the multicast handler retain targeted synchronization, tracking
deleted flows for deferred SB deletion instead of setting needs_full_sync?
Simply adding the rebuilt multicast reference to the dirty set would not
preserve deletions, since lflow_ref_unlink_and_prune() destroys the old
in-memory flows.
Could full rebuilds also be marked explicitly, so an empty dirty-reference
set does not by itself require full-table synchronization?
-----
While checking the performance, I also ran some normal LLM review and
the following findings may also be worth looking at. The second one
may not be an actual issue, as we do not expect SB data to be manipulated
by external writers, but the fact that there might be a reachable release
of a borrowed reference doesn't sound great. So, if you can take a look
and confirm or dismiss, that would also be great.
commit b9809453f5466390f4a71e85c3fdd19a53ee1ea1
("northd: Split en_lflow into compute and sync nodes.")
Author: Jacob Tanenbaum <[email protected]>
This patch separates logical-flow computation from synchronization and
defers dirty-reference synchronization to the datapath-group engine node.
> if (hmapx_is_empty(&lflow_data->trk_data.dirty_lflow_refs) ||
> lflow_data->trk_data.needs_full_sync) {
>
> dp_group_sync_to_sb(node, lflow_data);
> return EN_HANDLED_UPDATED;
> }
> [...]
> if (!lflow_ref_sync_lflows(ref, lflow_data->lflow_table,
Can dp_group_lflow_handler() in northd/en-dp-group.c:92-108 retire
unlinked reference nodes even when needs_full_sync is set? Full table
synchronization retains those nodes on shared flows that still have
datapaths, whereas lflow_ref_sync_lflows__() in
northd/lflow-mgr.c:1529-1538 removes them.
With two switch load balancers sharing a VIP and switch membership,
removing that VIP from one balancer while updating an unrelated existing
port's addresses can unlink its reference and select this full-sync path.
If that balancer retains another VIP, a later options change combined with
another port update calls lflow_ref_unlink_lflows() in
northd/lflow-mgr.c:696-715 on the retained, already-unlinked node.
Wouldn't this release the other balancer's remaining datapath counts and
let full synchronization delete its still-required VIP flow? Could the
full-sync path perform the unlinked-node cleanup before clearing the dirty
references?
---
> lflow->dpg = ovn_dp_group_get(dp_groups, &lflow->dpg_bitmap,
> n_datapaths);
> [...]
> if (!lflow->dpg->dp_group) {
> [...]
> ovn_dp_group_release(dp_groups, lflow->dpg);
> lflow->dpg = NULL;
> pre_sync_dpg = NULL;
Could sync_lflow_to_sb() in northd/lflow-mgr.c:1358-1382 release only an
owned reference here? ovn_dp_group_get() returns a borrowed cache entry,
and an incrementally rebuilt flow can have pre_sync_dpg == NULL while
unchanged flows still own that entry.
If an authorized Southbound writer clears all logical_dp_group references
to a group, allowing its row to be garbage-collected, rebuilding only some
flows with that bitmap can reach this branch before the remaining owners.
The release then consumes another flow's reference and leaves the cached
group's reference count too low.
When the remaining owners are subsequently updated or synchronized, can
ovn_dp_group_release() in northd/lflow-mgr.c:1429-1444 free the group while
one flow still points to it, then write to freed memory on that flow's
release? Setting pre_sync_dpg to NULL does not restore the reference
consumed by releasing the borrowed lookup.
----
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
