[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 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