* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
@ 2006-04-09 17:56 ` Sergey Vlasov
2006-04-09 18:00 ` Andrey Borzenkov
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Sergey Vlasov @ 2006-04-09 17:56 UTC (permalink / raw)
To: Thomas de Grenier de Latour; +Cc: Linux-hotplug-devel, netdev
[-- Attachment #1: Type: text/plain, Size: 5493 bytes --]
On Sun, 9 Apr 2006 19:21:40 +0200 Thomas de Grenier de Latour wrote:
> Hi,
>
> I'm running Gentoo Linux, a 2.6.16-ck kernel, and udev-0.89-r2, and
> have had hard time with network interfaces renaming through udev
> rules. The first thing i've tried were rules like this one:
>
> SUBSYSTEM=="net", KERNEL=="eth*", SYSFS{address}=="00:0d:60:12:75:0a", NAME="lan"
>
> Plus this one from the standard early rules:
>
> ACTION=="add", SUBSYSTEM=="net", WAIT_FOR_SYSFS="address"
>
> It doesn't work when i "modprobe e1000" (my ethernet driver):
> ...
> udevd[21612]: udev_event_run: seq 956 forked, pid [21816], 'add' 'net', 0 seconds old
> udevd-event[21816]: wait_for_sysfs: file '/sys/class/net/eth0/address' appeared after 0 loops
> udevd-event[21816]: udev_rules_get_name: no node name set, will use kernel name 'eth0'
> ...
> But if i later do a "echo add > /sys/class/net/eth0/uevent", then
> interface is properly renamed. It also works fine if i start with
> the module initialy not loaded, and then trigger the uevent on the
> corresponding pci device (will load the module, etc.)
>
>
> Then i've tried replacing the address match by DRIVER=="e1000", with
> no more success. But i've noticed something interesting in debug
> output, when the rule matcher walks up in parent devices to check
> the driver:
> % grep -i driver /var/tmp/udev-net-debug.log
> ...
> udevd-event[18411]: match_key: key DRIVER value='e1000'
> udevd-event[18411]: match_key: match DRIVER 'e1000' <-> ''
> udevd-event[18411]: match_key: DRIVER is false
> udevd-event[18411]: sysfs_device_get: add to cache 'devpath=/devices/pci0000:00/0000:00:1e.0/0000:02:01.0', subsystem='pci', driver=''
> udevd-event[18411]: match_key: key DRIVER value='e1000'
> udevd-event[18411]: match_key: match DRIVER 'e1000' <-> ''
> udevd-event[18411]: match_key: DRIVER is false
> ...
> This is weird, because if i latter look in /sys, the "driver" links
> are here. Which made me think it was a race, so i've added this rule:
>
> ACTION=="add", SUBSYSTEM=="net", WAIT_FOR_SYSFS="device/driver"
>
> And it fixed the problem:
> ...
> udevd[21612]: udev_event_run: seq 950 forked, pid [21790], 'add' 'net', 0 seconds old
> udevd-event[21790]: wait_for_sysfs: file '/sys/class/net/eth0/address' appeared after 0 loops
> udevd-event[21790]: wait_for_sysfs: wait for '/sys/class/net/eth0/device/driver' for 20 mseconds
> udevd-event[21790]: wait_for_sysfs: file '/sys/class/net/eth0/device/driver' appeared after 1 loops
> udevd-event[21790]: udev_rules_get_name: rule applied, 'eth0' becomes 'lan'
> udevd-event[21790]: rename_net_if: changing net interface name from 'eth0' to 'lan'
> udevd-event[21790]: udev_add_device: renamed netif to 'lan'
> ...
>
> It also fixes the problem with using a SYSFS{address} match btw.
> With no such wait, i can see in debug that "address" is found in sysfs,
> but with no value:
> ...
> udevd-event[21977]: sysfs_attr_get_value: open '/class/net/eth0'/'address'
> udevd-event[21977]: sysfs_attr_get_value: new uncached attribute '/sys/class/net/eth0/address'
> udevd-event[21977]: sysfs_attr_get_value: add to cache '/sys/class/net/eth0/address'
> udevd-event[21977]: sysfs_attr_get_value: open '/class/net/eth0'/'address'
> ...
> Whereas with the wait-for-driver trick, i can see it read with a
> useful value:
> ...
> udevd-event[21954]: sysfs_attr_get_value: open '/class/net/eth0'/'address'
> udevd-event[21954]: sysfs_attr_get_value: new uncached attribute '/sys/class/net/eth0/address'
> udevd-event[21954]: sysfs_attr_get_value: add to cache '/sys/class/net/eth0/address'
> udevd-event[21954]: sysfs_attr_get_value: cache '/sys/class/net/eth0/address' with value '00:0d:60:12:75:0a'
> ...
>
> So i wonder, maybe such a rule should be added to the standard early
> ones? It should maybe use more checks though, to be sure there is
> actually a driver to wait. Something like ENV{PHYSDEVPATH}=="?*"
> and/or ENV{PHYSDEVDRIVER}=="?*".
>
> Btw, using ENV{PHYSDEVDRIVER}=="e1000" in my renaming rule was working
> fine, with no trick (this variables are correctly set, like 'udevmonitor
> --env' shows).
>
>
> So, what do you think, does such a rule makes sense?
> Or is "address" being added to sysfs with no useful value yet the real
> issue, and my rule only an ugly workround?
(quoting the whole message for netdev)
Apparently there is a race there. The "add" uevent for /class/net/eth0
is emitted by class_device_register(), which is called by
netdev_register_sysfs(). Class device attributes (like "address") are
also added by class_device_register() (really class_device_add(), which
is invoked from there), but this is done before generating the uevent,
so at the first glance there is no race here (and waiting for "address"
is unnecessary). However, show_address() does not output anything
unless dev->reg_state == NETREG_REGISTERED - and this state is set by
netdev_run_todo() only after netdev_register_sysfs() returns, so in the
meantime (while netdev_register_sysfs() is busy adding the "statistics"
attribute group) some process may see an empty "address" attribute.
Waiting for "device/driver" ensures that udevd continues to process the
uevent only after the probe function completes, which ensures completion
of the registration (netdev_run_todo() is invoked by rtnl_unlock() at
the end of register_netdev() and processes all pending requests).
[-- Attachment #2: Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
2006-04-09 17:56 ` Sergey Vlasov
@ 2006-04-09 18:00 ` Andrey Borzenkov
2006-04-09 19:28 ` Thomas de Grenier de Latour
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Andrey Borzenkov @ 2006-04-09 18:00 UTC (permalink / raw)
To: linux-hotplug
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Sunday 09 April 2006 21:21, Thomas de Grenier de Latour wrote:
> Hi,
>
> I'm running Gentoo Linux, a 2.6.16-ck kernel, and udev-0.89-r2, and
> have had hard time with network interfaces renaming through udev
> rules. The first thing i've tried were rules like this one:
>
> SUBSYSTEM="net", KERNEL="eth*", SYSFS{address}="00:0d:60:12:75:0a",
> NAME="lan"
>
> Plus this one from the standard early rules:
>
> ACTION="add", SUBSYSTEM="net", WAIT_FOR_SYSFS="address"
>
> It doesn't work when i "modprobe e1000" (my ethernet driver):
> ....
> udevd[21612]: udev_event_run: seq 956 forked, pid [21816], 'add' 'net', 0
> seconds old udevd-event[21816]: wait_for_sysfs: file
> '/sys/class/net/eth0/address' appeared after 0 loops udevd-event[21816]:
> udev_rules_get_name: no node name set, will use kernel name 'eth0' ....
> But if i later do a "echo add > /sys/class/net/eth0/uevent", then
> interface is properly renamed. It also works fine if i start with
> the module initialy not loaded, and then trigger the uevent on the
> corresponding pci device (will load the module, etc.)
>
There appears to be race between creating 'address' and getting (usable)
value. See below.
[...]
>
> So i wonder, maybe such a rule should be added to the standard early
> ones? It should maybe use more checks though, to be sure there is
> actually a driver to wait. Something like ENV{PHYSDEVPATH}="?*"
> and/or ENV{PHYSDEVDRIVER}="?*".
>
You probably get reply that they are obsolete and should not be used in new
rules :)
[...]
> Or is "address" being added to sysfs with no useful value yet the real
> issue, and my rule only an ugly workround?
>
Seems so.
net/core/net-syfs.c:dev_isalive():
return dev->reg_state = NETREG_REGISTERED;
net/core/net-syfs.c:show_address():
if (dev_isalive(net))
ret = format_addr(buf, net->dev_addr, net->addr_len);
net/core/dev.c:netdev_run_todo():
err = netdev_register_sysfs(dev);
if (err)
printk(KERN_ERR "%s: failed sysfs registration
(%d)\n",
dev->name, err);
dev->reg_state = NETREG_REGISTERED;
break;
So basically there is a window between net device appearing in sysfs and
'address' returning usable value. To test you could move dev->reg_state
assignment before netdev_register_sysfs call; if this helps it makes sense
ask on net list if this is safe change.
- -andrey
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
iD8DBQFEOUvTR6LMutpd94wRAg+nAKCXnI9mrChpQ9TAz4VVQhSaP/MQjgCfbX6n
5lcJ30K1qpTtbxISei6pN9I=Rwsc
-----END PGP SIGNATURE-----
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x110944&bid$1720&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
2006-04-09 17:56 ` Sergey Vlasov
2006-04-09 18:00 ` Andrey Borzenkov
@ 2006-04-09 19:28 ` Thomas de Grenier de Latour
2006-04-13 8:39 ` Marco d'Itri
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Thomas de Grenier de Latour @ 2006-04-09 19:28 UTC (permalink / raw)
To: linux-hotplug
On Sun, 9 Apr 2006 22:00:49 +0400,
Andrey Borzenkov <arvidjaar@mail.ru> wrote:
> net/core/dev.c:netdev_run_todo():
> err = netdev_register_sysfs(dev);
> if (err)
> printk(KERN_ERR "%s: failed sysfs
> registration (%d)\n",
> dev->name, err);
> dev->reg_state = NETREG_REGISTERED;
> break;
>
> So basically there is a window between net device appearing in sysfs
> and 'address' returning usable value. To test you could move
> dev->reg_state assignment before netdev_register_sysfs call; if this
> helps it makes sense ask on net list if this is safe change.
As far as i can test, yes, it works fine. I will forward your
suggestion to -netdev@.
Btw, many thanks to both of you, Andrey and Sergey, for the fast
answers and crystal-clear explanations of the actual issue.
--
TGL.
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x110944&bid$1720&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
` (2 preceding siblings ...)
2006-04-09 19:28 ` Thomas de Grenier de Latour
@ 2006-04-13 8:39 ` Marco d'Itri
2006-04-13 9:06 ` Thomas de Grenier de Latour
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Marco d'Itri @ 2006-04-13 8:39 UTC (permalink / raw)
To: linux-hotplug
On Apr 09, Andrey Borzenkov <arvidjaar@mail.ru> wrote:
> > So i wonder, maybe such a rule should be added to the standard early
> > ones? It should maybe use more checks though, to be sure there is
> > actually a driver to wait. Something like ENV{PHYSDEVPATH}="?*"
> > and/or ENV{PHYSDEVDRIVER}="?*".
> >
>
> You probably get reply that they are obsolete and should not be used in new
> rules :)
So, is this an acceptable workaround until the kernel is fixed or not?
I implemented interfaces persistence for the Debian package, but users
will get mad if the rules are randomly ignored (which apparently is
already happening).
Also, is there a kernel patch yet?
--
ciao,
Marco
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x110944&bid$1720&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
` (3 preceding siblings ...)
2006-04-13 8:39 ` Marco d'Itri
@ 2006-04-13 9:06 ` Thomas de Grenier de Latour
2006-04-13 10:29 ` Kay Sievers
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Thomas de Grenier de Latour @ 2006-04-13 9:06 UTC (permalink / raw)
To: linux-hotplug
On Thu, 13 Apr 2006 10:39:53 +0200,
md@Linux.IT (Marco d'Itri) wrote:
>
> Also, is there a kernel patch yet?
>
Yes, this one has been applied:
http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdiff;h'31570eba5b35a21c311dd587057c39805082f1;hpflb62998866ae2e298139164a85ec0757b7f3fc7;hbî69d458b90bfb9117cbb488cfa645d94c3921b1;f=net/core/dev.c
--
TGL.
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x110944&bid$1720&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
` (4 preceding siblings ...)
2006-04-13 9:06 ` Thomas de Grenier de Latour
@ 2006-04-13 10:29 ` Kay Sievers
2006-04-13 10:36 ` Marco d'Itri
2006-04-13 11:10 ` Kay Sievers
7 siblings, 0 replies; 9+ messages in thread
From: Kay Sievers @ 2006-04-13 10:29 UTC (permalink / raw)
To: linux-hotplug
On Thu, Apr 13, 2006 at 10:39:53AM +0200, Marco d'Itri wrote:
> On Apr 09, Andrey Borzenkov <arvidjaar@mail.ru> wrote:
>
> > > So i wonder, maybe such a rule should be added to the standard early
> > > ones? It should maybe use more checks though, to be sure there is
> > > actually a driver to wait. Something like ENV{PHYSDEVPATH}="?*"
> > > and/or ENV{PHYSDEVDRIVER}="?*".
> > >
> >
> > You probably get reply that they are obsolete and should not be used in new
> > rules :)
>
> So, is this an acceptable workaround until the kernel is fixed or not?
Sure, it will not go away soon, and for temporary fixes, if that helps
working around something that is on the way to be fixed, it's fine.
But right, the longer term goal is that the notion of "physical devices"
and "virtual devices" should not be exported that way by the kernel
and therefore the use of these values, or the "device" link, or the
<subsystem:kernel_name> symlinks, or a hardcoded sequence of sysfs parent
devices must be avoided for other things than temporary workarounds.
Thanks,
Kay
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x110944&bid$1720&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
` (5 preceding siblings ...)
2006-04-13 10:29 ` Kay Sievers
@ 2006-04-13 10:36 ` Marco d'Itri
2006-04-13 11:10 ` Kay Sievers
7 siblings, 0 replies; 9+ messages in thread
From: Marco d'Itri @ 2006-04-13 10:36 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]
On Apr 13, Kay Sievers <kay.sievers@vrfy.org> wrote:
> But right, the longer term goal is that the notion of "physical devices"
> and "virtual devices" should not be exported that way by the kernel
> and therefore the use of these values, or the "device" link, or the
> <subsystem:kernel_name> symlinks, or a hardcoded sequence of sysfs parent
> devices must be avoided for other things than temporary workarounds.
How should I rewrite this code then?
device_description() {
local bus=$(sysreadlink device/bus)
bus=${bus##*/}
if [ "$bus" = pci ]; then
local vendor_id=$(sysread device/vendor)
local device_id=$(sysread device/device)
echo -n "PCI device ${vendor_id#0x}:${device_id#0x}"
elif [ "$bus" = usb ]; then
local device=$(sysreadlink device)
if [ "$device" -a -e $device/../idVendor ]; then
local idVendor idProduct
read idVendor < $device/../idVendor || true
read idProduct < $device/../idProduct || true
printf 'USB device %x/%x' 0x$idVendor 0x$idProduct
else
echo -n "UNKNOWN USB device ($DEVPATH)"
fi
elif [ "$bus" ]; then
echo -n "UNKNOWN $bus device ($DEVPATH)"
else
echo -n "UNKNOWN device ($DEVPATH)"
fi
local driver=$(sysreadlink device/driver)
if [ "$driver" ]; then echo -n " (${driver##*/})"; fi
}
--
ciao,
Marco
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: net interface renaming issue (+fix?)
2006-04-09 17:21 net interface renaming issue (+fix?) Thomas de Grenier de Latour
` (6 preceding siblings ...)
2006-04-13 10:36 ` Marco d'Itri
@ 2006-04-13 11:10 ` Kay Sievers
7 siblings, 0 replies; 9+ messages in thread
From: Kay Sievers @ 2006-04-13 11:10 UTC (permalink / raw)
To: linux-hotplug
On Thu, Apr 13, 2006 at 12:36:27PM +0200, Marco d'Itri wrote:
> On Apr 13, Kay Sievers <kay.sievers@vrfy.org> wrote:
>
> > But right, the longer term goal is that the notion of "physical devices"
> > and "virtual devices" should not be exported that way by the kernel
> > and therefore the use of these values, or the "device" link, or the
> > <subsystem:kernel_name> symlinks, or a hardcoded sequence of sysfs parent
> > devices must be avoided for other things than temporary workarounds.
>
> How should I rewrite this code then?
>
> device_description() {
> local bus=$(sysreadlink device/bus)
> bus=${bus##*/}
>
> if [ "$bus" = pci ]; then
> local vendor_id=$(sysread device/vendor)
> local device_id=$(sysread device/device)
> echo -n "PCI device ${vendor_id#0x}:${device_id#0x}"
> elif [ "$bus" = usb ]; then
> local device=$(sysreadlink device)
> if [ "$device" -a -e $device/../idVendor ]; then
> local idVendor idProduct
> read idVendor < $device/../idVendor || true
> read idProduct < $device/../idProduct || true
> printf 'USB device %x/%x' 0x$idVendor 0x$idProduct
> else
> echo -n "UNKNOWN USB device ($DEVPATH)"
> fi
> elif [ "$bus" ]; then
> echo -n "UNKNOWN $bus device ($DEVPATH)"
> else
> echo -n "UNKNOWN device ($DEVPATH)"
> fi
>
> local driver=$(sysreadlink device/driver)
> if [ "$driver" ]; then echo -n " (${driver##*/})"; fi
> }
In the end, we will have only one tree in sysfs and the devpath will
contain the whole dependency of devices for every device. eth0 will
have:
/devices/pci0000:00/0000:00:1c.0/0000:02:00.0/eth0
as devpath instead of the current /class/net/eth0, which will only be
a silent symlink to that location. When you get the event, you have to
walk up the chain of parents in the devpath until you find the device
you are looking for, you can't rely on a specific sequence, there may be
a device inserted if the kernel device model needs that. That's why the
"device" link does not make sense at that point.
So you would walk up until you find a device of the subsystem you are
looking for like "pci" or "usb" and then read these values. See the
usb_id logic:
http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=blob;h…c6beda40e38d307d355aed4d8b789bb193cca4;hb\f4a805bccbc52e48f972a87f83007fdb0ffe19e;f=extras/usb_id/usb_id.c#l250
It does not use any of these values and requests parent devices only by
subsystem and can skip newly inserted parents in the devpath without breakage.
Udev already does all that today, so it may make sense to get udev to
read these values and pass them to the script.
Or we export them with rules to the environment (needs a change to udev
to apply the format to the ENV key at that time).
Or we provide some logic from udev_sysfs.c as a similar shell function to
include by script users:
http://www.kernel.org/git/?p=linux/hotplug/udev.git;a=blob;hÝc0b46790066e30bede2c9bb0026a271633795c;hb\f4a805bccbc52e48f972a87f83007fdb0ffe19e;f=udev_sysfs.c#l196
Let me know if we should add something to udev itself to solve that.
Thanks,
Kay
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid\x110944&bid$1720&dat\x121642
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
^ permalink raw reply [flat|nested] 9+ messages in thread