Re: net interface renaming issue (+fix?)

[Sergey Vlasov <vsu@altlinux.ru>]

[-- 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 --]