[ovs-dev] [PATCH 2/5] dpif-netdev: Make PMD auto load balance use common rxq scheduling.

Pai G, Sunil sunil.pai.g at intel.com
Wed Jun 23 11:21:26 UTC 2021


Hey Kevin , 

Patch looks good to me.
Builds fine , all test cases here http://patchwork.ozlabs.org/project/openvswitch/patch/20210316154532.127858-1-ktraynor@redhat.com/ pass as well.

Some minor nits inline :

<snipped>

> +static bool
> +pmd_rebalance_dry_run(struct dp_netdev *dp)
> +    OVS_REQUIRES(dp->port_mutex)
> +{
> +    struct sched_numa_list *numa_list_cur;
> +    struct sched_numa_list *numa_list_est;
> +    bool thresh_met = false;
> +    uint64_t current, estimate;

current and estimate aren't specific, may be current_var and estimate_var ?

> +    uint64_t improvement = 0;
> +
> +    VLOG_DBG("PMD auto load balance performing dry run.");
> +
> +    /* Populate current assignments. */
> +    numa_list_cur = xzalloc(sizeof *numa_list_cur);
> +    sched_numa_list_populate(numa_list_cur, dp);
> +    sched_numa_list_assignments(numa_list_cur, dp);
> +
> +    /* Populate estimated assignments. */
> +    numa_list_est = xzalloc(sizeof *numa_list_est);
> +    sched_numa_list_populate(numa_list_est, dp);
> +    sched_numa_list_schedule(numa_list_est, dp,
> +                             dp->pmd_rxq_assign_cyc, VLL_DBG);
> +
> +    /* Check if cross-numa polling, there is only one numa with PMDs. */
> +    if (!sched_numa_list_cross_numa_polling(numa_list_est) ||
> +            sched_numa_list_count(numa_list_est) == 1) {
> +
> +        /* Calculate variances. */
> +        current = sched_numa_list_variance(numa_list_cur);
> +        estimate = sched_numa_list_variance(numa_list_est);
> +
> +        if (estimate < current) {
> +             improvement = ((current - estimate) * 100) / current;
> +        }
> +        VLOG_DBG("Current variance %"PRIu64" Estimated variance
> %"PRIu64"",
> +                 current, estimate);

space alignment issues. extra space is required at the start of second statement to align with the first one ?
Also, comma or full stop after Current variance %"PRIu64" ?

> +        VLOG_DBG("Variance improvement %"PRIu64"%%", improvement);
> +
> +        if (improvement >= dp->pmd_alb.rebalance_improve_thresh) {
> +            thresh_met = true;
> +            VLOG_DBG("PMD load variance improvement threshold %u%% "
> +                      "is met", dp->pmd_alb.rebalance_improve_thresh);

space alignment issue. Extra space added here before "is_met".


> +        } else {
> +            VLOG_DBG("PMD load variance improvement threshold %u%% is not
> met",
> +                      dp->pmd_alb.rebalance_improve_thresh);
> +        }
> +    } else {
> +        VLOG_DBG("PMD auto load balance detected cross-numa polling with "
> +                 "multiple numa nodes. Unable to accurately estimate.");
> +    }
> +
> +    sched_numa_list_free_entries(numa_list_cur);
> +    sched_numa_list_free_entries(numa_list_est);
> +
> +    free(numa_list_cur);
> +    free(numa_list_est);
> +
> +    return thresh_met;
> +}
> +
>  static void
>  reload_affected_pmds(struct dp_netdev *dp) @@ -5897,215 +5925,4 @@
> variance(uint64_t a[], int n)  }

<snipped>



More information about the dev mailing list