Add a new OVS external-id, "ovn-memlimit-lflow-cache-kb", through which
users can specify the maximum amount of memory (in KB) ovn-controller
can use for caching logical flows.

To maintain backwards compatibility the default behavior is to not
enforce any memory limit on the size of the cache.

Add lflow cache reports to "ovn-appctl -t ovn-controller memory/show".

Signed-off-by: Dumitru Ceara <[email protected]>
---
 NEWS                            |    8 ++++--
 controller/chassis.c            |   21 +++++++++++++++
 controller/lflow-cache.c        |   53 +++++++++++++++++++++++++++++++--------
 controller/lflow-cache.h        |   10 ++++++-
 controller/ovn-controller.8.xml |    7 +++++
 controller/ovn-controller.c     |    8 ++++--
 include/ovn/expr.h              |    2 +
 lib/expr.c                      |   42 +++++++++++++++++++++++++++++++
 8 files changed, 133 insertions(+), 18 deletions(-)

diff --git a/NEWS b/NEWS
index 68c9a8e..a1bdc71 100644
--- a/NEWS
+++ b/NEWS
@@ -13,9 +13,11 @@ Post-v20.12.0
   - ovn-ctl: Added new command line argument '--ovsdb-disable-file-column-diff'
     to support ovsdb-server upgrades from version 2.14 and earlier to
     2.15 and later.  See ovsdb(7) for more details.
-  - ovn-controller: Add a configuration knob, through OVS external-id
-    "ovn-limit-lflow-cache", to allow enforcing a limit for the size of the
-    logical flow cache.
+  - ovn-controller: Add configuration knobs, through OVS external-id
+    "ovn-limit-lflow-cache" and "ovn-memlimit-lflow-cache-kb", to allow
+    enforcing a limit for the size of the logical flow cache based on
+    maximum number of entries and/or memory usage.
+  - ovn-controller: Add lflow cache related memory reports.
 
 OVN v20.12.0 - 18 Dec 2020
 --------------------------
diff --git a/controller/chassis.c b/controller/chassis.c
index c66837a..a4a264c 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -50,6 +50,7 @@ struct ovs_chassis_cfg {
     const char *chassis_macs;
     const char *enable_lflow_cache;
     const char *limit_lflow_cache;
+    const char *memlimit_lflow_cache;
 
     /* Set of encap types parsed from the 'ovn-encap-type' external-id. */
     struct sset encap_type_set;
@@ -142,6 +143,12 @@ get_limit_lflow_cache(const struct smap *ext_ids)
 }
 
 static const char *
+get_memlimit_lflow_cache(const struct smap *ext_ids)
+{
+    return smap_get_def(ext_ids, "ovn-memlimit-lflow-cache-kb", "");
+}
+
+static const char *
 get_encap_csum(const struct smap *ext_ids)
 {
     return smap_get_def(ext_ids, "ovn-encap-csum", "true");
@@ -264,6 +271,8 @@ chassis_parse_ovs_config(const struct 
ovsrec_open_vswitch_table *ovs_table,
     ovs_cfg->chassis_macs = get_chassis_mac_mappings(&cfg->external_ids);
     ovs_cfg->enable_lflow_cache = get_enable_lflow_cache(&cfg->external_ids);
     ovs_cfg->limit_lflow_cache = get_limit_lflow_cache(&cfg->external_ids);
+    ovs_cfg->memlimit_lflow_cache =
+        get_memlimit_lflow_cache(&cfg->external_ids);
 
     if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) {
         return false;
@@ -293,6 +302,7 @@ chassis_build_other_config(struct smap *config, const char 
*bridge_mappings,
                            const char *iface_types,
                            const char *enable_lflow_cache,
                            const char *limit_lflow_cache,
+                           const char *memlimit_lflow_cache,
                            bool is_interconn)
 {
     smap_replace(config, "ovn-bridge-mappings", bridge_mappings);
@@ -301,6 +311,7 @@ chassis_build_other_config(struct smap *config, const char 
*bridge_mappings,
     smap_replace(config, "ovn-monitor-all", monitor_all);
     smap_replace(config, "ovn-enable-lflow-cache", enable_lflow_cache);
     smap_replace(config, "ovn-limit-lflow-cache", limit_lflow_cache);
+    smap_replace(config, "ovn-memlimit-lflow-cache-kb", memlimit_lflow_cache);
     smap_replace(config, "iface-types", iface_types);
     smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
     smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
@@ -317,6 +328,7 @@ chassis_other_config_changed(const char *bridge_mappings,
                              const char *chassis_macs,
                              const char *enable_lflow_cache,
                              const char *limit_lflow_cache,
+                             const char *memlimit_lflow_cache,
                              const struct ds *iface_types,
                              bool is_interconn,
                              const struct sbrec_chassis *chassis_rec)
@@ -363,6 +375,13 @@ chassis_other_config_changed(const char *bridge_mappings,
         return true;
     }
 
+    const char *chassis_memlimit_lflow_cache =
+        get_memlimit_lflow_cache(&chassis_rec->other_config);
+
+    if (strcmp(memlimit_lflow_cache, chassis_memlimit_lflow_cache)) {
+        return true;
+    }
+
     const char *chassis_mac_mappings =
         get_chassis_mac_mappings(&chassis_rec->other_config);
     if (strcmp(chassis_macs, chassis_mac_mappings)) {
@@ -543,6 +562,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
                                      ovs_cfg->chassis_macs,
                                      ovs_cfg->enable_lflow_cache,
                                      ovs_cfg->limit_lflow_cache,
+                                     ovs_cfg->memlimit_lflow_cache,
                                      &ovs_cfg->iface_types,
                                      ovs_cfg->is_interconn,
                                      chassis_rec)) {
@@ -557,6 +577,7 @@ chassis_update(const struct sbrec_chassis *chassis_rec,
                                    ds_cstr_ro(&ovs_cfg->iface_types),
                                    ovs_cfg->enable_lflow_cache,
                                    ovs_cfg->limit_lflow_cache,
+                                   ovs_cfg->memlimit_lflow_cache,
                                    ovs_cfg->is_interconn);
         sbrec_chassis_verify_other_config(chassis_rec);
         sbrec_chassis_set_other_config(chassis_rec, &other_config);
diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c
index ee7ea9b..5e63397 100644
--- a/controller/lflow-cache.c
+++ b/controller/lflow-cache.c
@@ -38,16 +38,20 @@ COVERAGE_DEFINE(lflow_cache_hit);
 COVERAGE_DEFINE(lflow_cache_miss);
 COVERAGE_DEFINE(lflow_cache_delete);
 COVERAGE_DEFINE(lflow_cache_full);
+COVERAGE_DEFINE(lflow_cache_mem_full);
 
 struct lflow_cache {
     struct hmap entries;
     uint32_t capacity;
+    uint64_t mem_usage;
+    uint64_t max_mem_usage;
     bool enabled;
 };
 
 struct lflow_cache_entry {
     struct hmap_node node;
     struct uuid lflow_uuid; /* key */
+    size_t size;
 
     struct lflow_cache_value value;
 };
@@ -55,7 +59,8 @@ struct lflow_cache_entry {
 static struct lflow_cache_value *lflow_cache_add__(
     struct lflow_cache *lc,
     const struct sbrec_logical_flow *lflow,
-    enum lflow_cache_type type);
+    enum lflow_cache_type type,
+    uint64_t value_size);
 static void lflow_cache_delete__(struct lflow_cache *lc,
                                  struct lflow_cache_entry *lce);
 
@@ -66,6 +71,7 @@ lflow_cache_create(void)
 
     hmap_init(&lc->entries);
     lc->enabled = true;
+    lc->mem_usage = 0;
     return lc;
 }
 
@@ -101,14 +107,20 @@ lflow_cache_destroy(struct lflow_cache *lc)
 }
 
 void
-lflow_cache_enable(struct lflow_cache *lc, bool enabled, uint32_t capacity)
+lflow_cache_enable(struct lflow_cache *lc, bool enabled, uint32_t capacity,
+                   uint64_t max_mem_usage_kb)
 {
-    if ((lc->enabled && !enabled) || capacity < hmap_count(&lc->entries)) {
+    uint64_t max_mem_usage = max_mem_usage_kb * 1024;
+
+    if ((lc->enabled && !enabled)
+            || capacity < hmap_count(&lc->entries)
+            || max_mem_usage < lc->mem_usage) {
         lflow_cache_flush(lc);
     }
 
     lc->enabled = enabled;
     lc->capacity = capacity;
+    lc->max_mem_usage = max_mem_usage;
 }
 
 bool
@@ -123,8 +135,7 @@ lflow_cache_add_conj_id(struct lflow_cache *lc,
                         uint32_t conj_id_ofs)
 {
     struct lflow_cache_value *lcv =
-        lflow_cache_add__(lc, lflow, LCACHE_T_CONJ_ID);
-
+        lflow_cache_add__(lc, lflow, LCACHE_T_CONJ_ID, 0);
     if (!lcv) {
         return;
     }
@@ -140,8 +151,7 @@ lflow_cache_add_expr(struct lflow_cache *lc,
                      struct expr *expr)
 {
     struct lflow_cache_value *lcv =
-        lflow_cache_add__(lc, lflow, LCACHE_T_EXPR);
-
+        lflow_cache_add__(lc, lflow, LCACHE_T_EXPR, expr_size(expr));
     if (!lcv) {
         return;
     }
@@ -157,8 +167,8 @@ lflow_cache_add_matches(struct lflow_cache *lc,
                         struct hmap *matches)
 {
     struct lflow_cache_value *lcv =
-        lflow_cache_add__(lc, lflow, LCACHE_T_MATCHES);
-
+        lflow_cache_add__(lc, lflow, LCACHE_T_MATCHES,
+                          expr_matches_size(matches));
     if (!lcv) {
         return;
     }
@@ -203,20 +213,38 @@ lflow_cache_delete(struct lflow_cache *lc,
     }
 }
 
+void
+lflow_cache_get_memory_usage(const struct lflow_cache *lc, struct simap *usage)
+{
+    simap_increase(usage, "lflow-cache-entries", hmap_count(&lc->entries));
+    simap_increase(usage, "lflow-cache-size-KB",
+                   ROUND_UP(lc->mem_usage, 1024) / 1024);
+}
+
 static struct lflow_cache_value *
 lflow_cache_add__(struct lflow_cache *lc,
                   const struct sbrec_logical_flow *lflow,
-                  enum lflow_cache_type type)
+                  enum lflow_cache_type type,
+                  uint64_t value_size)
 {
+    struct lflow_cache_entry *lce;
+
     if (hmap_count(&lc->entries) == lc->capacity) {
         COVERAGE_INC(lflow_cache_full);
         return NULL;
     }
 
-    struct lflow_cache_entry *lce = xzalloc(sizeof *lce);
+    size_t size = sizeof *lce + value_size;
+    if (size + lc->mem_usage > lc->max_mem_usage) {
+        COVERAGE_INC(lflow_cache_mem_full);
+        return NULL;
+    }
+    lc->mem_usage += size;
 
     COVERAGE_INC(lflow_cache_add);
+    lce = xzalloc(sizeof *lce);
     lce->lflow_uuid = lflow->header_.uuid;
+    lce->size = size;
     lce->value.type = type;
     hmap_insert(&lc->entries, &lce->node, uuid_hash(&lflow->header_.uuid));
     return &lce->value;
@@ -247,5 +275,8 @@ lflow_cache_delete__(struct lflow_cache *lc, struct 
lflow_cache_entry *lce)
         free(lce->value.expr_matches);
         break;
     }
+
+    ovs_assert(lc->mem_usage >= lce->size);
+    lc->mem_usage -= lce->size;
     free(lce);
 }
diff --git a/controller/lflow-cache.h b/controller/lflow-cache.h
index 6d3ea05..1219b73 100644
--- a/controller/lflow-cache.h
+++ b/controller/lflow-cache.h
@@ -20,6 +20,7 @@
 
 #include "openvswitch/hmap.h"
 #include "openvswitch/uuid.h"
+#include "simap.h"
 
 struct sbrec_logical_flow;
 struct lflow_cache;
@@ -29,7 +30,8 @@ struct lflow_cache;
  *    results in conjunctive OpenvSwitch flows.
  *
  *  - Caches
- *     (1) Nothing if the logical flow has port group/address set references.
+ *     (1) Conjunction ID offset if the logical flow has port group/address
+ *         set references.
  *     (2) expr tree if the logical flow has is_chassis_resident() match.
  *     (3) expr matches if (1) and (2) are false.
  */
@@ -53,7 +55,8 @@ struct lflow_cache_value {
 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);
+void lflow_cache_enable(struct lflow_cache *, bool enabled, uint32_t capacity,
+                        uint64_t max_mem_usage_kb);
 bool lflow_cache_is_enabled(struct lflow_cache *);
 
 void lflow_cache_add_conj_id(struct lflow_cache *,
@@ -72,4 +75,7 @@ struct lflow_cache_value *lflow_cache_get(struct lflow_cache 
*,
 void lflow_cache_delete(struct lflow_cache *,
                         const struct sbrec_logical_flow *);
 
+void lflow_cache_get_memory_usage(const struct lflow_cache *,
+                                  struct simap *usage);
+
 #endif /* controller/lflow-cache.h */
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index e92db6d..75c6596 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -265,6 +265,13 @@
         when the logical flow cache is enabled.  By default the size of the
         cache is unlimited.
       </dd>
+      <dt><code>external_ids:ovn-memlimit-lflow-cache-kb</code></dt>
+      <dd>
+        When used, this configuration value determines the maximum size of
+        the logical flow cache (in KB) <code>ovn-controller</code> may create
+        when the logical flow cache is enabled.  By default the size of the
+        cache is unlimited.
+      </dd>
     </dl>
 
     <p>
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index f551142..ba6d8a3 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -97,6 +97,7 @@ OVS_NO_RETURN static void usage(void);
 
 /* By default don't set an upper bound for the lflow cache. */
 #define DEFAULT_LFLOW_CACHE_MAX_ENTRIES UINT32_MAX
+#define DEFAULT_LFLOW_CACHE_MAX_MEM_KB (UINT64_MAX / 1024)
 
 struct controller_engine_ctx {
     struct lflow_cache *lflow_cache;
@@ -587,7 +588,10 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl 
*ovnsb_idl,
                                          true),
                            smap_get_uint(&cfg->external_ids,
                                          "ovn-limit-lflow-cache",
-                                         DEFAULT_LFLOW_CACHE_MAX_ENTRIES));
+                                         DEFAULT_LFLOW_CACHE_MAX_ENTRIES),
+                           smap_get_ullong(&cfg->external_ids,
+                                           "ovn-memlimit-lflow-cache-kb",
+                                           DEFAULT_LFLOW_CACHE_MAX_MEM_KB));
     }
 }
 
@@ -2705,7 +2709,7 @@ main(int argc, char *argv[])
         if (memory_should_report()) {
             struct simap usage = SIMAP_INITIALIZER(&usage);
 
-            /* Nothing special to report yet. */
+            lflow_cache_get_memory_usage(ctrl_engine_ctx.lflow_cache, &usage);
             memory_report(&usage);
             simap_destroy(&usage);
         }
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index c2c8218..a8f6a7e 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -413,6 +413,7 @@ expr_from_node(const struct ovs_list *node)
 
 void expr_format(const struct expr *, struct ds *);
 void expr_print(const struct expr *);
+size_t expr_size(const struct expr *);
 struct expr *expr_parse(struct lexer *, const struct shash *symtab,
                         const struct shash *addr_sets,
                         const struct shash *port_groups,
@@ -479,6 +480,7 @@ uint32_t expr_to_matches(const struct expr *,
 void expr_matches_destroy(struct hmap *matches);
 void expr_matches_prepare(struct hmap *matches, uint32_t conj_id_ofs);
 void expr_matches_print(const struct hmap *matches, FILE *);
+size_t expr_matches_size(const struct hmap *matches);
 
 /* Action parsing helper. */
 
diff --git a/lib/expr.c b/lib/expr.c
index 356e6fe..082ce7e 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -471,6 +471,36 @@ expr_print(const struct expr *e)
     ds_destroy(&output);
 }
 
+/* Expr Size. */
+size_t
+expr_size(const struct expr *expr) {
+    size_t total_sz = sizeof *expr;
+    const struct expr *subexpr;
+
+    switch (expr->type) {
+    case EXPR_T_CMP:
+        return total_sz + (expr->cmp.symbol->width
+               ? 0
+               : strlen(expr->cmp.string));
+
+    case EXPR_T_AND:
+    case EXPR_T_OR:
+        LIST_FOR_EACH (subexpr, node, &expr->andor) {
+            total_sz += expr_size(subexpr);
+        }
+        return total_sz;
+
+    case EXPR_T_BOOLEAN:
+        return total_sz;
+
+    case EXPR_T_CONDITION:
+        return total_sz + strlen(expr->cond.string);
+
+    default:
+        OVS_NOT_REACHED();
+    }
+}
+
 /* Parsing. */
 
 #define MAX_PAREN_DEPTH 100
@@ -3162,6 +3192,18 @@ expr_matches_destroy(struct hmap *matches)
     hmap_destroy(matches);
 }
 
+size_t
+expr_matches_size(const struct hmap *matches)
+{
+    size_t total_size = 0;
+
+    const struct expr_match *m;
+    HMAP_FOR_EACH (m, hmap_node, matches) {
+        total_size += sizeof *m + m->allocated * sizeof *m->conjunctions;
+    }
+    return total_size;
+}
+
 /* Prints a representation of the 'struct expr_match'es in 'matches' to
  * 'stream'. */
 void

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to