On 7/2/25 5:27 AM, Dumitru Ceara wrote:
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?
Yes, that's completely fair to do. I posted it because while it depends
on the datapath and port binding refactor patches, my hope was that the
code should be reviewable by applying both datapath and port binding
refactor first. However, the urgency of this has declined somewhat, so
it makes complete sense to just defer this instead of trying to have it
ready overly soon.
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
