[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