[ovs-dev] [PATCH] dpif-netdev: Sync PMD ALB state with user commands.
David Marchand
david.marchand at redhat.com
Fri Oct 29 13:20:51 UTC 2021
On Fri, Oct 8, 2021 at 12:19 PM Kevin Traynor <ktraynor at redhat.com> wrote:
>
> 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>
>
Reviewed-by: David Marchand <david.marchand at redhat.com>
Two nits below:
[snip]
> - 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. "
Nit: I'd either let this log as is, or otherwise put a ',' here, and
finish this log with a '.' after other infos.
> "interval %"PRIu64" mins, "
> "pmd load threshold %"PRIu8"%%, "
[snip]
> @@ -5482,4 +5453,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.");
Nit: extra space as indent.
> + return false;
> + }
> + return true;
> + }
--
David Marchand
More information about the dev
mailing list