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

Dumitru Ceara dceara at redhat.com
Fri Jun 18 19:58:18 UTC 2021


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>
---
 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
+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