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

[Pai G, Sunil]

Hi Kevin, 

Patch LGTM in general,  just a query below.
<snipped>

> + * 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; }
> +

I wonder if the logs in the above function be INFO instead of DBG ?
Don't have a strong preference TBH. But if they were info, we need not remove any test cases no ?

<snipped>

Thanks and regards,
Sunil