On 7/30/26 12:48 PM, Jun Gu wrote: > Patch (localnet / L2 gateway) port ofport changes force a full recompute > of the logical flow output through two independent engine edges, even > though logical flows never depend on patch ofports: > > - non_vif_data bundles patch ofports together with tunnel data, and > en_lflow_output has no change handler for non_vif_data, so any patch > ofport change invalidates it too. > > - Patch ports never carry an iface-id, so their ofport changes also hit > the "unhandled" branch of binding_handle_ovs_interface_changes(), > forcing a full recompute of runtime_data and, transitively, of > lflow_output again. > > This is hit on the datapath every time a bridged / L2 logical port is > bound or moved between chassis, since the peer patch port is (re)created > and gets a new ofport. In deployments with many logical flows each > recompute can take several seconds, dominating the port's dataplane > downtime. > > Fix both edges: > > - Move patch ofports into their own engine node (en_patch_port_data) > that feeds only the physical flow output. en_non_vif_data keeps > feeding en_lflow_output, but now only changes on tunnel ofport > changes. > > - Recognize patch-type OVS interfaces in > binding_handle_ovs_interface_changes() and skip them instead of > treating them as unhandled: their ofport is already tracked by > en_patch_port_data. > > Tunnel (geneve/vxlan) interfaces are deliberately left alone in the > latter fix. runtime_data's 'active_tunnels' (BFD-derived reachability, > used for ECMP / gateway-chassis flow selection) has no incremental > tracking and is only recalculated on a full recompute; skipping tunnel > ofport changes there would silently stop refreshing it on BFD status > changes. > > Recognizing patch ports in binding_handle_ovs_interface_changes() also > exposes a pre-existing gap: unlike binding_handle_port_binding_changes(), > it never re-scans a newly-local datapath's sibling ports (localnet / > external / vtep / multichassis) after calling add_local_datapath(). This > used to be masked by the very recompute this patch removes. Extract the > existing catch-up logic into catch_up_new_local_datapaths() and call it > from both incremental handlers. > > Assisted-by: Claude Opus 4.8, Claude Code > Signed-off-by: Jun Gu <[email protected]> > --- > controller/binding.c | 122 +++++++++++++++------ > controller/local_data.c | 180 ++++++++++++++++++++----------- > controller/local_data.h | 22 ++-- > controller/ovn-controller.c | 94 +++++++++++++--- > tests/ovn-controller.at | 81 ++++++++++++++ > tests/ovn-inc-proc-graph-dump.at | 5 + > 6 files changed, 387 insertions(+), 117 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index de51be823..a91193aa0 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -2706,6 +2706,30 @@ is_iface_vif(const struct ovsrec_interface *iface_rec) > return true; > } > > +/* Patch (localnet / L2 gateway) ports are OVS interfaces that > + * ovn-controller itself creates and manages. They never carry an iface-id > + * and are of no interest to port binding processing: their ofport changes > + * are already tracked by the patch_port_data engine node. Recognizing them > + * here lets binding_handle_ovs_interface_changes() skip them instead of > + * treating them as an unhandled change and forcing a full recompute of > + * runtime_data (and, transitively, of lflow_output) on every patch port > + * bind / migration.
This doesn't seem right. What prevents someone from adding an LSP using a patch port? It will have iface-id and it will be externally managed by the user or CMS. Is there something that doesn't allow that? Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
