On Wed, Apr 30, 2025 at 7:54 AM Ales Musil <amu...@redhat.com> wrote:
> The statistics were stored as allocated object for each individual
> statistic and linked together via list. Store them in vector instead
> which allows us to avoid a lot of small allocations and free right
> after using the statistic.
>
> The same applies to the buffered packets, use vector instead of moving
> around single allocated objects. This should reduce the fragmentation.
>
> Signed-off-by: Ales Musil <amu...@redhat.com>
> ---
> v4: Rebase on top of current main.
> Adjust the thhreshold define names.
> Add missing vector_destroy.
> v3: Rebase on top of current main.
> v2: Rebase on top of current main.
> ---
> controller/mac-cache.c | 191 +++++++++++++++++------------------------
> controller/mac-cache.h | 39 +++++----
> controller/pinctrl.c | 15 +++-
> controller/statctrl.c | 54 ++++++------
> 4 files changed, 141 insertions(+), 158 deletions(-)
>
> diff --git a/controller/mac-cache.c b/controller/mac-cache.c
> index 18236c440..3237677dc 100644
> --- a/controller/mac-cache.c
> +++ b/controller/mac-cache.c
> @@ -18,6 +18,7 @@
>
> #include "lflow.h"
> #include "lib/mac-binding-index.h"
> +#include "lib/vec.h"
> #include "local_data.h"
> #include "lport.h"
> #include "mac-cache.h"
> @@ -339,43 +340,30 @@ fdbs_clear(struct hmap *map)
> }
> }
>
> -struct mac_cache_stats {
> - struct ovs_list list_node;
> -
> - int64_t idle_age_ms;
> -
> - union {
> - /* Common data to identify MAC binding. */
> - struct mac_binding_data mb;
> - /* Common data to identify FDB. */
> - struct fdb_data fdb;
> - } data;
> -};
> -
> /* MAC binding stat processing. */
> void
> -mac_binding_stats_process_flow_stats(struct ovs_list *stats_list,
> +mac_binding_stats_process_flow_stats(struct vector *stats_vec,
> struct ofputil_flow_stats *ofp_stats)
> {
> - struct mac_cache_stats *stats = xmalloc(sizeof *stats);
> -
> - stats->idle_age_ms = ofp_stats->idle_age * 1000;
> - stats->data.mb = (struct mac_binding_data) {
> - .cookie = ntohll(ofp_stats->cookie),
> - /* The port_key must be zero to match
> mac_binding_data_from_sbrec. */
> - .port_key = 0,
> - .dp_key = ntohll(ofp_stats->match.flow.metadata),
> - .mac = ofp_stats->match.flow.dl_src
> + struct mac_cache_stats stats = (struct mac_cache_stats) {
> + .idle_age_ms = ofp_stats->idle_age * 1000,
> + .data.mb = (struct mac_binding_data) {
> + .cookie = ntohll(ofp_stats->cookie),
> + /* The port_key must be zero to match
> + * mac_binding_data_from_sbrec. */
> + .port_key = 0,
> + .dp_key = ntohll(ofp_stats->match.flow.metadata),
> + .mac = ofp_stats->match.flow.dl_src
> + },
> };
>
> if (ofp_stats->match.flow.dl_type == htons(ETH_TYPE_IP) ||
> ofp_stats->match.flow.dl_type == htons(ETH_TYPE_ARP)) {
> - stats->data.mb.ip =
> in6_addr_mapped_ipv4(ofp_stats->match.flow.nw_src);
> + stats.data.mb.ip =
> in6_addr_mapped_ipv4(ofp_stats->match.flow.nw_src);
> } else {
> - stats->data.mb.ip = ofp_stats->match.flow.ipv6_src;
> + stats.data.mb.ip = ofp_stats->match.flow.ipv6_src;
> }
> -
> - ovs_list_push_back(stats_list, &stats->list_node);
> + vector_push(stats_vec, &stats);
> }
>
> static void
> @@ -409,19 +397,18 @@ void
> mac_binding_stats_run(
> struct rconn *swconn OVS_UNUSED,
> struct ovsdb_idl_index *sbrec_port_binding_by_name OVS_UNUSED,
> - struct ovs_list *stats_list, uint64_t *req_delay, void *data)
> + struct vector *stats_vec, uint64_t *req_delay, void *data)
> {
> struct mac_cache_data *cache_data = data;
> long long timewall_now = time_wall_msec();
>
> struct mac_cache_stats *stats;
> - LIST_FOR_EACH_POP (stats, list_node, stats_list) {
> + VECTOR_FOR_EACH_PTR (stats_vec, stats) {
> struct mac_binding *mb =
> mac_binding_find(&cache_data->mac_bindings,
> &stats->data.mb);
> if (!mb) {
> mac_binding_update_log("Not found in the cache:",
> &stats->data.mb,
> false, NULL, 0, 0);
> - free(stats);
> continue;
> }
>
> @@ -449,7 +436,6 @@ mac_binding_stats_run(
> threshold, stats->idle_age_ms,
> since_updated_ms);
> }
> - free(stats);
> }
>
> mac_cache_update_req_delay(&cache_data->thresholds, req_delay);
> @@ -460,19 +446,18 @@ mac_binding_stats_run(
>
> /* FDB stat processing. */
> void
> -fdb_stats_process_flow_stats(struct ovs_list *stats_list,
> +fdb_stats_process_flow_stats(struct vector *stats_vec,
> struct ofputil_flow_stats *ofp_stats)
> {
> - struct mac_cache_stats *stats = xmalloc(sizeof *stats);
> -
> - stats->idle_age_ms = ofp_stats->idle_age * 1000;
> - stats->data.fdb = (struct fdb_data) {
> + struct mac_cache_stats stats = (struct mac_cache_stats) {
> + .idle_age_ms = ofp_stats->idle_age * 1000,
> + .data.fdb = (struct fdb_data) {
> .port_key = ofp_stats->match.flow.regs[MFF_LOG_INPORT -
> MFF_REG0],
> .dp_key = ntohll(ofp_stats->match.flow.metadata),
> .mac = ofp_stats->match.flow.dl_src
> + },
> };
> -
> - ovs_list_push_back(stats_list, &stats->list_node);
> + vector_push(stats_vec, &stats);
> }
>
> static void
> @@ -506,20 +491,19 @@ fdb_update_log(const char *action,
> void
> fdb_stats_run(struct rconn *swconn OVS_UNUSED,
> struct ovsdb_idl_index *sbrec_port_binding_by_name
> OVS_UNUSED,
> - struct ovs_list *stats_list,
> + struct vector *stats_vec,
> uint64_t *req_delay, void *data)
> {
> struct mac_cache_data *cache_data = data;
> long long timewall_now = time_wall_msec();
>
> struct mac_cache_stats *stats;
> - LIST_FOR_EACH_POP (stats, list_node, stats_list) {
> + VECTOR_FOR_EACH_PTR (stats_vec, stats) {
> struct fdb *fdb = fdb_find(&cache_data->fdbs, &stats->data.fdb);
>
> if (!fdb) {
> fdb_update_log("Not found in the cache:", &stats->data.fdb,
> false, NULL, 0, 0);
> - free(stats);
> continue;
> }
>
> @@ -546,8 +530,6 @@ fdb_stats_run(struct rconn *swconn OVS_UNUSED,
> fdb_update_log("Not updating non-active", &fdb->data, true,
> threshold, stats->idle_age_ms,
> since_updated_ms);
> }
> -
> - free(stats);
> }
>
> mac_cache_update_req_delay(&cache_data->thresholds, req_delay);
> @@ -557,34 +539,10 @@ fdb_stats_run(struct rconn *swconn OVS_UNUSED,
> }
>
> /* Packet buffering. */
> -struct bp_packet_data *
> -bp_packet_data_create(const struct ofputil_packet_in *pin,
> - const struct ofpbuf *continuation) {
> - struct bp_packet_data *pd = xmalloc(sizeof *pd);
> -
> - pd->pin = (struct ofputil_packet_in) {
> - .packet = xmemdup(pin->packet, pin->packet_len),
> - .packet_len = pin->packet_len,
> - .flow_metadata = pin->flow_metadata,
> - .reason = pin->reason,
> - .table_id = pin->table_id,
> - .cookie = pin->cookie,
> - /* Userdata are empty on purpose,
> - * it is not needed for the continuation. */
> - .userdata = NULL,
> - .userdata_len = 0,
> - };
> - pd->continuation = ofpbuf_clone(continuation);
> -
> - return pd;
> -}
> -
> -
> void
> bp_packet_data_destroy(struct bp_packet_data *pd) {
> free(pd->pin.packet);
> ofpbuf_delete(pd->continuation);
> - free(pd);
> }
>
> struct buffered_packets *
> @@ -604,7 +562,8 @@ buffered_packets_add(struct buffered_packets_ctx *ctx,
> /* Schedule the freshly added buffered packet to do lookup
> * immediately. */
> bp->lookup_at_ms = 0;
> - ovs_list_init(&bp->queue);
> + bp->queue = VECTOR_CAPACITY_INITIALIZER(struct bp_packet_data,
> + BUFFER_QUEUE_DEPTH);
> }
>
> bp->expire_at_ms = time_msec() + BUFFERED_PACKETS_TIMEOUT_MS;
> @@ -614,14 +573,32 @@ buffered_packets_add(struct buffered_packets_ctx
> *ctx,
>
> void
> buffered_packets_packet_data_enqueue(struct buffered_packets *bp,
> - struct bp_packet_data *pd) {
> - if (ovs_list_size(&bp->queue) == BUFFER_QUEUE_DEPTH) {
> - struct bp_packet_data *p =
> CONTAINER_OF(ovs_list_pop_front(&bp->queue),
> - struct bp_packet_data,
> node);
> -
> - bp_packet_data_destroy(p);
> + const struct ofputil_packet_in *pin,
> + const struct ofpbuf *continuation)
> +{
> + if (vector_len(&bp->queue) == BUFFER_QUEUE_DEPTH) {
> + struct bp_packet_data pd;
> + vector_remove(&bp->queue, 0, &pd);
> + bp_packet_data_destroy(&pd);
> }
> - ovs_list_push_back(&bp->queue, &pd->node);
> +
> + struct bp_packet_data pd = (struct bp_packet_data) {
> + .pin = (struct ofputil_packet_in) {
> + .packet = xmemdup(pin->packet, pin->packet_len),
> + .packet_len = pin->packet_len,
> + .flow_metadata = pin->flow_metadata,
> + .reason = pin->reason,
> + .table_id = pin->table_id,
> + .cookie = pin->cookie,
> + /* Userdata are empty on purpose,
> + * it is not needed for the continuation. */
> + .userdata = NULL,
> + .userdata_len = 0,
> + },
> + .continuation = ofpbuf_clone(continuation),
> + };
> +
> + vector_push(&bp->queue, &pd);
> }
>
> void
> @@ -661,16 +638,18 @@ buffered_packets_ctx_run(struct buffered_packets_ctx
> *ctx,
> }
>
> struct bp_packet_data *pd;
> - LIST_FOR_EACH_POP (pd, node, &bp->queue) {
> + VECTOR_FOR_EACH_PTR (&bp->queue, pd) {
> struct dp_packet packet;
> dp_packet_use_const(&packet, pd->pin.packet,
> pd->pin.packet_len);
>
> struct eth_header *eth = dp_packet_data(&packet);
> eth->eth_dst = mac;
> -
> - ovs_list_push_back(&ctx->ready_packets_data, &pd->node);
> }
>
> + vector_push_array(&ctx->ready_packets_data,
> + vector_get_array(&bp->queue),
> + vector_len(&bp->queue));
> + vector_clear(&bp->queue);
> buffered_packets_remove(ctx, bp);
> }
>
> @@ -679,7 +658,7 @@ buffered_packets_ctx_run(struct buffered_packets_ctx
> *ctx,
>
> bool
> buffered_packets_ctx_is_ready_to_send(struct buffered_packets_ctx *ctx) {
> - return !ovs_list_is_empty(&ctx->ready_packets_data);
> + return !vector_is_empty(&ctx->ready_packets_data);
> }
>
> bool
> @@ -689,16 +668,17 @@ buffered_packets_ctx_has_packets(struct
> buffered_packets_ctx *ctx) {
>
> void
> buffered_packets_ctx_init(struct buffered_packets_ctx *ctx) {
> + ctx->ready_packets_data = VECTOR_EMPTY_INITIALIZER(struct
> bp_packet_data);
> hmap_init(&ctx->buffered_packets);
> - ovs_list_init(&ctx->ready_packets_data);
> }
>
> void
> buffered_packets_ctx_destroy(struct buffered_packets_ctx *ctx) {
> struct bp_packet_data *pd;
> - LIST_FOR_EACH_POP (pd, node, &ctx->ready_packets_data) {
> + VECTOR_FOR_EACH_PTR (&ctx->ready_packets_data, pd) {
> bp_packet_data_destroy(pd);
> }
> + vector_destroy(&ctx->ready_packets_data);
>
> struct buffered_packets *bp;
> HMAP_FOR_EACH_SAFE (bp, hmap_node, &ctx->buffered_packets) {
> @@ -707,16 +687,6 @@ buffered_packets_ctx_destroy(struct
> buffered_packets_ctx *ctx) {
> hmap_destroy(&ctx->buffered_packets);
> }
>
> -
> -void
> -mac_cache_stats_destroy(struct ovs_list *stats_list)
> -{
> - struct mac_cache_stats *stats;
> - LIST_FOR_EACH_POP (stats, list_node, stats_list) {
> - free(stats);
> - }
> -}
> -
> static uint32_t
> mac_binding_data_hash(const struct mac_binding_data *mb_data)
> {
> @@ -815,11 +785,12 @@ static void
> buffered_packets_remove(struct buffered_packets_ctx *ctx,
> struct buffered_packets *bp) {
> struct bp_packet_data *pd;
> - LIST_FOR_EACH_POP (pd, node, &bp->queue) {
> + VECTOR_FOR_EACH_PTR (&bp->queue, pd) {
> bp_packet_data_destroy(pd);
> }
>
> hmap_remove(&ctx->buffered_packets, &bp->hmap_node);
> + vector_destroy(&bp->queue);
> free(bp);
> }
>
> @@ -861,48 +832,49 @@ buffered_packets_db_lookup(struct buffered_packets
> *bp, struct ds *ip,
>
> void
> mac_binding_probe_stats_process_flow_stats(
> - struct ovs_list *stats_list,
> + struct vector *stats_vec,
> struct ofputil_flow_stats *ofp_stats)
> {
> - struct mac_cache_stats *stats = xmalloc(sizeof *stats);
> -
> - stats->idle_age_ms = ofp_stats->idle_age * 1000;
> - stats->data.mb = (struct mac_binding_data) {
> - .cookie = ntohll(ofp_stats->cookie),
> - /* The port_key must be zero to match
> mac_binding_data_from_sbrec. */
> - .port_key = 0,
> - .dp_key = ntohll(ofp_stats->match.flow.metadata),
> + struct mac_cache_stats stats = (struct mac_cache_stats) {
> + .idle_age_ms = ofp_stats->idle_age * 1000,
> + .data.mb = (struct mac_binding_data) {
> + .cookie = ntohll(ofp_stats->cookie),
> + /* The port_key must be zero to match
> + * mac_binding_data_from_sbrec. */
> + .port_key = 0,
> + .dp_key = ntohll(ofp_stats->match.flow.metadata),
> + .mac = ofp_stats->match.flow.dl_src
> + },
> };
>
> if (ofp_stats->match.flow.regs[0]) {
> - stats->data.mb.ip =
> + stats.data.mb.ip =
> in6_addr_mapped_ipv4(htonl(ofp_stats->match.flow.regs[0]));
> } else {
> ovs_be128 ip6 = hton128(flow_get_xxreg(&ofp_stats->match.flow,
> 1));
> - memcpy(&stats->data.mb.ip, &ip6, sizeof stats->data.mb.ip);
> + memcpy(&stats.data.mb.ip, &ip6, sizeof stats.data.mb.ip);
> }
>
> - ovs_list_push_back(stats_list, &stats->list_node);
> + vector_push(stats_vec, &stats);
> }
>
> void
> mac_binding_probe_stats_run(
> struct rconn *swconn,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> - struct ovs_list *stats_list,
> + struct vector *stats_vec,
> uint64_t *req_delay, void *data)
> {
> long long timewall_now = time_wall_msec();
> struct mac_cache_data *cache_data = data;
>
> struct mac_cache_stats *stats;
> - LIST_FOR_EACH_POP (stats, list_node, stats_list) {
> + VECTOR_FOR_EACH_PTR (stats_vec, stats) {
> struct mac_binding *mb =
> mac_binding_find(&cache_data->mac_bindings,
> &stats->data.mb);
> if (!mb) {
> mac_binding_update_log("Probe: not found in the cache:",
> &stats->data.mb, false, NULL, 0, 0);
> - free(stats);
> continue;
> }
>
> @@ -915,7 +887,6 @@ mac_binding_probe_stats_run(
> mac_binding_update_log("Not sending ARP/ND request for
> non-active",
> &mb->data, true, threshold,
> stats->idle_age_ms, since_updated_ms);
> - free(stats);
> continue;
> }
>
> @@ -924,7 +895,6 @@ mac_binding_probe_stats_run(
> "Not sending ARP/ND request for recently updated",
> &mb->data, true, threshold, stats->idle_age_ms,
> since_updated_ms);
> - free(stats);
> continue;
> }
>
> @@ -932,13 +902,11 @@ mac_binding_probe_stats_run(
> lport_lookup_by_name(sbrec_port_binding_by_name,
> sbrec->logical_port);
> if (!pb) {
> - free(stats);
> continue;
> }
>
> struct lport_addresses laddr;
> if (!extract_lsp_addresses(pb->mac[0], &laddr)) {
> - free(stats);
> continue;
> }
>
> @@ -960,7 +928,6 @@ mac_binding_probe_stats_run(
> OFTABLE_LOCAL_OUTPUT);
> }
>
> - free(stats);
> destroy_lport_addresses(&laddr);
> }
>
> diff --git a/controller/mac-cache.h b/controller/mac-cache.h
> index f5dbaf813..bdaec064b 100644
> --- a/controller/mac-cache.h
> +++ b/controller/mac-cache.h
> @@ -28,6 +28,7 @@
> #include "ovn-sb-idl.h"
>
> struct ovsdb_idl_index;
> +struct vector;
>
> struct mac_cache_data {
> /* 'struct mac_cache_threshold' by datapath's tunnel_key. */
> @@ -89,9 +90,18 @@ struct fdb {
> long long timestamp;
> };
>
> -struct bp_packet_data {
> - struct ovs_list node;
> +struct mac_cache_stats {
> + int64_t idle_age_ms;
> +
> + union {
> + /* Common data to identify MAC binding. */
> + struct mac_binding_data mb;
> + /* Common data to identify FDB. */
> + struct fdb_data fdb;
> + } data;
> +};
>
> +struct bp_packet_data {
> struct ofpbuf *continuation;
> struct ofputil_packet_in pin;
> };
> @@ -102,7 +112,7 @@ struct buffered_packets {
> struct mac_binding_data mb_data;
>
> /* Queue of packet_data associated with this struct. */
> - struct ovs_list queue;
> + struct vector queue;
>
> /* Timestamp in ms when the buffered packet should expire. */
> long long int expire_at_ms;
> @@ -115,7 +125,7 @@ struct buffered_packets_ctx {
> /* Map of all buffered packets waiting for the MAC address. */
> struct hmap buffered_packets;
> /* List of packet data that are ready to be sent. */
> - struct ovs_list ready_packets_data;
> + struct vector ready_packets_data;
> };
>
> /* Thresholds. */
> @@ -177,30 +187,24 @@ void fdbs_clear(struct hmap *map);
>
> /* MAC binding stat processing. */
> void
> -mac_binding_stats_process_flow_stats(struct ovs_list *stats_list,
> +mac_binding_stats_process_flow_stats(struct vector *stats_vec,
> struct ofputil_flow_stats
> *ofp_stats);
>
> void mac_binding_stats_run(
> struct rconn *swconn OVS_UNUSED,
> struct ovsdb_idl_index *sbrec_port_binding_by_name OVS_UNUSED,
> - struct ovs_list *stats_list, uint64_t *req_delay, void *data);
> + struct vector *stats_vec, uint64_t *req_delay, void *data);
>
> /* FDB stat processing. */
> -void fdb_stats_process_flow_stats(struct ovs_list *stats_list,
> +void fdb_stats_process_flow_stats(struct vector *stats_vec,
> struct ofputil_flow_stats *ofp_stats);
>
> void fdb_stats_run(
> struct rconn *swconn OVS_UNUSED,
> struct ovsdb_idl_index *sbrec_port_binding_by_name OVS_UNUSED,
> - struct ovs_list *stats_list, uint64_t *req_delay, void *data);
> -
> -void mac_cache_stats_destroy(struct ovs_list *stats_list);
> + struct vector *stats_vec, uint64_t *req_delay, void *data);
>
> /* Packet buffering. */
> -struct bp_packet_data *
> -bp_packet_data_create(const struct ofputil_packet_in *pin,
> - const struct ofpbuf *continuation);
> -
> void bp_packet_data_destroy(struct bp_packet_data *pd);
>
> struct buffered_packets *
> @@ -208,7 +212,8 @@ buffered_packets_add(struct buffered_packets_ctx *ctx,
> struct mac_binding_data mb_data);
>
> void buffered_packets_packet_data_enqueue(struct buffered_packets *bp,
> - struct bp_packet_data *pd);
> + const struct ofputil_packet_in
> *pin,
> + const struct ofpbuf
> *continuation);
>
> void buffered_packets_ctx_run(struct buffered_packets_ctx *ctx,
> const struct hmap *recent_mbs,
> @@ -226,12 +231,12 @@ bool buffered_packets_ctx_is_ready_to_send(struct
> buffered_packets_ctx *ctx);
> bool buffered_packets_ctx_has_packets(struct buffered_packets_ctx *ctx);
>
> void mac_binding_probe_stats_process_flow_stats(
> - struct ovs_list *stats_list,
> + struct vector *stats_vec,
> struct ofputil_flow_stats *ofp_stats);
>
> void mac_binding_probe_stats_run(
> struct rconn *swconn,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> - struct ovs_list *stats_list, uint64_t *req_delay, void *data);
> + struct vector *stats_vec, uint64_t *req_delay, void *data);
>
> #endif /* controller/mac-cache.h */
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 34fb2e6bf..0647f053d 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -1570,8 +1570,7 @@ OVS_REQUIRES(pinctrl_mutex)
> return;
> }
>
> - struct bp_packet_data *pd = bp_packet_data_create(pin, continuation);
> - buffered_packets_packet_data_enqueue(bp, pd);
> + buffered_packets_packet_data_enqueue(bp, pin, continuation);
>
> /* There is a chance that the MAC binding was already created. */
> notify_pinctrl_main();
> @@ -4738,6 +4737,8 @@ pinctrl_handle_put_mac_binding(const struct flow *md,
> notify_pinctrl_main();
> }
>
> +#define READY_PACKETS_VEC_CAPACITY_THRESHOLD 1024
> +
> /* Called with in the pinctrl_handler thread context. */
> static void
> send_mac_binding_buffered_pkts(struct rconn *swconn)
> @@ -4745,15 +4746,21 @@ send_mac_binding_buffered_pkts(struct rconn
> *swconn)
> {
> enum ofp_version version = rconn_get_version(swconn);
> enum ofputil_protocol proto =
> ofputil_protocol_from_ofp_version(version);
> + struct vector *rpd = &buffered_packets_ctx.ready_packets_data;
>
> struct bp_packet_data *pd;
> - LIST_FOR_EACH_POP (pd, node,
> &buffered_packets_ctx.ready_packets_data) {
> + VECTOR_FOR_EACH_PTR (rpd, pd) {
> queue_msg(swconn, ofputil_encode_resume(&pd->pin,
> pd->continuation,
> proto));
> bp_packet_data_destroy(pd);
> }
>
> - ovs_list_init(&buffered_packets_ctx.ready_packets_data);
> + vector_clear(rpd);
> + if (vector_capacity(rpd) >= READY_PACKETS_VEC_CAPACITY_THRESHOLD) {
> + VLOG_DBG("The ready_packets_data vector capacity (%"PRIuSIZE") "
> + "is over threshold.", vector_capacity(rpd));
> + vector_shrink_to_fit(rpd);
> + }
> }
>
> /* Update or add an IP-MAC binding for 'logical_port'.
> diff --git a/controller/statctrl.c b/controller/statctrl.c
> index f0c5bd904..454314dda 100644
> --- a/controller/statctrl.c
> +++ b/controller/statctrl.c
> @@ -19,6 +19,7 @@
> #include "dirs.h"
> #include "latch.h"
> #include "lflow.h"
> +#include "lib/vec.h"
> #include "mac-cache.h"
> #include "openvswitch/ofp-errors.h"
> #include "openvswitch/ofp-flow.h"
> @@ -37,6 +38,8 @@
>
> VLOG_DEFINE_THIS_MODULE(statctrl);
>
> +#define STATS_VEC_CAPACITY_THRESHOLD 1024
> +
> enum stat_type {
> STATS_MAC_BINDING = 0,
> STATS_FDB,
> @@ -53,39 +56,33 @@ struct stats_node {
> int64_t next_request_timestamp;
> /* Request delay in ms. */
> uint64_t request_delay;
> - /* List of processed statistics. */
> - struct ovs_list stats_list;
> - /* Function to clean up the node.
> - * This function runs in main thread. */
> - void (*destroy)(struct ovs_list *stats_list);
> + /* Vector of processed statistics. */
> + struct vector stats;
> /* 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 ovs_list *stats_list,
> + void (*process_flow_stats)(struct vector *stats,
> struct ofputil_flow_stats *ofp_stats);
> /* Function to process the parsed stats.
> * This function runs in main thread locked behind mutex. */
> void (*run)(struct rconn *swconn,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> - struct ovs_list *stats_list,
> + struct vector *stats,
> uint64_t *req_delay, void *data);
> - /* Name of the stats node corresponding stopwatch. */
> - const char *stopwatch_name;
> + /* Name of the stats node. */
> + const char *name;
> };
>
> -#define STATS_NODE(NAME, REQUEST, DESTROY, PROCESS, RUN)
> \
> +#define STATS_NODE(NAME, REQUEST, STAT_TYPE, PROCESS, RUN)
> \
> do {
> \
> statctrl_ctx.nodes[STATS_##NAME] = (struct stats_node) {
> \
> .request = REQUEST,
> \
> .xid = 0,
> \
> .next_request_timestamp = INT64_MAX,
> \
> .request_delay = 0,
> \
> - .stats_list =
> \
> - OVS_LIST_INITIALIZER(
> \
> - &statctrl_ctx.nodes[STATS_##NAME].stats_list),
> \
> - .destroy = DESTROY,
> \
> + .stats = VECTOR_EMPTY_INITIALIZER(STAT_TYPE),
> \
> .process_flow_stats = PROCESS,
> \
> .run = RUN,
> \
> - .stopwatch_name = OVS_STRINGIZE(stats_##NAME),
> \
> + .name = OVS_STRINGIZE(stats_##NAME), \
> };
> \
> stopwatch_create(OVS_STRINGIZE(stats_##NAME), SW_MS);
> \
> } while (0)
> @@ -143,7 +140,7 @@ statctrl_init(void)
> .out_group = OFPG_ANY,
> .table_id = OFTABLE_MAC_CACHE_USE,
> };
> - STATS_NODE(MAC_BINDING, mac_binding_request, mac_cache_stats_destroy,
> + STATS_NODE(MAC_BINDING, mac_binding_request, struct mac_cache_stats,
> mac_binding_stats_process_flow_stats,
> mac_binding_stats_run);
>
> struct ofputil_flow_stats_request fdb_request = {
> @@ -153,7 +150,7 @@ statctrl_init(void)
> .out_group = OFPG_ANY,
> .table_id = OFTABLE_LOOKUP_FDB,
> };
> - STATS_NODE(FDB, fdb_request, mac_cache_stats_destroy,
> + STATS_NODE(FDB, fdb_request, struct mac_cache_stats,
> fdb_stats_process_flow_stats, fdb_stats_run);
>
> struct ofputil_flow_stats_request mac_binding_probe_request = {
> @@ -164,7 +161,7 @@ statctrl_init(void)
> .table_id = OFTABLE_MAC_BINDING,
> };
> STATS_NODE(MAC_BINDING_PROBE, mac_binding_probe_request,
> - mac_cache_stats_destroy,
> + struct mac_cache_stats,
> mac_binding_probe_stats_process_flow_stats,
> mac_binding_probe_stats_run);
>
> @@ -196,11 +193,18 @@ statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct stats_node *node = &statctrl_ctx.nodes[i];
> uint64_t prev_delay = node->request_delay;
>
> - stopwatch_start(node->stopwatch_name, time_msec());
> + stopwatch_start(node->name, time_msec());
> node->run(statctrl_ctx.swconn,
> - sbrec_port_binding_by_name, &node->stats_list,
> + sbrec_port_binding_by_name, &node->stats,
> &node->request_delay, node_data[i]);
> - stopwatch_stop(node->stopwatch_name, time_msec());
> + vector_clear(&node->stats);
> + if (vector_capacity(&node->stats) >=
> STATS_VEC_CAPACITY_THRESHOLD) {
> + VLOG_DBG("The statistics vector for node '%s' capacity "
> + "(%"PRIuSIZE") is over threshold.", node->name,
> + vector_capacity(&node->stats));
> + vector_shrink_to_fit(&node->stats);
> + }
> + stopwatch_stop(node->name, time_msec());
>
> schedule_updated |=
> statctrl_update_next_request_timestamp(node, now,
> prev_delay);
> @@ -233,7 +237,7 @@ statctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn)
> ovs_mutex_lock(&mutex);
> for (size_t i = 0; i < STATS_MAX; i++) {
> struct stats_node *node = &statctrl_ctx.nodes[i];
> - if (!ovs_list_is_empty(&node->stats_list)) {
> + if (!vector_is_empty(&node->stats)) {
> poll_immediate_wake();
> }
> }
> @@ -254,7 +258,7 @@ statctrl_destroy(void)
>
> for (size_t i = 0; i < STATS_MAX; i++) {
> struct stats_node *node = &statctrl_ctx.nodes[i];
> - node->destroy(&node->stats_list);
> + vector_destroy(&node->stats);
> }
> }
>
> @@ -364,7 +368,7 @@ statctrl_decode_statistics_reply(struct stats_node
> *node, struct ofpbuf *msg)
> break;
> }
>
> - node->process_flow_stats(&node->stats_list, &fs);
> + node->process_flow_stats(&node->stats, &fs);
> }
>
> ofpbuf_uninit(&ofpacts);
> @@ -399,7 +403,7 @@ static void
> statctrl_notify_main_thread(struct statctrl_ctx *ctx)
> {
> for (size_t i = 0; i < STATS_MAX; i++) {
> - if (!ovs_list_is_empty(&ctx->nodes[i].stats_list)) {
> + if (!vector_is_empty(&ctx->nodes[i].stats)) {
> seq_change(ctx->main_seq);
> return;
> }
> --
> 2.49.0
>
>
Recheck-request: github-robot
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
