| Message ID | 1399390594-1409-1-git-send-email-boris.brezillon@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
From: Boris BREZILLON <boris.brezillon@free-electrons.com> Date: Tue, 6 May 2014 17:36:34 +0200 > There is currently no proper way to bind a net interface to a specific > name. The interface name is chosen based on the interface type (eth, > wlan, ...) and the interfaces already registered (the core codes takes > the first unused interface id of the given type). > > Add support for DT retrieval of the interface id based on DT aliases. > The alias name must match the interface type (e.g. ethX if you're defining > an ethernet dev alias). > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> This really isn't kosher at all. And there absolutely is a proper way to bind a net interface to a specific name, udev has provided this facility for years. I'm not applying this, sorry.
Hi David, On 09/05/2014 04:42, David Miller wrote: > From: Boris BREZILLON <boris.brezillon@free-electrons.com> > Date: Tue, 6 May 2014 17:36:34 +0200 > >> There is currently no proper way to bind a net interface to a specific >> name. The interface name is chosen based on the interface type (eth, >> wlan, ...) and the interfaces already registered (the core codes takes >> the first unused interface id of the given type). >> >> Add support for DT retrieval of the interface id based on DT aliases. >> The alias name must match the interface type (e.g. ethX if you're defining >> an ethernet dev alias). >> >> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > This really isn't kosher at all. Just for my personal knowledge, what is wrong with this code ? Is it because I'm using "of_" functions in the core code, and you want to keep it DT agnostic ? Or is it something else ? > > And there absolutely is a proper way to bind a net interface to > a specific name, udev has provided this facility for years. Thanks for pointing this out. But, what if the system does not use udev (this is often the case on embedded systems where udev is replaced by mdev) ? Moreover, on embedded systems, most users rely on the default interface name provided by the kernel. IIRC (tell me if I'm wrong), before moving to DT we could control the probe order of net interfaces derived from platform devices by modifying the platform dev registration order (okay, this is only true if the platform devices are controlled by the same driver, which is often the case when a SoC provides several net interfaces). With DT we can't know for sure the exact probe order because it depends on the net interface node position in the DT, and this node position might change over the time (or at least it used to change, now that we're enforced to declare DT nodes in strict memory @ order it should not change that much). Another issue: what if I want to rename eth0 into eth1 and eth1 into eth0 ? I guess I'll have to execute this sequence: eth1 -> eth2, eth0 -> eth1, eth2 -> eth0, otherwise the SIOCSIFNAME ioctl will return an error. Best Regards, Boris
2014-05-09 1:26 GMT-07:00 Boris BREZILLON <boris.brezillon@free-electrons.com>: > Hi David, > > On 09/05/2014 04:42, David Miller wrote: >> From: Boris BREZILLON <boris.brezillon@free-electrons.com> >> Date: Tue, 6 May 2014 17:36:34 +0200 >> >>> There is currently no proper way to bind a net interface to a specific >>> name. The interface name is chosen based on the interface type (eth, >>> wlan, ...) and the interfaces already registered (the core codes takes >>> the first unused interface id of the given type). >>> >>> Add support for DT retrieval of the interface id based on DT aliases. >>> The alias name must match the interface type (e.g. ethX if you're defining >>> an ethernet dev alias). >>> >>> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> >> This really isn't kosher at all. > > Just for my personal knowledge, what is wrong with this code ? > Is it because I'm using "of_" functions in the core code, and you want > to keep it DT agnostic ? > Or is it something else ? I think the major problem is that you are using DT to name interfaces and enforcing a naming policy within the kernel, while this should be left solely to user-space. I know that coming from an embedded use-case this might sound appealing, but the interface naming policy had better remain in user-space to avoid mixing policy with mechanisms. > >> >> And there absolutely is a proper way to bind a net interface to >> a specific name, udev has provided this facility for years. > > Thanks for pointing this out. > > But, what if the system does not use udev (this is often the case on > embedded systems where udev is replaced by mdev) ? Traditional embedded systems are also using a lot of custom software, why not write a small device mapper program that looks at aliases in the device-tree and matches it with sysfs entries for these corresponding network interfaces? > Moreover, on embedded systems, most users rely on the default interface > name provided by the kernel. > > IIRC (tell me if I'm wrong), before moving to DT we could control the > probe order of net interfaces derived from platform devices by modifying > the platform dev registration order (okay, this is only true if the > platform devices are controlled by the same driver, which is often the > case when a SoC provides several net interfaces). I do not quite agree with this, before moving to DT, we were mostly relying on the linking order imposed by the lines in the Makefile, which is still the case for a few things. It is sometimes fragile, and it is sometimes very convenient, and it also provides some perceived probing order stability, but that's no longer true with e.g: deffered probing which can happen regardless of DT. > With DT we can't know for sure the exact probe order because it depends > on the net interface node position in the DT, and this node position > might change over the time (or at least it used to change, now that > we're enforced to declare DT nodes in strict memory @ order it should > not change that much). Which is precisely where aliases are coming handy, and I understand why it is tempting to use them, but aliases are nothing more than the mechanism to help you, not the policy. > > Another issue: what if I want to rename eth0 into eth1 and eth1 into eth0 ? > I guess I'll have to execute this sequence: eth1 -> eth2, eth0 -> eth1, > eth2 -> eth0, otherwise the SIOCSIFNAME ioctl will return an error. Just like you swap two variables, use a temporary name.
diff --git a/net/core/dev.c b/net/core/dev.c index d2c8a06..8f30cb8 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -132,6 +132,7 @@ #include <linux/hashtable.h> #include <linux/vmalloc.h> #include <linux/if_macvlan.h> +#include <linux/of.h> #include "net-sysfs.h" @@ -960,13 +961,19 @@ EXPORT_SYMBOL(dev_valid_name); * Returns the number of the unit assigned or a negative errno code. */ -static int __dev_alloc_name(struct net *net, const char *name, char *buf) +static int __dev_alloc_name(struct net *net, struct net_device *dev, + const char *name, char *buf) { int i = 0; + int of_id = -EINVAL; const char *p; const int max_netdevices = 8*PAGE_SIZE; unsigned long *inuse; struct net_device *d; + struct device_node *np = NULL; + + if (dev->dev.parent) + np = dev->dev.parent->of_node; p = strnchr(name, IFNAMSIZ-1, '%'); if (p) { @@ -978,11 +985,38 @@ static int __dev_alloc_name(struct net *net, const char *name, char *buf) if (p[1] != 'd' || strchr(p + 2, '%')) return -EINVAL; + if (np) { + strlcpy(buf, name, (size_t)(p + 1 - name)); + of_id = of_alias_get_id(np, buf); + } + /* Use one page as a bit array of possible slots */ inuse = (unsigned long *) get_zeroed_page(GFP_ATOMIC); if (!inuse) return -ENOMEM; +#ifdef CONFIG_OF + /* iterate over aliases to reserve interfaces names */ + np = of_find_node_by_path("/aliases"); + if (np) { + struct property *pp; + for_each_property_of_node(np, pp) { + if (!sscanf(pp->name, name, &i)) + continue; + if (i < 0 || i >= max_netdevices) + continue; + + /* avoid cases where sscanf is not exact + * inverse of printf + */ + snprintf(buf, IFNAMSIZ, name, i); + if (!strncmp(buf, pp->name, IFNAMSIZ) && + i != of_id) + set_bit(i, inuse); + } + } +#endif + for_each_netdev(net, d) { if (!sscanf(d->name, name, &i)) continue; @@ -995,7 +1029,10 @@ static int __dev_alloc_name(struct net *net, const char *name, char *buf) set_bit(i, inuse); } - i = find_first_zero_bit(inuse, max_netdevices); + if (of_id >= 0 && !test_bit(of_id, inuse)) + i = of_id; + else + i = find_first_zero_bit(inuse, max_netdevices); free_page((unsigned long) inuse); } @@ -1033,7 +1070,7 @@ int dev_alloc_name(struct net_device *dev, const char *name) BUG_ON(!dev_net(dev)); net = dev_net(dev); - ret = __dev_alloc_name(net, name, buf); + ret = __dev_alloc_name(net, dev, name, buf); if (ret >= 0) strlcpy(dev->name, buf, IFNAMSIZ); return ret; @@ -1047,7 +1084,7 @@ static int dev_alloc_name_ns(struct net *net, char buf[IFNAMSIZ]; int ret; - ret = __dev_alloc_name(net, name, buf); + ret = __dev_alloc_name(net, dev, name, buf); if (ret >= 0) strlcpy(dev->name, buf, IFNAMSIZ); return ret;
There is currently no proper way to bind a net interface to a specific name. The interface name is chosen based on the interface type (eth, wlan, ...) and the interfaces already registered (the core codes takes the first unused interface id of the given type). Add support for DT retrieval of the interface id based on DT aliases. The alias name must match the interface type (e.g. ethX if you're defining an ethernet dev alias). Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> --- net/core/dev.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-)