On 10/05/2022 17:00, Mike Pattrick wrote:
On Sat, May 7, 2022 at 1:10 PM lin huang <mit...@outlook.com> wrote:

The ALB parameters should never be negative.
So it's to use smap_get_ulonglong() or smap_get_uint() to get it properly.

Fixes: 5bf84282482a ("Adding support for PMD auto load balancing")
Signed-off-by: Lin Huang linhu...@ruijie.com.cn
---
  lib/dpif-netdev.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 88a5459cc..2e4be433c 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4879,8 +4879,8 @@ dpif_netdev_set_config(struct dpif *dpif, const struct 
smap *other_config)

      struct pmd_auto_lb *pmd_alb = &dp->pmd_alb;

-    rebalance_intvl = smap_get_int(other_config, "pmd-auto-lb-rebal-interval",
-                                   ALB_REBALANCE_INTERVAL);
+    rebalance_intvl = smap_get_ullong(other_config, 
"pmd-auto-lb-rebal-interval",
+                                      ALB_REBALANCE_INTERVAL);

      /* Input is in min, convert it to msec. */
      rebalance_intvl =
@@ -4893,9 +4893,9 @@ dpif_netdev_set_config(struct dpif *dpif, const struct 
smap *other_config)
          log_autolb = true;
      }

-    rebalance_improve = smap_get_int(other_config,
-                                     "pmd-auto-lb-improvement-threshold",
-                                     ALB_IMPROVEMENT_THRESHOLD);
+    rebalance_improve = smap_get_uint(other_config,
+                                      "pmd-auto-lb-improvement-threshold",
+                                      ALB_IMPROVEMENT_THRESHOLD);
      if (rebalance_improve > 100) {
          rebalance_improve = ALB_IMPROVEMENT_THRESHOLD;
      }
@@ -4906,8 +4906,8 @@ 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",
-                                  ALB_LOAD_THRESHOLD);
+    rebalance_load = smap_get_uint(other_config, "pmd-auto-lb-load-threshold",
+                                   ALB_LOAD_THRESHOLD);
      if (rebalance_load > 100) {
          rebalance_load = ALB_LOAD_THRESHOLD;
      }

Shouldn't rebalance_load and rebalance_improve be defined as uint
types? The truncation from int to char may produce unintended results.


That's a fair point. In that case, how about: store from smap_get_uint() to local uint, check/adjust to valid range (0-100), then store/atomic_store into pmd_alb struct member with a uint8_t cast to avoid any -Wconversion warnings. Update VLOG formatting. Sounds ok?

Cheers,
M

--
2.27.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to