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"); } - 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
