Hi Mark, I was looking into doing v8 with the memory leak fix but just saw your response.
> I plan to merge this with the following changes: Thanks a lot, much appreciated! I am still planning to post some performance numbers but v7 took a bit more time than I expected - partly because it was tricky to avoid full recomputes while introducing the new index and split out status handling into a separate change. I am going to work on preparing ovn-heater changes for submission next to complement those patches. Thanks a lot for the quick review! On Friday, August 14th, 2026 at 8:23 PM, Mark Michelson <[email protected]> wrote: > Hi Dmitrii, > > Thanks for the updated patches. I had another look, and there are very > few issues at this point. They are so few and minor, that I think it's > not worth your time to try to fix them up, so for the whole series, > > Acked-by: Mark Michelson > > I plan to merge this with the following changes: > * In patch 4, there is a memory leak fix in lib/actions.c that should > be extracted into its own standalone commit and should be backported > to other versions where the leak exists. I will keep you as the author > on this separate commit. > * In patch 5, there are a lot of seemingly-random blank lines removed > from files, very far from where relevant changes have been made. I am > going to remove these changes when merging. > * In patch 6, I plan to add a NEWS item for the new > Advertised_Route_Status table. > > Thanks again for the contribution! > > On Fri, Aug 14, 2026 at 3:24 AM Dmitrii Shcherbakov > <[email protected]> wrote: > > > > OVN 26.03 introduced distributed load balancers [1]: when a > > Load_Balancer has options:distributed=true and ip_port_mappings is > > configured, each chassis delivers traffic only to the local backend. > > Deployments that use BGP to advertise tenant routes into the fabric > > [2] could previously advertise LB VIPs only as a single prefix per > > VIP, with no per-chassis health awareness. The fabric could ECMP > > across chassis but could not prefer those hosting healthy backends > > or withdraw routes from chassis with unhealthy ones. > > > > This series splits the Advertised_Route emission for LB VIPs from > > one-per-VIP to one-per-(VIP IP, backend LSP) and adds a controller- > > side gate that skips route installation when the Service_Monitor for > > the corresponding backend is offline. With > > dynamic-routing-redistribute-local-only=true, this allows the local > > dynamic routing speaker to advertise the VIP prefix only from chassis > > that host a healthy backend, so fabric ECMP converges onto healthy > > chassis. Without local-only, health gating retains the existing > > remote-route behavior for healthy backends. > > > > Chassis-local advertisement requires options:dynamic-routing=true on > > the advertising logical router, dynamic-routing-redistribute=lb and > > dynamic-routing-redistribute-local-only=true on its advertising port, > > and a distributed LB with ip_port_mappings and a health check. > > > > The series also installs local forwarding routes on the advertising > > logical router for peer-LR LB VIPs and NAT external IPs enumerated > > for redistribution, so that the advertising LR can forward traffic to > > those addresses through the peer. > > > > Patch 1 fixes an existing bug where attaching a distributed LB to a > > router with a chassis-redirect port can leave stale lr_in_admission > > guards, dropping ingress LB traffic on non-gateway chassis. > > > > Patch 2 installs local forwarding routes for peer-LR LB VIPs and NAT > > external IPs that an advertising LRP has enumerated for redistribution. > > LB and NAT forwarding routes for the same prefix are treated as ECMP > > members so both peer-LR paths survive when a VIP IP and a NAT external > > IP overlap. This is an edge case a CMS would typically prevent. > > > > Patch 3 tracks additions and removals while those forwarding routes > > are rebuilt, preserving parsed-route pointer identity for downstream > > incremental nodes. > > > > Patch 4 splits Advertised_Route emission from one-per-VIP to > > one-per-(VIP IP, backend LSP), deduplicated by the existing SB unique > > index. Each LB-derived row is tagged with external_ids:source=lb and > > the backend LSP is used as tracked_port for per-chassis locality. > > Centralized load balancers retain one VIP-level row, and both modes > > retain the forwarding route through the peer LR. The per-backend > > split applies to LBs attached directly to the advertising LR as well > > as to LBs on neighbouring routers. Its local-only system test covers > > backend movement between chassis. It also handles the short interval in > > which a newly local datapath is present in runtime data before its > > Advertised_Route monitor condition catches up. The northd tests expose > > and fix an existing ct_lb action-string leak on the no-active-backend > > path. > > > > Patch 5 adds the controller-side gate. Northd writes the backend > > selectors to a single external_ids:health-checks key on each > > distributed-LB Advertised_Route as semicolon-delimited > > protocol,IP,port tuples. The controller uses an IDL index over logical > > port, monitor type, protocol and port, then compares IP addresses in > > binary form. It records the UUIDs of matching monitors so tracked > > deletions and selector changes trigger route recomputation. > > It skips kernel-route installation when matching monitors exist > > and all are offline. external_ids:enabled is intentionally not > > written by northd. It remains an operator- or > > CMS-owned administrative override for marked routes. Controllers reject > > remote local-only routes before registering their health selectors. > > > > Patch 6 replaces the v6 external_ids:status approach with a per-chassis > > Advertised_Route_Status table for operator observability. It requires an > > SB schema change (21.11.0 to 21.12.0). If the schema change cannot land > > during the soft freeze, patch 6 can be deferred to the next cycle without > > affecting the health-gating feature in patches 1-5. > > > > The nexthop comparison fix previously included as patch 2 in v4 has > > been dropped: it was merged upstream independently in commit > > ace80f762 ("northd: Fix the route and route policy lookup > > functions."). > > > > Changes since v6: > > - Split forwarding-route incremental tracking from patch 2 into a new > > patch 3. > > - Patch 1 uses datapath-index bitmaps for the affected LR snapshot and > > fixes the existing LB flag guard and reset paths. The transition check > > derives the post-update state without mutating the datapath snapshot. > > - Patches 2 and 3 pass the forwarding peer LRP separately from > > tracked_port. This covers distributed NAT rows whose tracked port > > is a backend LSP. LB and NAT routes for the same prefix remain as > > separate, ECMP-compatible forwarding routes. This is an edge case > > a CMS would typically prevent. > > - Patch 4 retains forwarding routes for both centralized and distributed > > load balancers. The IPv4, IPv6 and LB/NAT overlap tests continue to > > exercise centralized load balancers, while distributed coverage remains > > in the per-backend tests. > > - Patch 4: the own-LR path (LBs attached directly to the advertising > > LR) uses the same per-backend emission as neighbour-owned LBs, so > > distributed LBs on the advertising LR also emit per-backend > > Advertised_Route rows with distributed-lb=true and backend LSP as > > tracked_port. > > - Patch 4: include the tracked_port logical-port name in the > > ar_entry hash so that per-backend routes for the same VIP > > distribute across hash buckets instead of colliding into one. > > - Patch 4 adds a local-only system test for backend movement between > > chassis and handles Advertised_Route monitor-condition lag when a > > datapath first becomes local. > > - Document that LB and datapath-association changes reach dynamic_routes > > through lr_stateful, and cover an association-only update. > > - Patch 5 stores exact health-check selectors in a single > > external_ids:health-checks key on each Advertised_Route as > > semicolon-delimited protocol,IP,port tuples. The controller reads > > those selectors directly and compares IP addresses in binary form, > > treats equivalent IPv6 forms consistently and does not let an > > unrelated LB with the same VIP affect the gate. > > - Patch 5 uses a Service_Monitor IDL index over logical port, monitor > > type, protocol and port for selector lookup. It keeps a UUID set of > > monitors matched during the previous route run. A tracked UUID catches > > deletion or movement away from a selector, while a current > > load-balancer monitor on a referenced logical port catches insertion > > or movement into scope. Other monitor changes do not recompute the > > route node, and route evaluation always reads live IDL rows. > > - Patch 5 skips the health-gate scan on controllers with no advertised > > datapaths. It applies advertising-port locality and > > dynamic-routing-redistribute-local-only eligibility before registering > > health selectors, so a remote local-only backend does not cause route > > recomputation on other chassis. > > - Patch 5: when two LB listeners share the same VIP IP and backend > > LSP (e.g. VIP:80 with a health check and VIP:443 without one), > > northd omits external_ids:health-checks so the controller does > > not gate the shared route. The kernel route is per-prefix > > regardless of listener port. > > - Patch 5: if an LB route and a non-LB route share the same (datapath, > > logical_port, ip_prefix, tracked_port) key, northd omits source=lb, > > distributed-lb and health-checks metadata so the controller does not > > health-gate the shared route. > > - Patch 6 replaces the v6 external_ids:status approach with the per-chassis > > Advertised_Route_Status table. It reports desired state, withdrawal > > reasons, operational state (whether the route is in the routing > > table) and reconciliation errors. The controller publishes and > > repairs only its own rows. Conditional monitoring and a chassis_name > > index avoid global status-table scans. > > - Patch 5: the dynamic-routing-redistribute-local-only eligibility > > check runs before administrative and health gating in route_run(). > > A route not selected for the chassis is ignored. Patch 6 therefore > > does not publish an Advertised_Route_Status row for it, matching the > > documented "selected for its chassis" meaning. > > - Patch 5: administrative override (external_ids:enabled=false) and > > health gating run outside the tracked_port block, so they also > > apply to distributed-LB routes that have no tracked_port (e.g. a > > distributed LB without ip_port_mappings). > > - Patch 6 restricts Advertised_Route_Status writes to each controller's > > own chassis name via RBAC, and cleans up rows on controller shutdown > > and chassis-del. The SB schema version changes from 21.11.0 to 21.12.0. > > - Patch 5 avoids route recomputation for unrelated Load_Balancer, > > Service_Monitor and Advertised_Route changes. > > - Patch 6 uses sorted route decisions and indexed local status rows for > > status publication. > > - Add regression tests for own-LR per-backend emission, ungated > > listener suppression, remote local-only health updates, exact > > NAT/LB key overlap and administrative override without > > tracked_port. > > - Extend tests for exact selector serialization and isolation, IPv6 > > address normalization, same-selector-bucket IP isolation, > > Service_Monitor insertion, deletion and selector movement, and > > redistribute-local-only behavior. > > - Patch 6 adds tests for RBAC, schema compatibility, desired and > > operational status, reason reporting, reconciliation errors, row > > repair and cleanup. > > > > A separate LRP option for VRF route sharing (share-advertise-routes) > > is deferred to a follow-up series. Deployments that need cross-VRF > > route sharing before the option lands can achieve the same effect > > by configuring the dynamic routing speaker to import routes between > > the OVN-managed VRFs on each chassis. > > > > Prior work: RFC for LB BGP advertisement [3] in which a review [4] > > suggested a separate incremental processing node for dynamic routes, > > which this series adopts. For tracked_port, patch 2 uses the peer LRP > > for forwarding routes and patch 4 uses the backend LSP (from > > ip_port_mappings) for per-backend locality rows. Patch 5 adds > > Service_Monitor gating on the per-backend rows. > > > > > > [1] commit 7b0eb4d9ed ("northd: Add distributed load balancer support.") > > [2] > > https://opendev.org/openstack/neutron-specs/src/branch/master/specs/2025.2/ovn-bgp-integration.rst > > [3] https://mail.openvswitch.org/pipermail/ovs-dev/2025-February/420552.html > > [4] https://mail.openvswitch.org/pipermail/ovs-dev/2025-February/420789.html > > > > Dmitrii Shcherbakov (6): > > northd: Recompute only when LR is_distributed flips on LB. > > northd: Install forwarding routes for redistribute={lb,nat}. > > northd: Track dynamic forwarding route changes. > > northd: Emit per-backend Advertised_Route for LB redistribution. > > controller: Skip LB route install when Service_Monitor is offline. > > controller: Add Advertised_Route_Status table. > > > > controller/chassis.c | 27 +- > > controller/chassis.h | 4 +- > > controller/ovn-controller.c | 349 +++++- > > controller/route-exchange.c | 38 +- > > controller/route-exchange.h | 5 +- > > controller/route.c | 336 +++++- > > controller/route.h | 36 + > > lib/actions.c | 1 + > > lib/ovn-util.h | 7 + > > northd/en-advertised-route-sync.c | 551 +++++++-- > > northd/en-advertised-route-sync.h | 29 +- > > northd/en-group-ecmp-route.c | 94 +- > > northd/en-group-ecmp-route.h | 4 + > > northd/en-lb-data.c | 22 +- > > northd/en-lb-data.h | 3 + > > northd/en-lr-stateful.c | 2 + > > northd/en-northd.c | 1 + > > northd/inc-proc-northd.c | 5 + > > northd/lb.c | 2 + > > northd/lb.h | 1 + > > northd/northd.c | 189 +++- > > northd/northd.h | 3 + > > northd/ovn-northd.c | 17 + > > ovn-sb.ovsschema | 34 +- > > ovn-sb.xml | 142 ++- > > tests/automake.mk | 6 +- > > tests/ovn-controller.at | 59 + > > tests/ovn-inc-proc-graph-dump.at | 11 +- > > tests/ovn-northd.at | 1743 ++++++++++++++++++++++++++++- > > tests/ovn-sbctl.at | 7 +- > > tests/system-ovn.at | 1327 +++++++++++++++++++++- > > utilities/ovn-sbctl.8.xml | 5 +- > > utilities/ovn-sbctl.c | 17 + > > 33 files changed, 4909 insertions(+), 168 deletions(-) > > > > > > base-commit: 20b9f0b9a771e07f15d2db270464965663d15f56 > > -- > > 2.53.0 > > > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
