[ovs-dev] [PATCH v5 2/3] dpif-netdev: Add more logs to PMD auto load balance parameters.

[Flavio Leitner]

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?

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
> 

-- 
fbl