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
