* [PATCH] add sysfs attribute 'carrier' for net devices @ 2004-09-26 22:51 ` Jesper Juhl 2004-09-27 17:29 ` Stephen Hemminger 0 siblings, 1 reply; 11+ messages in thread From: Jesper Juhl @ 2004-09-26 22:51 UTC (permalink / raw) To: linux-kernel; +Cc: Stephen Hemminger, Nico Schottelius Hi, Here's a patch that adds a new sysfs attribute for net devices. The new attribute 'carrier' exposes the result of netif_carrier_ok() so that when a network device has carrier the attribute value is 1 and when there is no carrier the attribute value is 0. Very rellevant attribute for network devices in my oppinion, and sysfs is the logical place for it. I've tested this only on my own machine, but I get the expected results: With network cable plugged into eth0 : juhl@dragon:~$ cat /sys/class/net/eth0/carrier 1 juhl@dragon:~$ With network cable unplugged : juhl@dragon:~$ cat /sys/class/net/eth0/carrier 0 juhl@dragon:~$ Please review and consider applying. Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk> diff -u linux-2.6.9-rc2-bk5-orig/net/core/net-sysfs.c linux-2.6.9-rc2-bk5/net/core/net-sysfs.c --- linux-2.6.9-rc2-bk5-orig/net/core/net-sysfs.c 2004-09-14 23:19:53.000000000 +0200 +++ linux-2.6.9-rc2-bk5/net/core/net-sysfs.c 2004-09-27 00:24:01.000000000 +0200 @@ -126,8 +126,21 @@ return -EINVAL; } +static ssize_t show_carrier(struct class_device *dev, char *buf) +{ + struct net_device *net = to_net_dev(dev); + if (netif_running(net)) { + if (netif_carrier_ok(net)) + return snprintf(buf, 3, "%d\n", 1); + else + return snprintf(buf, 3, "%d\n", 0); + } + return -EINVAL; +} + static CLASS_DEVICE_ATTR(address, S_IRUGO, show_address, NULL); static CLASS_DEVICE_ATTR(broadcast, S_IRUGO, show_broadcast, NULL); +static CLASS_DEVICE_ATTR(carrier, S_IRUGO, show_carrier, NULL); /* read-write attributes */ NETDEVICE_SHOW(mtu, fmt_dec); @@ -186,6 +199,7 @@ &class_device_attr_type, &class_device_attr_address, &class_device_attr_broadcast, + &class_device_attr_carrier, NULL }; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices 2004-09-26 22:51 ` [PATCH] add sysfs attribute 'carrier' for net devices Jesper Juhl @ 2004-09-27 17:29 ` Stephen Hemminger 2004-09-28 11:18 ` Jesper Juhl 2004-09-28 12:10 ` [PATCH] add sysfs attribute 'carrier' for net devices Paulo Marques 0 siblings, 2 replies; 11+ messages in thread From: Stephen Hemminger @ 2004-09-27 17:29 UTC (permalink / raw) To: Jesper Juhl; +Cc: linux-kernel, Nico Schottelius On Mon, 2004-09-27 at 00:51 +0200, Jesper Juhl wrote: > Hi, > > Here's a patch that adds a new sysfs attribute for net devices. The new > attribute 'carrier' exposes the result of netif_carrier_ok() so that when > a network device has carrier the attribute value is 1 and when there is no > carrier the attribute value is 0. > Very rellevant attribute for network devices in my oppinion, and sysfs is > the logical place for it. > > I've tested this only on my own machine, but I get the expected results: > > With network cable plugged into eth0 : > > juhl@dragon:~$ cat /sys/class/net/eth0/carrier > 1 > juhl@dragon:~$ > > With network cable unplugged : > > juhl@dragon:~$ cat /sys/class/net/eth0/carrier > 0 > juhl@dragon:~$ > > Please review and consider applying. > > Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk> > > diff -u linux-2.6.9-rc2-bk5-orig/net/core/net-sysfs.c linux-2.6.9-rc2-bk5/net/core/net-sysfs.c > --- linux-2.6.9-rc2-bk5-orig/net/core/net-sysfs.c 2004-09-14 23:19:53.000000000 +0200 > +++ linux-2.6.9-rc2-bk5/net/core/net-sysfs.c 2004-09-27 00:24:01.000000000 +0200 > @@ -126,8 +126,21 @@ > return -EINVAL; > } > > +static ssize_t show_carrier(struct class_device *dev, char *buf) > +{ > + struct net_device *net = to_net_dev(dev); > + if (netif_running(net)) { > + if (netif_carrier_ok(net)) > + return snprintf(buf, 3, "%d\n", 1); > + else > + return snprintf(buf, 3, "%d\n", 0); Using snprintf in this way is kind of silly. since buffer is PAGESIZE. The most concise format of this would be: return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); > + } > + return -EINVAL; > +} > + > static CLASS_DEVICE_ATTR(address, S_IRUGO, show_address, NULL); > static CLASS_DEVICE_ATTR(broadcast, S_IRUGO, show_broadcast, NULL); > +static CLASS_DEVICE_ATTR(carrier, S_IRUGO, show_carrier, NULL); > > /* read-write attributes */ > NETDEVICE_SHOW(mtu, fmt_dec); > @@ -186,6 +199,7 @@ > &class_device_attr_type, > &class_device_attr_address, > &class_device_attr_broadcast, > + &class_device_attr_carrier, > NULL > }; > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices 2004-09-27 17:29 ` Stephen Hemminger @ 2004-09-28 11:18 ` Jesper Juhl 2004-09-28 19:23 ` [PATCH] add sysfs attribute 'carrier' for net devices - try 2 Jesper Juhl 2004-09-28 12:10 ` [PATCH] add sysfs attribute 'carrier' for net devices Paulo Marques 1 sibling, 1 reply; 11+ messages in thread From: Jesper Juhl @ 2004-09-28 11:18 UTC (permalink / raw) To: Stephen Hemminger; +Cc: linux-kernel, Nico Schottelius On Mon, 27 Sep 2004, Stephen Hemminger wrote: > Date: Mon, 27 Sep 2004 10:29:13 -0700 > From: Stephen Hemminger <shemminger@osdl.org> > To: Jesper Juhl <juhl-lkml@dif.dk> > Cc: linux-kernel <linux-kernel@vger.kernel.org>, > Nico Schottelius <nico-kernel@schottelius.org> > Subject: Re: [PATCH] add sysfs attribute 'carrier' for net devices > > On Mon, 2004-09-27 at 00:51 +0200, Jesper Juhl wrote: > > > > +static ssize_t show_carrier(struct class_device *dev, char *buf) > > +{ > > + struct net_device *net = to_net_dev(dev); > > + if (netif_running(net)) { > > + if (netif_carrier_ok(net)) > > + return snprintf(buf, 3, "%d\n", 1); > > + else > > + return snprintf(buf, 3, "%d\n", 0); > > Using snprintf in this way is kind of silly. since buffer is PAGESIZE. > The most concise format of this would be: > return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); > I see your point and I'll create a new patch this evening when I get home from work. Thank you for your feedback. -- Jesper Juhl ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] add sysfs attribute 'carrier' for net devices - try 2. 2004-09-28 11:18 ` Jesper Juhl @ 2004-09-28 19:23 ` Jesper Juhl 2004-09-28 19:22 ` Stephen Hemminger 2004-09-28 20:54 ` Nico Schottelius 0 siblings, 2 replies; 11+ messages in thread From: Jesper Juhl @ 2004-09-28 19:23 UTC (permalink / raw) To: Stephen Hemminger Cc: linux-kernel, Nico Schottelius, Paulo Marques, Martin Waitz On Tue, 28 Sep 2004, Jesper Juhl wrote: > On Mon, 27 Sep 2004, Stephen Hemminger wrote: > > > Date: Mon, 27 Sep 2004 10:29:13 -0700 > > From: Stephen Hemminger <shemminger@osdl.org> > > To: Jesper Juhl <juhl-lkml@dif.dk> > > Cc: linux-kernel <linux-kernel@vger.kernel.org>, > > Nico Schottelius <nico-kernel@schottelius.org> > > Subject: Re: [PATCH] add sysfs attribute 'carrier' for net devices > > > > On Mon, 2004-09-27 at 00:51 +0200, Jesper Juhl wrote: > > > > > > +static ssize_t show_carrier(struct class_device *dev, char *buf) > > > +{ > > > + struct net_device *net = to_net_dev(dev); > > > + if (netif_running(net)) { > > > + if (netif_carrier_ok(net)) > > > + return snprintf(buf, 3, "%d\n", 1); > > > + else > > > + return snprintf(buf, 3, "%d\n", 0); > > > > Using snprintf in this way is kind of silly. since buffer is PAGESIZE. > > The most concise format of this would be: > > return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); > > > > I see your point and I'll create a new patch this evening when I get home > from work. > Thank you for your feedback. > Here's a second try at a patch to properly implement a 'carrier' sysfs attribute for net devices. Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk> diff -u linux-2.6.9-rc2-bk14-orig/net/core/net-sysfs.c linux-2.6.9-rc2-bk14/net/core/net-sysfs.c --- linux-2.6.9-rc2-bk14-orig/net/core/net-sysfs.c 2004-09-14 23:19:53.000000000 +0200 +++ linux-2.6.9-rc2-bk14/net/core/net-sysfs.c 2004-09-28 19:37:45.000000000 +0200 @@ -126,8 +126,18 @@ return -EINVAL; } +static ssize_t show_carrier(struct class_device *dev, char *buf) +{ + struct net_device *netdev = to_net_dev(dev); + if (netif_running(netdev)) { + return sprintf(buf, fmt_dec, !!netif_carrier_ok(netdev)); + } + return -EINVAL; +} + static CLASS_DEVICE_ATTR(address, S_IRUGO, show_address, NULL); static CLASS_DEVICE_ATTR(broadcast, S_IRUGO, show_broadcast, NULL); +static CLASS_DEVICE_ATTR(carrier, S_IRUGO, show_carrier, NULL); /* read-write attributes */ NETDEVICE_SHOW(mtu, fmt_dec); @@ -186,6 +196,7 @@ &class_device_attr_type, &class_device_attr_address, &class_device_attr_broadcast, + &class_device_attr_carrier, NULL }; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices - try 2. 2004-09-28 19:23 ` [PATCH] add sysfs attribute 'carrier' for net devices - try 2 Jesper Juhl @ 2004-09-28 19:22 ` Stephen Hemminger 2004-09-28 20:54 ` Nico Schottelius 1 sibling, 0 replies; 11+ messages in thread From: Stephen Hemminger @ 2004-09-28 19:22 UTC (permalink / raw) To: Jesper Juhl; +Cc: linux-kernel, Nico Schottelius, Paulo Marques, Martin Waitz On Tue, 2004-09-28 at 21:23 +0200, Jesper Juhl wrote: > On Tue, 28 Sep 2004, Jesper Juhl wrote: > > > On Mon, 27 Sep 2004, Stephen Hemminger wrote: > > > > > Date: Mon, 27 Sep 2004 10:29:13 -0700 > > > From: Stephen Hemminger <shemminger@osdl.org> > > > To: Jesper Juhl <juhl-lkml@dif.dk> > > > Cc: linux-kernel <linux-kernel@vger.kernel.org>, > > > Nico Schottelius <nico-kernel@schottelius.org> > > > Subject: Re: [PATCH] add sysfs attribute 'carrier' for net devices > > > > > > On Mon, 2004-09-27 at 00:51 +0200, Jesper Juhl wrote: > > > > > > > > +static ssize_t show_carrier(struct class_device *dev, char *buf) > > > > +{ > > > > + struct net_device *net = to_net_dev(dev); > > > > + if (netif_running(net)) { > > > > + if (netif_carrier_ok(net)) > > > > + return snprintf(buf, 3, "%d\n", 1); > > > > + else > > > > + return snprintf(buf, 3, "%d\n", 0); > > > > > > Using snprintf in this way is kind of silly. since buffer is PAGESIZE. > > > The most concise format of this would be: > > > return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); > > > > > > > I see your point and I'll create a new patch this evening when I get home > > from work. > > Thank you for your feedback. > > > > Here's a second try at a patch to properly implement a 'carrier' sysfs > attribute for net devices. > > Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk> > > diff -u linux-2.6.9-rc2-bk14-orig/net/core/net-sysfs.c linux-2.6.9-rc2-bk14/net/core/net-sysfs.c > --- linux-2.6.9-rc2-bk14-orig/net/core/net-sysfs.c 2004-09-14 23:19:53.000000000 +0200 > +++ linux-2.6.9-rc2-bk14/net/core/net-sysfs.c 2004-09-28 19:37:45.000000000 +0200 > @@ -126,8 +126,18 @@ > return -EINVAL; > } > > +static ssize_t show_carrier(struct class_device *dev, char *buf) > +{ > + struct net_device *netdev = to_net_dev(dev); > + if (netif_running(netdev)) { > + return sprintf(buf, fmt_dec, !!netif_carrier_ok(netdev)); > + } > + return -EINVAL; > +} > + > static CLASS_DEVICE_ATTR(address, S_IRUGO, show_address, NULL); > static CLASS_DEVICE_ATTR(broadcast, S_IRUGO, show_broadcast, NULL); > +static CLASS_DEVICE_ATTR(carrier, S_IRUGO, show_carrier, NULL); > > /* read-write attributes */ > NETDEVICE_SHOW(mtu, fmt_dec); > @@ -186,6 +196,7 @@ > &class_device_attr_type, > &class_device_attr_address, > &class_device_attr_broadcast, > + &class_device_attr_carrier, > NULL > }; Looks great, it kind of shows how easy sysfs is but how baroque it is as well. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices - try 2. 2004-09-28 19:23 ` [PATCH] add sysfs attribute 'carrier' for net devices - try 2 Jesper Juhl 2004-09-28 19:22 ` Stephen Hemminger @ 2004-09-28 20:54 ` Nico Schottelius 2004-09-28 22:24 ` Stephen Hemminger 1 sibling, 1 reply; 11+ messages in thread From: Nico Schottelius @ 2004-09-28 20:54 UTC (permalink / raw) To: Jesper Juhl Cc: Stephen Hemminger, linux-kernel, Nico Schottelius, Paulo Marques, Martin Waitz [-- Attachment #1: Type: text/plain, Size: 647 bytes --] Hello! I am just a bit tired and a bit confused, but Jesper Juhl [Tue, Sep 28, 2004 at 09:23:25PM +0200]: > > > Using snprintf in this way is kind of silly. since buffer is PAGESIZE. > > > The most concise format of this would be: > > > return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); wouldn't this potentially open a security hole / buffer overflow? I am currently not really seeing where buf is passed from and what dec_fmt is, but am I totally wrong? Nico -- Keep it simple & stupid, use what's available. Please use pgp encryption: 8D0E 27A4 is my id. http://nico.schotteli.us | http://linux.schottelius.org [-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices - try 2. 2004-09-28 20:54 ` Nico Schottelius @ 2004-09-28 22:24 ` Stephen Hemminger 2004-09-29 6:54 ` Nico Schottelius 0 siblings, 1 reply; 11+ messages in thread From: Stephen Hemminger @ 2004-09-28 22:24 UTC (permalink / raw) To: Nico Schottelius; +Cc: Jesper Juhl, linux-kernel, Paulo Marques, Martin Waitz On Tue, 2004-09-28 at 22:54 +0200, Nico Schottelius wrote: > Hello! > > I am just a bit tired and a bit confused, but > > Jesper Juhl [Tue, Sep 28, 2004 at 09:23:25PM +0200]: > > > > Using snprintf in this way is kind of silly. since buffer is PAGESIZE. > > > > The most concise format of this would be: > > > > return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); > > wouldn't this potentially open a security hole / buffer overflow? buf comes from fs/sysfs/file.c:fill_read_buf and is PAGESIZE. Since netif_carrier_ok() returns 0 or 1, don't see how that could ever turn into a security hole. > I am currently not really seeing where buf is passed from and what > dec_fmt is, but am I totally wrong? dec_fmt is in net-sysfs.c and is defined as "%d\n" to avoid repetition of same string literal. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices - try 2. 2004-09-28 22:24 ` Stephen Hemminger @ 2004-09-29 6:54 ` Nico Schottelius 0 siblings, 0 replies; 11+ messages in thread From: Nico Schottelius @ 2004-09-29 6:54 UTC (permalink / raw) To: Stephen Hemminger Cc: Nico Schottelius, Jesper Juhl, linux-kernel, Paulo Marques, Martin Waitz [-- Attachment #1: Type: text/plain, Size: 1219 bytes --] Thanks for enlightening [1] me! Nico P.S.: [1] enlightenment.org is still alive, just wanted to note that.. Stephen Hemminger [Tue, Sep 28, 2004 at 03:24:43PM -0700]: > On Tue, 2004-09-28 at 22:54 +0200, Nico Schottelius wrote: > > Hello! > > > > I am just a bit tired and a bit confused, but > > > > Jesper Juhl [Tue, Sep 28, 2004 at 09:23:25PM +0200]: > > > > > Using snprintf in this way is kind of silly. since buffer is PAGESIZE. > > > > > The most concise format of this would be: > > > > > return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); > > > > wouldn't this potentially open a security hole / buffer overflow? > > buf comes from fs/sysfs/file.c:fill_read_buf and is PAGESIZE. > Since netif_carrier_ok() returns 0 or 1, don't see how that could ever > turn into a security hole. > > > > I am currently not really seeing where buf is passed from and what > > dec_fmt is, but am I totally wrong? > > dec_fmt is in net-sysfs.c and is defined as "%d\n" to avoid > repetition of same string literal. > > -- Keep it simple & stupid, use what's available. Please use pgp encryption: 8D0E 27A4 is my id. http://nico.schotteli.us | http://linux.schottelius.org [-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices 2004-09-27 17:29 ` Stephen Hemminger 2004-09-28 11:18 ` Jesper Juhl @ 2004-09-28 12:10 ` Paulo Marques 2004-09-28 16:09 ` Martin Waitz 1 sibling, 1 reply; 11+ messages in thread From: Paulo Marques @ 2004-09-28 12:10 UTC (permalink / raw) To: Stephen Hemminger; +Cc: Jesper Juhl, linux-kernel, Nico Schottelius Stephen Hemminger wrote: > .... >>+{ >>+ struct net_device *net = to_net_dev(dev); >>+ if (netif_running(net)) { >>+ if (netif_carrier_ok(net)) >>+ return snprintf(buf, 3, "%d\n", 1); >>+ else >>+ return snprintf(buf, 3, "%d\n", 0); > > > Using snprintf in this way is kind of silly. since buffer is PAGESIZE. > The most concise format of this would be: > return sprintf(buf, dec_fmt, !!netif_carrier_ok(dev)); <nitpick> Since netif_carrier_ok already has a "!" in it, it is guaranteed to return a 0 / 1 result. so this could be: return sprintf(buf, dec_fmt, netif_carrier_ok(dev)); Of course your way is more robust to future 'netif_carrier_ok' changes and the compiler should optimize it way anyway since it is an inline function, so I actually prefer the !! version :) </nitpick> -- Paulo Marques - www.grupopie.com To err is human, but to really foul things up requires a computer. Farmers' Almanac, 1978 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices 2004-09-28 12:10 ` [PATCH] add sysfs attribute 'carrier' for net devices Paulo Marques @ 2004-09-28 16:09 ` Martin Waitz 2004-10-23 18:49 ` Nico Schottelius 0 siblings, 1 reply; 11+ messages in thread From: Martin Waitz @ 2004-09-28 16:09 UTC (permalink / raw) To: Paulo Marques Cc: Stephen Hemminger, Jesper Juhl, linux-kernel, Nico Schottelius [-- Attachment #1: Type: text/plain, Size: 372 bytes --] hi :) On Tue, Sep 28, 2004 at 01:10:21PM +0100, Paulo Marques wrote: > Of course your way is more robust to future 'netif_carrier_ok' changes > and the compiler should optimize it way anyway since it is an inline > function, so I actually prefer the !! version :) But perhaps those future changes would be interesting for userspace, too? -- Martin Waitz [-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] add sysfs attribute 'carrier' for net devices 2004-09-28 16:09 ` Martin Waitz @ 2004-10-23 18:49 ` Nico Schottelius 0 siblings, 0 replies; 11+ messages in thread From: Nico Schottelius @ 2004-10-23 18:49 UTC (permalink / raw) To: Paulo Marques, Stephen Hemminger, Jesper Juhl, linux-kernel, Nico Schottelius [-- Attachment #1: Type: text/plain, Size: 581 bytes --] > On Fri, 22 Oct 2004, Jesper Juhl wrote: > [sysfs kernel patch] > Good news, > Linus has merged the patch in 2.6.10-rc1 so unless someone finds a flaw > with it or otherwise complains about it it should be in 2.6.10 once that > is released. I tested it in 2.6.9 and it works without any problems. I even included it in an isntallation script and tested pluggin cable out and in, works like charme. Nico -- Keep it simple & stupid, use what's available. Please use pgp encryption: 8D0E 27A4 is my id. http://nico.schotteli.us | http://linux.schottelius.org [-- Attachment #2: Type: application/pgp-signature, Size: 827 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2004-10-23 18:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20041021112750.GN15294@schottelius.org>
[not found] ` <Pine.LNX.4.61.0410221744500.15769@jjulnx.backbone.dif.dk>
2004-09-26 22:51 ` [PATCH] add sysfs attribute 'carrier' for net devices Jesper Juhl
2004-09-27 17:29 ` Stephen Hemminger
2004-09-28 11:18 ` Jesper Juhl
2004-09-28 19:23 ` [PATCH] add sysfs attribute 'carrier' for net devices - try 2 Jesper Juhl
2004-09-28 19:22 ` Stephen Hemminger
2004-09-28 20:54 ` Nico Schottelius
2004-09-28 22:24 ` Stephen Hemminger
2004-09-29 6:54 ` Nico Schottelius
2004-09-28 12:10 ` [PATCH] add sysfs attribute 'carrier' for net devices Paulo Marques
2004-09-28 16:09 ` Martin Waitz
2004-10-23 18:49 ` Nico Schottelius
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).