Hi Mark, On 6/9/25 7:35 PM, Mark Michelson via dev wrote: > NOTE: This series is built on top of the datapath binding refactor and > port binding refactor patches. You're likely to see tests failing on > these patches if those series are not merged already. >
It seems the logical flow sync refactor (this series) doesn't even apply cleanly on top of main without the datapath binding and port binding refactor. https://mail.openvswitch.org/pipermail/ovs-build/2025-June/046904.html Should we defer it until the binding refactor lands? > This series refactors how logical flows are synced, accomplishing two > main goals: > > 1) Syncing of logical flows to the southbound database is handled in a > new node, rather than in en-lflow. This allows for syncing of logical > flow tables from en-lflow and other nodes. > > 2) The libraries for building logical flow tables and for syncing > logical flows no longer relies on ovn-northd constructs such as > ovn_datapath. > > This series accomplishes the above goals through a series of smaller > patches rather than a single giant patch like other refactoring series. > > Overall, this series gets the job done, but there are a few things I'm > not especially happy about: > > 1) The goal of the new en-lflow-sync node is to be able to sync logical > flows from multiple logical flow tables. However, in current OVN, the > only logical flow table is the one created by en-lflow. Therefore, while > the code *should* be able to handle multiple logical flow tables, it's > not proven with this code change. > > 2) I'm not happy about the non-constness of the inputs to the > en-lflow-sync node. In particular, > * lflow_refs are created/destroyed in various nodes, then all are > populated in en-lflow, and then all are synced in en-lflow-sync. > This means that en-lflow and en-lflow-sync are modifying lflow_refs > beyond the node that manages their lifetimes. > * struct ovn_lflow has a datapath group field (dpg) that is modified > in en-lflow-sync. > > There are ways to fix (2), but to do so in a performant way would be > outside the scope of simply trying to refactor when/how logical flows > are synced. > > Mark Michelson (14): > northd: Find ovn_datapath from a single hmap. > lflow-mgr: Remove the ovn_datapath from ovn_lflow. > lflow-mgr: Use one ovn_datapaths arg for ovn_dp_group_create(). > lflow-mgr: Pass one ovn_datapaths arg to sync_lflow_to_sb(). > lflow_mgr: Pass dp_groups to sync_lflow_to_sb(). > lflows: Create a separate node for lflow syncing. > en-lflow-sync: Separate syncing from datapaths and from the lflow > table. > northd: Convert ovn_stage from an enum to a struct. > lflow_ref: Allow multiple datapaths per lflow_ref_node. > lflows: Separate lflow_ref building and syncing. > lflows: Move dp_groups from lflow_data to lflow_sync_data. > datapaths: Add a hashvec of datapath bindings. > lflow: Remove northd dependency from en-lflow-sync. > lflows: Remove ovn_datapath parameter when adding lflows. > > northd/datapath_sync.c | 52 +++ > northd/datapath_sync.h | 33 +- > northd/en-datapath-logical-router.c | 23 +- > northd/en-datapath-logical-router.h | 10 +- > northd/en-datapath-logical-switch.c | 23 +- > northd/en-datapath-logical-switch.h | 10 +- > northd/en-datapath-sync.c | 36 +- > northd/en-learned-route-sync.c | 8 +- > northd/en-lflow.c | 309 +++++++++++--- > northd/en-lflow.h | 32 ++ > northd/en-multicast.c | 3 +- > northd/en-northd-output.c | 14 +- > northd/en-northd-output.h | 5 +- > northd/en-northd.c | 5 +- > northd/en-port-binding-chassisredirect.c | 8 +- > northd/en-port-binding-logical-router-port.c | 2 +- > northd/en-port-binding-logical-switch-port.c | 2 +- > northd/en-port-binding-mirror.c | 4 +- > northd/en-sync-sb.c | 104 +++-- > northd/inc-proc-northd.c | 22 +- > northd/lflow-mgr.c | 419 +++++++++---------- > northd/lflow-mgr.h | 84 ++-- > northd/northd.c | 376 +++++++---------- > northd/northd.h | 92 ++-- > tests/ovn-northd.at | 125 +++++- > 25 files changed, 1112 insertions(+), 689 deletions(-) > Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
