On Fri, Dec 18, 2020 at 11:18:27PM +0000, Kevin Traynor wrote: > 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.
The patch set already proposes to add logs, so at least we agree they are useful. :-) From support point of view, I think it's a bug if we can change runtime behavior without any log message. If the previous patch adding interval parameter included the log message, then you would have only a single patch now. To me that sounds like a separate patch (1/3 as you proposed above) using 'Fixes' tag and then we have 2/3 complete. Old commits work as references/documentation, and with your proposal 2/3 becomes a good one. Does that make sense to you? Thanks, fbl _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
