[ovs-dev] [PATCH v3] dpif-netdev: Sync PMD ALB state with user commands.

Kevin Traynor ktraynor at redhat.com
Tue Nov 2 18:10:21 UTC 2021


Previously, when a user enabled PMD auto load balancer with
pmd-auto-lb="true", some conditions such as number of PMDs/RxQs
that were required for a rebalance to take place were checked.

If the configuration meant that a rebalance would not take place
then PMD ALB was logged as 'disabled' and not run.

Later, if the PMD/RxQ configuration changed whereby a rebalance
could be effective, PMD ALB was logged as 'enabled' and would run at
the appropriate time.

This worked ok from a functional view but it is unintuitive for the
user reading the logs.

e.g. with one PMD (PMD ALB would not be effective)

User enables ALB, but logs say it is disabled because it won't run.
$ ovs-vsctl set open_vSwitch . other_config:pmd-auto-lb="true"
|dpif_netdev|INFO|PMD auto load balance is disabled

No dry run takes place.

Add more PMDs (PMD ALB may be effective).
$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=50
|dpif_netdev|INFO|PMD auto load balance is enabled ...

Dry run takes place.
|dpif_netdev|DBG|PMD auto load balance performing dry run.

A better approach is to simply reflect back the user enable/disable
state in the logs and deal with whether the rebalance will be effective
when needed. That is the approach taken in this patch.

To cut down on unneccessary work, some basic checks are also made before
starting a PMD ALB dry run and debug logs can indicate this to the user.

e.g. with one PMD (PMD ALB would not be effective)

User enables ALB, and logs confirm the user has enabled it.
$ ovs-vsctl set open_vSwitch . other_config:pmd-auto-lb="true"
|dpif_netdev|INFO|PMD auto load balance is enabled...

No dry run takes place.
|dpif_netdev|DBG|PMD auto load balance nothing to do, not enough non-isolated PMDs or RxQs.

Add more PMDs (PMD ALB may be effective).
$ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=50

Dry run takes place.
|dpif_netdev|DBG|PMD auto load balance performing dry run.

Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
Acked-by: Sunil Pai G <sunil.pai.g at intel.com>
Reviewed-by: David Marchand <david.marchand at redhat.com>
Acked-by: Gaetan Rivet <grive at u256.net>

---
v3:
- add minor fixes from v2 that were not committed
- add unit tests for dry run config checking
- Keeping Acks as there is just additional UTs added here (Let me know if any issue with that)

v2:
- minor: punctuation change for one log and one indent fix
- Add Ack/Review tags

GH actions:
https://github.com/kevintraynor/ovs/actions/runs/1413417595
---
 lib/dpif-netdev.c | 120 +++++++++++++++++++++++++++++-----------------
 tests/alb.at      |  91 +++++++++++++++++++++++++++--------
 2 files changed, 145 insertions(+), 66 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b078c2da5..21b7ee487 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -217,5 +217,6 @@ struct dp_meter {
 
 struct pmd_auto_lb {
-    bool auto_lb_requested;     /* Auto load balancing requested by user. */
+    bool do_dry_run;
+    bool recheck_config;
     bool is_enabled;            /* Current status of Auto load balancing. */
     uint64_t rebalance_intvl;
@@ -4149,52 +4150,25 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops,
 /* Enable or Disable PMD auto load balancing. */
 static void
-set_pmd_auto_lb(struct dp_netdev *dp, bool always_log)
+set_pmd_auto_lb(struct dp_netdev *dp, bool state, bool always_log)
 {
-    unsigned int cnt = 0;
-    struct dp_netdev_pmd_thread *pmd;
     struct pmd_auto_lb *pmd_alb = &dp->pmd_alb;
-    uint8_t rebalance_load_thresh;
 
-    bool enable_alb = false;
-    bool multi_rxq = false;
-    enum sched_assignment_type pmd_rxq_assign_type = dp->pmd_rxq_assign_type;
-
-    /* Ensure that there is at least 2 non-isolated PMDs and
-     * one of them is polling more than one rxq. */
-    CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
-        if (pmd->core_id == NON_PMD_CORE_ID || pmd->isolated) {
-            continue;
-        }
-
-        if (hmap_count(&pmd->poll_list) > 1) {
-            multi_rxq = true;
-        }
-        if (cnt && multi_rxq) {
-                enable_alb = true;
-                break;
-        }
-        cnt++;
-    }
-
-    /* Enable auto LB if requested and not using roundrobin assignment. */
-    enable_alb = enable_alb && pmd_rxq_assign_type != SCHED_ROUNDROBIN &&
-                    pmd_alb->auto_lb_requested;
-
-    if (pmd_alb->is_enabled != enable_alb || always_log) {
-        pmd_alb->is_enabled = enable_alb;
+    if (pmd_alb->is_enabled != state || always_log) {
+        pmd_alb->is_enabled = state;
         if (pmd_alb->is_enabled) {
+            uint8_t rebalance_load_thresh;
+
             atomic_read_relaxed(&pmd_alb->rebalance_load_thresh,
                                 &rebalance_load_thresh);
-            VLOG_INFO("PMD auto load balance is enabled "
+            VLOG_INFO("PMD auto load balance is enabled, "
                       "interval %"PRIu64" mins, "
                       "pmd load threshold %"PRIu8"%%, "
-                      "improvement threshold %"PRIu8"%%",
+                      "improvement threshold %"PRIu8"%%.",
                        pmd_alb->rebalance_intvl / MIN_TO_MSEC,
                        rebalance_load_thresh,
                        pmd_alb->rebalance_improve_thresh);
-
         } else {
             pmd_alb->rebalance_poll_timer = 0;
-            VLOG_INFO("PMD auto load balance is disabled");
+            VLOG_INFO("PMD auto load balance is disabled.");
         }
     }
@@ -4317,10 +4291,4 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
 
     struct pmd_auto_lb *pmd_alb = &dp->pmd_alb;
-    bool cur_rebalance_requested = pmd_alb->auto_lb_requested;
-    pmd_alb->auto_lb_requested = smap_get_bool(other_config, "pmd-auto-lb",
-                              false);
-    if (cur_rebalance_requested != pmd_alb->auto_lb_requested) {
-        log_autolb = true;
-    }
 
     rebalance_intvl = smap_get_int(other_config, "pmd-auto-lb-rebal-interval",
@@ -4364,5 +4332,8 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
         log_autolb = true;
     }
-    set_pmd_auto_lb(dp, log_autolb);
+
+    bool autolb_state = smap_get_bool(other_config, "pmd-auto-lb", false);
+
+    set_pmd_auto_lb(dp, autolb_state, log_autolb);
     return 0;
 }
@@ -5492,4 +5463,62 @@ sched_numa_list_variance(struct sched_numa_list *numa_list)
 }
 
+/*
+ * This function checks that some basic conditions needed for a rebalance to be
+ * effective are met. Such as Rxq scheduling assignment type, more than one
+ * PMD, more than 2 Rxqs on a PMD. If there was no reconfiguration change
+ * since the last check, it reuses the last result.
+ *
+ * It is not intended to be an inclusive check of every condition that may make
+ * a rebalance ineffective. It is done as a quick check so a full
+ * pmd_rebalance_dry_run() can be avoided when it is not needed.
+ */
+static bool
+pmd_reblance_dry_run_needed(struct dp_netdev *dp)
+    OVS_REQUIRES(dp->port_mutex)
+{
+    struct dp_netdev_pmd_thread *pmd;
+    struct pmd_auto_lb *pmd_alb = &dp->pmd_alb;
+    unsigned int cnt = 0;
+    bool multi_rxq = false;
+
+    /* Check if there was no reconfiguration since last check. */
+    if (!pmd_alb->recheck_config) {
+        if (!pmd_alb->do_dry_run) {
+            VLOG_DBG("PMD auto load balance nothing to do, "
+                     "no configuration changes since last check.");
+            return false;
+        }
+        return true;
+    }
+    pmd_alb->recheck_config = false;
+
+    /* Check for incompatible assignment type. */
+    if (dp->pmd_rxq_assign_type == SCHED_ROUNDROBIN) {
+        VLOG_DBG("PMD auto load balance nothing to do, "
+                 "pmd-rxq-assign=roundrobin assignment type configured.");
+        return pmd_alb->do_dry_run = false;
+    }
+
+    /* Check that there is at least 2 non-isolated PMDs and
+     * one of them is polling more than one rxq. */
+    CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
+        if (pmd->core_id == NON_PMD_CORE_ID || pmd->isolated) {
+            continue;
+        }
+
+        if (hmap_count(&pmd->poll_list) > 1) {
+            multi_rxq = true;
+        }
+        if (cnt && multi_rxq) {
+            return pmd_alb->do_dry_run = true;
+        }
+        cnt++;
+    }
+
+    VLOG_DBG("PMD auto load balance nothing to do, "
+             "not enough non-isolated PMDs or RxQs.");
+    return pmd_alb->do_dry_run = false;
+}
+
 static bool
 pmd_rebalance_dry_run(struct dp_netdev *dp)
@@ -5876,6 +5905,6 @@ reconfigure_datapath(struct dp_netdev *dp)
     reload_affected_pmds(dp);
 
-    /* Check if PMD Auto LB is to be enabled */
-    set_pmd_auto_lb(dp, false);
+    /* PMD ALB will need to recheck if dry run needed. */
+    dp->pmd_alb.recheck_config = true;
 }
 
@@ -6005,4 +6034,5 @@ dpif_netdev_run(struct dpif *dpif)
                 !dp_netdev_is_reconf_required(dp) &&
                 !ports_require_restart(dp) &&
+                pmd_reblance_dry_run_needed(dp) &&
                 pmd_rebalance_dry_run(dp)) {
                 VLOG_INFO("PMD auto load balance dry run. "
diff --git a/tests/alb.at b/tests/alb.at
index 903238fcb..fcc7b0bec 100644
--- a/tests/alb.at
+++ b/tests/alb.at
@@ -37,6 +37,6 @@ AT_CLEANUP
 AT_SETUP([ALB - enable/disable])
 OVS_VSWITCHD_START([add-port br0 p0 \
-                    -- set Interface p0 type=dummy-pmd options:n_rxq=3 \
-                    -- set Open_vSwitch . other_config:pmd-cpu-mask=3 \
+                    -- set Interface p0 type=dummy-pmd options:n_rxq=1 \
+                    -- set Open_vSwitch . other_config:pmd-cpu-mask=1 \
                     -- set open_vswitch . other_config:pmd-auto-lb="true"],
                    [], [], [DUMMY_NUMA])
@@ -58,29 +58,38 @@ OVS_VSWITCHD_START([add-port br0 p0 \
                     -- set Interface p0 type=dummy-pmd options:n_rxq=2 \
                     -- set Open_vSwitch . other_config:pmd-cpu-mask=1 \
-                    -- set open_vswitch . other_config:pmd-auto-lb="true"],
+                    -- set open_vswitch . other_config:pmd-auto-lb="true" \
+                    -- set open_vswitch . other_config:pmd-auto-lb-load-threshold=0],
                    [], [], [DUMMY_NUMA])
-OVS_WAIT_UNTIL([grep "PMD auto load balance is disabled" ovs-vswitchd.log])
+OVS_WAIT_UNTIL([grep "PMD auto load balance is enabled" ovs-vswitchd.log])
+AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg])
 
-# Add more PMD
+# 1 pmds 2 rxqs
+get_log_next_line_num
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance nothing to do, not enough non-isolated PMDs or RxQs."])
+
+# 2 pmds 2 rxq
+get_log_next_line_num
 AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x3])
-OVS_WAIT_UNTIL([grep "There are 2 pmd threads on numa node" ovs-vswitchd.log])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "There are 2 pmd threads on numa node"])
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance nothing to do, not enough non-isolated PMDs or RxQs."])
 
-# Add one more rxq to have 2 rxq on a PMD
+# 2 pmds 3 rxqs
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set interface p0 options:n_rxq=3])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is enabled"])
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."])
 
-# Reduce PMD
+# 1 pmds 3 rxqs
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x1])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "There are 1 pmd threads on numa node"])
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance nothing to do, not enough non-isolated PMDs or RxQs."])
 
-# Check logs when try to enable but min PMD/RxQ prevents
-get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="false"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
-get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
+# Same config as last time
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance nothing to do, no configuration changes since last check."])
 
 OVS_VSWITCHD_STOP
@@ -91,5 +100,6 @@ OVS_VSWITCHD_START([add-port br0 p0 \
                     -- set Interface p0 type=dummy-pmd options:n_rxq=3 \
                     -- set Open_vSwitch . other_config:pmd-cpu-mask=3 \
-                    -- set open_vswitch . other_config:pmd-auto-lb="true"],
+                    -- set open_vswitch . other_config:pmd-auto-lb="true" \
+                    -- set open_vswitch . other_config:pmd-auto-lb-load-threshold=0],
                    [], [], [DUMMY_NUMA])
 OVS_WAIT_UNTIL([grep "PMD auto load balance is enabled" ovs-vswitchd.log])
@@ -99,5 +109,11 @@ get_log_next_line_num
 AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=roundrobin])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "mode changed to: 'roundrobin'"])
+
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="false"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is enabled"])
 
 # Change back assignment type
@@ -105,16 +121,49 @@ get_log_next_line_num
 AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=cycles])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "mode changed to: 'cycles'"])
+
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="false"])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is enabled"])
 
-# Check logs when try to enable but assignment prevents
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=group])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "mode changed to: 'group'"])
+
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="false"])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is enabled"])
+
+# Enable debug and test dryrun config checks
+AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg])
+
+# Just to be sure it is still set to group
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=group])
+get_log_next_line_num
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."])
+
+get_log_next_line_num
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=cycles])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "mode changed to: 'cycles'"])
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."])
+
 get_log_next_line_num
 AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=roundrobin])
 OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "mode changed to: 'roundrobin'"])
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance nothing to do, pmd-rxq-assign=roundrobin assignment type configured."])
+
 get_log_next_line_num
-AT_CHECK([ovs-vsctl set open_vswitch . other_config:pmd-auto-lb="true"])
-OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is disabled"])
+AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:pmd-rxq-assign=group])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "mode changed to: 'group'"])
+ovs-appctl time/warp 60000 10000
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance performing dry run."])
 
 OVS_VSWITCHD_STOP
-- 
2.31.1



More information about the dev mailing list