diff mbox series

[ovs-dev,3/3] upcall: Configure datapath min-revalidate-pps through ovs-vsctl.

Message ID 1563698063-28256-4-git-send-email-roid@mellanox.com
State Accepted
Commit e31ecf5885bdb578947296913177d10e9cb057d9
Headers show
Series be able to tune revalidator timing | expand

Commit Message

Roi Dayan July 21, 2019, 8:34 a.m. UTC 
From: Vlad Buslov <vladbu@mellanox.com>

This patch adds a new configuration option, "min-revalidate-pps" to the
Open_vSwitch "other-config" column. This sets minimum pps that flow must
have in order to be revalidated when revalidation duration exceeds half of
max-revalidator config variable.

Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Roi Dayan <roid@mellanox.com>
---
 ofproto/ofproto-dpif-upcall.c |  4 ++--
 ofproto/ofproto-provider.h    |  4 ++++
 ofproto/ofproto.c             |  9 +++++++++
 ofproto/ofproto.h             |  2 ++
 vswitchd/bridge.c             |  3 +++
 vswitchd/vswitch.xml          | 11 +++++++++++
 6 files changed, 31 insertions(+), 2 deletions(-)

Comments

0-day Robot July 21, 2019, 9:03 a.m. UTC | #1 
Bleep bloop.  Greetings Roi Dayan, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 80 characters long (recommended limit is 79)
#125 FILE: vswitchd/vswitch.xml:211:
          revalidation duration exceeds half of max-revalidator config variable.

Lines checked: 137, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Ben Pfaff Aug. 21, 2019, 10:41 p.m. UTC | #2 
I applied this series to master.  Thank you!

On Sun, Jul 21, 2019 at 11:34:23AM +0300, Roi Dayan wrote:
> From: Vlad Buslov <vladbu@mellanox.com>
> 
> This patch adds a new configuration option, "min-revalidate-pps" to the
> Open_vSwitch "other-config" column. This sets minimum pps that flow must
> have in order to be revalidated when revalidation duration exceeds half of
> max-revalidator config variable.
> 
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Acked-by: Roi Dayan <roid@mellanox.com>
> ---
>  ofproto/ofproto-dpif-upcall.c |  4 ++--
>  ofproto/ofproto-provider.h    |  4 ++++
>  ofproto/ofproto.c             |  9 +++++++++
>  ofproto/ofproto.h             |  2 ++
>  vswitchd/bridge.c             |  3 +++
>  vswitchd/vswitch.xml          | 11 +++++++++++
>  6 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 00c8e6ddcfaa..9e58a4dcf91b 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -2060,8 +2060,8 @@ should_revalidate(const struct udpif *udpif, uint64_t packets,
>      duration = now - used;
>      metric = duration / packets;
>  
> -    if (metric < 200) {
> -        /* The flow is receiving more than ~5pps, so keep it. */
> +    if (metric < 1000 / ofproto_min_revalidate_pps) {
> +        /* The flow is receiving more than min-revalidate-pps, so keep it. */
>          return true;
>      }
>      return false;
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 92b384448f27..6cc454371dc7 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -528,6 +528,10 @@ extern unsigned ofproto_max_idle;
>   * Revalidator timeout is a minimum of max_idle and max_revalidator values. */
>  extern unsigned ofproto_max_revalidator;
>  
> +/* Minimum pps that flow must have in order to be revalidated when revalidation
> + * duration exceeds half of max-revalidator config variable. */
> +extern unsigned ofproto_min_revalidate_pps;
> +
>  /* Number of upcall handler and revalidator threads. Only affects the
>   * ofproto-dpif implementation. */
>  extern size_t n_handlers, n_revalidators;
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 6f1d327ee87d..12758a3f7651 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -307,6 +307,7 @@ struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER;
>  unsigned ofproto_flow_limit = OFPROTO_FLOW_LIMIT_DEFAULT;
>  unsigned ofproto_max_idle = OFPROTO_MAX_IDLE_DEFAULT;
>  unsigned ofproto_max_revalidator = OFPROTO_MAX_REVALIDATOR_DEFAULT;
> +unsigned ofproto_min_revalidate_pps = OFPROTO_MIN_REVALIDATE_PPS_DEFAULT;
>  
>  size_t n_handlers, n_revalidators;
>  
> @@ -712,6 +713,14 @@ ofproto_set_max_revalidator(unsigned max_revalidator)
>      }
>  }
>  
> +/* Set minimum pps that flow must have in order to be revalidated when
> + * revalidation duration exceeds half of max-revalidator config variable. */
> +void
> +ofproto_set_min_revalidate_pps(unsigned min_revalidate_pps)
> +{
> +    ofproto_min_revalidate_pps = min_revalidate_pps ? min_revalidate_pps : 1;
> +}
> +
>  /* If forward_bpdu is true, the NORMAL action will forward frames with
>   * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is false,
>   * the NORMAL action will drop these frames. */
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 6e17fd317fbc..1977bc2b5c1c 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -309,6 +309,7 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
>  #define OFPROTO_FLOW_LIMIT_DEFAULT 200000
>  #define OFPROTO_MAX_IDLE_DEFAULT 10000 /* ms */
>  #define OFPROTO_MAX_REVALIDATOR_DEFAULT 500 /* ms */
> +#define OFPROTO_MIN_REVALIDATE_PPS_DEFAULT 5
>  
>  const char *ofproto_port_open_type(const struct ofproto *,
>                                     const char *port_type);
> @@ -337,6 +338,7 @@ void ofproto_set_bundle_idle_timeout(unsigned timeout);
>  void ofproto_set_flow_limit(unsigned limit);
>  void ofproto_set_max_idle(unsigned max_idle);
>  void ofproto_set_max_revalidator(unsigned max_revalidator);
> +void ofproto_set_min_revalidate_pps(unsigned min_revalidate_pps);
>  void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
>  void ofproto_set_mac_table_config(struct ofproto *, unsigned idle_time,
>                                    size_t max_entries);
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index be093af1d821..d921c4ef8d5f 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -604,6 +604,9 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
>      ofproto_set_max_revalidator(smap_get_int(&ovs_cfg->other_config,
>                                               "max-revalidator",
>                                               OFPROTO_MAX_REVALIDATOR_DEFAULT));
> +    ofproto_set_min_revalidate_pps(
> +        smap_get_int(&ovs_cfg->other_config, "min-revalidate-pps",
> +                     OFPROTO_MIN_REVALIDATE_PPS_DEFAULT));
>      ofproto_set_vlan_limit(smap_get_int(&ovs_cfg->other_config, "vlan-limit",
>                                         LEGACY_MAX_VLAN_HEADERS));
>      ofproto_set_bundle_idle_timeout(smap_get_int(&ovs_cfg->other_config,
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 533823c3d853..9a743c05b4bf 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -204,6 +204,17 @@
>          </p>
>        </column>
>  
> +      <column name="other_config" key="min-revalidate-pps"
> +              type='{"type": "integer", "minInteger": 1}'>
> +        <p>
> +          Set minimum pps that flow must have in order to be revalidated when
> +          revalidation duration exceeds half of max-revalidator config variable.
> +        </p>
> +        <p>
> +          The default is 5.
> +        </p>
> +      </column>
> +
>        <column name="other_config" key="hw-offload"
>                type='{"type": "boolean"}'>
>          <p>
> -- 
> 2.7.0
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 00c8e6ddcfaa..9e58a4dcf91b 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2060,8 +2060,8 @@  should_revalidate(const struct udpif *udpif, uint64_t packets,
     duration = now - used;
     metric = duration / packets;
 
-    if (metric < 200) {
-        /* The flow is receiving more than ~5pps, so keep it. */
+    if (metric < 1000 / ofproto_min_revalidate_pps) {
+        /* The flow is receiving more than min-revalidate-pps, so keep it. */
         return true;
     }
     return false;
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 92b384448f27..6cc454371dc7 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -528,6 +528,10 @@  extern unsigned ofproto_max_idle;
  * Revalidator timeout is a minimum of max_idle and max_revalidator values. */
 extern unsigned ofproto_max_revalidator;
 
+/* Minimum pps that flow must have in order to be revalidated when revalidation
+ * duration exceeds half of max-revalidator config variable. */
+extern unsigned ofproto_min_revalidate_pps;
+
 /* Number of upcall handler and revalidator threads. Only affects the
  * ofproto-dpif implementation. */
 extern size_t n_handlers, n_revalidators;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 6f1d327ee87d..12758a3f7651 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -307,6 +307,7 @@  struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER;
 unsigned ofproto_flow_limit = OFPROTO_FLOW_LIMIT_DEFAULT;
 unsigned ofproto_max_idle = OFPROTO_MAX_IDLE_DEFAULT;
 unsigned ofproto_max_revalidator = OFPROTO_MAX_REVALIDATOR_DEFAULT;
+unsigned ofproto_min_revalidate_pps = OFPROTO_MIN_REVALIDATE_PPS_DEFAULT;
 
 size_t n_handlers, n_revalidators;
 
@@ -712,6 +713,14 @@  ofproto_set_max_revalidator(unsigned max_revalidator)
     }
 }
 
+/* Set minimum pps that flow must have in order to be revalidated when
+ * revalidation duration exceeds half of max-revalidator config variable. */
+void
+ofproto_set_min_revalidate_pps(unsigned min_revalidate_pps)
+{
+    ofproto_min_revalidate_pps = min_revalidate_pps ? min_revalidate_pps : 1;
+}
+
 /* If forward_bpdu is true, the NORMAL action will forward frames with
  * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is false,
  * the NORMAL action will drop these frames. */
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 6e17fd317fbc..1977bc2b5c1c 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -309,6 +309,7 @@  int ofproto_port_dump_done(struct ofproto_port_dump *);
 #define OFPROTO_FLOW_LIMIT_DEFAULT 200000
 #define OFPROTO_MAX_IDLE_DEFAULT 10000 /* ms */
 #define OFPROTO_MAX_REVALIDATOR_DEFAULT 500 /* ms */
+#define OFPROTO_MIN_REVALIDATE_PPS_DEFAULT 5
 
 const char *ofproto_port_open_type(const struct ofproto *,
                                    const char *port_type);
@@ -337,6 +338,7 @@  void ofproto_set_bundle_idle_timeout(unsigned timeout);
 void ofproto_set_flow_limit(unsigned limit);
 void ofproto_set_max_idle(unsigned max_idle);
 void ofproto_set_max_revalidator(unsigned max_revalidator);
+void ofproto_set_min_revalidate_pps(unsigned min_revalidate_pps);
 void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
 void ofproto_set_mac_table_config(struct ofproto *, unsigned idle_time,
                                   size_t max_entries);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index be093af1d821..d921c4ef8d5f 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -604,6 +604,9 @@  bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
     ofproto_set_max_revalidator(smap_get_int(&ovs_cfg->other_config,
                                              "max-revalidator",
                                              OFPROTO_MAX_REVALIDATOR_DEFAULT));
+    ofproto_set_min_revalidate_pps(
+        smap_get_int(&ovs_cfg->other_config, "min-revalidate-pps",
+                     OFPROTO_MIN_REVALIDATE_PPS_DEFAULT));
     ofproto_set_vlan_limit(smap_get_int(&ovs_cfg->other_config, "vlan-limit",
                                        LEGACY_MAX_VLAN_HEADERS));
     ofproto_set_bundle_idle_timeout(smap_get_int(&ovs_cfg->other_config,
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 533823c3d853..9a743c05b4bf 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -204,6 +204,17 @@ 
         </p>
       </column>
 
+      <column name="other_config" key="min-revalidate-pps"
+              type='{"type": "integer", "minInteger": 1}'>
+        <p>
+          Set minimum pps that flow must have in order to be revalidated when
+          revalidation duration exceeds half of max-revalidator config variable.
+        </p>
+        <p>
+          The default is 5.
+        </p>
+      </column>
+
       <column name="other_config" key="hw-offload"
               type='{"type": "boolean"}'>
         <p>