DPDK patches and discussions
 help / color / mirror / Atom feed
From: "John Daley (johndale)" <johndale@cisco.com>
To: Stephen Hemminger <stephen@networkplumber.org>,
	"Hyong Youb Kim (hyonkim)" <hyonkim@cisco.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] net/enic: retain previous message logging
Date: Fri, 26 Jul 2019 21:15:05 +0000	[thread overview]
Message-ID: <MWHPR1101MB21434023282B1B968E65B0AFCFC00@MWHPR1101MB2143.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20190726135044.287956ae@hermes.lan>



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, July 26, 2019 1:51 PM
> To: Hyong Youb Kim (hyonkim) <hyonkim@cisco.com>
> Cc: John Daley (johndale) <johndale@cisco.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/enic: retain previous message logging
> 
> On Fri, 26 Jul 2019 04:21:23 +0000
> "Hyong Youb Kim (hyonkim)" <hyonkim@cisco.com> wrote:
> 
> > > -----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
> >
> 
> Drivers should be silent unless they see a problem.
> We don't want every driver outputting messages by default.
> 
> For your current issue, why not just register the default log level as info?

Yes, that is better and what Ferruh suggested also. It is what the v2 patch is: http://patches.dpdk.org/patch/57199/.
Thanks, John


  reply	other threads:[~2019-07-26 21:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-25  2:46 John Daley
2019-07-25 10:07 ` Ferruh Yigit
2019-07-25 20:25   ` John Daley (johndale)
2019-07-26  4:21     ` Hyong Youb Kim (hyonkim)
2019-07-26  9:51       ` Ferruh Yigit
2019-07-26 20:50       ` Stephen Hemminger
2019-07-26 21:15         ` John Daley (johndale) [this message]
2019-07-26  8:17     ` John Daley (johndale)
2019-07-26 10:00       ` Ferruh Yigit
2019-07-26 20:06         ` John Daley (johndale)
2019-07-26 20:12 ` [dpdk-dev] [PATCH v2] " John Daley
2019-07-29 11:03   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MWHPR1101MB21434023282B1B968E65B0AFCFC00@MWHPR1101MB2143.namprd11.prod.outlook.com \
    --to=johndale@cisco.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hyonkim@cisco.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).