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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
