On 18/10/2021 13:03, Pai G, Sunil wrote:
Hi Kevin,
Hi Sunil, Thanks for reviewing.
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 ?
The reason they are debug is so they won't pollute the logs when a rebalance cannot occur.
For example, if user sets ALB enabled and there is only one PMD. If it is at 100% loaded, this message would be printed every minute to say that a ALB will do nothing.
As nothing is changing and it can occur every minute I thought it was more appropriate to keep it at debug level for when you need to check/debug the operation of ALB. WDYT?
Kevin.
<snipped> Thanks and regards, Sunil
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
