On 8/13/25 1:02 AM, Mark Michelson via dev wrote: > This patch series seeks to refactor how northbound datapath types are > synced with southbound Datapath_Bindings. > > In current OVN, the en-northd node is responsible for creating, updating > and deleting all southbound Datapath_Bindings. This means that if you > have a new type of Datapath_Binding that you want to add to OVN, it > needs to be added to the en-northd node. > > An upcoming feature (composable services) will be adding new types of > southbound Datapath_Bindings. However, it does not fit well into the > current en_northd environment and would do better separated into its own > set of engine nodes. In order to allow this, the Datapath_Binding > syncing code needs to be extracted to separate nodes. > > This series does just that. The en_northd node is stripped of its > previous functionality of creating, updating, and deleting southbound > Datapath_Bindings. These are now accomplished in dedicated incremental > engine nodes that are inputs to the en_northd engine node. > > Lorenzo Bianconi (3): > northd: Add IP for new logical switches in en-datapath-logical-switch > node. > northd: datapath-sync: Move dp_tnlids map in ovn_synced_datapaths. > northd: datapath-sync: Add IP for LS and LR inputs. > > Mark Michelson (5): > Datapath_Binding: Separate type and UUID external-ids. > northd: Refactor datapath syncing. > en-datapath-logical-router: Incrementally process unsynced routers. > datapaths: Add incremental processing for synced datapaths. > northd: Use synced datapaths everywhere. >
Hi Mark, As mentioned earlier on the v14 thread [0], I think the approach from v16's patch 1/8 "Datapath_Binding: Separate type and UUID external-ids." will create a significant performance problem on upgrades of OpenStack + OVN large deployments. The "enum" schema change in the same patch is also problematic and breaks ovn-kubernetes (not our bug but still a bit annoying) [1]. Also, there was still a use after free due to an I-P bug in v16's patch 6/8 "northd: datapath-sync: Add IP for LS and LR inputs.". As we're getting close to branching for 25.09, Ales and I thought it might be useful to be proactive and prepare a v17 branch that essentially combines the first part of the v15 series with the second part of the v16 one. It also addresses the I-P bug mentioned above. I pushed it in my fork here: https://github.com/dceara/ovn/commits/refs/heads/review-datapath-i-p-v17/ I added v17 changelog messages to all commits. In summary the changes we did were: v17: - patch 1: cherry picked the v15 version - patch 2: cherry picked the v15 version - patch 3: cherry picked the v15 version - patch 4: cherry picked the v15 version - patch 5: cherry picked the v15 version - patch 6: - cherry picked the v16 version - cherry picked some of the v16 patch 8/8 to avoid use after free - fix I-P bug in en_datapath_sync_run() - patch 7: - cherry picked the v16 version - patch 8: - cherry picked the v16 version I'm also running ovn-heater control plane performance tests with this branch as we speak but I'm quite confident they'll be OK based on how they look until now. If the new branch looks OK to you and if you agree with the comments I had about the NB -> SB UUID sync performance problem and about the schema change, feel free to post a v17 based on that. The only things that need to happen before posting v17 are: - update changelogs for v1-v16 in each of the commits (git-pw removed those in my branch) - squash https://github.com/dceara/ovn/commit/acf726d into patch 6/8 (I kept it as a separate commit so the incremental changes are easy to review) - update commit messages for patches 6/8 and 8/8 If so, I can work with Ales on getting this fully reviewed and merged tomorrow morning. Regards, Dumitru [0] https://mail.openvswitch.org/pipermail/ovs-dev/2025-August/425293.html [1] https://mail.openvswitch.org/pipermail/ovs-dev/2025-August/425296.html > TODO.rst | 12 + > controller/local_data.c | 2 +- > ic/ovn-ic.c | 3 +- > lib/ovn-util.c | 50 +++ > lib/ovn-util.h | 11 + > northd/aging.c | 8 +- > northd/automake.mk | 8 + > northd/datapath-sync.c | 143 +++++++ > northd/datapath-sync.h | 125 ++++++ > northd/en-advertised-route-sync.c | 12 +- > northd/en-datapath-logical-router.c | 383 ++++++++++++++++++ > northd/en-datapath-logical-router.h | 67 ++++ > northd/en-datapath-logical-switch.c | 382 ++++++++++++++++++ > northd/en-datapath-logical-switch.h | 64 +++ > northd/en-datapath-sync.c | 598 ++++++++++++++++++++++++++++ > northd/en-datapath-sync.h | 36 ++ > northd/en-global-config.c | 11 + > northd/en-lb-data.c | 267 ++++++++----- > northd/en-lb-data.h | 4 +- > northd/en-multicast.c | 4 +- > northd/en-northd.c | 12 +- > northd/en-port-group.c | 3 +- > northd/inc-proc-northd.c | 52 ++- > northd/lflow-mgr.c | 4 +- > northd/northd.c | 463 +++++---------------- > northd/northd.h | 21 +- > ovn-sb.ovsschema | 6 +- > ovn-sb.xml | 20 +- > tests/ovn-controller.at | 4 + > tests/ovn-northd.at | 208 ++++++++-- > tests/ovn.at | 6 +- > utilities/ovn-sbctl.c | 31 +- > utilities/ovn-trace.c | 3 +- > 33 files changed, 2445 insertions(+), 578 deletions(-) > create mode 100644 northd/datapath-sync.c > create mode 100644 northd/datapath-sync.h > create mode 100644 northd/en-datapath-logical-router.c > create mode 100644 northd/en-datapath-logical-router.h > create mode 100644 northd/en-datapath-logical-switch.c > create mode 100644 northd/en-datapath-logical-switch.h > create mode 100644 northd/en-datapath-sync.c > create mode 100644 northd/en-datapath-sync.h > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
