diff mbox series

[ovs-dev,2/2] dpif-netdev: Fix ALB 'rebalance_intvl' max hard limit.

Message ID MEYP282MB3499AE42BCD2853B76BA81ECCDD79@MEYP282MB3499.AUSP282.PROD.OUTLOOK.COM
State Accepted
Commit ba462b358929e249448254d92e9058c6b4f3de61
Headers show
Series [ovs-dev,1/2] dpif-netdev: Fix ALB parameters type mismatch. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

lin huang May 24, 2022, 1:48 p.m. UTC
From: Lin Huang <linhuang@ruijie.com.cn>

Currently the pmd-auto-lb-rebal-interval's value was not been
checked properly.

It maybe a negative, or too big value (>2 weeks between rebalances),
which will be lead to a big unsigned value. So reset it to default
if the value exceeds the max permitted as described in vswitchd.xml.

Fixes: 5bf84282482a ("Adding support for PMD auto load balancing")
Signed-off-by: Lin Huang <linhuang@ruijie.com.cn>
---
 lib/dpif-netdev.c |  6 +++++-
 tests/alb.at      | 20 +++++++++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

Comments

Kevin Traynor May 24, 2022, 4:37 p.m. UTC | #1
On 24/05/2022 14:48, miterv@outlook.com wrote:
> From: Lin Huang <linhuang@ruijie.com.cn>
> 
> Currently the pmd-auto-lb-rebal-interval's value was not been
> checked properly.
> 
> It maybe a negative, or too big value (>2 weeks between rebalances),
> which will be lead to a big unsigned value. So reset it to default
> if the value exceeds the max permitted as described in vswitchd.xml.
> 
> Fixes: 5bf84282482a ("Adding support for PMD auto load balancing")
> Signed-off-by: Lin Huang <linhuang@ruijie.com.cn>

Acked-by: Kevin Traynor <ktraynor@redhat.com>

> ---
>   lib/dpif-netdev.c |  6 +++++-
>   tests/alb.at      | 20 +++++++++++++++++++-
>   2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 95b154185..a69b570f6 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -93,7 +93,8 @@ VLOG_DEFINE_THIS_MODULE(dpif_netdev);
>   /* Auto Load Balancing Defaults */
>   #define ALB_IMPROVEMENT_THRESHOLD    25
>   #define ALB_LOAD_THRESHOLD           95
> -#define ALB_REBALANCE_INTERVAL       1 /* 1 Min */
> +#define ALB_REBALANCE_INTERVAL       1     /* 1 Min */
> +#define MAX_ALB_REBALANCE_INTERVAL   20000 /* 20000 Min */
>   #define MIN_TO_MSEC                  60000
>   
>   #define FLOW_DUMP_MAX_BATCH 50
> @@ -4883,6 +4884,9 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
>       rebalance_intvl = smap_get_ullong(other_config,
>                                         "pmd-auto-lb-rebal-interval",
>                                         ALB_REBALANCE_INTERVAL);
> +    if (rebalance_intvl > MAX_ALB_REBALANCE_INTERVAL) {
> +        rebalance_intvl = ALB_REBALANCE_INTERVAL;
> +    }
>   
>       /* Input is in min, convert it to msec. */
>       rebalance_intvl =
> diff --git a/tests/alb.at b/tests/alb.at
> index 0036bd1f2..922185d61 100644
> --- a/tests/alb.at
> +++ b/tests/alb.at
> @@ -243,7 +243,25 @@ get_log_next_line_num
>   AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="0"])
>   CHECK_ALB_PARAM([interval], [1 mins], [+$LINENUM])
>   
> -# No check for above max as it is only a documented max value and not a hard limit
> +# Set new value
> +get_log_next_line_num
> +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="100"])
> +CHECK_ALB_PARAM([interval], [100 mins], [+$LINENUM])
> +
> +# Set above max value
> +get_log_next_line_num
> +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="20001"])
> +CHECK_ALB_PARAM([interval], [1 mins], [+$LINENUM])
> +
> +# Set new value
> +get_log_next_line_num
> +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="1000"])
> +CHECK_ALB_PARAM([interval], [1000 mins], [+$LINENUM])
> +
> +# Set Negative value
> +get_log_next_line_num
> +AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="-1"])
> +CHECK_ALB_PARAM([interval], [1 mins], [+$LINENUM])
>   
>   OVS_VSWITCHD_STOP
>   AT_CLEANUP
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 95b154185..a69b570f6 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -93,7 +93,8 @@  VLOG_DEFINE_THIS_MODULE(dpif_netdev);
 /* Auto Load Balancing Defaults */
 #define ALB_IMPROVEMENT_THRESHOLD    25
 #define ALB_LOAD_THRESHOLD           95
-#define ALB_REBALANCE_INTERVAL       1 /* 1 Min */
+#define ALB_REBALANCE_INTERVAL       1     /* 1 Min */
+#define MAX_ALB_REBALANCE_INTERVAL   20000 /* 20000 Min */
 #define MIN_TO_MSEC                  60000
 
 #define FLOW_DUMP_MAX_BATCH 50
@@ -4883,6 +4884,9 @@  dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
     rebalance_intvl = smap_get_ullong(other_config,
                                       "pmd-auto-lb-rebal-interval",
                                       ALB_REBALANCE_INTERVAL);
+    if (rebalance_intvl > MAX_ALB_REBALANCE_INTERVAL) {
+        rebalance_intvl = ALB_REBALANCE_INTERVAL;
+    }
 
     /* Input is in min, convert it to msec. */
     rebalance_intvl =
diff --git a/tests/alb.at b/tests/alb.at
index 0036bd1f2..922185d61 100644
--- a/tests/alb.at
+++ b/tests/alb.at
@@ -243,7 +243,25 @@  get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="0"])
 CHECK_ALB_PARAM([interval], [1 mins], [+$LINENUM])
 
-# No check for above max as it is only a documented max value and not a hard limit
+# Set new value
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="100"])
+CHECK_ALB_PARAM([interval], [100 mins], [+$LINENUM])
+
+# Set above max value
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="20001"])
+CHECK_ALB_PARAM([interval], [1 mins], [+$LINENUM])
+
+# Set new value
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="1000"])
+CHECK_ALB_PARAM([interval], [1000 mins], [+$LINENUM])
+
+# Set Negative value
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb-rebal-interval="-1"])
+CHECK_ALB_PARAM([interval], [1 mins], [+$LINENUM])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP