Message ID | 20210618195816.26987.45869.stgit@dceara.remote.csb |
---|---|
State | Accepted |
Headers | show |
Series | ovn-controller: Auto trim memory when chache size is reduced significantly. | expand |
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@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@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 > >
On 6/20/21 9:49 AM, Mark Gray wrote: >> 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@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@redhat.com> > Thanks for the review! I assume maintainers can fix the typo when the patch is pushed if there are no other issues. Regards, Dumitru
On 6/21/21 5:57 AM, Dumitru Ceara wrote: > On 6/20/21 9:49 AM, Mark Gray wrote: >>> 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@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@redhat.com> >> > > Thanks for the review! I assume maintainers can fix the typo when the > patch is pushed if there are no other issues. > > Regards, > Dumitru > Thanks guys. I fixed the typo and pushed the change to master.
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
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@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(-)