On Fri, Sep 25, 2026 at 1:29 PM Ales Musil <[email protected]> wrote:
> Large MAC binding and FDB timestamp transactions can block both the > controller main thread and the Southbound database server. Spread these > updates across the statistics request interval to reduce individual > transaction costs while preserving the existing eligibility checks. > > Pending UUID sets also deduplicate repeated statistics replies and allow > rows deleted before a drain to be skipped safely. > > Reported-at: https://issues.redhat.com/browse/FDP-4385 Small mistake, this should have been https://redhat.atlassian.net/browse/FDP-4388 instead. > > Assisted-by: GPT-5.6 Sol, OpenCode > Signed-off-by: Ales Musil <[email protected]> > --- > controller/mac-cache.c | 93 ++++++++++++++++++++++++++++++++++++------ > controller/mac-cache.h | 15 +++++-- > controller/statctrl.c | 70 +++++++++++++++++++++++++++---- > 3 files changed, 155 insertions(+), 23 deletions(-) > > diff --git a/controller/mac-cache.c b/controller/mac-cache.c > index fad44401a..5869604a9 100644 > --- a/controller/mac-cache.c > +++ b/controller/mac-cache.c > @@ -25,6 +25,7 @@ > #include "openvswitch/hmap.h" > #include "openvswitch/vlog.h" > #include "ovn/logical-fields.h" > +#include "uuidset.h" > #include "ovn-sb-idl.h" > #include "pinctrl.h" > > @@ -403,7 +404,8 @@ mac_binding_update_log(const char *action, > > void > mac_binding_stats_run(struct vector *stats_vec, uint64_t *req_delay, > - void *data, long long timewall_now) > + void *data, struct uuidset *pending, > + long long timewall_now) > { > struct mac_cache_data *cache_data = data; > > @@ -432,10 +434,10 @@ mac_binding_stats_run(struct vector *stats_vec, > uint64_t *req_delay, > * used on this chassis. */ > if (stats->idle_age_ms < threshold->value) { > if (since_updated_ms >= threshold->cooldown_period) { > - mac_binding_update_log("Updating active", &mb->data, true, > - threshold, stats->idle_age_ms, > - since_updated_ms); > - sbrec_mac_binding_set_timestamp(mb->sbrec, timewall_now); > + mac_binding_update_log("Queuing timestamp update for > active", > + &mb->data, true, threshold, > + stats->idle_age_ms, > since_updated_ms); > + uuidset_insert(pending, &mb->sbrec->header_.uuid); > } else { > /* Postponing the update to avoid sending database > transactions > * too frequently. */ > @@ -456,6 +458,38 @@ mac_binding_stats_run(struct vector *stats_vec, > uint64_t *req_delay, > } > } > > +void > +mac_binding_stats_drain(struct ovsdb_idl_txn *txn, struct uuidset > *pending, > + size_t batch_size, long long timewall_now) > +{ > + struct ovsdb_idl *idl = ovsdb_idl_txn_get_idl(txn); > + size_t drained = 0; > + > + struct uuidset_node *node; > + UUIDSET_FOR_EACH_SAFE (node, pending) { > + if (drained == batch_size) { > + break; > + } > + > + const struct sbrec_mac_binding *mb = > + sbrec_mac_binding_get_for_uuid(idl, &node->uuid); > + if (mb) { > + sbrec_mac_binding_set_timestamp(mb, timewall_now); > + > + if (VLOG_IS_DBG_ENABLED()) { > + struct mac_binding_data data; > + if (mac_binding_data_from_sbrec(&data, mb)) { > + mac_binding_update_log("Updating active", &data, > false, > + NULL, 0, > + mb->timestamp - timewall_now); > + } > + } > + } > + uuidset_delete(pending, node); > + drained++; > + } > +} > + > /* FDB stat processing. */ > void > fdb_stats_process_flow_stats(struct vector *stats_vec, > @@ -508,7 +542,7 @@ fdb_update_log(const char *action, > > void > fdb_stats_run(struct vector *stats_vec, uint64_t *req_delay, void *data, > - long long timewall_now) > + struct uuidset *pending, long long timewall_now) > { > struct mac_cache_data *cache_data = data; > > @@ -536,10 +570,10 @@ fdb_stats_run(struct vector *stats_vec, uint64_t > *req_delay, void *data, > * used on this chassis. */ > if (stats->idle_age_ms < threshold->value) { > if (since_updated_ms >= threshold->cooldown_period) { > - fdb_update_log("Updating active", &fdb->data, true, > - threshold, stats->idle_age_ms, > - since_updated_ms); > - sbrec_fdb_set_timestamp(fdb->sbrec_fdb, timewall_now); > + fdb_update_log("Queuing timestamp update for active", > + &fdb->data, true, threshold, > + stats->idle_age_ms, since_updated_ms); > + uuidset_insert(pending, &fdb->sbrec_fdb->header_.uuid); > } else { > /* Postponing the update to avoid sending database > transactions > * too frequently. */ > @@ -559,6 +593,36 @@ fdb_stats_run(struct vector *stats_vec, uint64_t > *req_delay, void *data, > } > } > > +void > +fdb_stats_drain(struct ovsdb_idl_txn *txn, struct uuidset *pending, > + size_t batch_size, long long timewall_now) > +{ > + struct ovsdb_idl *idl = ovsdb_idl_txn_get_idl(txn); > + size_t drained = 0; > + > + struct uuidset_node *node; > + UUIDSET_FOR_EACH_SAFE (node, pending) { > + if (drained == batch_size) { > + break; > + } > + > + const struct sbrec_fdb *fdb = sbrec_fdb_get_for_uuid(idl, > &node->uuid); > + if (fdb) { > + sbrec_fdb_set_timestamp(fdb, timewall_now); > + > + if (VLOG_IS_DBG_ENABLED()) { > + struct fdb_data data; > + if (fdb_data_from_sbrec(&data, fdb)) { > + fdb_update_log("Updating active", &data, false, > + NULL, 0, fdb->timestamp - > timewall_now); > + } > + } > + } > + uuidset_delete(pending, node); > + drained++; > + } > +} > + > /* Packet buffering. */ > void > bp_packet_data_destroy(struct bp_packet_data *pd) { > @@ -901,7 +965,8 @@ mac_binding_probe_stats_process_flow_stats( > > void > mac_binding_probe_stats_run(struct vector *stats_vec, uint64_t *req_delay, > - void *data, long long timewall_now) > + void *data, struct uuidset *pending > OVS_UNUSED, > + long long timewall_now) > { > struct mac_binding_probe_data *probe_data = data; > struct mac_cache_data *cache_data = probe_data->cache_data; > @@ -936,9 +1001,11 @@ mac_binding_probe_stats_run(struct vector > *stats_vec, uint64_t *req_delay, > continue; > } > > - if (since_updated_ms < threshold->cooldown_period) { > + if (since_updated_ms < threshold->cooldown_period || > + uuidset_contains(probe_data->mac_binding_pending, > + &sbrec->header_.uuid)) { > mac_binding_update_log( > - "Not sending ARP/ND request for recently updated", > + "Not sending ARP/ND request for recently/to be > updated", > &mb->data, true, threshold, stats->idle_age_ms, > since_updated_ms); > mb->arp_attempts = 0; > diff --git a/controller/mac-cache.h b/controller/mac-cache.h > index bf9afaf3a..f27998e40 100644 > --- a/controller/mac-cache.h > +++ b/controller/mac-cache.h > @@ -29,6 +29,7 @@ > > struct ovsdb_idl_index; > struct vector; > +struct uuidset; > > struct mac_cache_data { > /* 'struct mac_cache_threshold' by datapath's tunnel_key. */ > @@ -65,6 +66,7 @@ struct mac_binding_data { > > struct mac_binding_probe_data { > struct mac_cache_data *cache_data; > + struct uuidset *mac_binding_pending; > struct rconn *swconn; > struct ovsdb_idl_index *sbrec_port_binding_by_name; > const struct sbrec_chassis *chassis; > @@ -201,14 +203,20 @@ mac_binding_stats_process_flow_stats(struct vector > *stats_vec, > struct ofputil_flow_stats > *ofp_stats); > > void mac_binding_stats_run(struct vector *stats_vec, uint64_t *req_delay, > - void *data, long long timewall_now); > + void *data, struct uuidset *pending, > + long long timewall_now); > +void mac_binding_stats_drain(struct ovsdb_idl_txn *txn, > + struct uuidset *pending, size_t batch_size, > + long long timewall_now); > > /* FDB stat processing. */ > void fdb_stats_process_flow_stats(struct vector *stats_vec, > struct ofputil_flow_stats *ofp_stats); > > void fdb_stats_run(struct vector *stats_vec, uint64_t *req_delay, void > *data, > - long long timewall_now); > + struct uuidset *pending, long long timewall_now); > +void fdb_stats_drain(struct ovsdb_idl_txn *txn, struct uuidset *pending, > + size_t batch_size, long long timewall_now); > > /* Packet buffering. */ > void bp_packet_data_destroy(struct bp_packet_data *pd); > @@ -237,6 +245,7 @@ void mac_binding_probe_stats_process_flow_stats( > struct ofputil_flow_stats *ofp_stats); > > void mac_binding_probe_stats_run(struct vector *stats_vec, uint64_t > *req_delay, > - void *data, long long timewall_now); > + void *data, struct uuidset *pending, > + long long timewall_now); > > #endif /* controller/mac-cache.h */ > diff --git a/controller/statctrl.c b/controller/statctrl.c > index 00c0a4450..77453319c 100644 > --- a/controller/statctrl.c > +++ b/controller/statctrl.c > @@ -35,10 +35,13 @@ > #include "socket-util.h" > #include "statctrl.h" > #include "stopwatch.h" > +#include "uuidset.h" > > VLOG_DEFINE_THIS_MODULE(statctrl); > > #define STATS_VEC_CAPACITY_THRESHOLD 1024 > +#define STATS_DRAIN_INTERVAL 3000 > +#define STATS_MIN_BATCH_SIZE 500 > > enum stat_type { > STATS_MAC_BINDING = 0, > @@ -58,6 +61,10 @@ struct stats_node { > uint64_t request_delay; > /* Vector of processed statistics. */ > struct vector stats; > + /* UUIDs whose timestamps should be updated. */ > + struct uuidset pending; > + /* Timestamp when the next batch should be drained. */ > + int64_t next_drain_timestamp; > /* Function to process the response and store it in the list. > * This function runs in statctrl thread locked behind mutex. */ > void (*process_flow_stats)(struct vector *stats, > @@ -65,12 +72,15 @@ struct stats_node { > /* Function to process the parsed stats. > * This function runs in main thread locked behind mutex. */ > void (*run)(struct vector *stats, uint64_t *req_delay, void *data, > - long long timewall_now); > + struct uuidset *pending, long long timewall_now); > + /* Function to drain a batch of pending timestamp updates. */ > + void (*drain)(struct ovsdb_idl_txn *txn, struct uuidset *pending, > + size_t batch_size, long long timewall_now); > /* Name of the stats node. */ > const char *name; > }; > > -#define STATS_NODE(NAME, REQUEST, STAT_TYPE, PROCESS, RUN) > \ > +#define STATS_NODE(NAME, REQUEST, STAT_TYPE, PROCESS, RUN, DRAIN) > \ > do { > \ > statctrl_ctx.nodes[STATS_##NAME] = (struct stats_node) { > \ > .request = REQUEST, > \ > @@ -78,10 +88,13 @@ struct stats_node { > .next_request_timestamp = INT64_MAX, > \ > .request_delay = 0, > \ > .stats = VECTOR_EMPTY_INITIALIZER(STAT_TYPE), > \ > + .next_drain_timestamp = INT64_MAX, > \ > .process_flow_stats = PROCESS, > \ > .run = RUN, > \ > - .name = OVS_STRINGIZE(stats_##NAME), \ > + .drain = DRAIN, > \ > + .name = OVS_STRINGIZE(stats_##NAME), > \ > }; > \ > + uuidset_init(&statctrl_ctx.nodes[STATS_##NAME].pending); > \ > stopwatch_create(OVS_STRINGIZE(stats_##NAME), SW_MS); > \ > } while (0) > > @@ -141,7 +154,8 @@ statctrl_init(void) > .table_id = OFTABLE_MAC_CACHE_USE, > }; > STATS_NODE(MAC_BINDING, mac_binding_request, struct mac_cache_stats, > - mac_binding_stats_process_flow_stats, > mac_binding_stats_run); > + mac_binding_stats_process_flow_stats, > mac_binding_stats_run, > + mac_binding_stats_drain); > > struct ofputil_flow_stats_request fdb_request = { > .cookie = htonll(0), > @@ -151,7 +165,8 @@ statctrl_init(void) > .table_id = OFTABLE_LOOKUP_FDB, > }; > STATS_NODE(FDB, fdb_request, struct mac_cache_stats, > - fdb_stats_process_flow_stats, fdb_stats_run); > + fdb_stats_process_flow_stats, fdb_stats_run, > + fdb_stats_drain); > > struct ofputil_flow_stats_request mac_binding_probe_request = { > .cookie = htonll(0), > @@ -163,7 +178,7 @@ statctrl_init(void) > STATS_NODE(MAC_BINDING_PROBE, mac_binding_probe_request, > struct mac_cache_stats, > mac_binding_probe_stats_process_flow_stats, > - mac_binding_probe_stats_run); > + mac_binding_probe_stats_run, NULL); > > statctrl_ctx.thread = ovs_thread_create("ovn_statctrl", > statctrl_thread_handler, > @@ -182,6 +197,7 @@ statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > struct mac_binding_probe_data mac_binding_probe_data = { > .cache_data = mac_cache_data, > + .mac_binding_pending = > &statctrl_ctx.nodes[STATS_MAC_BINDING].pending, > .sbrec_port_binding_by_name = sbrec_port_binding_by_name, > .swconn = statctrl_ctx.swconn, > .chassis = chassis, > @@ -202,10 +218,12 @@ statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > for (size_t i = 0; i < STATS_MAX; i++) { > struct stats_node *node = &statctrl_ctx.nodes[i]; > uint64_t prev_delay = node->request_delay; > + size_t prev_pending_count = uuidset_count(&node->pending); > > stopwatch_start(node->name, time_msec()); > node->run(&node->stats, &node->request_delay, node_data[i], > - timewall_now); > + &node->pending, timewall_now); > + now = time_msec(); > vector_clear(&node->stats); > if (vector_capacity(&node->stats) >= > STATS_VEC_CAPACITY_THRESHOLD) { > VLOG_DBG("The statistics vector for node '%s' capacity " > @@ -217,6 +235,40 @@ statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > schedule_updated |= > statctrl_update_next_request_timestamp(node, now, > prev_delay); > + > + if (!node->drain) { > + continue; > + } > + > + size_t pending_count = uuidset_count(&node->pending); > + if (pending_count != prev_pending_count && pending_count && > + node->next_drain_timestamp == INT64_MAX) { > + node->next_drain_timestamp = now; > + } > + > + if (now < node->next_drain_timestamp) { > + continue; > + } > + > + /* Distribute pending updates over the remaining fixed drain > intervals > + * before the next statistics request, subject to the minimum > batch > + * size. */ > + uint64_t slots = 1; > + if (node->next_request_timestamp != INT64_MAX && > + node->next_request_timestamp > now) { > + slots = MAX((node->next_request_timestamp - now) > + / STATS_DRAIN_INTERVAL, 1); > + } > + size_t batch_size = MAX(STATS_MIN_BATCH_SIZE, > + (pending_count / slots) + 1); > + > + stopwatch_start(node->name, time_msec()); > + node->drain(ovnsb_idl_txn, &node->pending, batch_size, > + timewall_now); > + node->next_drain_timestamp = uuidset_is_empty(&node->pending) > + ? INT64_MAX > + : now + STATS_DRAIN_INTERVAL; > + stopwatch_stop(node->name, time_msec()); > } > ovs_mutex_unlock(&mutex); > > @@ -249,6 +301,9 @@ statctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn) > if (!vector_is_empty(&node->stats)) { > poll_immediate_wake(); > } > + if (node->next_drain_timestamp != INT64_MAX) { > + poll_timer_wait_until(node->next_drain_timestamp); > + } > } > seq_wait(statctrl_ctx.main_seq, statctrl_ctx.new_main_seq); > ovs_mutex_unlock(&mutex); > @@ -267,6 +322,7 @@ statctrl_destroy(void) > for (size_t i = 0; i < STATS_MAX; i++) { > struct stats_node *node = &statctrl_ctx.nodes[i]; > vector_destroy(&node->stats); > + uuidset_destroy(&node->pending); > } > } > > -- > 2.55.0 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
