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

["Hyong Youb Kim (hyonkim)" <hyonkim@cisco.com>]

> -----Original Message-----
> From: John Daley (johndale)
> Sent: Friday, July 26, 2019 5:26 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Hyong Youb Kim (hyonkim) <hyonkim@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.

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).

Thanks a lot.
-Hyong