netdev - Re: [net-next 01/13] i40e: add checks for AQ error status bits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 09 Jun 2014 14:10:38 -0700
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	Joe Perches <joe@...ches.com>
Cc:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	davem@...emloft.net, Shannon Nelson <shannon.nelson@...el.com>,
	netdev@...r.kernel.org, gospo@...hat.com, sassmann@...hat.com
Subject: Re: [net-next 01/13] i40e: add checks for AQ error status bits

On Mon, 2014-06-09 at 14:02 -0700, Joe Perches wrote:
> On Mon, 2014-06-09 at 13:35 -0700, Jeff Kirsher wrote:
> > On Mon, 2014-06-09 at 17:21 +0400, Sergei Shtylyov wrote:
> > > On 06/09/2014 12:49 PM, Jeff Kirsher wrote:
> > > > From: Shannon Nelson <shannon.nelson@...el.com>
> 
> > > > If the Firmware sets these bits, it will trigger an AdminQ
> > > > interrupt to get the driver's attention to process the ARQ, which will
> > > > likely be enough to clear the actual issue.
>  
> > >     Hm, why not dev_err() here and below?
> 
> > The thought was that these should be more of "FYI..." type of messages
> > not "Oh Crap!..." messages.  So that is why dev_err() was not used,
> > although we are not opposed to changing it if you feel it warrants it in
> > a follow-up patch.
> []
> > > > +	if (val & I40E_PF_ATQLEN_ATQCRIT_MASK) {
> > > > +		dev_info(&pf->pdev->dev, "ASQ Critical Error detected\n");
> > > > +		val &= ~I40E_PF_ATQLEN_ATQCRIT_MASK;
> > > > +	}
> 
> I thought it was odd to have a "critical error"
> emitted at KERN_INFO
> 
> Maybe adding something like
> 	"ARQ should fix this automatically"
> would be enough.
> 
> Should any/all of these be ratelimited or maybe even
> changed to dev_dbg?
> 
> 

Well, as Shannon noted in the patch description:
"If the Firmware sets these bits, it will trigger an AdminQ
interrupt to get the driver's attention to process the ARQ, which will
likely be enough to clear the actual issue."

To me that means the log messages should not be filling up with these
types of messages and if they do occur, it should be transient and
resolve itself.  So does that warrant dev_dbg? 

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)