netdev - Re: [PATCH] SMSC LAN911x and LAN921x vendor driver

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] [day] [month] [year] [list]

Date:	Wed, 4 Jun 2008 17:34:24 +0100
From:	Steve.Glendinning@...c.com
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	Bahadir Balban <Bahadir.Balban@....com>,
	Bill Gatliff <bgat@...lgatliff.com>, catalin.marinas@....com,
	hennerich@...ckfin.uclinux.org, ian.saturley@...c.com,
	Michael.Hennerich@...log.com, netdev@...r.kernel.org,
	jeff@...zik.org
Subject: Re: [PATCH] SMSC LAN911x and LAN921x vendor driver

Hi Ben,

> [...]
> > +static void smsc911x_ethtool_setmsglevel(struct net_device *dev, u32 
level)
> > +{
> > +   struct smsc911x_data *pdata = netdev_priv(dev);
> > +   pdata->msg_enable = level;
> > +}
> 
> It would be nice if the logging macros actually tested msg_enable too. 
;-)

I'm implementing this now, using e100's approach (and DPRINTF macro) as a 
guide.
I have a few questions on the "right way" to do this.

1. What's the recommended way to initialise msg_enable? Some drivers set 
it to
a constant, but e100 has a module param called debug and sets msg_enable 
to
((1 << debug) - 1) during probe.  This implies the bits are in order of
importance (i.e. to debug IFDOWN messages one would also have to view DRV,
PROBE, LINK and TIMER messages), is this right?

Once the interface is up, we can turn any combination of bits on and off 
using
ethtool, but a lot of this embedded driver's debugging is done in probe(), 
when
embedded developers/platform integrators are verifying the platform's bus 
layout
and timing.

2. My driver had two levels of message importance (warning and trace), 
chosen
at compile-time so they were #defined out on non-debug builds.  Should I 
remove
this compile-time decision and print the two as KERN_WARNING and 
KERN_DEBUG,
or would printing KERN_DEBUG messages in non-debug builds (when no-one is
interested) be discouraged?

3. DPRINTF adds dev->name to the messages, which is useful.  If I use it 
before
I call register_netdev, dev->name is set to "eth%d" so the debugging 
messages
look strange.  Should I skip the whole msg_enable/DPRINTF thing in probe 
or is
there a better way?

Regards,
--
Steve Glendinning
SMSC GmbH
m: +44 777 933 9124
e: steve.glendinning@...c.com

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html