Re: [PATCH] USB: usbnet uses netif_msg_*() ethtool filtering

[David Brownell <david-b@pacbell.net>]

On Wednesday 30 March 2005 5:09 pm, Jeff Garzik wrote:
> David Brownell wrote:
> >   http://www.tux.org/hypermail/linux-vortex/2001-Nov/0021.html
> > 
> > If there are other rules, they belong in Documentation/netif-msg.txt
> > don't they?  That way folk won't be forced to guess.  Or risk
> > accidentally following the "wrong" set of rules...
> 
> I don't see from the code that the struct net_device interface is going 
> down (via dev->stop) at that point.  Am I mistaken?

It's going down, either by netdev->stop() or by hot-unplug, in every
place I used the netif_msg_ifdown() filter.


> Moreover, if you look at any other user of netif_msg_if{up,down}, you 
> will see that it does not produce multiple lines of status register 
> information opaque to anyone but the programmer.  Its not a debugging 
> message, but something a user should feel comfortable enabling (if not 
> enabled by default).

Again, if that's going to become an official guideline it ought to be
documented in Documentation/netif-msg.txt ... along with information
about how to handle categorization of debug messages.

I suspect a general "how to use debug messages" document would help too.
With guidance like "fault paths should have diagnostics that let you
distinguish each fault", and "never provide success messages at default
message logging levels".  (Log spamming ... we hates it forever!)


If it's wrong to use netif_msg_if{up,down} for debug messages (which go
away when DEBUG isn't defined, and yes are intended for debugging aids)
which are related to interfaces going up and down ... then what's the
"correct" category to filter those out?


> >>>@@ -3044,7 +3047,7 @@
> >>> 
> >>> 	memset(urb->transfer_buffer, 0, urb->transfer_buffer_length);
> >>> 	status = usb_submit_urb (urb, GFP_ATOMIC);
> >>>-	if (status != 0)
> >>>+	if (status != 0 && netif_msg_timer (dev))
> >>> 		deverr(dev, "intr resubmit --> %d", status);
> >>> }
> >>> 
> >>
> >>this looks more like a debugging message?
> > 
> > 
> > It's an error of the "what do I do now??" variety, triggered by
> > what's effectively a timer callback.  USB interrupt transfers
> > are polled by the host controller according to a schedule that's
> > maintained by the HCD.
> 
> The above example seems more like netif_msg_tx_err() or even just KERN_ERR ?

It's not coupled to a packet TX or RX, so the guidelines I've seen
suggest netif_msg_{tx,rx}_err() would be wrong.  Maybe KERN_ERR with
no filter would be right ... but I'm used to having one goal of all
message filtering as being able to _completely shut up_ the component.
That's why there's a filter ... deverr() does wrap KERN_ERR.

- Dave