[ovs-dev] [PATCH ovn v3 2/2] lflow-cache: Auto trim when cache size is reduced significantly.

Mark Gray mark.d.gray at redhat.com
Sun Jun 20 07:49:27 UTC 2021


On 18/06/2021 20:58, Dumitru Ceara wrote:
> Due to a bug in glibc [0], M_TRIM_THRESHOLD (default 128KB) is not
> honored.  The lflow cache is one of the largest memory consumers in
> ovn-controller and it used to trim memory whenever the cache was
> flushed.  However, that required periodic intervention from the CMS
> side.
> 
> Instead, we now automatically trim memory every time the lflow cache
> utilization decreases by 50%, with a threshold of at least
> lflow-cache-trim-limit (10000) elements in the cache.
> 
> The percentage of the high watermark under which memory is trimmed
> automatically as well as the lflow-cache minimum number of elements
> above which trimming is performed are configurable through OVS external
> IDs.
> 
> [0] https://sourceware.org/bugzilla/show_bug.cgi?id=14827
> 
> Reported-at: https://bugzilla.redhat.com/1967882
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>

There's a small typo below in the test code. Please *don't* update
unless you need to do a v4 as it wouldnt be worth the effort otherwise.

Acked-by: Mark D. Gray <mark.d.gray at redhat.com>

> ---
>  NEWS                            |    4 +
>  controller/chassis.c            |   38 +++++
>  controller/lflow-cache.c        |  104 +++++++++++----
>  controller/lflow-cache.h        |    3 
>  controller/ovn-controller.8.xml |   17 ++
>  controller/ovn-controller.c     |   14 ++
>  controller/test-lflow-cache.c   |   61 +++++++--
>  tests/ovn-lflow-cache.at        |  277 +++++++++++++++++++++++++++++++++++++++
>  8 files changed, 478 insertions(+), 40 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 0da7d8f97..79f562f1e 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -25,6 +25,10 @@ OVN v21.06.0 - 11 May 2021
>      * ovn-sbctl now also supports daemon mode.
>    - Added support in native DNS to respond to PTR request types.
>    - New --dry-run option for ovn-northd and ovn-northd-ddlog.
> +  - ovn-controller: Add configuration knobs, through OVS external-id
> +    "ovn-trim-limit-lflow-cache" and "ovn-trim-wmark-perc-lflow-cache", to
> +    allow enforcing a lflow cache size limit and high watermark percentage
> +    for which automatic memory trimming is performed.
>  
>  OVN v21.03.0 - 12 Mar 2021
>  -------------------------
> diff --git a/controller/chassis.c b/controller/chassis.c
> index 80d516f49..c11c10a34 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -52,6 +52,8 @@ struct ovs_chassis_cfg {
>      const char *enable_lflow_cache;
>      const char *limit_lflow_cache;
>      const char *memlimit_lflow_cache;
> +    const char *trim_limit_lflow_cache;
> +    const char *trim_wmark_perc_lflow_cache;
>  
>      /* Set of encap types parsed from the 'ovn-encap-type' external-id. */
>      struct sset encap_type_set;
> @@ -149,6 +151,18 @@ get_memlimit_lflow_cache(const struct smap *ext_ids)
>      return smap_get_def(ext_ids, "ovn-memlimit-lflow-cache-kb", "");
>  }
>  
> +static const char *
> +get_trim_limit_lflow_cache(const struct smap *ext_ids)
> +{
> +    return smap_get_def(ext_ids, "ovn-trim-limit-lflow-cache", "");
> +}
> +
> +static const char *
> +get_trim_wmark_perc_lflow_cache(const struct smap *ext_ids)
> +{
> +    return smap_get_def(ext_ids, "ovn-trim-wmark-perc-lflow-cache", "");
> +}
> +
>  static const char *
>  get_encap_csum(const struct smap *ext_ids)
>  {
> @@ -274,6 +288,10 @@ chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table,
>      ovs_cfg->limit_lflow_cache = get_limit_lflow_cache(&cfg->external_ids);
>      ovs_cfg->memlimit_lflow_cache =
>          get_memlimit_lflow_cache(&cfg->external_ids);
> +    ovs_cfg->trim_limit_lflow_cache =
> +        get_trim_limit_lflow_cache(&cfg->external_ids);
> +    ovs_cfg->trim_wmark_perc_lflow_cache =
> +        get_trim_wmark_perc_lflow_cache(&cfg->external_ids);
>  
>      if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) {
>          return false;
> @@ -314,6 +332,10 @@ chassis_build_other_config(const struct ovs_chassis_cfg *ovs_cfg,
>      smap_replace(config, "ovn-limit-lflow-cache", ovs_cfg->limit_lflow_cache);
>      smap_replace(config, "ovn-memlimit-lflow-cache-kb",
>                   ovs_cfg->memlimit_lflow_cache);
> +    smap_replace(config, "ovn-trim-limit-lflow-cache",
> +                 ovs_cfg->trim_limit_lflow_cache);
> +    smap_replace(config, "ovn-trim-wmark-perc-lflow-cache",
> +                 ovs_cfg->trim_wmark_perc_lflow_cache);
>      smap_replace(config, "iface-types", ds_cstr_ro(&ovs_cfg->iface_types));
>      smap_replace(config, "ovn-chassis-mac-mappings", ovs_cfg->chassis_macs);
>      smap_replace(config, "is-interconn",
> @@ -377,6 +399,22 @@ chassis_other_config_changed(const struct ovs_chassis_cfg *ovs_cfg,
>          return true;
>      }
>  
> +    const char *chassis_trim_limit_lflow_cache =
> +        get_trim_limit_lflow_cache(&chassis_rec->other_config);
> +
> +    if (strcmp(ovs_cfg->trim_limit_lflow_cache,
> +               chassis_trim_limit_lflow_cache)) {
> +        return true;
> +    }
> +
> +    const char *chassis_trim_wmark_perc_lflow_cache =
> +        get_trim_wmark_perc_lflow_cache(&chassis_rec->other_config);
> +
> +    if (strcmp(ovs_cfg->trim_wmark_perc_lflow_cache,
> +               chassis_trim_wmark_perc_lflow_cache)) {
> +        return true;
> +    }
> +
>      const char *chassis_mac_mappings =
>          get_chassis_mac_mappings(&chassis_rec->other_config);
>      if (strcmp(ovs_cfg->chassis_macs, chassis_mac_mappings)) {
> diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c
> index 56ddf1075..ece39dbf0 100644
> --- a/controller/lflow-cache.c
> +++ b/controller/lflow-cache.c
> @@ -24,8 +24,11 @@
>  #include "coverage.h"
>  #include "lflow-cache.h"
>  #include "lib/uuid.h"
> +#include "openvswitch/vlog.h"
>  #include "ovn/expr.h"
>  
> +VLOG_DEFINE_THIS_MODULE(lflow_cache);
> +
>  COVERAGE_DEFINE(lflow_cache_flush);
>  COVERAGE_DEFINE(lflow_cache_add_conj_id);
>  COVERAGE_DEFINE(lflow_cache_add_expr);
> @@ -40,6 +43,7 @@ COVERAGE_DEFINE(lflow_cache_delete);
>  COVERAGE_DEFINE(lflow_cache_full);
>  COVERAGE_DEFINE(lflow_cache_mem_full);
>  COVERAGE_DEFINE(lflow_cache_made_room);
> +COVERAGE_DEFINE(lflow_cache_trim);
>  
>  static const char *lflow_cache_type_names[LCACHE_T_MAX] = {
>      [LCACHE_T_CONJ_ID] = "cache-conj-id",
> @@ -47,11 +51,18 @@ static const char *lflow_cache_type_names[LCACHE_T_MAX] = {
>      [LCACHE_T_MATCHES] = "cache-matches",
>  };
>  
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +
>  struct lflow_cache {
>      struct hmap entries[LCACHE_T_MAX];
> +    uint32_t n_entries;
> +    uint32_t high_watermark;
>      uint32_t capacity;
>      uint64_t mem_usage;
>      uint64_t max_mem_usage;
> +    uint32_t trim_limit;
> +    uint32_t trim_wmark_perc;
> +    uint64_t trim_count;
>      bool enabled;
>  };
>  
> @@ -63,7 +74,6 @@ struct lflow_cache_entry {
>      struct lflow_cache_value value;
>  };
>  
> -static size_t lflow_cache_n_entries__(const struct lflow_cache *lc);
>  static bool lflow_cache_make_room__(struct lflow_cache *lc,
>                                      enum lflow_cache_type type);
>  static struct lflow_cache_value *lflow_cache_add__(
> @@ -71,18 +81,17 @@ static struct lflow_cache_value *lflow_cache_add__(
>      enum lflow_cache_type type, uint64_t value_size);
>  static void lflow_cache_delete__(struct lflow_cache *lc,
>                                   struct lflow_cache_entry *lce);
> +static void lflow_cache_trim__(struct lflow_cache *lc, bool force);
>  
>  struct lflow_cache *
>  lflow_cache_create(void)
>  {
> -    struct lflow_cache *lc = xmalloc(sizeof *lc);
> +    struct lflow_cache *lc = xzalloc(sizeof *lc);
>  
>      for (size_t i = 0; i < LCACHE_T_MAX; i++) {
>          hmap_init(&lc->entries[i]);
>      }
>  
> -    lc->enabled = true;
> -    lc->mem_usage = 0;
>      return lc;
>  }
>  
> @@ -101,12 +110,8 @@ lflow_cache_flush(struct lflow_cache *lc)
>          HMAP_FOR_EACH_SAFE (lce, lce_next, node, &lc->entries[i]) {
>              lflow_cache_delete__(lc, lce);
>          }
> -        hmap_shrink(&lc->entries[i]);
>      }
> -
> -#if HAVE_DECL_MALLOC_TRIM
> -    malloc_trim(0);
> -#endif
> +    lflow_cache_trim__(lc, true);
>  }
>  
>  void
> @@ -125,23 +130,45 @@ lflow_cache_destroy(struct lflow_cache *lc)
>  
>  void
>  lflow_cache_enable(struct lflow_cache *lc, bool enabled, uint32_t capacity,
> -                   uint64_t max_mem_usage_kb)
> +                   uint64_t max_mem_usage_kb, uint32_t lflow_trim_limit,
> +                   uint32_t trim_wmark_perc)
>  {
>      if (!lc) {
>          return;
>      }
>  
> +    if (trim_wmark_perc > 100) {
> +        VLOG_WARN_RL(&rl, "Invalid requested trim watermark percentage: "
> +                     "requested %"PRIu32", using 100 instead",
> +                     trim_wmark_perc);
> +        trim_wmark_perc = 100;
> +    }
> +
>      uint64_t max_mem_usage = max_mem_usage_kb * 1024;
> +    bool need_flush = false;
> +    bool need_trim = false;
>  
>      if ((lc->enabled && !enabled)
> -            || capacity < lflow_cache_n_entries__(lc)
> +            || capacity < lc->n_entries
>              || max_mem_usage < lc->mem_usage) {
> -        lflow_cache_flush(lc);
> +        need_flush = true;
> +    } else if (lc->enabled
> +                    && (lc->trim_limit != lflow_trim_limit
> +                        || lc->trim_wmark_perc != trim_wmark_perc)) {
> +        need_trim = true;
>      }
>  
>      lc->enabled = enabled;
>      lc->capacity = capacity;
>      lc->max_mem_usage = max_mem_usage;
> +    lc->trim_limit = lflow_trim_limit;
> +    lc->trim_wmark_perc = trim_wmark_perc;
> +
> +    if (need_flush) {
> +        lflow_cache_flush(lc);
> +    } else if (need_trim) {
> +        lflow_cache_trim__(lc, false);
> +    }
>  }
>  
>  bool
> @@ -164,11 +191,15 @@ lflow_cache_get_stats(const struct lflow_cache *lc, struct ds *output)
>  
>      ds_put_format(output, "Enabled: %s\n",
>                    lflow_cache_is_enabled(lc) ? "true" : "false");
> +    ds_put_format(output, "%-16s: %"PRIu32"\n", "high-watermark",
> +                  lc->high_watermark);
> +    ds_put_format(output, "%-16s: %"PRIu32"\n", "total", lc->n_entries);
>      for (size_t i = 0; i < LCACHE_T_MAX; i++) {
>          ds_put_format(output, "%-16s: %"PRIuSIZE"\n",
>                        lflow_cache_type_names[i],
>                        hmap_count(&lc->entries[i]));
>      }
> +    ds_put_format(output, "%-16s: %"PRIu64"\n", "trim count", lc->trim_count);
>      ds_put_format(output, "%-16s: %"PRIu64"\n", "Mem usage (KB)",
>                    ROUND_UP(lc->mem_usage, 1024) / 1024);
>  }
> @@ -254,20 +285,10 @@ lflow_cache_delete(struct lflow_cache *lc, const struct uuid *lflow_uuid)
>          COVERAGE_INC(lflow_cache_delete);
>          lflow_cache_delete__(lc, CONTAINER_OF(lcv, struct lflow_cache_entry,
>                                                value));
> +        lflow_cache_trim__(lc, false);
>      }
>  }
>  
> -static size_t
> -lflow_cache_n_entries__(const struct lflow_cache *lc)
> -{
> -    size_t n_entries = 0;
> -
> -    for (size_t i = 0; i < LCACHE_T_MAX; i++) {
> -        n_entries += hmap_count(&lc->entries[i]);
> -    }
> -    return n_entries;
> -}
> -
>  static bool
>  lflow_cache_make_room__(struct lflow_cache *lc, enum lflow_cache_type type)
>  {
> @@ -319,7 +340,7 @@ lflow_cache_add__(struct lflow_cache *lc, const struct uuid *lflow_uuid,
>          return NULL;
>      }
>  
> -    if (lflow_cache_n_entries__(lc) == lc->capacity) {
> +    if (lc->n_entries == lc->capacity) {
>          if (!lflow_cache_make_room__(lc, type)) {
>              COVERAGE_INC(lflow_cache_full);
>              return NULL;
> @@ -336,17 +357,17 @@ lflow_cache_add__(struct lflow_cache *lc, const struct uuid *lflow_uuid,
>      lce->size = size;
>      lce->value.type = type;
>      hmap_insert(&lc->entries[type], &lce->node, uuid_hash(lflow_uuid));
> +    lc->n_entries++;
> +    lc->high_watermark = MAX(lc->high_watermark, lc->n_entries);
>      return &lce->value;
>  }
>  
>  static void
>  lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce)
>  {
> -    if (!lce) {
> -        return;
> -    }
> -
> +    ovs_assert(lc->n_entries > 0);
>      hmap_remove(&lc->entries[lce->value.type], &lce->node);
> +    lc->n_entries--;
>      switch (lce->value.type) {
>      case LCACHE_T_NONE:
>          OVS_NOT_REACHED();
> @@ -369,3 +390,30 @@ lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce)
>      lc->mem_usage -= lce->size;
>      free(lce);
>  }
> +
> +static void
> +lflow_cache_trim__(struct lflow_cache *lc, bool force)
> +{
> +    /* Trim if we had at least 'TRIM_LIMIT' elements at some point and if the
> +     * current usage is less than half of 'high_watermark'.
> +     */
> +    uint32_t upper_trim_limit = lc->high_watermark * lc->trim_wmark_perc / 100;
> +    ovs_assert(lc->high_watermark >= lc->n_entries);
> +    if (!force
> +            && (lc->high_watermark <= lc->trim_limit
> +                || lc->n_entries > upper_trim_limit)) {
> +        return;
> +    }
> +
> +    COVERAGE_INC(lflow_cache_trim);
> +    for (size_t i = 0; i < LCACHE_T_MAX; i++) {
> +        hmap_shrink(&lc->entries[i]);
> +    }
> +
> +#if HAVE_DECL_MALLOC_TRIM
> +    malloc_trim(0);
> +#endif
> +
> +    lc->high_watermark = lc->n_entries;
> +    lc->trim_count++;
> +}
> diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h
> index 3c1fb4142..6166fa7c5 100644
> --- a/controller/lflow-cache.h
> +++ b/controller/lflow-cache.h
> @@ -57,7 +57,8 @@ struct lflow_cache *lflow_cache_create(void);
>  void lflow_cache_flush(struct lflow_cache *);
>  void lflow_cache_destroy(struct lflow_cache *);
>  void lflow_cache_enable(struct lflow_cache *, bool enabled, uint32_t capacity,
> -                        uint64_t max_mem_usage_kb);
> +                        uint64_t max_mem_usage_kb, uint32_t lflow_trim_limit,
> +                        uint32_t trim_wmark_perc);
>  bool lflow_cache_is_enabled(const struct lflow_cache *);
>  void lflow_cache_get_stats(const struct lflow_cache *, struct ds *output);
>  
> diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
> index 8886df568..ce279a818 100644
> --- a/controller/ovn-controller.8.xml
> +++ b/controller/ovn-controller.8.xml
> @@ -272,6 +272,23 @@
>          when the logical flow cache is enabled.  By default the size of the
>          cache is unlimited.
>        </dd>
> +
> +      <dt><code>external_ids:ovn-trim-limit-lflow-cache</code></dt>
> +      <dd>
> +        When used, this configuration value sets the minimum number of entries
> +        in the logical flow cache starting with which automatic memory trimming
> +        is performed.  By default this is set to 10000 entries.
> +      </dd>
> +      <dt><code>external_ids:ovn-trim-wmark-perc-lflow-cache</code></dt>
> +      <dd>
> +        When used, this configuration value sets the percentage from the high
> +        watermark number of entries in the logical flow cache under which
> +        automatic memory trimming is performed.  E.g., if the trim watermark
> +        percentage is set to 50%, automatic memory trimming happens only when
> +        the number of entries in the logical flow cache gets reduced to less
> +        than half of the last measured high watermark.  By default this is set
> +        to 50.
> +      </dd>
>      </dl>
>  
>      <p>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index addb08755..74d9c13de 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -103,9 +103,13 @@ static const char *ssl_private_key_file;
>  static const char *ssl_certificate_file;
>  static const char *ssl_ca_cert_file;
>  
> -/* By default don't set an upper bound for the lflow cache. */
> +/* By default don't set an upper bound for the lflow cache and enable auto
> + * trimming above 10K logical flows when reducing cache size by 50%.
> + */
>  #define DEFAULT_LFLOW_CACHE_MAX_ENTRIES UINT32_MAX
>  #define DEFAULT_LFLOW_CACHE_MAX_MEM_KB (UINT64_MAX / 1024)
> +#define DEFAULT_LFLOW_CACHE_TRIM_LIMIT 10000
> +#define DEFAULT_LFLOW_CACHE_WMARK_PERC 50
>  
>  struct controller_engine_ctx {
>      struct lflow_cache *lflow_cache;
> @@ -530,7 +534,13 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
>                                           DEFAULT_LFLOW_CACHE_MAX_ENTRIES),
>                             smap_get_ullong(&cfg->external_ids,
>                                             "ovn-memlimit-lflow-cache-kb",
> -                                           DEFAULT_LFLOW_CACHE_MAX_MEM_KB));
> +                                           DEFAULT_LFLOW_CACHE_MAX_MEM_KB),
> +                           smap_get_uint(&cfg->external_ids,
> +                                         "ovn-trim-limit-lflow-cache",
> +                                         DEFAULT_LFLOW_CACHE_TRIM_LIMIT),
> +                           smap_get_uint(&cfg->external_ids,
> +                                         "ovn-trim-wmark-perc-lflow-cache",
> +                                         DEFAULT_LFLOW_CACHE_WMARK_PERC));
>      }
>  }
>  
> diff --git a/controller/test-lflow-cache.c b/controller/test-lflow-cache.c
> index 6a1416197..b46b1f743 100644
> --- a/controller/test-lflow-cache.c
> +++ b/controller/test-lflow-cache.c
> @@ -26,6 +26,12 @@
>  /* Simulate 1KB large cache values. */
>  #define TEST_LFLOW_CACHE_VALUE_SIZE 1024
>  
> +/* Set memory trimming limit to 1 by default. */
> +#define TEST_LFLOW_CACHE_TRIM_LIMIT 1
> +
> +/* Set memory trimming high watermark percentage to 50% by default. */
> +#define TEST_LFLOW_CACHE_TRIM_WMARK_PERC 50
> +
>  static void
>  test_lflow_cache_add__(struct lflow_cache *lc, const char *op_type,
>                         const struct uuid *lflow_uuid,
> @@ -104,10 +110,15 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx)
>      struct lflow_cache *lc = lflow_cache_create();
>      struct expr *e = expr_create_boolean(true);
>      bool enabled = !strcmp(ctx->argv[1], "true");
> +    struct uuid *lflow_uuids = NULL;
> +    size_t n_allocated_lflow_uuids = 0;
> +    size_t n_lflow_uuids = 0;
>      unsigned int shift = 2;
>      unsigned int n_ops;
>  
> -    lflow_cache_enable(lc, enabled, UINT32_MAX, UINT32_MAX);
> +    lflow_cache_enable(lc, enabled, UINT32_MAX, UINT32_MAX,
> +                       TEST_LFLOW_CACHE_TRIM_LIMIT,
> +                       TEST_LFLOW_CACHE_TRIM_WMARK_PERC);
>      test_lflow_cache_stats__(lc);
>  
>      if (!test_read_uint_value(ctx, shift++, "n_ops", &n_ops)) {
> @@ -121,9 +132,6 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx)
>              goto done;
>          }
>  
> -        struct uuid lflow_uuid;
> -        uuid_generate(&lflow_uuid);
> -
>          if (!strcmp(op, "add")) {
>              const char *op_type = test_read_value(ctx, shift++, "op_type");
>              if (!op_type) {
> @@ -136,8 +144,15 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx)
>                  goto done;
>              }
>  
> -            test_lflow_cache_add__(lc, op_type, &lflow_uuid, conj_id_ofs, e);
> -            test_lflow_cache_lookup__(lc, &lflow_uuid);
> +            if (n_lflow_uuids == n_allocated_lflow_uuids) {
> +                lflow_uuids = x2nrealloc(lflow_uuids, &n_allocated_lflow_uuids,
> +                                         sizeof *lflow_uuids);
> +            }
> +            struct uuid *lflow_uuid = &lflow_uuids[n_lflow_uuids++];
> +
> +            uuid_generate(lflow_uuid);
> +            test_lflow_cache_add__(lc, op_type, lflow_uuid, conj_id_ofs, e);
> +            test_lflow_cache_lookup__(lc, lflow_uuid);
>          } else if (!strcmp(op, "add-del")) {
>              const char *op_type = test_read_value(ctx, shift++, "op_type");
>              if (!op_type) {
> @@ -150,13 +165,21 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx)
>                  goto done;
>              }
>  
> +            struct uuid lflow_uuid;
> +            uuid_generate(&lflow_uuid);
>              test_lflow_cache_add__(lc, op_type, &lflow_uuid, conj_id_ofs, e);
>              test_lflow_cache_lookup__(lc, &lflow_uuid);
>              test_lflow_cache_delete__(lc, &lflow_uuid);
>              test_lflow_cache_lookup__(lc, &lflow_uuid);
> +        } else if (!strcmp(op, "del")) {
> +            ovs_assert(n_lflow_uuids);
> +            test_lflow_cache_delete__(lc, &lflow_uuids[n_lflow_uuids - 1]);
> +            n_lflow_uuids--;
>          } else if (!strcmp(op, "enable")) {
>              unsigned int limit;
>              unsigned int mem_limit_kb;
> +            unsigned int trim_limit = TEST_LFLOW_CACHE_TRIM_LIMIT;
> +            unsigned int trim_wmark_perc = TEST_LFLOW_CACHE_TRIM_WMARK_PERC;
>              if (!test_read_uint_value(ctx, shift++, "limit", &limit)) {
>                  goto done;
>              }
> @@ -164,11 +187,28 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx)
>                                        &mem_limit_kb)) {
>                  goto done;
>              }
> +            if (!strcmp(ctx->argv[shift], "trim-limit")) {
> +                shift++;
> +                if (!test_read_uint_value(ctx, shift++, "trim-limit",
> +                                          &trim_limit)) {
> +                    goto done;
> +                }
> +            }
> +            if (!strcmp(ctx->argv[shift], "trim-wmark-perc")) {
> +                shift++;
> +                if (!test_read_uint_value(ctx, shift++, "trim-wmark-perc",
> +                                          &trim_wmark_perc)) {
> +                    goto done;
> +                }
> +            }
>              printf("ENABLE\n");
> -            lflow_cache_enable(lc, true, limit, mem_limit_kb);
> +            lflow_cache_enable(lc, true, limit, mem_limit_kb, trim_limit,
> +                               trim_wmark_perc);
>          } else if (!strcmp(op, "disable")) {
>              printf("DISABLE\n");
> -            lflow_cache_enable(lc, false, UINT32_MAX, UINT32_MAX);
> +            lflow_cache_enable(lc, false, UINT32_MAX, UINT32_MAX,
> +                               TEST_LFLOW_CACHE_TRIM_LIMIT,
> +                               TEST_LFLOW_CACHE_TRIM_WMARK_PERC);
>          } else if (!strcmp(op, "flush")) {
>              printf("FLUSH\n");
>              lflow_cache_flush(lc);
> @@ -179,6 +219,7 @@ test_lflow_cache_operations(struct ovs_cmdl_context *ctx)
>      }
>  done:
>      lflow_cache_destroy(lc);
> +    free(lflow_uuids);
>      expr_destroy(e);
>  }
>  
> @@ -187,7 +228,9 @@ test_lflow_cache_negative(struct ovs_cmdl_context *ctx OVS_UNUSED)
>  {
>      lflow_cache_flush(NULL);
>      lflow_cache_destroy(NULL);
> -    lflow_cache_enable(NULL, true, UINT32_MAX, UINT32_MAX);
> +    lflow_cache_enable(NULL, true, UINT32_MAX, UINT32_MAX,
> +                       TEST_LFLOW_CACHE_TRIM_LIMIT,
> +                       TEST_LFLOW_CACHE_TRIM_WMARK_PERC);
>      ovs_assert(!lflow_cache_is_enabled(NULL));
>  
>      struct ds ds = DS_EMPTY_INITIALIZER;
> diff --git a/tests/ovn-lflow-cache.at b/tests/ovn-lflow-cache.at
> index e5e9ed1e8..7f452c9e7 100644
> --- a/tests/ovn-lflow-cache.at
> +++ b/tests/ovn-lflow-cache.at
> @@ -12,36 +12,48 @@ AT_CHECK(
>          add matches 3 | grep -v 'Mem usage (KB)'],
>      [0], [dnl
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD conj-id:
>    conj-id-ofs: 1
>  LOOKUP:
>    conj_id_ofs: 1
>    type: conj-id
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD expr:
>    conj-id-ofs: 2
>  LOOKUP:
>    conj_id_ofs: 2
>    type: expr
>  Enabled: true
> +high-watermark  : 2
> +total           : 2
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 0
> +trim count      : 0
>  ADD matches:
>    conj-id-ofs: 3
>  LOOKUP:
>    conj_id_ofs: 0
>    type: matches
>  Enabled: true
> +high-watermark  : 3
> +total           : 3
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 1
> +trim count      : 0
>  ])
>  AT_CLEANUP
>  
> @@ -54,9 +66,12 @@ AT_CHECK(
>          add-del matches 3 | grep -v 'Mem usage (KB)'],
>      [0], [dnl
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD conj-id:
>    conj-id-ofs: 1
>  LOOKUP:
> @@ -66,9 +81,12 @@ DELETE
>  LOOKUP:
>    not found
>  Enabled: true
> +high-watermark  : 1
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD expr:
>    conj-id-ofs: 2
>  LOOKUP:
> @@ -78,9 +96,12 @@ DELETE
>  LOOKUP:
>    not found
>  Enabled: true
> +high-watermark  : 1
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD matches:
>    conj-id-ofs: 3
>  LOOKUP:
> @@ -90,9 +111,12 @@ DELETE
>  LOOKUP:
>    not found
>  Enabled: true
> +high-watermark  : 1
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ])
>  AT_CLEANUP
>  
> @@ -105,33 +129,45 @@ AT_CHECK(
>          add matches 3 | grep -v 'Mem usage (KB)'],
>      [0], [dnl
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD conj-id:
>    conj-id-ofs: 1
>  LOOKUP:
>    not found
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD expr:
>    conj-id-ofs: 2
>  LOOKUP:
>    not found
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD matches:
>    conj-id-ofs: 3
>  LOOKUP:
>    not found
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ])
>  AT_CLEANUP
>  
> @@ -153,102 +189,142 @@ AT_CHECK(
>          flush | grep -v 'Mem usage (KB)'],
>      [0], [dnl
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD conj-id:
>    conj-id-ofs: 1
>  LOOKUP:
>    conj_id_ofs: 1
>    type: conj-id
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD expr:
>    conj-id-ofs: 2
>  LOOKUP:
>    conj_id_ofs: 2
>    type: expr
>  Enabled: true
> +high-watermark  : 2
> +total           : 2
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 0
> +trim count      : 0
>  ADD matches:
>    conj-id-ofs: 3
>  LOOKUP:
>    conj_id_ofs: 0
>    type: matches
>  Enabled: true
> +high-watermark  : 3
> +total           : 3
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 1
> +trim count      : 0
>  DISABLE
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +dnl At "disable" the cache was flushed.
> +trim count      : 1
>  ADD conj-id:
>    conj-id-ofs: 4
>  LOOKUP:
>    not found
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 1
>  ADD expr:
>    conj-id-ofs: 5
>  LOOKUP:
>    not found
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 1
>  ADD matches:
>    conj-id-ofs: 6
>  LOOKUP:
>    not found
>  Enabled: false
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 1
>  ENABLE
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 1
>  ADD conj-id:
>    conj-id-ofs: 7
>  LOOKUP:
>    conj_id_ofs: 7
>    type: conj-id
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 1
>  ADD expr:
>    conj-id-ofs: 8
>  LOOKUP:
>    conj_id_ofs: 8
>    type: expr
>  Enabled: true
> +high-watermark  : 2
> +total           : 2
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 0
> +trim count      : 1
>  ADD matches:
>    conj-id-ofs: 9
>  LOOKUP:
>    conj_id_ofs: 0
>    type: matches
>  Enabled: true
> +high-watermark  : 3
> +total           : 3
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 1
> +trim count      : 1
>  FLUSH
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 2
>  ])
>  AT_CLEANUP
>  
> @@ -270,53 +346,71 @@ AT_CHECK(
>          add matches 10 | grep -v 'Mem usage (KB)'],
>      [0], [dnl
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD conj-id:
>    conj-id-ofs: 1
>  LOOKUP:
>    conj_id_ofs: 1
>    type: conj-id
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 0
>  ADD expr:
>    conj-id-ofs: 2
>  LOOKUP:
>    conj_id_ofs: 2
>    type: expr
>  Enabled: true
> +high-watermark  : 2
> +total           : 2
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 0
> +trim count      : 0
>  ADD matches:
>    conj-id-ofs: 3
>  LOOKUP:
>    conj_id_ofs: 0
>    type: matches
>  Enabled: true
> +high-watermark  : 3
> +total           : 3
>  cache-conj-id   : 1
>  cache-expr      : 1
>  cache-matches   : 1
> +trim count      : 0
>  ENABLE
>  dnl
>  dnl Max capacity smaller than current usage, cache should be flushed.
>  dnl
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 1
>  ADD conj-id:
>    conj-id-ofs: 4
>  LOOKUP:
>    conj_id_ofs: 4
>    type: conj-id
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 1
>  ADD expr:
>    conj-id-ofs: 5
>  LOOKUP:
> @@ -327,9 +421,12 @@ dnl Cache is full but we can evict the conj-id entry because we're adding
>  dnl an expr one.
>  dnl
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 0
>  cache-expr      : 1
>  cache-matches   : 0
> +trim count      : 1
>  ADD matches:
>    conj-id-ofs: 6
>  LOOKUP:
> @@ -340,9 +437,12 @@ dnl Cache is full but we can evict the expr entry because we're adding
>  dnl a matches one.
>  dnl
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 1
> +trim count      : 1
>  ADD conj-id:
>    conj-id-ofs: 7
>  LOOKUP:
> @@ -352,27 +452,36 @@ dnl Cache is full and we're adding a conj-id entry so we shouldn't evict
>  dnl anything else.
>  dnl
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 1
> +trim count      : 1
>  ENABLE
>  dnl
>  dnl Max memory usage smaller than current memory usage, cache should be
>  dnl flushed.
>  dnl
>  Enabled: true
> +high-watermark  : 0
> +total           : 0
>  cache-conj-id   : 0
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 2
>  ADD conj-id:
>    conj-id-ofs: 8
>  LOOKUP:
>    conj_id_ofs: 8
>    type: conj-id
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 2
>  ADD expr:
>    conj-id-ofs: 9
>  LOOKUP:
> @@ -382,9 +491,12 @@ dnl Cache is full and we're adding a cache entry that would go over the max
>  dnl memory limit so adding should fail.
>  dnl
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 2
>  ADD matches:
>    conj-id-ofs: 10
>  LOOKUP:
> @@ -394,9 +506,174 @@ dnl Cache is full and we're adding a cache entry that would go over the max
>  dnl memory limit so adding should fail.
>  dnl
>  Enabled: true
> +high-watermark  : 1
> +total           : 1
>  cache-conj-id   : 1
>  cache-expr      : 0
>  cache-matches   : 0
> +trim count      : 2
> +])
> +AT_CLEANUP
> +
> +AT_SETUP([ovn -- unit test -- lflow-cache trimming])
> +AT_CHECK(
> +    [ovstest test-lflow-cache lflow_cache_operations \
> +        true 12 \
> +        enable 1000 1024 trim-limit 100 trim-wmark-perc 100 \
> +        add conj-id 1 \
> +        add conj-id 2 \
> +        add conj-id 3 \
> +        add conj-id 4 \
> +        add conj-id 5 \
> +        del \
> +        enable 1000 1024 trim-limit 0 trim-wmark-perc 100 \
> +        del \
> +        enable 1000 1024 trim-limit 0 trim-wmark-perc 50 \
> +        del \
> +        del | grep -v 'Mem usage (KB)'],
> +    [0], [dnl
> +Enabled: true
> +high-watermark  : 0
> +total           : 0
> +cache-conj-id   : 0
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +ENABLE
> +Enabled: true
> +high-watermark  : 0
> +total           : 0
> +cache-conj-id   : 0
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +ADD conj-id:
> +  conj-id-ofs: 1
> +LOOKUP:
> +  conj_id_ofs: 1
> +  type: conj-id
> +Enabled: true
> +high-watermark  : 1
> +total           : 1
> +cache-conj-id   : 1
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +ADD conj-id:
> +  conj-id-ofs: 2
> +LOOKUP:
> +  conj_id_ofs: 2
> +  type: conj-id
> +Enabled: true
> +high-watermark  : 2
> +total           : 2
> +cache-conj-id   : 2
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +ADD conj-id:
> +  conj-id-ofs: 3
> +LOOKUP:
> +  conj_id_ofs: 3
> +  type: conj-id
> +Enabled: true
> +high-watermark  : 3
> +total           : 3
> +cache-conj-id   : 3
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +ADD conj-id:
> +  conj-id-ofs: 4
> +LOOKUP:
> +  conj_id_ofs: 4
> +  type: conj-id
> +Enabled: true
> +high-watermark  : 4
> +total           : 4
> +cache-conj-id   : 4
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +ADD conj-id:
> +  conj-id-ofs: 5
> +LOOKUP:
> +  conj_id_ofs: 5
> +  type: conj-id
> +Enabled: true
> +high-watermark  : 5
> +total           : 5
> +cache-conj-id   : 5
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +DELETE
> +dnl
> +dnl Trim limit is set to 100 so we shouldn't automatically trim memory.
> +dnl
> +Enabled: true
> +high-watermark  : 5
> +total           : 4
> +cache-conj-id   : 4
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 0
> +ENABLE
> +dnl
> +dnl Trim limit changed to 0 high watermark percentage is 100% so the chache


typo: chache ^


> +dnl should be trimmed.
> +dnl
> +Enabled: true
> +high-watermark  : 4
> +total           : 4
> +cache-conj-id   : 4
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 1
> +DELETE
> +dnl
> +dnl Trim limit is 0 and high watermark percentage is 100% so any delete
> +dnl should trigger trimming.
> +dnl
> +Enabled: true
> +high-watermark  : 3
> +total           : 3
> +cache-conj-id   : 3
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 2
> +ENABLE
> +Enabled: true
> +high-watermark  : 3
> +total           : 3
> +cache-conj-id   : 3
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 2
> +DELETE
> +dnl
> +dnl Trim limit is 0 but high watermark percentage is 50% so only the delete
> +dnl that reduces the number of entries under 2 should trigger trimming.
> +dnl
> +Enabled: true
> +high-watermark  : 3
> +total           : 2
> +cache-conj-id   : 2
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 2
> +dnl
> +dnl Number of entries dropped under 50% of high watermark, trimming should
> +dnl happen.
> +dnl
> +DELETE
> +Enabled: true
> +high-watermark  : 1
> +total           : 1
> +cache-conj-id   : 1
> +cache-expr      : 0
> +cache-matches   : 0
> +trim count      : 3
>  ])
>  AT_CLEANUP
>  
> 




More information about the dev mailing list