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