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 <[email protected]>
Acked-by: Sunil Pai G <[email protected]>
Reviewed-by: David Marchand <[email protected]>
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 <[email protected]>
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 :/
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
