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

Kevin Traynor ktraynor at redhat.com
Mon Dec 21 10:21:11 UTC 2020


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
> 



More information about the dev mailing list