[ovs-dev] [PATCH 2/2] dpif-netdev: auto load balance improve enable/disable logs.

[Kevin Traynor]

On 12/02/2021 17:42, Stokes, Ian wrote:
>> In order for auto load balance to be enabled, there are
>> minimum requirements of more than one PMD and more than
>> one Rxq on at least one PMD.
>>
>> If these conditions are not met a rebalance would be pointless,
>> so auto load balance is not enabled.
>>
>> Currently the state is logged but in the case where the criteria
>> for enabling is not met, there is no reason given.
>>
>> It would be useful for the user to see the reason, so they
>> can understand why auto load balance has not been enabled
>> when they have requested it.
>>
>> For example, if a user has one PMD and sets pmd-auto-lb=true,
>> previously:
>> |INFO|PMD auto load balance is disabled
>>
>> With patch:
>> |INFO|PMD auto load balance not enough PMDs or Rx Queues to enable
>> |INFO|PMD auto load balance is disabled
> 
> Thanks for the patch Kevin.
> 
> In testing this worked as expected.
> 
> One query I had was did you give thought towards more detailed in the log?
> 

I hadn't thought about splitting them. They are both connected as
increasing the PMDs, will reduce the number of RxQ's per PMD etc. so
it's hard to be prescriptive in splitting them to tell the user exactly
what they need to change.
e.g.
2 rxq on 1 pmd "not enough PMDs"
user increases pmds to 2
1 rxq on 2 pmds "not enough rxqs"

They'd get there in the end, but I wonder if it would be more annoying
to get a new message after fixing the only one that was reported first
time around :-)

> i.e. if its 1 PMD should we flag that PMD <=1 is an issue.
> 
> Similar with the number of RXQs.
> 
> Maybe that's overkill as you could argue the minimum requirements could change over time but if its something that could be flagged easily would it be worth it?
> 

At the moment it's easily added, with the caveat as per above that just
saying pmds or rxqs alone may not give the full picture. You are right
that the minimum requirements could change a bit in time, so we could
end up with further interconnected min reqs which might make it more
difficult to split.

If you think it's useful, I'm fine to add now, or else we can go with a
single line now and review again later when there is more development on
it and docs are improved etc.

> Thanks
> Ian
>>
>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>> ---
>>  lib/dpif-netdev.c | 19 +++++++++++++++----
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 4381c618f..833f45616 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -4213,4 +4213,5 @@ set_pmd_auto_lb(struct dp_netdev *dp, bool
>> always_log)
>>      bool enable_alb = false;
>>      bool multi_rxq = false;
>> +    bool minreq = false;
>>      bool pmd_rxq_assign_cyc = dp->pmd_rxq_assign_cyc;
>>
>> @@ -4226,6 +4227,6 @@ set_pmd_auto_lb(struct dp_netdev *dp, bool
>> always_log)
>>          }
>>          if (cnt && multi_rxq) {
>> -                enable_alb = true;
>> -                break;
>> +            minreq = true;
>> +            break;
>>          }
>>          cnt++;
>> @@ -4233,6 +4234,5 @@ set_pmd_auto_lb(struct dp_netdev *dp, bool
>> always_log)
>>
>>      /* Enable auto LB if it is requested and cycle based assignment is true. */
>> -    enable_alb = enable_alb && pmd_rxq_assign_cyc &&
>> -                    pmd_alb->auto_lb_requested;
>> +    enable_alb = minreq && pmd_rxq_assign_cyc && pmd_alb-
>>> auto_lb_requested;
>>
>>      if (pmd_alb->is_enabled != enable_alb || always_log) {
>> @@ -4251,4 +4251,15 @@ set_pmd_auto_lb(struct dp_netdev *dp, bool
>> always_log)
>>          } else {
>>              pmd_alb->rebalance_poll_timer = 0;
>> +            if (pmd_alb->auto_lb_requested) {
>> +                if (!minreq) {
>> +                    VLOG_INFO("PMD auto load balance not enough "
>> +                              "PMDs or Rx Queues to enable");
>> +                }
>> +                if (!pmd_rxq_assign_cyc) {
>> +                    VLOG_INFO("PMD auto load balance needs "
>> +                              "'other_config:pmd-rxq-assign=cycles' "
>> +                              "to enable");
>> +                }
>> +            }
>>              VLOG_INFO("PMD auto load balance is disabled");
>>          }
>> --
>> 2.26.2
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>