On Thu, Aug 20, 2026 at 11:28 AM Dmitrii Shcherbakov < [email protected]> wrote:
> A persistent IDL index adds work to every Service_Monitor update, > including on chassis that have no health-gated advertised routes. OVS > removes and reinserts a modified row in each index even when the indexed > columns did not change. > > Build a temporary hash of the selectors only while evaluating active > route gates, then scan the already monitored Service_Monitor table once. > This keeps route evaluation linear in the number of selectors and > monitors without steady-state index maintenance. Skip tracked-row > processing when the chassis has no relevant monitor or health-check port. > > The preceding commit 6f955a91d fixed cleanup of temporary index query > rows. This change removes that query path, so the cleanup is no longer > needed. > > The scan records every monitor matching an active selector. The indexed > loop stopped at later selectors after a gate found an online monitor. > This only widens relevant_service_monitors for deleted-row tracking. > Changes to live load-balancer monitors on a gated port are already > covered by health_check_ports. Gate decisions keep the same OR > semantics. > > The handler can return unchanged when both tracking sets are empty > because each unhandled path requires an entry in one of them. > > Each recompute now costs O(|Service_Monitor| + |selectors|). > ovn-heater comparisons covered both a fully distributed workload and an > asymmetric workload with 4000 monitors and only 10 advertised load > balancers. > > Signed-off-by: Dmitrii Shcherbakov <[email protected]> > --- > controller/ovn-controller.c | 25 +++---- > controller/route.c | 127 ++++++++++++++++++++++++++---------- > controller/route.h | 3 +- > 3 files changed, 103 insertions(+), 52 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 6551a4e30..38c1acfe8 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -5331,9 +5331,8 @@ en_route_run(struct engine_node *node, void *data) > > const struct sbrec_advertised_route_table *advertised_route_table = > EN_OVSDB_GET(engine_get_input("SB_advertised_route", node)); > - struct ovsdb_idl_index *service_monitor_by_selector = > - engine_ovsdb_node_get_index( > - engine_get_input("SB_service_monitor", node), "selector"); > + const struct sbrec_service_monitor_table *service_monitor_table = > + EN_OVSDB_GET(engine_get_input("SB_service_monitor", node)); > const struct ovsrec_open_vswitch *cfg > = ovsrec_open_vswitch_table_first(ovs_table); > const char *dynamic_routing_port_mapping = > @@ -5341,7 +5340,7 @@ en_route_run(struct engine_node *node, void *data) > > struct route_ctx_in r_ctx_in = { > .advertised_route_table = advertised_route_table, > - .service_monitor_by_selector = service_monitor_by_selector, > + .service_monitor_table = service_monitor_table, > .sbrec_port_binding_by_name = sbrec_port_binding_by_name, > .chassis = chassis, > .dynamic_routing_port_mapping = dynamic_routing_port_mapping, > @@ -5687,6 +5686,11 @@ route_sb_service_monitor_handler(struct engine_node > *node, > void *data) > { > struct ed_type_route *re_data = data; > + if (uuidset_is_empty(&re_data->relevant_service_monitors) && > + sset_is_empty(&re_data->health_check_ports)) { > + return EN_HANDLED_UNCHANGED; > + } > + > const struct sbrec_service_monitor_table *sm_table = > EN_OVSDB_GET(engine_get_input("SB_service_monitor", node)); > > @@ -7670,19 +7674,6 @@ inc_proc_ovn_controller_init( > engine_ovsdb_node_add_index(&en_sb_datapath_binding, "key", > sbrec_datapath_binding_by_key); > > - const struct ovsdb_idl_index_column > service_monitor_selector_columns[] = { > - { .column = &sbrec_service_monitor_col_logical_port }, > - { .column = &sbrec_service_monitor_col_type }, > - { .column = &sbrec_service_monitor_col_protocol }, > - { .column = &sbrec_service_monitor_col_port }, > - }; > - struct ovsdb_idl_index *service_monitor_by_selector = > - ovsdb_idl_index_create( > - sb_idl_loop->idl, service_monitor_selector_columns, > - ARRAY_SIZE(service_monitor_selector_columns)); > - engine_ovsdb_node_add_index(&en_sb_service_monitor, "selector", > - service_monitor_by_selector); > - > struct ovsdb_idl_index *sbrec_fdb_by_dp_key > = ovsdb_idl_index_create1(sb_idl_loop->idl, > &sbrec_fdb_col_dp_key); > diff --git a/controller/route.c b/controller/route.c > index cb424bec8..00fd9a105 100644 > --- a/controller/route.c > +++ b/controller/route.c > @@ -87,6 +87,15 @@ struct lb_route_gate { > bool any_online; > }; > > +/* A parsed health-check selector and the route gate that uses it. */ > +struct lb_route_selector { > + struct hmap_node node; > + struct lb_route_gate *gate; > + struct in6_addr backend_ip; > + char *protocol; > + uint16_t port; > +}; > + > static bool > route_has_health_checks(const struct sbrec_advertised_route *route) > { > @@ -186,13 +195,25 @@ destroy_lb_route_gates(struct hmap *gates) > hmap_destroy(gates); > } > > -/* Resolve northd-provided protocol,backend-IP,backend-port selectors > against > - * Service_Monitor rows with the same logical port, port and protocol. > IP is > - * compared separately in binary form so equivalent IPv6 spellings match. > */ > +/* Hash the four fields that identify the Service_Monitor row a selector > + * refers to. The address is hashed in binary form so equivalent IPv6 > + * spellings produce the same value. */ > +static uint32_t > +lb_route_selector_hash(const char *logical_port, const char *protocol, > + uint16_t port, const struct in6_addr *backend_ip) > +{ > + uint32_t hash = hash_string(logical_port, 0); > + hash = hash_string(protocol, hash); > + hash = hash_add_in6_addr(hash, backend_ip); > + return hash_int(port, hash); > +} > + > +/* Parse the "protocol,backend-IP,backend-port" health-check selectors of > + * active route gates into 'selectors'. The hash map lives only for the > + * duration of route evaluation, so ordinary Service_Monitor updates do > not > + * maintain a persistent index. */ > static void > -evaluate_lb_route_gates(struct hmap *gates, > - struct ovsdb_idl_index > *service_monitor_by_selector, > - struct uuidset *relevant_service_monitors) > +build_lb_route_selectors(struct hmap *selectors, struct hmap *gates) > { > struct lb_route_gate *g; > HMAP_FOR_EACH (g, node, gates) { > @@ -231,39 +252,77 @@ evaluate_lb_route_gates(struct hmap *gates, > continue; > } > > - const char *tracked_lp = g->route->tracked_port->logical_port; > - struct sbrec_service_monitor *filter = > - sbrec_service_monitor_index_init_row( > - service_monitor_by_selector); > - sbrec_service_monitor_index_set_logical_port(filter, > tracked_lp); > - sbrec_service_monitor_index_set_type(filter, "load-balancer"); > - sbrec_service_monitor_index_set_protocol(filter, protocol); > - sbrec_service_monitor_index_set_port(filter, backend_port); > - > - const struct sbrec_service_monitor *monitor; > - SBREC_SERVICE_MONITOR_FOR_EACH_EQUAL ( > - monitor, filter, service_monitor_by_selector) { > - struct in6_addr sm_ip; > - if (!ip46_parse(monitor->ip, &sm_ip) || > - !ipv6_addr_equals(&sm_ip, &backend_ip_addr)) { > - continue; > - } > + struct lb_route_selector *s = xmalloc(sizeof *s); > + *s = (struct lb_route_selector) { > + .gate = g, > + .backend_ip = backend_ip_addr, > + .protocol = xstrdup(protocol), > + .port = backend_port, > + }; > + const char *logical_port = > + g->route->tracked_port->logical_port; > + hmap_insert(selectors, &s->node, > + lb_route_selector_hash(logical_port, protocol, > + backend_port, > + &backend_ip_addr)); > + } > + free(buf); > + } > +} > > - uuidset_insert(relevant_service_monitors, > - &monitor->header_.uuid); > - g->seen_monitor = true; > - g->any_online |= monitor->status && > - !strcmp(monitor->status, "online"); > - } > +/* Match the parsed selectors to Service_Monitor rows with a single scan > of > + * the monitored table. Record every match in 'relevant_service_monitors' > + * so that changes to those rows trigger recompute. */ > +static void > +evaluate_lb_route_gates( > + struct hmap *gates, > + const struct sbrec_service_monitor_table *service_monitor_table, > + struct uuidset *relevant_service_monitors) > +{ > + struct hmap selectors = HMAP_INITIALIZER(&selectors); > + build_lb_route_selectors(&selectors, gates); > + > + const struct sbrec_service_monitor *monitor; > + SBREC_SERVICE_MONITOR_TABLE_FOR_EACH (monitor, service_monitor_table) > { > + if (!monitor->type || strcmp(monitor->type, "load-balancer") || > + !monitor->protocol || monitor->port < 0 || > + monitor->port > UINT16_MAX) { > + continue; > + } > > - sbrec_service_monitor_index_destroy_row(filter); > + struct in6_addr monitor_ip; > + if (!ip46_parse(monitor->ip, &monitor_ip)) { > + continue; > + } > > - if (g->any_online) { > - break; > + uint32_t hash = lb_route_selector_hash( > + monitor->logical_port, monitor->protocol, monitor->port, > + &monitor_ip); > + struct lb_route_selector *s; > + HMAP_FOR_EACH_WITH_HASH (s, node, hash, &selectors) { > + const char *logical_port = > + s->gate->route->tracked_port->logical_port; > + if (s->port != monitor->port || > + strcmp(s->protocol, monitor->protocol) || > + strcmp(logical_port, monitor->logical_port) || > + !ipv6_addr_equals(&s->backend_ip, &monitor_ip)) { > + continue; > } > + > + uuidset_insert(relevant_service_monitors, > + &monitor->header_.uuid); > + s->gate->seen_monitor = true; > + s->gate->any_online |= monitor->status && > + !strcmp(monitor->status, "online"); nit: Wrong alignment. > } > - free(buf); > } > + > + struct lb_route_selector *s; > + HMAP_FOR_EACH_POP (s, node, &selectors) { > + free(s->protocol); > + free(s); > + } > + hmap_destroy(&selectors); > } > > /* Look up the gate decision for a specific route. Returns: > @@ -726,7 +785,7 @@ route_run(struct route_ctx_in *r_ctx_in, > > if (!hmap_is_empty(&lb_route_gates)) { > evaluate_lb_route_gates(&lb_route_gates, > - r_ctx_in->service_monitor_by_selector, > + r_ctx_in->service_monitor_table, > r_ctx_out->relevant_service_monitors); > } > > diff --git a/controller/route.h b/controller/route.h > index cf575f0a9..cd078eb96 100644 > --- a/controller/route.h > +++ b/controller/route.h > @@ -34,10 +34,11 @@ struct sbrec_chassis; > struct sbrec_port_binding; > struct sbrec_datapath_binding; > struct sbrec_advertised_route; > +struct sbrec_service_monitor_table; > > struct route_ctx_in { > const struct sbrec_advertised_route_table *advertised_route_table; > - struct ovsdb_idl_index *service_monitor_by_selector; > + const struct sbrec_service_monitor_table *service_monitor_table; > struct ovsdb_idl_index *sbrec_port_binding_by_name; > const struct sbrec_chassis *chassis; > const char *dynamic_routing_port_mapping; > -- > 2.53.0 > > > Thank you Dmitrii, applied to main with the nit taken care of. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
