[dpdk-dev] [PATCH] net/enic: retain previous message logging

Ferruh Yigit ferruh.yigit at intel.com
Fri Jul 26 11:51:47 CEST 2019


On 7/26/2019 5:21 AM, Hyong Youb Kim (hyonkim) wrote:
>> -----Original Message-----
>> From: John Daley (johndale)
>> Sent: Friday, July 26, 2019 5:26 AM
>> To: Ferruh Yigit <ferruh.yigit at intel.com>
>> Cc: dev at dpdk.org; Hyong Youb Kim (hyonkim) <hyonkim at cisco.com>
>> Subject: RE: [PATCH] net/enic: retain previous message logging
>>
>> Ok, lets NAK this patch. See comment inline.
>> Thanks,
>> John
>>
> [...]
>>> On 7/25/2019 3:46 AM, John Daley wrote:
>>>> Prior to fix, RTE_LOGTYPE_INFO messages would display in testpmd by
>>>> default. After the fix, using dynamic logging, only NOTICE level and
>>>> higher were displayed by default and INFO level were not. Change the
>>>> messages to NOTICE level so they continue to display.
>>>>
>>>> DTS uses testpmd and parses messages and some tests failed because
>>>> messages were no longer displayed. Other apps may also depend on the
>>>> messages.
>>>
>>> If you need messages for the test framework, why not just increase the log
>>> level for enic PMD via application parameter [1], or as command to
>>> testpmd[2]?
>>> Since it is dynamic debug now, you don't need to change the default, can
>>> change the level on demand.
>>
>> I have no problem modifying our test scripts. The bigger concern was about
>> any other scripts out there that might break because the default enic PMD
>> messages changed. I suppose chances are slim and any such scripts can easily
>> be modified to set the log level to info.
>>
> 
> Hi John, Ferruh,
> 
> Can you guys reconsider? John's commit message makes it sound like he
> is modifying PMD to avoid modifying test scripts. That is not the
> issue at all. The real problem is that his previous commit causes
> a customer visible change, which can lead to a lot of headache for both
> us (doing tech support) and customers (wondering what's changed).
> 
> Prior to commit bbd8ecc05434 ("net/enic: remove PMD log type references"):
> 
> enic prints vNIC config related messages (rq/cq/wq info and such) via
> dev_info(). And, dev_info() uses LOGTYPE_PMD and the INFO level.
> LOGTYPE_PMD defaults to the INFO level, so these messages appear by
> default. Customers and tech support use them for debugging and so on.

I think this is the point, "use them for debugging and so on", if a log is used
for debug should it be always enabled? Specially when log level can be
controlled dynamically in runtime.

If every component selects same path, the dpdk log will be too verbose to be
useful, I think it make sense to log only warnings by default, and if explicitly
set more verbose logging can be enabled for desired component.

And if they are for regular use case (not for debug etc..) the logs are not most
effective way because they require a human parsing them, I believe it worth
thinking if they can be managed in programmatic way, like API return values etc,
instead of logs. And why a user really need a log be default enabled to be able
to run a driver?

> 
> After the commit:
> 
> dev_info() is now enic_pmd_logtype, which defaults to NOTICE. The
> macro is still using the INFO level. So, config messages are
> suppressed by default. This was never the intention. The current patch
> tries to fix that by elevating dev_info to dev_notice, because we do
> want these messages to appear by default. Should have done it as part
> of the previous commit, but we missed it.
> 
> Down the line, we will have to guide our customers to exploit dynamic log
> levels, but not this way (i.e. suddenly hiding messages that they used
> to see/rely on).

I think this is the correct path to follow, DPDK provides API to manage the log
levels dynamically, any user of the DPDK can integrate this into their
application in a way they desire.


More information about the dev mailing list