On 19/12/2020 00:05, Flavio Leitner wrote: > 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? >
Yep, sounds good, i'll split like that and add a Fixes: for 1/3. Thanks. > Thanks, > fbl > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
