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

Kevin Traynor ktraynor at redhat.com
Tue Nov 2 17:00:56 UTC 2021


On 02/11/2021 16:52, Gaëtan Rivet wrote:
> 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>
> 

Thanks Gaetan. Actually, I found a way to test these config checks 
without simulating a load, so I will be sending a v3 once I refine them 
a bit more. While doing that I also noticed I had made but not committed 
the minor log changes from David's comments in v2 :/



More information about the dev mailing list