On 07/05/2022 18:10, lin huang wrote:
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 [email protected]
Similar comment about title and sign-off as patch 1/2.
Fix looks good and thanks for the additional unit tests :) One small
comment on them below.
---
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 2e4be433c..b358b8b1c 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
@@ -4881,6 +4882,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 2bef06f39..17bb754ae 100644
--- a/tests/alb.at
+++ b/tests/alb.at
@@ -197,7 +197,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="50000"])
+CHECK_ALB_PARAM([interval], [1 mins], [+$LINENUM])
As it's checking above the max, might as well check the whole invalid
range with "20001", either as a replacement for this test or an
additional test.
thanks,
Kevin.
+
+# 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
