[ovs-dev] [PATCH v5 2/3] dpif-netdev: Add more logs to PMD auto load balance parameters.
Kevin Traynor
ktraynor at redhat.com
Fri Dec 18 23:18:27 UTC 2020
On 18/12/2020 21:54, Flavio Leitner wrote:
> On Fri, Dec 18, 2020 at 05:51:51PM +0000, Kevin Traynor wrote:
>> Previously if PMD auto load balance was disabled, updates to
>> parameters did not get logged.
>>
>> Add logs so that if there are any changes to the parameters,
>> the change is shown as well as the current enabled/disabled
>> state of PMD auto load balance.
>
> Well, I was expecting the parameter change log messages to
> come together with the patch adding those parameters.
>
> Then this patch would be about adding the set_pmd_auto_lb()
> which seems to be an addition improvement to me.
>
> What do you think?
>
Thanks for reviewing Flavio. I did it this way because the existing
interval parameter that was not added in the previous patch does not
have logging on changes.
If I added change logging for the new params in the previous patch it
would have meant some params changes were logged and some not, which is
probably a worse state. So I added the new params to match the existing
way (no logging on change) and then added a patch to improve the logging
for all params.
I could split like,
1/3 add change logs for interval param
2/3 add new params with change logs
3/3 set_pmd_auto_lb() additional logging.
To me it's 50:50, probably depends on whether the existing param is
considered broken by not logging the change, or the addition of the logs
is an improvement. Let me know if you have a strong preference.
Kevin.
> fbl
>
>
>>
>> Also, show the parameters when PMD auto load balance is changed
>> to be enabled.
>>
>> e.g.
>> |dpif_netdev|INFO|PMD auto load balance pmd load threshold changed to 70%
>> |dpif_netdev|INFO|PMD auto load balance is disabled
>>
>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>> Acked-by: David Marchand <david.marchand at redhat.com>
>> ---
>> lib/dpif-netdev.c | 30 +++++++++++++++++++++++-------
>> 1 file changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index eb19a3afc..9c24b58d2 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -4202,9 +4202,10 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op **ops, size_t n_ops,
>> /* Enable or Disable PMD auto load balancing. */
>> static void
>> -set_pmd_auto_lb(struct dp_netdev *dp)
>> +set_pmd_auto_lb(struct dp_netdev *dp, bool always_log)
>> {
>> unsigned int cnt = 0;
>> struct dp_netdev_pmd_thread *pmd;
>> struct pmd_auto_lb *pmd_alb = &dp->pmd_alb;
>> + uint8_t rebalance_load_thresh;
>>
>> bool enable_alb = false;
>> @@ -4233,10 +4234,16 @@ set_pmd_auto_lb(struct dp_netdev *dp)
>> pmd_alb->auto_lb_requested;
>>
>> - if (pmd_alb->is_enabled != enable_alb) {
>> + if (pmd_alb->is_enabled != enable_alb || always_log) {
>> pmd_alb->is_enabled = enable_alb;
>> if (pmd_alb->is_enabled) {
>> + atomic_read_relaxed(&pmd_alb->rebalance_load_thresh,
>> + &rebalance_load_thresh);
>> VLOG_INFO("PMD auto load balance is enabled "
>> - "(with rebalance interval:%"PRIu64" msec)",
>> - pmd_alb->rebalance_intvl);
>> + "interval %"PRIu64" mins, "
>> + "pmd load threshold %"PRIu8"%%, "
>> + "improvement threshold %"PRIu8"%%",
>> + pmd_alb->rebalance_intvl / MIN_TO_MSEC,
>> + rebalance_load_thresh,
>> + pmd_alb->rebalance_improve_thresh);
>> } else {
>> pmd_alb->rebalance_poll_timer = 0;
>> @@ -4244,5 +4251,4 @@ set_pmd_auto_lb(struct dp_netdev *dp)
>> }
>> }
>> -
>> }
>>
>> @@ -4264,4 +4270,5 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
>> uint8_t rebalance_load, cur_rebalance_load;
>> uint8_t rebalance_improve;
>> + bool log_autolb = false;
>>
>> tx_flush_interval = smap_get_int(other_config, "tx-flush-interval",
>> @@ -4349,4 +4356,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
>> if (pmd_alb->rebalance_intvl != rebalance_intvl) {
>> pmd_alb->rebalance_intvl = rebalance_intvl;
>> + VLOG_INFO("PMD auto load balance interval set to "
>> + "%"PRIu64" mins\n", rebalance_intvl / MIN_TO_MSEC);
>> + log_autolb = true;
>> }
>>
>> @@ -4358,4 +4368,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
>> if (rebalance_improve != pmd_alb->rebalance_improve_thresh) {
>> pmd_alb->rebalance_improve_thresh = rebalance_improve;
>> + VLOG_INFO("PMD auto load balance improvement threshold set to "
>> + "%"PRIu8"%%\n", rebalance_improve);
>> + log_autolb = true;
>> }
>>
>> @@ -4369,6 +4382,9 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
>> atomic_store_relaxed(&pmd_alb->rebalance_load_thresh,
>> rebalance_load);
>> + VLOG_INFO("PMD auto load balance pmd load threshold set to "
>> + "%"PRIu8"%%\n", rebalance_load);
>> + log_autolb = true;
>> }
>> - set_pmd_auto_lb(dp);
>> + set_pmd_auto_lb(dp, log_autolb);
>> return 0;
>> }
>> @@ -5454,5 +5470,5 @@ reconfigure_datapath(struct dp_netdev *dp)
>>
>> /* Check if PMD Auto LB is to be enabled */
>> - set_pmd_auto_lb(dp);
>> + set_pmd_auto_lb(dp, false);
>> }
>>
>> --
>> 2.26.2
>>
>
More information about the dev
mailing list