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

Kevin Traynor ktraynor at redhat.com
Mon Nov 1 14:42:04 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>

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

GH actions:
https://github.com/kevintraynor/ovs/actions/runs/1408299444
---
 lib/dpif-netdev.c | 118 +++++++++++++++++++++++++++++-----------------
 tests/alb.at      |  67 +++++++++-----------------
 2 files changed, 97 insertions(+), 88 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index b078c2da5..36230873e 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,42 +4150,16 @@ 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"%%, "
@@ -4193,8 +4168,7 @@ set_pmd_auto_lb(struct dp_netdev *dp, bool always_log)
                        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..444711129 100644
--- a/tests/alb.at
+++ b/tests/alb.at
@@ -37,50 +37,17 @@ 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 open_vswitch . other_config:pmd-auto-lb="true"],
-                   [], [], [DUMMY_NUMA])
-OVS_WAIT_UNTIL([grep "PMD auto load balance is enabled" ovs-vswitchd.log])
-
-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"])
-
-OVS_VSWITCHD_STOP
-AT_CLEANUP
-
-AT_SETUP([ALB - min num PMD/RxQ])
-OVS_VSWITCHD_START([add-port br0 p0 \
-                    -- set Interface p0 type=dummy-pmd options:n_rxq=2 \
+                    -- 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])
-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])
 
-# Add more PMD
-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])
-
-# Add one more rxq to have 2 rxq on a PMD
-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"])
-
-# Reduce PMD
-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"])
-
-# 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"])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is enabled"])
 
 OVS_VSWITCHD_STOP
@@ -89,6 +56,6 @@ AT_CLEANUP
 AT_SETUP([ALB - PMD/RxQ assignment type])
 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])
@@ -99,5 +66,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 +78,22 @@ 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_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-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-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="true"])
+OVS_WAIT_UNTIL([tail -n +$LINENUM ovs-vswitchd.log | grep "PMD auto load balance is enabled"])
+
+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"])
 
 OVS_VSWITCHD_STOP
-- 
2.31.1



More information about the dev mailing list