On 28/04/2022 17:12, lin huang wrote:
Hi Kevin,
Hi Lin,
I don't see this issue locally. Not sure what I am missing.
# /root/ovs/utilities/ovs-vsctl --no-wait set open_vSwitch .
other_config:pmd-auto-lb-rebal-interval="-0"
# 2022-04-28T14:01:07Z|00359|dpif_netdev|INFO|PMD auto load balance
interval set to 1 mins
However, we can add a guard for other negative numbers for the ALB params.
What do you think?
I try it again using the latest version.I don't see this issue locally too.
I'm sorry. Maybe the problem is caused by the fact that I am using an older
version.
ok, cool. I don't think the interval part has changed much since it was
introduced in OVS 2.11, so perhaps there was some local edits or
something else. Thanks for checking.
# 2022-04-28T14:01:07Z|00359|dpif_netdev|INFO|PMD auto load balance
interval set to 1 mins
However, we can add a guard for other negative numbers for the ALB params.
What do you think?
When i set a negative value (-1) to pmd-auto-lb-rebal-interval.
The actual value set to pmd-auto-lb-rebal-interval is 307445734561824.
e.g.
2022-04-28T15:29:14.741Z|00053|dpif_netdev|INFO|PMD auto load balance interval
set to 307445734561824 mins
So, we can add a guard for the negative number for the ALB params.
Here is the demo code:
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9f35713ef..39845bf62 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4777,7 +4777,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct
smap *other_config)
DEFAULT_EM_FLOW_INSERT_INV_PROB);
uint32_t insert_min, cur_min;
uint32_t tx_flush_interval, cur_tx_flush_interval;
- uint64_t rebalance_intvl;
+ int rebalance_intvl;
uint8_t rebalance_load, cur_rebalance_load;
uint8_t rebalance_improve;
bool log_autolb = false;
@@ -4884,17 +4884,19 @@ dpif_netdev_set_config(struct dpif *dpif, const struct
smap *other_config)
ALB_REBALANCE_INTERVAL);
/* Input is in min, convert it to msec. */
- rebalance_intvl =
- rebalance_intvl ? rebalance_intvl * MIN_TO_MSEC : MIN_TO_MSEC;
+ if (rebalance_intvl <= 0) {
+ rebalance_intvl = ALB_REBALANCE_INTERVAL;
+ }
For rebal-interval, it might be fine to keep the existing code as is,
but use smap_get_ullong() and add a check/reset to default if the value
exceeds the max permitted as per vswitchd.xml [0]. This is similar as to
what is done for emc-insert-inv-prob.
The value wasn't hard checked for max in the past as there isn't an
issue with having it larger. The only thing is that a user might have
entered a negative or too big value (>2 weeks between rebalances) by
mistake. So checking might be helpful to guard against that scenario.
[0]
https://github.com/openvswitch/ovs/blob/master/vswitchd/vswitch.xml#L719-L720
+ rebalance_intvl *= MIN_TO_MSEC;
if (pmd_alb->rebalance_intvl != rebalance_intvl) {
pmd_alb->rebalance_intvl = rebalance_intvl;
VLOG_INFO("PMD auto load balance interval set to "
- "%"PRIu64" mins\n", rebalance_intvl / MIN_TO_MSEC);
+ "%"PRIu32" mins\n", rebalance_intvl / MIN_TO_MSEC);
log_autolb = true;
}
- rebalance_improve = smap_get_int(other_config,
+ rebalance_improve = smap_get_uint(other_config,
"pmd-auto-lb-improvement-threshold",
ALB_IMPROVEMENT_THRESHOLD);
if (rebalance_improve > 100) {
@@ -4907,7 +4909,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct
smap *other_config)
log_autolb = true;
}
- rebalance_load = smap_get_int(other_config, "pmd-auto-lb-load-threshold",
+ rebalance_load = smap_get_uint(other_config, "pmd-auto-lb-load-threshold",
ALB_LOAD_THRESHOLD);
Changes for improvement-threshold and load-threshold look good.
thanks,
Kevin.
if (rebalance_load > 100) {
rebalance_load = ALB_LOAD_THRESHOLD;
I'm not sure if you saw the robot ci report for this patch, but it is not being
applied/tested because of below [0]. I see the same error locally.
The patch "[PATCH] dpif-netdev : Fix ALB rebalance interval zero value." Is
corrupted.
So the robot return error. I'll send a new patch later.
-----Original Message-----
From: dev <[email protected]> On Behalf Of Kevin Traynor
Sent: Thursday, April 28, 2022 10:23 PM
To: [email protected];
[email protected]; [email protected]
Cc: [email protected]
Subject: Re: [ovs-dev] [PATCH] dpif-netdev : Fix ALB rebalance interval zero
value.
Hi Lin,
On 20/04/2022 13:55, wrote:
When we set a special value "-0" to pmd-auto-lb-rebal-interval, its
value is 0 not 1.
e.g.
ovs-vsctl set open_vswitch .
other_config:pmd-auto-lb-rebal-interval="-0"
2022-04-20T11:31:44.987Z|00526|dpif_netdev|INFO|PMD auto load
balance
interval set to 0 mins
I don't see this issue locally. Not sure what I am missing.
# /root/ovs/utilities/ovs-vsctl --no-wait set open_vSwitch .
other_config:pmd-auto-lb-rebal-interval="-0"
# 2022-04-28T14:01:07Z|00359|dpif_netdev|INFO|PMD auto load balance
interval set to 1 mins
However, we can add a guard for other negative numbers for the ALB params.
What do you think?
So, fix pmd-auto-lb-rebal-interval's value to 1.
Fixes: 5bf84282482a ("Adding support for PMD auto load balancing")
Signed-off-by: Lin Huang <[email protected]>
I'm not sure if you saw the robot ci report for this patch, but it is not being
applied/tested because of below [0]. I see the same error locally.
[0]
Test-Label: apply-robot
Test-Status: fail
https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatchw
ork.ozlabs.org%2Fapi%2Fpatches%2F1621541%2F&data=05%7C01%7C%
7C789f2cb4338444d4bf8a08da2922a93e%7C84df9e7fe9f640afb435aaaaaaaa
aaaa%7C1%7C0%7C637867526137171838%7CUnknown%7CTWFpbGZsb3d8e
yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
7C3000%7C%7C%7C&sdata=QmHO9oLfCfBrlIaGheg7JfnWYgKWp3ziZL1P
6NlBCs8%3D&reserved=0
_apply and check: fail_
git-am:
error: corrupt patch at line 25
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at
0001 dpif-netdev : Fix ALB rebalance interval zero value.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
---
lib/dpif-netdev.c | 8 ++++----
tests/alb.at | 10 ++++++++++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
9f35713ef..6a5a9f740 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4882,10 +4882,10 @@ dpif_netdev_set_config(struct dpif *dpif,
const struct smap *other_config)
rebalance_intvl = smap_get_int(other_config,
"pmd-auto-lb-rebal-interval",
ALB_REBALANCE_INTERVAL);
-
- /* Input is in min, convert it to msec. */
- rebalance_intvl =
- rebalance_intvl ? rebalance_intvl * MIN_TO_MSEC :
MIN_TO_MSEC;
+ if (rebalance_intvl < ALB_REBALANCE_INTERVAL) {
+ rebalance_intvl = ALB_REBALANCE_INTERVAL;
+ }
+ rebalance_intvl *= MIN_TO_MSEC;
if (pmd_alb->rebalance_intvl != rebalance_intvl) {
pmd_alb->rebalance_intvl = rebalance_intvl; diff --git
a/tests/alb.at b/tests/alb.at index 2bef06f39..d105b72bf 100644
--- a/tests/alb.at
+++ b/tests/alb.at
@@ -197,6 +197,16 @@ 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])
+# 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 below min value
+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
OVS_VSWITCHD_STOP
--
2.27.0
_______________________________________________
dev mailing list
[email protected]
https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.o
penvswitch.org%2Fmailman%2Flistinfo%2Fovs-dev&data=05%7C01%7C%
7C789f2cb4338444d4bf8a08da2922a93e%7C84df9e7fe9f640afb435aaaaaaaa
aaaa%7C1%7C0%7C637867526137171838%7CUnknown%7CTWFpbGZsb3d8e
yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
7C3000%7C%7C%7C&sdata=I%2FdHC3kfHPDnyLHqLACBJm3GDbq6DajCG
ocj5WJ3P%2Bk%3D&reserved=0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
