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

Gaëtan Rivet grive at u256.net
Tue Nov 2 16:52:06 UTC 2021


On Tue, Nov 2, 2021, at 11:35, Kevin Traynor wrote:
> On 02/11/2021 01:34, Gaëtan Rivet wrote:
>> On Mon, Nov 1, 2021, at 15:42, Kevin Traynor wrote:
>>> Previously, when a user enabled PMD auto load balancer with
>>> pmd-auto-lb="true", some conditions such as number of PMDs/RxQs
>>> that were required for a rebalance to take place were checked.
>>>
>>> If the configuration meant that a rebalance would not take place
>>> then PMD ALB was logged as 'disabled' and not run.
>>>
>>> Later, if the PMD/RxQ configuration changed whereby a rebalance
>>> could be effective, PMD ALB was logged as 'enabled' and would run at
>>> the appropriate time.
>>>
>>> This worked ok from a functional view but it is unintuitive for the
>>> user reading the logs.
>>>
>>> e.g. with one PMD (PMD ALB would not be effective)
>>>
>>> User enables ALB, but logs say it is disabled because it won't run.
>>> $ ovs-vsctl set open_vSwitch . other_config:pmd-auto-lb="true"
>>> |dpif_netdev|INFO|PMD auto load balance is disabled
>>>
>>> No dry run takes place.
>>>
>>> Add more PMDs (PMD ALB may be effective).
>>> $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=50
>>> |dpif_netdev|INFO|PMD auto load balance is enabled ...
>>>
>>> Dry run takes place.
>>> |dpif_netdev|DBG|PMD auto load balance performing dry run.
>>>
>>> A better approach is to simply reflect back the user enable/disable
>>> state in the logs and deal with whether the rebalance will be effective
>>> when needed. That is the approach taken in this patch.
>>>
>>> To cut down on unneccessary work, some basic checks are also made before
>>> starting a PMD ALB dry run and debug logs can indicate this to the user.
>>>
>>> e.g. with one PMD (PMD ALB would not be effective)
>>>
>>> User enables ALB, and logs confirm the user has enabled it.
>>> $ ovs-vsctl set open_vSwitch . other_config:pmd-auto-lb="true"
>>> |dpif_netdev|INFO|PMD auto load balance is enabled...
>>>
>>> No dry run takes place.
>>> |dpif_netdev|DBG|PMD auto load balance nothing to do, not enough
>>> non-isolated PMDs or RxQs.
>>>
>>> Add more PMDs (PMD ALB may be effective).
>>> $ ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=50
>>>
>>> Dry run takes place.
>>> |dpif_netdev|DBG|PMD auto load balance performing dry run.
>>>
>>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>>> Acked-by: Sunil Pai G <sunil.pai.g at intel.com>
>>> Reviewed-by: David Marchand <david.marchand at redhat.com>
>>>
>> 
>> Hi Kevin,
>> 
>> The changes make sense and the code looks good.
>> For the test coverage, given the new behavior it seems more straightforward
>> as far as checking the ALB state.
>> 
>> If I'm not mistaken however, the previous tests were not only checking the
>> ALB state (following pmd-auto-lb=true), but also that it would be active given the
>> proper conditions (n_rxq, pmd-cpu-mask > 1).
>> 
>> The test changes kept the first part, but removed the second.
>> It seems it could be worth testing, WDYT?
>> 
>
> Hi Gaetan, thanks for reviewing. Yes, you are right. The issue is that 
> those checks are currently config time checks, so those conditions can 
> easily be created in UT. Now that the checks are changed to be during 
> runtime and only when a CPU threshold is detected for 1 min, it is a 
> more difficult condition to create in UT.
>
> I can take a look and see if there is something quick I can think of for 
> this specific case, but otherwise I would rather have a separate task of 
> seeing if there is a way to fake CPU and RxQ cycles over a period of 
> time. Then several other runtime tests could be added including ones for 
> these checks.
>
> Kevin.

It make sense. Adding a way to simulate load on a PMD is out of scope for
this patch. I think it's otherwise fine so:

Acked-by: Gaetan Rivet <grive at u256.net>

-- 
Gaetan Rivet


More information about the dev mailing list