netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] [RFC] NET: Implement a standard ndev_printk family
@ 2007-06-11 21:37 Auke Kok
  2007-06-11 21:37 ` [PATCH 2/2] [RFC] NET: Convert several drivers to ndev_printk Auke Kok
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Auke Kok @ 2007-06-11 21:37 UTC (permalink / raw)
  To: netdev, jeff, davem; +Cc: arjan, joe, shemminger

A lot of netdevices implement their own variant of printk and use
use variations of dev_printk, printk or others that use msg_enable,
which has been an eyesore with countless variations across drivers.

This patch implements a standard ndev_printk and derivatives
such as ndev_err, ndev_info, ndev_warn that allows drivers to
transparently use both the msg_enable and a generic netdevice
message layout. It moves the msg_enable over to the net_device
struct and allows drivers to obsolete ethtool handling code of
the msg_enable value.

The current code has each driver contain a copy of msg_enable and
handle the setting/changing through ethtool that way. Since the
netdev name is stored in the net_device struct, those two are
not coherently available in a uniform way across all drivers (a
single macro or function would not work since all drivers name
their net_device members differently). This makes netdevice
driver writes reinvent the wheel over and over again.

It thus makes sense to move msg_enable to the net_device. This
gives us the opportunity to (1) initialize it by default with a
globally sane value, (2) remove msg_enable handling code w/r
ethtool for drivers that know and use the msg_enable member
of the net_device struct. (3) Ethtool code can just modify the
net_device msg_enable for drivers that do not have custom
msg_enable get/set handlers so converted drivers lose some
code for that as well.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
---

 include/linux/netdevice.h |   38 ++++++++++++++++++++++++++++++++++++++
 net/core/dev.c            |    5 +++++
 net/core/ethtool.c        |   14 +++++++-------
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3a70f55..d185f41 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -540,6 +540,8 @@ struct net_device
 	struct device		dev;
 	/* space for optional statistics and wireless sysfs groups */
 	struct attribute_group  *sysfs_groups[3];
+
+	int			msg_enable;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
@@ -838,6 +840,42 @@ enum {
 	NETIF_MSG_WOL		= 0x4000,
 };
 
+#define ndev_err(netdev, level, format, arg...) \
+	do { if ((netdev)->msg_enable & NETIF_MSG_##level) { \
+		printk(KERN_ERR "%s: %s: " format, (netdev)->name, \
+		(netdev)->dev.parent->bus_id, ## arg); } } while (0)
+
+#define ndev_warn(netdev, level, format, arg...) \
+	do { if ((netdev)->msg_enable & NETIF_MSG_##level) { \
+		printk(KERN_WARNING "%s: %s: " format, (netdev)->name, \
+		(netdev)->dev.parent->bus_id, ## arg); } } while (0)
+
+#define ndev_notice(netdev, level, format, arg...) \
+	do { if ((netdev)->msg_enable & NETIF_MSG_##level) { \
+		printk(KERN_NOTICE "%s: %s: " format, (netdev)->name, \
+		(netdev)->dev.parent->bus_id, ## arg); } } while (0)
+
+#define ndev_info(netdev, level, format, arg...) \
+	do { if ((netdev)->msg_enable & NETIF_MSG_##level) { \
+		printk(KERN_INFO "%s: %s: " format, (netdev)->name, \
+		(netdev)->dev.parent->bus_id, ## arg); } } while (0)
+
+#ifdef DEBUG
+#define ndev_debug(netdev, level, format, arg...) \
+	do { if ((netdev)->msg_enable & NETIF_MSG_##level) { \
+		printk(KERN_DEBUG "%s: %s: " format, (netdev)->name, \
+		(netdev)->dev.parent->bus_id, ## arg); } } while (0)
+#else
+/* Force type-checking on ndev_dbg() when DEBUG is not set */
+static inline int __attribute__ ((format (printf, 3, 4)))
+__ndev_dbg(struct net_device * netdev, u32 level, const char *format, ...)
+{
+	return 0;
+}
+#define ndev_dbg(netdev, level, fmt, args...) \
+	__ndev_dbg(netdev, NETIF_MSG_##level, fmt, ##args)
+#endif
+
 #define netif_msg_drv(p)	((p)->msg_enable & NETIF_MSG_DRV)
 #define netif_msg_probe(p)	((p)->msg_enable & NETIF_MSG_PROBE)
 #define netif_msg_link(p)	((p)->msg_enable & NETIF_MSG_LINK)
diff --git a/net/core/dev.c b/net/core/dev.c
index 2609062..316f250 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3378,6 +3378,11 @@ struct net_device *alloc_netdev(int sizeof_priv, const char *name,
 		dev->priv = netdev_priv(dev);
 
 	dev->get_stats = internal_stats;
+	dev->msg_enable = NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK;
+#ifdef DEBUG
+	/* put these to good use: */
+	dev->msg_enable |= NETIF_MSG_RX_ERR | NETIF_MSG_TX_ERR;
+#endif
 	setup(dev);
 	strcpy(dev->name, name);
 	return dev;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 8d5e5a0..ff8d52f 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -234,9 +234,9 @@ static int ethtool_get_msglevel(struct net_device *dev, char __user *useraddr)
 	struct ethtool_value edata = { ETHTOOL_GMSGLVL };
 
 	if (!dev->ethtool_ops->get_msglevel)
-		return -EOPNOTSUPP;
-
-	edata.data = dev->ethtool_ops->get_msglevel(dev);
+		edata.data = dev->msg_enable;
+	else
+		edata.data = dev->ethtool_ops->get_msglevel(dev);
 
 	if (copy_to_user(useraddr, &edata, sizeof(edata)))
 		return -EFAULT;
@@ -247,13 +247,13 @@ static int ethtool_set_msglevel(struct net_device *dev, char __user *useraddr)
 {
 	struct ethtool_value edata;
 
-	if (!dev->ethtool_ops->set_msglevel)
-		return -EOPNOTSUPP;
-
 	if (copy_from_user(&edata, useraddr, sizeof(edata)))
 		return -EFAULT;
 
-	dev->ethtool_ops->set_msglevel(dev, edata.data);
+	if (!dev->ethtool_ops->set_msglevel)
+		dev->msg_enable = edata.data;
+	else
+		dev->ethtool_ops->set_msglevel(dev, edata.data);
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 1/2] [RFC] NET: Implement a standard ndev_printk family
@ 2007-06-08 22:00 Auke Kok
  2007-06-08 23:24 ` Stephen Hemminger
  0 siblings, 1 reply; 20+ messages in thread
From: Auke Kok @ 2007-06-08 22:00 UTC (permalink / raw)
  To: netdev, jeff, davem; +Cc: arjan

A lot of netdevices implement their own variant of printk and use
use variations of dev_printk, printk or others that use msg_enable,
which has been an eyesore with countless variations across drivers.

This patch implements a standard ndev_printk and derivatives
such as ndev_err, ndev_info, ndev_warn that allows drivers to
transparently use both the msg_enable and a generic netdevice
message layout. It moves the msg_enable over to the net_device
struct and allows drivers to obsolete ethtool handling code of
the msg_enable value.

The current code has each driver contain a copy of msg_enable and
handle the setting/changing through ethtool that way. Since the
netdev name is stored in the net_device struct, those two are
not coherently available in a uniform way across all drivers (a
single macro or function would not work since all drivers name
their net_device members differently). This makes netdevice
driver writes reinvent the wheel over and over again.

It thus makes sense to move msg_enable to the net_device. This
gives us the opportunity to (1) initialize it by default with a
globally sane value, (2) remove msg_enable handling code w/r
ethtool for drivers that know and use the msg_enable member
of the net_device struct. (3) Ethtool code can just modify the
net_device msg_enable for drivers that do not have custom
msg_enable get/set handlers so converted drivers lose some
code for that as well.

Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
---

 include/linux/netdevice.h |   24 ++++++++++++++++++++++++
 net/core/dev.c            |   10 ++++++++++
 net/core/ethtool.c        |   14 +++++++-------
 3 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3a70f55..5551b63 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -540,6 +540,8 @@ struct net_device
 	struct device		dev;
 	/* space for optional statistics and wireless sysfs groups */
 	struct attribute_group  *sysfs_groups[3];
+
+	int			msg_enable;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
@@ -838,6 +840,28 @@ enum {
 	NETIF_MSG_WOL		= 0x4000,
 };
 
+#define ndev_printk(kern_level, netif_level, netdev, format, arg...) \
+	do { if ((netdev)->msg_enable & NETIF_MSG_##netif_level) { \
+		printk(kern_level "%s: " format, \
+		(netdev)->name, ## arg); } } while (0)
+
+#ifdef DEBUG
+#define ndev_dbg(level, netdev, format, arg...) \
+	ndev_printk(KERN_DEBUG, level, netdev, format, ## arg)
+#else
+#define ndev_dbg(level, netdev, format, arg...) \
+	do { (void)(netdev); } while (0)
+#endif
+
+#define ndev_err(level, netdev, format, arg...) \
+	ndev_printk(KERN_ERR, level, netdev, format, ## arg)
+#define ndev_info(level, netdev, format, arg...) \
+	ndev_printk(KERN_INFO, level, netdev, format, ## arg)
+#define ndev_warn(level, netdev, format, arg...) \
+	ndev_printk(KERN_WARNING, level, netdev, format, ## arg)
+#define ndev_notice(level, netdev, format, arg...) \
+	ndev_printk(KERN_NOTICE, level, netdev, format, ## arg)
+
 #define netif_msg_drv(p)	((p)->msg_enable & NETIF_MSG_DRV)
 #define netif_msg_probe(p)	((p)->msg_enable & NETIF_MSG_PROBE)
 #define netif_msg_link(p)	((p)->msg_enable & NETIF_MSG_LINK)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5a7f20f..e854c09 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3376,6 +3376,16 @@ struct net_device *alloc_netdev(int sizeof_priv, const char *name,
 		dev->priv = netdev_priv(dev);
 
 	dev->get_stats = internal_stats;
+	dev->msg_enable = NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK;
+#ifdef DEBUG
+	/* put these to good use: */
+	dev->msg_enable |= NETIF_MSG_TIMER | NETIF_MSG_IFDOWN |
+	                   NETIF_MSG_IFUP | NETIF_MSG_RX_ERR |
+	                   NETIF_MSG_TX_ERR | NETIF_MSG_TX_QUEUED |
+	                   NETIF_MSG_INTR | NETIF_MSG_TX_DONE |
+	                   NETIF_MSG_RX_STATUS | NETIF_MSG_PKTDATA |
+	                   NETIF_MSG_HW | NETIF_MSG_WOL;
+#endif
 	setup(dev);
 	strcpy(dev->name, name);
 	return dev;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 8d5e5a0..ff8d52f 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -234,9 +234,9 @@ static int ethtool_get_msglevel(struct net_device *dev, char __user *useraddr)
 	struct ethtool_value edata = { ETHTOOL_GMSGLVL };
 
 	if (!dev->ethtool_ops->get_msglevel)
-		return -EOPNOTSUPP;
-
-	edata.data = dev->ethtool_ops->get_msglevel(dev);
+		edata.data = dev->msg_enable;
+	else
+		edata.data = dev->ethtool_ops->get_msglevel(dev);
 
 	if (copy_to_user(useraddr, &edata, sizeof(edata)))
 		return -EFAULT;
@@ -247,13 +247,13 @@ static int ethtool_set_msglevel(struct net_device *dev, char __user *useraddr)
 {
 	struct ethtool_value edata;
 
-	if (!dev->ethtool_ops->set_msglevel)
-		return -EOPNOTSUPP;
-
 	if (copy_from_user(&edata, useraddr, sizeof(edata)))
 		return -EFAULT;
 
-	dev->ethtool_ops->set_msglevel(dev, edata.data);
+	if (!dev->ethtool_ops->set_msglevel)
+		dev->msg_enable = edata.data;
+	else
+		dev->ethtool_ops->set_msglevel(dev, edata.data);
 	return 0;
 }
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2007-06-11 22:43 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-11 21:37 [PATCH 1/2] [RFC] NET: Implement a standard ndev_printk family Auke Kok
2007-06-11 21:37 ` [PATCH 2/2] [RFC] NET: Convert several drivers to ndev_printk Auke Kok
2007-06-11 21:52 ` [PATCH 1/2] [RFC] NET: Implement a standard ndev_printk family Kok, Auke
2007-06-11 21:54 ` Joe Perches
2007-06-11 22:01   ` Kok, Auke
2007-06-11 22:24     ` Joe Perches
2007-06-11 22:00 ` Randy Dunlap
2007-06-11 22:04   ` Kok, Auke
2007-06-11 22:19 ` Stephen Hemminger
2007-06-11 22:43   ` Kok, Auke
  -- strict thread matches above, loose matches on Subject: below --
2007-06-08 22:00 Auke Kok
2007-06-08 23:24 ` Stephen Hemminger
2007-06-08 23:42   ` Kok, Auke
2007-06-09  0:10     ` Stephen Hemminger
2007-06-09  3:06       ` Kok, Auke
2007-06-09  3:18       ` Kok, Auke
2007-06-11 17:30       ` Kok, Auke
2007-06-11 17:50         ` Stephen Hemminger
2007-06-11 18:28           ` Joe Perches
2007-06-11 20:28             ` Kok, Auke

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