[PATCH] netdev: add support for interface name retrieval from DT aliases

[PATCH] netdev: add support for interface name retrieval from DT aliases

[Boris BREZILLON]

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(-)

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;
-- 
1.8.3.2

Re: [PATCH] netdev: add support for interface name retrieval from DT aliases

[David Miller]

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.

Re: [PATCH] netdev: add support for interface name retrieval from DT aliases

[Boris BREZILLON]

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

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH] netdev: add support for interface name retrieval from DT aliases

[Florian Fainelli]

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.
-- 
Florian