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