Re: [ovs-dev] [PATCH ovn v2 09/10] lflow-cache: Make maximum number of cache entries configurable.

2021-02-08 Thread Numan Siddique 
On Thu, Feb 4, 2021 at 6:57 PM Dumitru Ceara  wrote:
>
> Add a new OVS external-id, "ovn-limit-lflow-cache", through which users
> can specify the maximum size of the ovn-controller logical flow cache.
>
> To maintain backwards compatibility the default behavior is to not
> enforce any limit on the size of the cache.
>
> When the cache becomes full, the rule is to prefer more "important"
> cache entries over less "important" ones.  That is, evict entries of
> type LCACHE_T_CONJ_ID if there's no room to add an entry of type
> LCACHE_T_EXPR.  Similarly, evict entries of type LCACHE_T_CONJ_ID or
> LCACHE_T_EXPR if there's no room to add an entry of type
> LCACHE_T_MATCHES.
>
> Signed-off-by: Dumitru Ceara 

Acked-by: Numan Siddique 

I think the above rules description can also be added as a comment in the
function lflow_cache_make_room__().

Thanks
Numan


> ---
>  NEWS|3 +
>  controller/chassis.c|   23 -
>  controller/lflow-cache.c|   48 +-
>  controller/lflow-cache.h|2 -
>  controller/ovn-controller.8.xml |   16 ++
>  controller/ovn-controller.c |8 +++
>  controller/test-lflow-cache.c   |   12 +++--
>  tests/ovn-lflow-cache.at|  104 
> +++
>  8 files changed, 206 insertions(+), 10 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 2044671..6e10557 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -16,6 +16,9 @@ Post-v20.12.0
>- Add a new option to Load_Balancer.options, "hairpin_snat_ip", to allow
>  users to explicitly select which source IP should be used for load
>  balancer hairpin traffic.
> +  - 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 v20.12.0 - 18 Dec 2020
>  --
> diff --git a/controller/chassis.c b/controller/chassis.c
> index b4d4b0e..c66837a 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -49,6 +49,7 @@ struct ovs_chassis_cfg {
>  const char *monitor_all;
>  const char *chassis_macs;
>  const char *enable_lflow_cache;
> +const char *limit_lflow_cache;
>
>  /* Set of encap types parsed from the 'ovn-encap-type' external-id. */
>  struct sset encap_type_set;
> @@ -135,6 +136,12 @@ get_enable_lflow_cache(const struct smap *ext_ids)
>  }
>
>  static const char *
> +get_limit_lflow_cache(const struct smap *ext_ids)
> +{
> +return smap_get_def(ext_ids, "ovn-limit-lflow-cache", "");
> +}
> +
> +static const char *
>  get_encap_csum(const struct smap *ext_ids)
>  {
>  return smap_get_def(ext_ids, "ovn-encap-csum", "true");
> @@ -256,6 +263,7 @@ chassis_parse_ovs_config(const struct 
> ovsrec_open_vswitch_table *ovs_table,
>  ovs_cfg->monitor_all = get_monitor_all(&cfg->external_ids);
>  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);
>
>  if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) 
> {
>  return false;
> @@ -283,13 +291,16 @@ chassis_build_other_config(struct smap *config, const 
> char *bridge_mappings,
> const char *datapath_type, const char 
> *cms_options,
> const char *monitor_all, const char *chassis_macs,
> const char *iface_types,
> -   const char *enable_lflow_cache, bool is_interconn)
> +   const char *enable_lflow_cache,
> +   const char *limit_lflow_cache,
> +   bool is_interconn)
>  {
>  smap_replace(config, "ovn-bridge-mappings", bridge_mappings);
>  smap_replace(config, "datapath-type", datapath_type);
>  smap_replace(config, "ovn-cms-options", cms_options);
>  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, "iface-types", iface_types);
>  smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
>  smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
> @@ -305,6 +316,7 @@ chassis_other_config_changed(const char *bridge_mappings,
>   const char *monitor_all,
>   const char *chassis_macs,
>   const char *enable_lflow_cache,
> + const char *limit_lflow_cache,
>   const struct ds *iface_types,
>   bool is_interconn,
>   const struct sbrec_chassis *chassis_rec)
> @@ -344,6 +356,13 @@ chassis_other_config_changed(cons

[ovs-dev] [PATCH ovn v2 09/10] lflow-cache: Make maximum number of cache entries configurable.

2021-02-04 Thread Dumitru Ceara 
Add a new OVS external-id, "ovn-limit-lflow-cache", through which users
can specify the maximum size of the ovn-controller logical flow cache.

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

When the cache becomes full, the rule is to prefer more "important"
cache entries over less "important" ones.  That is, evict entries of
type LCACHE_T_CONJ_ID if there's no room to add an entry of type
LCACHE_T_EXPR.  Similarly, evict entries of type LCACHE_T_CONJ_ID or
LCACHE_T_EXPR if there's no room to add an entry of type
LCACHE_T_MATCHES.

Signed-off-by: Dumitru Ceara 
---
 NEWS|3 +
 controller/chassis.c|   23 -
 controller/lflow-cache.c|   48 +-
 controller/lflow-cache.h|2 -
 controller/ovn-controller.8.xml |   16 ++
 controller/ovn-controller.c |8 +++
 controller/test-lflow-cache.c   |   12 +++--
 tests/ovn-lflow-cache.at|  104 +++
 8 files changed, 206 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 2044671..6e10557 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,9 @@ Post-v20.12.0
   - Add a new option to Load_Balancer.options, "hairpin_snat_ip", to allow
 users to explicitly select which source IP should be used for load
 balancer hairpin traffic.
+  - 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 v20.12.0 - 18 Dec 2020
 --
diff --git a/controller/chassis.c b/controller/chassis.c
index b4d4b0e..c66837a 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -49,6 +49,7 @@ struct ovs_chassis_cfg {
 const char *monitor_all;
 const char *chassis_macs;
 const char *enable_lflow_cache;
+const char *limit_lflow_cache;
 
 /* Set of encap types parsed from the 'ovn-encap-type' external-id. */
 struct sset encap_type_set;
@@ -135,6 +136,12 @@ get_enable_lflow_cache(const struct smap *ext_ids)
 }
 
 static const char *
+get_limit_lflow_cache(const struct smap *ext_ids)
+{
+return smap_get_def(ext_ids, "ovn-limit-lflow-cache", "");
+}
+
+static const char *
 get_encap_csum(const struct smap *ext_ids)
 {
 return smap_get_def(ext_ids, "ovn-encap-csum", "true");
@@ -256,6 +263,7 @@ chassis_parse_ovs_config(const struct 
ovsrec_open_vswitch_table *ovs_table,
 ovs_cfg->monitor_all = get_monitor_all(&cfg->external_ids);
 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);
 
 if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) {
 return false;
@@ -283,13 +291,16 @@ chassis_build_other_config(struct smap *config, const 
char *bridge_mappings,
const char *datapath_type, const char *cms_options,
const char *monitor_all, const char *chassis_macs,
const char *iface_types,
-   const char *enable_lflow_cache, bool is_interconn)
+   const char *enable_lflow_cache,
+   const char *limit_lflow_cache,
+   bool is_interconn)
 {
 smap_replace(config, "ovn-bridge-mappings", bridge_mappings);
 smap_replace(config, "datapath-type", datapath_type);
 smap_replace(config, "ovn-cms-options", cms_options);
 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, "iface-types", iface_types);
 smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
 smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
@@ -305,6 +316,7 @@ chassis_other_config_changed(const char *bridge_mappings,
  const char *monitor_all,
  const char *chassis_macs,
  const char *enable_lflow_cache,
+ const char *limit_lflow_cache,
  const struct ds *iface_types,
  bool is_interconn,
  const struct sbrec_chassis *chassis_rec)
@@ -344,6 +356,13 @@ chassis_other_config_changed(const char *bridge_mappings,
 return true;
 }
 
+const char *chassis_limit_lflow_cache =
+get_limit_lflow_cache(&chassis_rec->other_config);
+
+if (strcmp(limit_lflow_cache, chassis_limit_lflow_cache)) {
+return true;
+}
+
 const char *chassis_mac_mappings =
 get_chassis_mac_mappings(&chassis_rec->other_config);
 if (strcmp(chassis_macs, chassis_mac_mapp