| Message ID | 1405606186-13703-3-git-send-email-vfalico@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Thu, 2014-07-17 at 16:09 +0200, Veaceslav Falico wrote: > This way we'll always know in what status the device is, unless it's > running normally (i.e. NETDEV_REGISTERED). [] > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h [] > @@ -3388,6 +3388,20 @@ static inline const char *netdev_name(const struct net_device *dev) > return dev->name; > } > > +static inline const char *netdev_reg_state(const struct net_device *dev) > +{ > + switch (dev->reg_state) { > + case NETREG_UNINITIALIZED: return " (unregistered)"; Why not " (uninitialized)"? > + case NETREG_REGISTERED: return ""; > + case NETREG_UNREGISTERING: return " (unregistering)"; > + case NETREG_UNREGISTERED: return " (unregistered)"; > + case NETREG_RELEASED: return " (released)"; > + case NETREG_DUMMY: return " (dummy)"; > + } > + > + return " (unknown)"; Shouldn't this " (unknown)" have stronger text and use a WARN_ON_ONCE? I'd put this in net/core/dev.c and make it not be static inline. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 17, 2014 at 10:00:24AM -0700, Joe Perches wrote: >On Thu, 2014-07-17 at 16:09 +0200, Veaceslav Falico wrote: >> This way we'll always know in what status the device is, unless it's >> running normally (i.e. NETDEV_REGISTERED). >[] >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >[] >> @@ -3388,6 +3388,20 @@ static inline const char *netdev_name(const struct net_device *dev) >> return dev->name; >> } >> >> +static inline const char *netdev_reg_state(const struct net_device *dev) >> +{ >> + switch (dev->reg_state) { >> + case NETREG_UNINITIALIZED: return " (unregistered)"; > >Why not " (uninitialized)"? Good one, thank you, missed it somehow. Will send v2. > >> + case NETREG_REGISTERED: return ""; >> + case NETREG_UNREGISTERING: return " (unregistering)"; >> + case NETREG_UNREGISTERED: return " (unregistered)"; >> + case NETREG_RELEASED: return " (released)"; >> + case NETREG_DUMMY: return " (dummy)"; >> + } >> + >> + return " (unknown)"; > >Shouldn't this " (unknown)" have stronger text >and use a WARN_ON_ONCE? Hrm, I don't remember why, but I've specifically dropped the warning here. Now it seems like a good idea, so I'll add it here. > >I'd put this in net/core/dev.c and make it not be static inline. Again, I don't think it's really needed, as it's used in 4 functions (which aren't inline), so the benefits are minimal, if any. I'll rather keep them inline, I guess... > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 70256aa..d868858 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3388,6 +3388,20 @@ static inline const char *netdev_name(const struct net_device *dev) return dev->name; } +static inline const char *netdev_reg_state(const struct net_device *dev) +{ + switch (dev->reg_state) { + case NETREG_UNINITIALIZED: return " (unregistered)"; + case NETREG_REGISTERED: return ""; + case NETREG_UNREGISTERING: return " (unregistering)"; + case NETREG_UNREGISTERED: return " (unregistered)"; + case NETREG_RELEASED: return " (released)"; + case NETREG_DUMMY: return " (dummy)"; + } + + return " (unknown)"; +} + __printf(3, 4) int netdev_printk(const char *level, const struct net_device *dev, const char *format, ...); @@ -3444,7 +3458,8 @@ do { \ * file/line information and a backtrace. */ #define netdev_WARN(dev, format, args...) \ - WARN(1, "netdevice: %s\n" format, netdev_name(dev), ##args) + WARN(1, "netdevice: %s%s\n" format, netdev_name(dev), \ + netdev_reg_state(dev), ##args) /* netif printk helpers, similar to netdev_printk */ diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 7288e38..c9afbe2 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -614,13 +614,15 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor, char buf[PREFIX_SIZE]; res = dev_printk_emit(7, dev->dev.parent, - "%s%s %s %s: %pV", + "%s%s %s %s%s: %pV", dynamic_emit_prefix(descriptor, buf), dev_driver_string(dev->dev.parent), dev_name(dev->dev.parent), - netdev_name(dev), &vaf); + netdev_name(dev), netdev_reg_state(dev), + &vaf); } else if (dev) { - res = printk(KERN_DEBUG "%s: %pV", netdev_name(dev), &vaf); + res = printk(KERN_DEBUG "%s%s: %pV", netdev_name(dev), + netdev_reg_state(dev), &vaf); } else { res = printk(KERN_DEBUG "(NULL net_device): %pV", &vaf); } diff --git a/net/core/dev.c b/net/core/dev.c index 239722a..81d6101 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -6950,12 +6950,14 @@ static int __netdev_printk(const char *level, const struct net_device *dev, if (dev && dev->dev.parent) { r = dev_printk_emit(level[1] - '0', dev->dev.parent, - "%s %s %s: %pV", + "%s %s %s%s: %pV", dev_driver_string(dev->dev.parent), dev_name(dev->dev.parent), - netdev_name(dev), vaf); + netdev_name(dev), netdev_reg_state(dev), + vaf); } else if (dev) { - r = printk("%s%s: %pV", level, netdev_name(dev), vaf); + r = printk("%s%s%s: %pV", level, netdev_name(dev), + netdev_reg_state(dev), vaf); } else { r = printk("%s(NULL net_device): %pV", level, vaf); }
This way we'll always know in what status the device is, unless it's running normally (i.e. NETDEV_REGISTERED). CC: "David S. Miller" <davem@davemloft.net> CC: Jason Baron <jbaron@akamai.com> CC: Eric Dumazet <edumazet@google.com> CC: Vlad Yasevich <vyasevic@redhat.com> CC: stephen hemminger <stephen@networkplumber.org> CC: Jerry Chu <hkchu@google.com> CC: Ben Hutchings <bhutchings@solarflare.com> Signed-off-by: Veaceslav Falico <vfalico@gmail.com> --- include/linux/netdevice.h | 17 ++++++++++++++++- lib/dynamic_debug.c | 8 +++++--- net/core/dev.c | 8 +++++--- 3 files changed, 26 insertions(+), 7 deletions(-)