[PATCH] Add firmware label support to iproute2

[PATCH] Add firmware label support to iproute2

[Narendra K]

Hello,

We have proposed solutions to resolve the specific issue of
"eth0 does not always map to Integrated NIC port 1 on the 
server chassis". Below are the approaches we proposed which
were not acceptable -

1. Create char device node for every network device and have udev
create alternate names in the form of /dev/netdev/.

http://marc.info/?l=linux-netdev&m=125510301513312&w=2

2. Achieve the above in userspace only using udev

3. Provide an option in the installers for users to rename network
interfaces based on various policies such as chassis label, mac
addresses etc.

https://www.redhat.com/archives/anaconda-devel-list/2009-November/msg00516.html

The approach to export firmware provided index and labels was more
acceptable.
 
http://marc.info/?l=linux-pci&m=126713402415401&w=3 -

This feature is now part of the kernel and firmware labels of
network devices are available to user space via sysfs.

http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;
h=911e1c9b05a8e3559a7aa89083930700a0b9e7ee
(PCI: export SMBIOS provided firmware instance and label to sysfs)

cat /sys/class/net/ethN/device/label
Embedded NIC N

Next step would be to enable applications to support the firmware
names.

With the patch below, Iproute2 can support firmware labels like
"Embedded NIC 1". We can refer to the network interfaces with firmware
names as labeled on the system chassis. With this "Embedded NIC 1"
would always refer to the 'integrated NIC port 1' on the system chassis.

Requirements -

1. Package be linked to libnetdevname library which maps firmware names
to kernel names. It is available here -

git clone http://linux.dell.com/git/libnetdevname.git

2. The kernel patch mentioned above.

Please let us know your views on this approach and if acceptable,
please consider this patch for inclusion.


From: Narendra K <narendra_k@dell.com>
Subject: [PATCH] Add firmware label support to iproute2

This patch enables iproute2 to support firmware provided
labels for network devices. Ex:

/sbin/ip maddr show dev "Embedded NIC N"
/sbin/ip addr show dev "Embedded NIC N"

/sbin/tc qdisc add dev "Embedded NIC N" root tbf rate Xkbit \
latency Yms burst 1540

The patch makes calls to libnetdevname library which maps
the "Embedded NIC N" names to ethN kernel names.

This makes sure that above commands that use "Embedded NIC 1" as
the dev argument, will refer to the integrated port 1 on the
chassis label.

Signed-off-by: Jordan Hargrave <jordan_hargrave@dell.com>
Signed-off-by: Narendra K <narendra_k@dell.com>
---
 Makefile       |    5 +++++
 ip/ipaddress.c |   14 ++++++++++++++
 ip/iplink.c    |   14 ++++++++++++++
 ip/ipmaddr.c   |   18 ++++++++++++++++++
 lib/ll_map.c   |   16 ++++++++++++++++
 tc/f_fw.c      |   15 +++++++++++++++
 tc/tc_class.c  |   25 +++++++++++++++++++++++++
 tc/tc_filter.c |   25 +++++++++++++++++++++++++
 tc/tc_qdisc.c  |   24 ++++++++++++++++++++++++
 9 files changed, 156 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 77a85c6..ee82640 100644
--- a/Makefile
+++ b/Makefile
@@ -35,6 +35,11 @@ YACCFLAGS = -d -t -v
 
 LDLIBS += -L../lib -lnetlink -lutil
 
+ifeq ($(shell test -L /usr/local/lib/libnetdevname.so; echo $$?),0)
+LDLIBS +=-lnetdevname
+DEFINES += -DLIBNETDEVNAME_PRESENT
+endif
+
 SUBDIRS=lib ip tc misc netem genl
 
 LIBNETLINK=../lib/libnetlink.a ../lib/libutil.a
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 3a411b1..6fafa2e 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -28,6 +28,10 @@
 #include <linux/if_arp.h>
 #include <linux/sockios.h>
 
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#endif
+
 #include "rt_names.h"
 #include "utils.h"
 #include "ll_map.h"
@@ -712,6 +716,10 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 	char *filter_dev = NULL;
 	int no_link = 0;
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	ipaddr_reset_filter(oneline);
 	filter.showqueue = 1;
 
@@ -787,7 +795,13 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 				usage();
 			if (filter_dev)
 				duparg2("dev", *argv);
+#ifndef LIBNETDEVNAME_PRESENT
 			filter_dev = *argv;
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			filter_dev = kernel_name;
+#endif
 		}
 		argv++; argc--;
 	}
diff --git a/ip/iplink.c b/ip/iplink.c
index cb2c4f5..e9f70e2 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -28,6 +28,10 @@
 #include <sys/ioctl.h>
 #include <linux/sockios.h>
 
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#endif
+
 #include "rt_names.h"
 #include "utils.h"
 #include "ip_common.h"
@@ -703,6 +707,10 @@ static int do_set(int argc, char **argv)
 	char *newname = NULL;
 	int htype, halen;
 
+#ifdef	LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	while (argc > 0) {
 		if (strcmp(*argv, "up") == 0) {
 			mask |= IFF_UP;
@@ -798,7 +806,13 @@ static int do_set(int argc, char **argv)
 				usage();
 			if (dev)
 				duparg2("dev", *argv);
+#ifndef	LIBNETDEVNAME_PRESENT
 			dev = *argv;
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			dev = kernel_name;
+#endif
 		}
 		argc--; argv++;
 	}
diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c
index 44ffdfc..b5c8380 100644
--- a/ip/ipmaddr.c
+++ b/ip/ipmaddr.c
@@ -26,6 +26,10 @@
 #include <linux/if_arp.h>
 #include <linux/sockios.h>
 
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#endif
+
 #include "rt_names.h"
 #include "utils.h"
 
@@ -245,6 +249,9 @@ static int multiaddr_list(int argc, char **argv)
 {
 	struct ma_info *list = NULL;
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
 	if (!filter.family)
 		filter.family = preferred_family;
 
@@ -257,7 +264,13 @@ static int multiaddr_list(int argc, char **argv)
 				usage();
 			if (filter.dev)
 				duparg2("dev", *argv);
+#ifndef LIBNETDEVNAME_PRESENT
 			filter.dev = *argv;
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			filter.dev = kernel_name;
+#endif
 		}
 		argv++; argc--;
 	}
@@ -289,7 +302,12 @@ int multiaddr_modify(int cmd, int argc, char **argv)
 			NEXT_ARG();
 			if (ifr.ifr_name[0])
 				duparg("dev", *argv);
+#ifndef	LIBNETDEVNAME_PRESENT
 			strncpy(ifr.ifr_name, *argv, IFNAMSIZ);
+#else
+			if (netdev_alias_to_kernelname(*argv, ifr.ifr_name) < 0)
+				show_firmware_alias_usage(*argv);
+#endif
 		} else {
 			if (matches(*argv, "address") == 0) {
 				NEXT_ARG();
diff --git a/lib/ll_map.c b/lib/ll_map.c
index b8b49aa..1476255 100644
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -19,9 +19,15 @@
 #include <netinet/in.h>
 #include <string.h>
 
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#include <linux/if.h>
+#endif
+
 #include "libnetlink.h"
 #include "ll_map.h"
 
+
 extern unsigned int if_nametoindex (const char *);
 
 struct idxmap
@@ -163,8 +169,18 @@ unsigned ll_name_to_index(const char *name)
 	int i;
 	unsigned idx;
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
 	if (name == NULL)
 		return 0;
+
+#ifdef LIBNETDEVNAME_PRESENT
+	if (netdev_alias_to_kernelname(name, kernel_name) < 0)
+		show_firmware_alias_usage(name);
+	strncpy(name, kernel_name, IFNAMSIZ);
+#endif
+
 	if (icache && strcmp(name, ncache) == 0)
 		return icache;
 	for (i=0; i<16; i++) {
diff --git a/tc/f_fw.c b/tc/f_fw.c
index 219b404..244bc8a 100644
--- a/tc/f_fw.c
+++ b/tc/f_fw.c
@@ -20,6 +20,11 @@
 #include <arpa/inet.h>
 #include <string.h>
 #include <linux/if.h> /* IFNAMSIZ */
+
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#endif
+
 #include "utils.h"
 #include "tc_util.h"
 
@@ -39,6 +44,10 @@ static int fw_parse_opt(struct filter_util *qu, char *handle, int argc, char **a
 	__u32 mask = 0;
 	int mask_set = 0;
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	memset(&tp, 0, sizeof(tp));
 
 	if (handle) {
@@ -100,7 +109,13 @@ static int fw_parse_opt(struct filter_util *qu, char *handle, int argc, char **a
 				fprintf(stderr, "Illegal indev\n");
 				return -1;
 			}
+#ifndef LIBNETDEVNAME_PRESENT
 			strncpy(d, *argv, sizeof (d) - 1);
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			strncpy(d, kernel_name, sizeof (d) - 1);
+#endif
 			addattr_l(n, MAX_MSG, TCA_FW_INDEV, d, strlen(d) + 1);
 		} else if (strcmp(*argv, "help") == 0) {
 			explain();
diff --git a/tc/tc_class.c b/tc/tc_class.c
index 9d4eea5..bff90f8 100644
--- a/tc/tc_class.c
+++ b/tc/tc_class.c
@@ -21,6 +21,11 @@
 #include <string.h>
 #include <math.h>
 
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#include <linux/if.h>
+#endif
+
 #include "utils.h"
 #include "tc_util.h"
 #include "tc_common.h"
@@ -52,6 +57,10 @@ int tc_class_modify(int cmd, unsigned flags, int argc, char **argv)
 	char  d[16];
 	char  k[16];
 
+#ifdef	LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	memset(&req, 0, sizeof(req));
 	memset(&est, 0, sizeof(est));
 	memset(d, 0, sizeof(d));
@@ -67,7 +76,13 @@ int tc_class_modify(int cmd, unsigned flags, int argc, char **argv)
 			NEXT_ARG();
 			if (d[0])
 				duparg("dev", *argv);
+#ifndef LIBNETDEVNAME_PRESENT
 			strncpy(d, *argv, sizeof(d)-1);
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			strncpy(d, kernel_name, sizeof (d) - 1);
+#endif
 		} else if (strcmp(*argv, "classid") == 0) {
 			__u32 handle;
 			NEXT_ARG();
@@ -237,6 +252,10 @@ int tc_class_list(int argc, char **argv)
 	struct tcmsg t;
 	char d[16];
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	memset(&t, 0, sizeof(t));
 	t.tcm_family = AF_UNSPEC;
 	memset(d, 0, sizeof(d));
@@ -246,7 +265,13 @@ int tc_class_list(int argc, char **argv)
 			NEXT_ARG();
 			if (d[0])
 				duparg("dev", *argv);
+#ifndef LIBNETDEVNAME_PRESENT
 			strncpy(d, *argv, sizeof(d)-1);
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			strncpy(d, kernel_name, sizeof (d) - 1);
+#endif
 		} else if (strcmp(*argv, "qdisc") == 0) {
 			NEXT_ARG();
 			if (filter_qdisc)
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index 919c57c..e144f9d 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -21,6 +21,11 @@
 #include <string.h>
 #include <linux/if_ether.h>
 
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#include <linux/if.h>
+#endif
+
 #include "rt_names.h"
 #include "utils.h"
 #include "tc_util.h"
@@ -61,6 +66,10 @@ int tc_filter_modify(int cmd, unsigned flags, int argc, char **argv)
 	char  k[16];
 	struct tc_estimator est;
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	memset(&req, 0, sizeof(req));
 	memset(&est, 0, sizeof(est));
 	memset(d, 0, sizeof(d));
@@ -80,7 +89,13 @@ int tc_filter_modify(int cmd, unsigned flags, int argc, char **argv)
 			NEXT_ARG();
 			if (d[0])
 				duparg("dev", *argv);
+#ifndef LIBNETDEVNAME_PRESENT
 			strncpy(d, *argv, sizeof(d)-1);
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			strncpy(d, kernel_name, sizeof (d) - 1);
+#endif
 		} else if (strcmp(*argv, "root") == 0) {
 			if (req.t.tcm_parent) {
 				fprintf(stderr, "Error: \"root\" is duplicate parent ID\n");
@@ -268,6 +283,10 @@ int tc_filter_list(int argc, char **argv)
 	__u32 protocol = 0;
 	char *fhandle = NULL;
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	memset(&t, 0, sizeof(t));
 	t.tcm_family = AF_UNSPEC;
 	memset(d, 0, sizeof(d));
@@ -277,7 +296,13 @@ int tc_filter_list(int argc, char **argv)
 			NEXT_ARG();
 			if (d[0])
 				duparg("dev", *argv);
+#ifndef LIBNETDEVNAME_PRESENT
 			strncpy(d, *argv, sizeof(d)-1);
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			strncpy(d, kernel_name, sizeof (d) - 1);
+#endif
 		} else if (strcmp(*argv, "root") == 0) {
 			if (t.tcm_parent) {
 				fprintf(stderr, "Error: \"root\" is duplicate parent ID\n");
diff --git a/tc/tc_qdisc.c b/tc/tc_qdisc.c
index c7f2988..3a9b05c 100644
--- a/tc/tc_qdisc.c
+++ b/tc/tc_qdisc.c
@@ -21,6 +21,10 @@
 #include <string.h>
 #include <math.h>
 #include <malloc.h>
+#ifdef LIBNETDEVNAME_PRESENT
+#include <netdevname.h>
+#include <linux/if.h>
+#endif
 
 #include "utils.h"
 #include "tc_util.h"
@@ -60,6 +64,10 @@ int tc_qdisc_modify(int cmd, unsigned flags, int argc, char **argv)
 		char   			buf[TCA_BUF_MAX];
 	} req;
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	memset(&req, 0, sizeof(req));
 	memset(&stab, 0, sizeof(stab));
 	memset(&est, 0, sizeof(est));
@@ -76,7 +84,13 @@ int tc_qdisc_modify(int cmd, unsigned flags, int argc, char **argv)
 			NEXT_ARG();
 			if (d[0])
 				duparg("dev", *argv);
+#ifndef LIBNETDEVNAME_PRESENT
 			strncpy(d, *argv, sizeof(d)-1);
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			strncpy(d, kernel_name, sizeof (d) - 1);
+#endif
 		} else if (strcmp(*argv, "handle") == 0) {
 			__u32 handle;
 			if (req.t.tcm_handle)
@@ -282,6 +296,10 @@ int tc_qdisc_list(int argc, char **argv)
 	struct tcmsg t;
 	char d[16];
 
+#ifdef LIBNETDEVNAME_PRESENT
+	char kernel_name[IFNAMSIZ];
+#endif
+
 	memset(&t, 0, sizeof(t));
 	t.tcm_family = AF_UNSPEC;
 	memset(&d, 0, sizeof(d));
@@ -289,7 +307,13 @@ int tc_qdisc_list(int argc, char **argv)
 	while (argc > 0) {
 		if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
+#ifndef LIBNETDEVNAME_PRESENT
 			strncpy(d, *argv, sizeof(d)-1);
+#else
+			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
+				show_firmware_alias_usage(*argv);
+			strncpy(d, kernel_name, sizeof (d) - 1);
+#endif
 #ifdef TC_H_INGRESS
                 } else if (strcmp(*argv, "ingress") == 0) {
                              if (t.tcm_parent) {
-- 
1.7.0.1

With regards,
Narendra K

Re: [PATCH] Add firmware label support to iproute2

[Stephen Hemminger]

On Thu, 12 Aug 2010 12:35:37 -0500
Narendra K <Narendra_K@dell.com> wrote:

> +ifeq ($(shell test -L /usr/local/lib/libnetdevname.so; echo $$?),0)
> +LDLIBS +=-lnetdevname
> +DEFINES += -DLIBNETDEVNAME_PRESENT
> +endif

I assume any user or distro using libnetdevname will put it in
the standard path not /usr/lib, don't hard code the path here.

why not build a sample program like the ATM checks?

Re: [PATCH] Add firmware label support to iproute2

[Stephen Hemminger]

On Thu, 12 Aug 2010 12:35:37 -0500
Narendra K <Narendra_K@dell.com> wrote:

> +#ifndef LIBNETDEVNAME_PRESENT
>  			filter_dev = *argv;
> +#else
> +			if (netdev_alias_to_kernelname(*argv, kernel_name) < 0)
> +				show_firmware_alias_usage(*argv);
> +			filter_dev = kernel_name;
> +#endif
>  		}

like the kernel, I don't like ifdef cases in main code.
You should put in stub inline that returns appropriate error.

What happens if alias matches existing interface name?

RE: [PATCH] Add firmware label support to iproute2

[Narendra_K]

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Thursday, August 12, 2010 11:43 PM
> To: K, Narendra
> Cc: netdev@vger.kernel.org; Domsch, Matt; Rose, Charles; Hargrave,
> Jordan
> Subject: Re: [PATCH] Add firmware label support to iproute2
> 
> On Thu, 12 Aug 2010 12:35:37 -0500
> Narendra K <Narendra_K@dell.com> wrote:
> 
> > +#ifndef LIBNETDEVNAME_PRESENT
> >  			filter_dev = *argv;
> > +#else
> > +			if (netdev_alias_to_kernelname(*argv,
kernel_name) <
> 0)
> > +				show_firmware_alias_usage(*argv);
> > +			filter_dev = kernel_name;
> > +#endif
> >  		}
> 
> like the kernel, I don't like ifdef cases in main code.
> You should put in stub inline that returns appropriate error.
> 

Stephen,

Thanks for the suggestions. I will rework the patch and send a revised
version soon.

> What happens if alias matches existing interface name?

If the user inputs an existing 'ethN' name, the function
'netdev_alias_to_kernelaname' will return it as it is. If the user
inputs 'Embedded NIC N', then the function would do a sysfs lookup and
return the corresponding 'ethN' name.

With regards,
Narendra K

Re: [PATCH] Add firmware label support to iproute2

[Stephen Hemminger]

On Fri, 13 Aug 2010 18:06:26 +0530
<Narendra_K@Dell.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: Thursday, August 12, 2010 11:43 PM
> > To: K, Narendra
> > Cc: netdev@vger.kernel.org; Domsch, Matt; Rose, Charles; Hargrave,
> > Jordan
> > Subject: Re: [PATCH] Add firmware label support to iproute2
> > 
> > On Thu, 12 Aug 2010 12:35:37 -0500
> > Narendra K <Narendra_K@dell.com> wrote:
> > 
> > > +#ifndef LIBNETDEVNAME_PRESENT
> > >  			filter_dev = *argv;
> > > +#else
> > > +			if (netdev_alias_to_kernelname(*argv,
> kernel_name) <
> > 0)
> > > +				show_firmware_alias_usage(*argv);
> > > +			filter_dev = kernel_name;
> > > +#endif
> > >  		}
> > 
> > like the kernel, I don't like ifdef cases in main code.
> > You should put in stub inline that returns appropriate error.
> > 
> 
> Stephen,
> 
> Thanks for the suggestions. I will rework the patch and send a revised
> version soon.
> 
> > What happens if alias matches existing interface name?
> 
> If the user inputs an existing 'ethN' name, the function
> 'netdev_alias_to_kernelaname' will return it as it is. If the user
> inputs 'Embedded NIC N', then the function would do a sysfs lookup and
> return the corresponding 'ethN' name.

The netdev_alias_to_kernelname should only happen after normal lookup failed.
Also how does ifindex to name mapping work?

-- 

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Wed, Aug 18, 2010 at 02:41:24PM -0700, Stephen Hemminger wrote:
> The netdev_alias_to_kernelname should only happen after normal lookup failed.

Stephen, can you enlighten me as to the "right" way to do interface
name lookups?  While I can still find examples of parsing
/proc/net/dev, or globbing /sys/class/net/*, I expect these aren't the
preferred method anymore.  Your own iproute2 suite uses RTM_GETLINK
netlink calls, though for the seeming simple case of "give me a list of all
interfaces", your path through there is far more capable (and
complex) than I would hope to need.

Please advise.

> Also how does ifindex to name mapping work?

libnetdevname does not use ifindex at all at present.

Thanks,
Matt

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: [PATCH] Add firmware label support to iproute2

[Stephen Hemminger]

On Thu, 19 Aug 2010 16:33:14 -0500
Matt Domsch <Matt_Domsch@Dell.com> wrote:

> On Wed, Aug 18, 2010 at 02:41:24PM -0700, Stephen Hemminger wrote:
> > The netdev_alias_to_kernelname should only happen after normal lookup failed.
> 
> Stephen, can you enlighten me as to the "right" way to do interface
> name lookups?  While I can still find examples of parsing
> /proc/net/dev, or globbing /sys/class/net/*, I expect these aren't the
> preferred method anymore.  Your own iproute2 suite uses RTM_GETLINK
> netlink calls, though for the seeming simple case of "give me a list of all
> interfaces", your path through there is far more capable (and
> complex) than I would hope to need.

There is no magic right way. We have to support multiple interfaces.
I am really concerned that all this alias stuff will turn into a
disaster when there are 10,000 interfaces (Vlans).  The kernel has
lots of tables and hashes to handle this but if the utilities
are doing a dumb scan of all names it will not work.

Also burying logic in an external library seems problematic as
well. The original sysfs library was a disaster for this.
I want this to work but it has to have a simple interface that
is not trying to hide things.

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Thu, Aug 19, 2010 at 02:53:08PM -0700, Stephen Hemminger wrote:
> On Thu, 19 Aug 2010 16:33:14 -0500
> Matt Domsch <Matt_Domsch@Dell.com> wrote:
> 
> > On Wed, Aug 18, 2010 at 02:41:24PM -0700, Stephen Hemminger wrote:
> > > The netdev_alias_to_kernelname should only happen after normal lookup failed.
> > 
> > Stephen, can you enlighten me as to the "right" way to do interface
> > name lookups?  While I can still find examples of parsing
> > /proc/net/dev, or globbing /sys/class/net/*, I expect these aren't the
> > preferred method anymore.  Your own iproute2 suite uses RTM_GETLINK
> > netlink calls, though for the seeming simple case of "give me a list of all
> > interfaces", your path through there is far more capable (and
> > complex) than I would hope to need.
> 
> There is no magic right way. We have to support multiple interfaces.
> I am really concerned that all this alias stuff will turn into a
> disaster when there are 10,000 interfaces (Vlans).  The kernel has
> lots of tables and hashes to handle this but if the utilities
> are doing a dumb scan of all names it will not work.

I believe you proposed a lookup algorithm in each app, something like:

if exists(passed device name)
  use it
else if (kernelname=netdev_alias_to_kernelname(passed device name))
  use kernelname

so I'm looking for the "right" way to do these existance tests.  I
agree iterating through all the interfaces should be avoided.

 
> Also burying logic in an external library seems problematic as
> well. The original sysfs library was a disaster for this.
> I want this to work but it has to have a simple interface that
> is not trying to hide things.

the library right now is all of:

  422 netdevname.c
   38 netdevname.h

with another 100 line app to print the mappings in both directions.

The interface is pretty simple too:

extern int netdev_alias_to_kernelname (const char *alias, char *kernelname);

struct netdev_alias {
        char *name;
        const char *namespace_name;
        int is_descriptive; /* this may disappear yet */
        struct netdev_alias *next;
};
extern int netdev_kernelname_to_aliases (const char *, struct netdev_alias **);
extern void free_netdev_aliases (struct netdev_alias *);

The common app usage will be to call netdev_alias_to_kernelname().
Only those apps that want to display the alternate names would use the
struct or other 2 functions.

Even though it's a small library, I do think it makes sense to make it
a library.  I expect to add additional "namespaces" (e.g. "fw:Embedded
NIC 1" is the name of a device provided by the firmware.  As I
mentioned in my talk at LinuxCon last week (slides at
http://domsch.com/linux/linuxcon10/) there could be other valid name
providers, and we'd like them to be usable without re-patching all the
applications again.

As always, I'm open to ideas.

Thanks,
Matt

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Thu, Aug 19, 2010 at 02:53:08PM -0700, Stephen Hemminger wrote:
> On Thu, 19 Aug 2010 16:33:14 -0500
> Matt Domsch <Matt_Domsch@Dell.com> wrote:
> 
> > On Wed, Aug 18, 2010 at 02:41:24PM -0700, Stephen Hemminger wrote:
> > > The netdev_alias_to_kernelname should only happen after normal lookup failed.
> > 
> > Stephen, can you enlighten me as to the "right" way to do interface
> > name lookups?  While I can still find examples of parsing
> > /proc/net/dev, or globbing /sys/class/net/*, I expect these aren't the
> > preferred method anymore.  Your own iproute2 suite uses RTM_GETLINK
> > netlink calls, though for the seeming simple case of "give me a list of all
> > interfaces", your path through there is far more capable (and
> > complex) than I would hope to need.
> 
> There is no magic right way. We have to support multiple interfaces.
> I am really concerned that all this alias stuff will turn into a
> disaster when there are 10,000 interfaces (Vlans).  The kernel has
> lots of tables and hashes to handle this but if the utilities
> are doing a dumb scan of all names it will not work.

We can remove the dumb scan of names if we expose the labels in a
second way.  For consideration of the approach:


>From 86b8ab8d24df722073d1118659c1eb269c5d9dd7 Mon Sep 17 00:00:00 2001
From: Matt Domsch <Matt_Domsch@dell.com>
Date: Wed, 25 Aug 2010 15:31:01 -0500
Subject: [PATCH] create /sys/firmware/pci for firmware-exposed label lookup

libnetdevname needs to be able to look up a (PCI) device by label.  It
currently does this by glob()ing /sys/class/net/*/device/label to find
all network interfaces that have a PCI device label exposed.  There can
be arbitrarily many network interfaces, making this a very expensive
operation.

This patch creates a new directory /sys/firmware/pci. This directory
contains symlinks, named for the PCI device labels that are exposed in
the 'label' file of each PCI device already.  Here is an example on a
Dell PowerEdge R610 server:

$ ls -l /sys/firmware/pci/
total 0
lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 1 -> ../../devices/pci0000:00/0000:00:01.0/0000:01:00.0/
lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 2 -> ../../devices/pci0000:00/0000:00:01.0/0000:01:00.1/
lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 3 -> ../../devices/pci0000:00/0000:00:03.0/0000:02:00.0/
lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 4 -> ../../devices/pci0000:00/0000:00:03.0/0000:02:00.1/
lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded Video -> ../../devices/pci0000:00/0000:00:1e.0/0000:09:03.0/
lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Integrated SAS -> ../../devices/pci0000:00/0000:00:1c.0/0000:03:00.0/

$ cat /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/label
Embedded NIC 1

Using this, libnetdevname can remove it's glob() call, and instead can
look up the PCI device corresponding to a given label directly.  It
can then find the network interface name for that PCI device by
looking in the net/ directory of that device.

This makes no attempt to deal with SMBIOS that reports the same label
for two different PCI devices (if such were to exist).

Signed-off-by: Matt Domsch <Matt_Domsch@dell.com>
---
 drivers/pci/pci-label.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index 111500e..a48eedb 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -19,6 +19,7 @@
 #include <linux/pci_ids.h>
 #include <linux/module.h>
 #include <linux/device.h>
+#include <linux/string.h>
 #include "pci.h"
 
 enum smbios_attr_enum {
@@ -131,13 +132,78 @@ pci_remove_smbiosname_file(struct pci_dev *pdev)
 	sysfs_remove_group(&pdev->dev.kobj, &smbios_attr_group);
 }
 
+static struct kobject *pci_label_kobj;
+
+static int pci_label_kobj_init(void)
+{
+	pci_label_kobj = kobject_create_and_add("pci", firmware_kobj);
+	if (pci_label_kobj == NULL)
+		return -ENOMEM;
+	return 0;
+}
+
+static int
+pci_create_smbios_label_symlink(struct pci_dev *pdev)
+{
+	char *label, *p;
+	mode_t mode;
+	int ret=-ENOMEM;
+
+	if (!pdev)
+		return -ENODEV;
+	if (pci_label_kobj == NULL)
+		if (pci_label_kobj_init())
+			return -ENOMEM;
+	label = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (label == NULL)
+		return -ENOMEM;
+	mode = find_smbios_instance_string(pdev, label, SMBIOS_ATTR_LABEL_SHOW);
+	if (mode == 0) {
+		ret = -ENODEV;
+		goto out_free;
+	}
+	p = strim(label);
+	ret = sysfs_create_link_nowarn(pci_label_kobj, &pdev->dev.kobj, p);
+out_free:
+	kfree(label);
+	return ret;
+}
+
+static void
+pci_delete_smbios_label_symlink(struct pci_dev *pdev)
+{
+	char *label, *p;
+	mode_t mode;
+
+	if (!pdev)
+		return;
+	if (pci_label_kobj == NULL)
+		return;
+	label = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (label == NULL)
+		return;
+	mode = find_smbios_instance_string(pdev, label, SMBIOS_ATTR_LABEL_SHOW);
+	if (mode == 0) {
+		goto out_free;
+	}
+	p = strim(label);
+	sysfs_delete_link(pci_label_kobj, &pdev->dev.kobj, p);
+
+out_free:
+	kfree(label);
+}
+
+
 void pci_create_firmware_label_files(struct pci_dev *pdev)
 {
 	if (!pci_create_smbiosname_file(pdev))
 		;
+	if (!pci_create_smbios_label_symlink(pdev))
+		;
 }
 
 void pci_remove_firmware_label_files(struct pci_dev *pdev)
 {
 	pci_remove_smbiosname_file(pdev);
+	pci_delete_smbios_label_symlink(pdev);
 }
-- 
1.7.1.1




-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: [PATCH] Add firmware label support to iproute2

[Greg KH]

On Wed, Aug 25, 2010 at 05:03:23PM -0500, Matt Domsch wrote:
> On Thu, Aug 19, 2010 at 02:53:08PM -0700, Stephen Hemminger wrote:
> > On Thu, 19 Aug 2010 16:33:14 -0500
> > Matt Domsch <Matt_Domsch@Dell.com> wrote:
> > 
> > > On Wed, Aug 18, 2010 at 02:41:24PM -0700, Stephen Hemminger wrote:
> > > > The netdev_alias_to_kernelname should only happen after normal lookup failed.
> > > 
> > > Stephen, can you enlighten me as to the "right" way to do interface
> > > name lookups?  While I can still find examples of parsing
> > > /proc/net/dev, or globbing /sys/class/net/*, I expect these aren't the
> > > preferred method anymore.  Your own iproute2 suite uses RTM_GETLINK
> > > netlink calls, though for the seeming simple case of "give me a list of all
> > > interfaces", your path through there is far more capable (and
> > > complex) than I would hope to need.
> > 
> > There is no magic right way. We have to support multiple interfaces.
> > I am really concerned that all this alias stuff will turn into a
> > disaster when there are 10,000 interfaces (Vlans).  The kernel has
> > lots of tables and hashes to handle this but if the utilities
> > are doing a dumb scan of all names it will not work.
> 
> We can remove the dumb scan of names if we expose the labels in a
> second way.  For consideration of the approach:
> 
> 
> >From 86b8ab8d24df722073d1118659c1eb269c5d9dd7 Mon Sep 17 00:00:00 2001
> From: Matt Domsch <Matt_Domsch@dell.com>
> Date: Wed, 25 Aug 2010 15:31:01 -0500
> Subject: [PATCH] create /sys/firmware/pci for firmware-exposed label lookup
> 
> libnetdevname needs to be able to look up a (PCI) device by label.  It
> currently does this by glob()ing /sys/class/net/*/device/label to find
> all network interfaces that have a PCI device label exposed.  There can
> be arbitrarily many network interfaces, making this a very expensive
> operation.

So you want to add code to the kernel to make userspace have an easier
'find' path?  I'm all for making stuff easier, but just how "expensive"
is such an operation today?

> This patch creates a new directory /sys/firmware/pci. This directory
> contains symlinks, named for the PCI device labels that are exposed in
> the 'label' file of each PCI device already.  Here is an example on a
> Dell PowerEdge R610 server:
> 
> $ ls -l /sys/firmware/pci/
> total 0
> lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 1 -> ../../devices/pci0000:00/0000:00:01.0/0000:01:00.0/
> lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 2 -> ../../devices/pci0000:00/0000:00:01.0/0000:01:00.1/
> lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 3 -> ../../devices/pci0000:00/0000:00:03.0/0000:02:00.0/
> lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded NIC 4 -> ../../devices/pci0000:00/0000:00:03.0/0000:02:00.1/
> lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Embedded Video -> ../../devices/pci0000:00/0000:00:1e.0/0000:09:03.0/
> lrwxrwxrwx. 1 root root 0 2010-08-25 15:28 Integrated SAS -> ../../devices/pci0000:00/0000:00:1c.0/0000:03:00.0/
> 
> $ cat /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/label
> Embedded NIC 1
> 
> Using this, libnetdevname can remove it's glob() call, and instead can
> look up the PCI device corresponding to a given label directly.  It
> can then find the network interface name for that PCI device by
> looking in the net/ directory of that device.
> 
> This makes no attempt to deal with SMBIOS that reports the same label
> for two different PCI devices (if such were to exist).

I'm sure it does, and I will end up getting the bugs reported to me as
it shows a sysfs WARN_ON() when that happens.

So please, handle this case to properly fail if there are duplicate
names, as we all know it will happen (look at all of the duplicate slot
names that have been found already as proof of this...)



> 
> Signed-off-by: Matt Domsch <Matt_Domsch@dell.com>
> ---
>  drivers/pci/pci-label.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 66 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
> index 111500e..a48eedb 100644
> --- a/drivers/pci/pci-label.c
> +++ b/drivers/pci/pci-label.c
> @@ -19,6 +19,7 @@
>  #include <linux/pci_ids.h>
>  #include <linux/module.h>
>  #include <linux/device.h>
> +#include <linux/string.h>
>  #include "pci.h"
>  
>  enum smbios_attr_enum {
> @@ -131,13 +132,78 @@ pci_remove_smbiosname_file(struct pci_dev *pdev)
>  	sysfs_remove_group(&pdev->dev.kobj, &smbios_attr_group);
>  }
>  
> +static struct kobject *pci_label_kobj;
> +
> +static int pci_label_kobj_init(void)
> +{
> +	pci_label_kobj = kobject_create_and_add("pci", firmware_kobj);
> +	if (pci_label_kobj == NULL)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +static int
> +pci_create_smbios_label_symlink(struct pci_dev *pdev)
> +{
> +	char *label, *p;
> +	mode_t mode;
> +	int ret=-ENOMEM;
> +
> +	if (!pdev)
> +		return -ENODEV;
> +	if (pci_label_kobj == NULL)
> +		if (pci_label_kobj_init())
> +			return -ENOMEM;
> +	label = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (label == NULL)
> +		return -ENOMEM;
> +	mode = find_smbios_instance_string(pdev, label, SMBIOS_ATTR_LABEL_SHOW);
> +	if (mode == 0) {
> +		ret = -ENODEV;
> +		goto out_free;
> +	}
> +	p = strim(label);
> +	ret = sysfs_create_link_nowarn(pci_label_kobj, &pdev->dev.kobj, p);
> +out_free:
> +	kfree(label);
> +	return ret;
> +}
> +
> +static void
> +pci_delete_smbios_label_symlink(struct pci_dev *pdev)
> +{
> +	char *label, *p;
> +	mode_t mode;
> +
> +	if (!pdev)
> +		return;
> +	if (pci_label_kobj == NULL)
> +		return;
> +	label = kmalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (label == NULL)
> +		return;
> +	mode = find_smbios_instance_string(pdev, label, SMBIOS_ATTR_LABEL_SHOW);
> +	if (mode == 0) {
> +		goto out_free;
> +	}
> +	p = strim(label);
> +	sysfs_delete_link(pci_label_kobj, &pdev->dev.kobj, p);
> +
> +out_free:
> +	kfree(label);
> +}
> +
> +
>  void pci_create_firmware_label_files(struct pci_dev *pdev)
>  {
>  	if (!pci_create_smbiosname_file(pdev))
>  		;
> +	if (!pci_create_smbios_label_symlink(pdev))
> +		;

Why even have this check at all, if nothing is going to be done with it?
I missed this last time around for the smbios name stuff, but come on
people, that's horrible code.

thanks,

greg k-h

Re: [PATCH] Add firmware label support to iproute2

[Stephen Hemminger]

On Wed, 25 Aug 2010 17:03:23 -0500
Matt Domsch <Matt_Domsch@Dell.com> wrote:

> On Thu, Aug 19, 2010 at 02:53:08PM -0700, Stephen Hemminger wrote:
> > On Thu, 19 Aug 2010 16:33:14 -0500
> > Matt Domsch <Matt_Domsch@Dell.com> wrote:
> > 
> > > On Wed, Aug 18, 2010 at 02:41:24PM -0700, Stephen Hemminger wrote:
> > > > The netdev_alias_to_kernelname should only happen after normal lookup failed.
> > > 
> > > Stephen, can you enlighten me as to the "right" way to do interface
> > > name lookups?  While I can still find examples of parsing
> > > /proc/net/dev, or globbing /sys/class/net/*, I expect these aren't the
> > > preferred method anymore.  Your own iproute2 suite uses RTM_GETLINK
> > > netlink calls, though for the seeming simple case of "give me a list of all
> > > interfaces", your path through there is far more capable (and
> > > complex) than I would hope to need.
> > 
> > There is no magic right way. We have to support multiple interfaces.
> > I am really concerned that all this alias stuff will turn into a
> > disaster when there are 10,000 interfaces (Vlans).  The kernel has
> > lots of tables and hashes to handle this but if the utilities
> > are doing a dumb scan of all names it will not work.
> 

Is it really a good idea to have to change every utility that could
alter network devices?  There is iproute2, iputils, tcpdump, wireshark,
quagga, snmp, ...  Many of the utilities come from a BSD world,
and will be less likely to accept some Linux specific wart.

I have lost faith in this library wrapper support everywhere method.
Let's just keep the firmware stuff in udev. If the user wants to
have a policy that renames device from eth0 to "Embedded BIOS LAN1" then
do it in udev. Or if you want to keep the ethX naming convention
and stuff the firmware label into ifAlias or other sysfs field
so it can be displayed that will be not a big issue.

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Wed, Aug 25, 2010 at 03:16:28PM -0700, Greg KH wrote:
> On Wed, Aug 25, 2010 at 05:03:23PM -0500, Matt Domsch wrote:
> > libnetdevname needs to be able to look up a (PCI) device by label.  It
> > currently does this by glob()ing /sys/class/net/*/device/label to find
> > all network interfaces that have a PCI device label exposed.  There can
> > be arbitrarily many network interfaces, making this a very expensive
> > operation.
> 
> So you want to add code to the kernel to make userspace have an easier
> 'find' path?  I'm all for making stuff easier, but just how "expensive"
> is such an operation today?

it's one newfstatat and one stat for each network interface,
including interfaces for each vlan, which as Stephen had noted,
could be tens of thousands.  Just to test, on a system with ~20k vlans
configured across 5 physical interfaces, the glob() took just over 4
minutes.  Now, that may not be the typical case, but it does show the
scalability problem Stephen is concerned about.

Thanks also for the patch review.  Yes, we'll clean up the items
noted, if we (collectively) come to agreement that this approach to
solve device naming is valid and warranted.  From Stephen's other
comments, we're not quite there yet. :-)

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> On Wed, 25 Aug 2010 17:03:23 -0500
On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> Is it really a good idea to have to change every utility that could
> alter network devices?  There is iproute2, iputils, tcpdump, wireshark,
> quagga, snmp, ...  Many of the utilities come from a BSD world,
> and will be less likely to accept some Linux specific wart.

I agree, I don't want to have to change all those userspace apps
either.  We've started creating patches and are willing to do so if it
will yield the result we want though.
 
> I have lost faith in this library wrapper support everywhere method.
> Let's just keep the firmware stuff in udev. If the user wants to
> have a policy that renames device from eth0 to "Embedded BIOS LAN1" then
> do it in udev. Or if you want to keep the ethX naming convention
> and stuff the firmware label into ifAlias or other sysfs field
> so it can be displayed that will be not a big issue.

1) we remain constrained to IFNAMSIZ named arguments.  There is no
   such constraint on BIOS-provided names.  Dell BIOS presently uses
   'Embedded NIC 1' which (fortunately) is 14 chars. We're cutting it
   awfully close already.  I can't dictate to non-Dell BIOS vendors
   what to use as their strings, or how long to make them.

digression 1a) udev has a replace-spaces-with-underscores feature I used in
    the biosdevname udev rules.  That's a reasonable munging of the
    provided strings.  Much more than that and I'm not sure we could
    consistently get it right.

2) there are apps which use a regexp for device names.  We'd have to
   find and fix those too.  Arguably we'd have to do this when we
   patch them to use libnetdevname. [1]

3) it continues to force a single naming convention for NICs, where
   for other types of devices we can have several independent naming
   conventions.  I have end users who wish to name their interfaces by
   the BIOS label, others by the function (public, dmz,
   backup, storage, ...) that the network segment provides.  While we
   could have different policies, each system can have only one policy
   at a time.
  

David Miller had suggested [2] that we could add a method to get
alternate labels for devices by querying them.  We've got something
similar now by exporting the labels for devices in sysfs.  Yea -
progress!

But we can't _use_ those labels for more than display
(meaning a human is doing the mapping in their heads), or to rename
devices.  We can't use the labels in scripts without doing the label->kernel
name lookups, and then using the kernel name.

I'm open to revisiting the "have udev rename devices", but I tried
that with biosdevname 2 years ago, with little success.  Adding in the
hotplug dev team to the thread.


[1] http://marc.info/?l=linux-netdev&m=125522163322610&w=2 (Marco d'Itri)
[2] http://marc.info/?l=linux-hotplug&m=123793549323431&w=2

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> On Wed, 25 Aug 2010 17:03:23 -0500
On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> Is it really a good idea to have to change every utility that could
> alter network devices?  There is iproute2, iputils, tcpdump, wireshark,
> quagga, snmp, ...  Many of the utilities come from a BSD world,
> and will be less likely to accept some Linux specific wart.

I agree, I don't want to have to change all those userspace apps
either.  We've started creating patches and are willing to do so if it
will yield the result we want though.
 
> I have lost faith in this library wrapper support everywhere method.
> Let's just keep the firmware stuff in udev. If the user wants to
> have a policy that renames device from eth0 to "Embedded BIOS LAN1" then
> do it in udev. Or if you want to keep the ethX naming convention
> and stuff the firmware label into ifAlias or other sysfs field
> so it can be displayed that will be not a big issue.

1) we remain constrained to IFNAMSIZ named arguments.  There is no
   such constraint on BIOS-provided names.  Dell BIOS presently uses
   'Embedded NIC 1' which (fortunately) is 14 chars. We're cutting it
   awfully close already.  I can't dictate to non-Dell BIOS vendors
   what to use as their strings, or how long to make them.

digression 1a) udev has a replace-spaces-with-underscores feature I used in
    the biosdevname udev rules.  That's a reasonable munging of the
    provided strings.  Much more than that and I'm not sure we could
    consistently get it right.

2) there are apps which use a regexp for device names.  We'd have to
   find and fix those too.  Arguably we'd have to do this when we
   patch them to use libnetdevname. [1]

3) it continues to force a single naming convention for NICs, where
   for other types of devices we can have several independent naming
   conventions.  I have end users who wish to name their interfaces by
   the BIOS label, others by the function (public, dmz,
   backup, storage, ...) that the network segment provides.  While we
   could have different policies, each system can have only one policy
   at a time.
  

David Miller had suggested [2] that we could add a method to get
alternate labels for devices by querying them.  We've got something
similar now by exporting the labels for devices in sysfs.  Yea -
progress!

But we can't _use_ those labels for more than display
(meaning a human is doing the mapping in their heads), or to rename
devices.  We can't use the labels in scripts without doing the label->kernel
name lookups, and then using the kernel name.

I'm open to revisiting the "have udev rename devices", but I tried
that with biosdevname 2 years ago, with little success.  Adding in the
hotplug dev team to the thread.


[1] http://marc.info/?l=linux-netdev&m\x125522163322610&w=2 (Marco d'Itri)
[2] http://marc.info/?l=linux-hotplug&m\x123793549323431&w=2

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

RE: [PATCH] Add firmware label support to iproute2

[Loke, Chetan]

What if we extend 'IFNAMSIZ'(beyond 16 chars. Older apps don't need to
worry because they have been working w/ 16 chars anyways) and also get
ifalias to work in udev(Or is ifalias a bad idea?)?

Chetan

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Matt Domsch
> Sent: August 26, 2010 11:01 AM
> To: Stephen Hemminger
> Cc: Narendra_K@Dell.com; netdev@vger.kernel.org;
Charles_Rose@Dell.com;
> Jordan_Hargrave@Dell.com; linux-pci@vger.kernel.org; linux-
> hotplug@vger.kernel.org
> Subject: Re: [PATCH] Add firmware label support to iproute2
> 
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > On Wed, 25 Aug 2010 17:03:23 -0500
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > Is it really a good idea to have to change every utility that could
> > alter network devices?  There is iproute2, iputils, tcpdump,
> wireshark,
> > quagga, snmp, ...  Many of the utilities come from a BSD world,
> > and will be less likely to accept some Linux specific wart.
> 
> I agree, I don't want to have to change all those userspace apps
> either.  We've started creating patches and are willing to do so if it
> will yield the result we want though.
> 
> > I have lost faith in this library wrapper support everywhere method.
> > Let's just keep the firmware stuff in udev. If the user wants to
> > have a policy that renames device from eth0 to "Embedded BIOS LAN1"
> then
> > do it in udev. Or if you want to keep the ethX naming convention
> > and stuff the firmware label into ifAlias or other sysfs field
> > so it can be displayed that will be not a big issue.
> 
> 1) we remain constrained to IFNAMSIZ named arguments.  There is no
>    such constraint on BIOS-provided names.  Dell BIOS presently uses
>    'Embedded NIC 1' which (fortunately) is 14 chars. We're cutting it
>    awfully close already.  I can't dictate to non-Dell BIOS vendors
>    what to use as their strings, or how long to make them.
> 
> digression 1a) udev has a replace-spaces-with-underscores feature I
> used in
>     the biosdevname udev rules.  That's a reasonable munging of the
>     provided strings.  Much more than that and I'm not sure we could
>     consistently get it right.
> 
> 2) there are apps which use a regexp for device names.  We'd have to
>    find and fix those too.  Arguably we'd have to do this when we
>    patch them to use libnetdevname. [1]
> 
> 3) it continues to force a single naming convention for NICs, where
>    for other types of devices we can have several independent naming
>    conventions.  I have end users who wish to name their interfaces by
>    the BIOS label, others by the function (public, dmz,
>    backup, storage, ...) that the network segment provides.  While we
>    could have different policies, each system can have only one policy
>    at a time.
> 
> 
> David Miller had suggested [2] that we could add a method to get
> alternate labels for devices by querying them.  We've got something
> similar now by exporting the labels for devices in sysfs.  Yea -
> progress!
> 
> But we can't _use_ those labels for more than display
> (meaning a human is doing the mapping in their heads), or to rename
> devices.  We can't use the labels in scripts without doing the label-
> >kernel
> name lookups, and then using the kernel name.
> 
> I'm open to revisiting the "have udev rename devices", but I tried
> that with biosdevname 2 years ago, with little success.  Adding in the
> hotplug dev team to the thread.
> 
> 
> [1] http://marc.info/?l=linux-netdev&m=125522163322610&w=2 (Marco
> d'Itri)
> [2] http://marc.info/?l=linux-hotplug&m=123793549323431&w=2
> 
> --
> Matt Domsch
> Technology Strategist
> Dell | Office of the CTO
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Add firmware label support to iproute2

[Loke, Chetan]

What if we extend 'IFNAMSIZ'(beyond 16 chars. Older apps don't need to
worry because they have been working w/ 16 chars anyways) and also get
ifalias to work in udev(Or is ifalias a bad idea?)?

Chetan

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Matt Domsch
> Sent: August 26, 2010 11:01 AM
> To: Stephen Hemminger
> Cc: Narendra_K@Dell.com; netdev@vger.kernel.org;
Charles_Rose@Dell.com;
> Jordan_Hargrave@Dell.com; linux-pci@vger.kernel.org; linux-
> hotplug@vger.kernel.org
> Subject: Re: [PATCH] Add firmware label support to iproute2
> 
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > On Wed, 25 Aug 2010 17:03:23 -0500
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > Is it really a good idea to have to change every utility that could
> > alter network devices?  There is iproute2, iputils, tcpdump,
> wireshark,
> > quagga, snmp, ...  Many of the utilities come from a BSD world,
> > and will be less likely to accept some Linux specific wart.
> 
> I agree, I don't want to have to change all those userspace apps
> either.  We've started creating patches and are willing to do so if it
> will yield the result we want though.
> 
> > I have lost faith in this library wrapper support everywhere method.
> > Let's just keep the firmware stuff in udev. If the user wants to
> > have a policy that renames device from eth0 to "Embedded BIOS LAN1"
> then
> > do it in udev. Or if you want to keep the ethX naming convention
> > and stuff the firmware label into ifAlias or other sysfs field
> > so it can be displayed that will be not a big issue.
> 
> 1) we remain constrained to IFNAMSIZ named arguments.  There is no
>    such constraint on BIOS-provided names.  Dell BIOS presently uses
>    'Embedded NIC 1' which (fortunately) is 14 chars. We're cutting it
>    awfully close already.  I can't dictate to non-Dell BIOS vendors
>    what to use as their strings, or how long to make them.
> 
> digression 1a) udev has a replace-spaces-with-underscores feature I
> used in
>     the biosdevname udev rules.  That's a reasonable munging of the
>     provided strings.  Much more than that and I'm not sure we could
>     consistently get it right.
> 
> 2) there are apps which use a regexp for device names.  We'd have to
>    find and fix those too.  Arguably we'd have to do this when we
>    patch them to use libnetdevname. [1]
> 
> 3) it continues to force a single naming convention for NICs, where
>    for other types of devices we can have several independent naming
>    conventions.  I have end users who wish to name their interfaces by
>    the BIOS label, others by the function (public, dmz,
>    backup, storage, ...) that the network segment provides.  While we
>    could have different policies, each system can have only one policy
>    at a time.
> 
> 
> David Miller had suggested [2] that we could add a method to get
> alternate labels for devices by querying them.  We've got something
> similar now by exporting the labels for devices in sysfs.  Yea -
> progress!
> 
> But we can't _use_ those labels for more than display
> (meaning a human is doing the mapping in their heads), or to rename
> devices.  We can't use the labels in scripts without doing the label-
> >kernel
> name lookups, and then using the kernel name.
> 
> I'm open to revisiting the "have udev rename devices", but I tried
> that with biosdevname 2 years ago, with little success.  Adding in the
> hotplug dev team to the thread.
> 
> 
> [1] http://marc.info/?l=linux-netdev&m\x125522163322610&w=2 (Marco
> d'Itri)
> [2] http://marc.info/?l=linux-hotplug&m\x123793549323431&w=2
> 
> --
> Matt Domsch
> Technology Strategist
> Dell | Office of the CTO
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Add firmware label support to iproute2

[Stephen Hemminger]

On Thu, 26 Aug 2010 11:17:51 -0400
"Loke, Chetan" <Chetan.Loke@netscout.com> wrote:

> What if we extend 'IFNAMSIZ'(beyond 16 chars. Older apps don't need to
> worry because they have been working w/ 16 chars anyways) and also get
> ifalias to work in udev(Or is ifalias a bad idea?)?
> 
> Chetan
>
That is  non-starter for the ioctl() style interface.

struct ifreq {
#define IFHWADDRLEN	6
	union
	{
		char	ifrn_name[IFNAMSIZ];		/* if name, e.g. "en0" */
	} ifr_ifrn;
	
	union {
		struct	sockaddr ifru_addr;
		struct	sockaddr ifru_dstaddr;
		struct	sockaddr ifru_broadaddr;
		struct	sockaddr ifru_netmask;
		struct  sockaddr ifru_hwaddr;
		short	ifru_flags;
		int	ifru_ivalue;
		int	ifru_mtu;
		struct  ifmap ifru_map;
		char	ifru_slave[IFNAMSIZ];	/* Just fits the size */
		char	ifru_newname[IFNAMSIZ];
		void __user *	ifru_data;
		struct	if_settings ifru_settings;
	} ifr_ifru;
};

Applications do:
   	strncpy(ifr.ifr_name, "my name", IFNAMSIZ);
	ioctl(fd, SIOCGHWADDR, &ifr)

Re: [PATCH] Add firmware label support to iproute2

[Stephen Hemminger]

On Thu, 26 Aug 2010 11:17:51 -0400
"Loke, Chetan" <Chetan.Loke@netscout.com> wrote:

> What if we extend 'IFNAMSIZ'(beyond 16 chars. Older apps don't need to
> worry because they have been working w/ 16 chars anyways) and also get
> ifalias to work in udev(Or is ifalias a bad idea?)?
> 
> Chetan
>
That is  non-starter for the ioctl() style interface.

struct ifreq {
#define IFHWADDRLEN	6
	union
	{
		char	ifrn_name[IFNAMSIZ];		/* if name, e.g. "en0" */
	} ifr_ifrn;
	
	union {
		struct	sockaddr ifru_addr;
		struct	sockaddr ifru_dstaddr;
		struct	sockaddr ifru_broadaddr;
		struct	sockaddr ifru_netmask;
		struct  sockaddr ifru_hwaddr;
		short	ifru_flags;
		int	ifru_ivalue;
		int	ifru_mtu;
		struct  ifmap ifru_map;
		char	ifru_slave[IFNAMSIZ];	/* Just fits the size */
		char	ifru_newname[IFNAMSIZ];
		void __user *	ifru_data;
		struct	if_settings ifru_settings;
	} ifr_ifru;
};

Applications do:
   	strncpy(ifr.ifr_name, "my name", IFNAMSIZ);
	ioctl(fd, SIOCGHWADDR, &ifr)

RE: [PATCH] Add firmware label support to iproute2

[Loke, Chetan]

Sorry, my bad.
I agree w/ your recommendation then - 
1) stuff it in ifalias(or create a new alias if net_device::ifalias is
used for snmp etc).

But how do we consume it? 
   1.1) spit it out via sysfs? But Doc/sysfsrules.txt says "Accessing
/sys/class/net/eth0/device is a bug in the application"


Chetan

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: August 26, 2010 11:22 AM
> To: Loke, Chetan
> Cc: Matt Domsch; Narendra_K@Dell.com; netdev@vger.kernel.org;
> Charles_Rose@Dell.com; Jordan_Hargrave@Dell.com; linux-
> pci@vger.kernel.org; linux-hotplug@vger.kernel.org
> Subject: Re: [PATCH] Add firmware label support to iproute2
> 
> On Thu, 26 Aug 2010 11:17:51 -0400
> "Loke, Chetan" <Chetan.Loke@netscout.com> wrote:
> 
> > What if we extend 'IFNAMSIZ'(beyond 16 chars. Older apps don't need
> to
> > worry because they have been working w/ 16 chars anyways) and also
> get
> > ifalias to work in udev(Or is ifalias a bad idea?)?
> >
> > Chetan
> >
> That is  non-starter for the ioctl() style interface.
> 
> struct ifreq {
> #define IFHWADDRLEN	6
> 	union
> 	{
> 		char	ifrn_name[IFNAMSIZ];		/* if name, e.g.
"en0"
> */
> 	} ifr_ifrn;
> 
> 	union {
> 		struct	sockaddr ifru_addr;
> 		struct	sockaddr ifru_dstaddr;
> 		struct	sockaddr ifru_broadaddr;
> 		struct	sockaddr ifru_netmask;
> 		struct  sockaddr ifru_hwaddr;
> 		short	ifru_flags;
> 		int	ifru_ivalue;
> 		int	ifru_mtu;
> 		struct  ifmap ifru_map;
> 		char	ifru_slave[IFNAMSIZ];	/* Just fits the size */
> 		char	ifru_newname[IFNAMSIZ];
> 		void __user *	ifru_data;
> 		struct	if_settings ifru_settings;
> 	} ifr_ifru;
> };
> 
> Applications do:
>    	strncpy(ifr.ifr_name, "my name", IFNAMSIZ);
> 	ioctl(fd, SIOCGHWADDR, &ifr)

RE: [PATCH] Add firmware label support to iproute2

[Loke, Chetan]

Sorry, my bad.
I agree w/ your recommendation then - 
1) stuff it in ifalias(or create a new alias if net_device::ifalias is
used for snmp etc).

But how do we consume it? 
   1.1) spit it out via sysfs? But Doc/sysfsrules.txt says "Accessing
/sys/class/net/eth0/device is a bug in the application"


Chetan

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: August 26, 2010 11:22 AM
> To: Loke, Chetan
> Cc: Matt Domsch; Narendra_K@Dell.com; netdev@vger.kernel.org;
> Charles_Rose@Dell.com; Jordan_Hargrave@Dell.com; linux-
> pci@vger.kernel.org; linux-hotplug@vger.kernel.org
> Subject: Re: [PATCH] Add firmware label support to iproute2
> 
> On Thu, 26 Aug 2010 11:17:51 -0400
> "Loke, Chetan" <Chetan.Loke@netscout.com> wrote:
> 
> > What if we extend 'IFNAMSIZ'(beyond 16 chars. Older apps don't need
> to
> > worry because they have been working w/ 16 chars anyways) and also
> get
> > ifalias to work in udev(Or is ifalias a bad idea?)?
> >
> > Chetan
> >
> That is  non-starter for the ioctl() style interface.
> 
> struct ifreq {
> #define IFHWADDRLEN	6
> 	union
> 	{
> 		char	ifrn_name[IFNAMSIZ];		/* if name, e.g.
"en0"
> */
> 	} ifr_ifrn;
> 
> 	union {
> 		struct	sockaddr ifru_addr;
> 		struct	sockaddr ifru_dstaddr;
> 		struct	sockaddr ifru_broadaddr;
> 		struct	sockaddr ifru_netmask;
> 		struct  sockaddr ifru_hwaddr;
> 		short	ifru_flags;
> 		int	ifru_ivalue;
> 		int	ifru_mtu;
> 		struct  ifmap ifru_map;
> 		char	ifru_slave[IFNAMSIZ];	/* Just fits the size */
> 		char	ifru_newname[IFNAMSIZ];
> 		void __user *	ifru_data;
> 		struct	if_settings ifru_settings;
> 	} ifr_ifru;
> };
> 
> Applications do:
>    	strncpy(ifr.ifr_name, "my name", IFNAMSIZ);
> 	ioctl(fd, SIOCGHWADDR, &ifr)

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Thu, Aug 26, 2010 at 10:01:16AM -0500, Matt Domsch wrote:
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > On Wed, 25 Aug 2010 17:03:23 -0500
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > Is it really a good idea to have to change every utility that could
> > alter network devices?  There is iproute2, iputils, tcpdump, wireshark,
> > quagga, snmp, ...  Many of the utilities come from a BSD world,
> > and will be less likely to accept some Linux specific wart.
> 
> I agree, I don't want to have to change all those userspace apps
> either.  We've started creating patches and are willing to do so if it
> will yield the result we want though.
>  
> > I have lost faith in this library wrapper support everywhere method.
> > Let's just keep the firmware stuff in udev. If the user wants to
> > have a policy that renames device from eth0 to "Embedded BIOS LAN1" then
> > do it in udev. Or if you want to keep the ethX naming convention
> > and stuff the firmware label into ifAlias or other sysfs field
> > so it can be displayed that will be not a big issue.
> 
> 1) we remain constrained to IFNAMSIZ named arguments.  There is no
>    such constraint on BIOS-provided names.  Dell BIOS presently uses
>    'Embedded NIC 1' which (fortunately) is 14 chars. We're cutting it
>    awfully close already.  I can't dictate to non-Dell BIOS vendors
>    what to use as their strings, or how long to make them.

vlan usage requires 5 characters from IFNAMSIZ (eth0.4095).  So my
example with 'Embedded NIC 1' plus a vlan doesn't fit.

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: [PATCH] Add firmware label support to iproute2

[Matt Domsch]

On Thu, Aug 26, 2010 at 10:01:16AM -0500, Matt Domsch wrote:
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > On Wed, 25 Aug 2010 17:03:23 -0500
> On Wed, Aug 25, 2010 at 05:16:46PM -0700, Stephen Hemminger wrote:
> > Is it really a good idea to have to change every utility that could
> > alter network devices?  There is iproute2, iputils, tcpdump, wireshark,
> > quagga, snmp, ...  Many of the utilities come from a BSD world,
> > and will be less likely to accept some Linux specific wart.
> 
> I agree, I don't want to have to change all those userspace apps
> either.  We've started creating patches and are willing to do so if it
> will yield the result we want though.
>  
> > I have lost faith in this library wrapper support everywhere method.
> > Let's just keep the firmware stuff in udev. If the user wants to
> > have a policy that renames device from eth0 to "Embedded BIOS LAN1" then
> > do it in udev. Or if you want to keep the ethX naming convention
> > and stuff the firmware label into ifAlias or other sysfs field
> > so it can be displayed that will be not a big issue.
> 
> 1) we remain constrained to IFNAMSIZ named arguments.  There is no
>    such constraint on BIOS-provided names.  Dell BIOS presently uses
>    'Embedded NIC 1' which (fortunately) is 14 chars. We're cutting it
>    awfully close already.  I can't dictate to non-Dell BIOS vendors
>    what to use as their strings, or how long to make them.

vlan usage requires 5 characters from IFNAMSIZ (eth0.4095).  So my
example with 'Embedded NIC 1' plus a vlan doesn't fit.

-- 
Matt Domsch
Technology Strategist
Dell | Office of the CTO

Re: [PATCH] Add firmware label support to iproute2

[Kay Sievers]

On Thu, Aug 26, 2010 at 17:38, Loke, Chetan <Chetan.Loke@netscout.com> wrote:
> Sorry, my bad.
> I agree w/ your recommendation then -
> 1) stuff it in ifalias(or create a new alias if net_device::ifalias is
> used for snmp etc).
>
> But how do we consume it?
>   1.1) spit it out via sysfs? But Doc/sysfsrules.txt says "Accessing
> /sys/class/net/eth0/device is a bug in the application"

You walk up the parent devices of the netdev until you find the one
you are looking for. The "device" link is broken by design and might
just point to the first parent device. The kernel is free to insert
devices into the device tree if needed, and then the device link
target may change.

/sys/class/net/eth0 is a symlink pointing to a location the
/sys/devices tree, like:
  ../../devices/pci0000:00/0000:00:19.0/net/eth0
One of the parent devices of the eth0 device has this attribute., but
you can never be sure that it's just the next parent (broken concept
of the device link), you have to walk up the chain of parents.

You can just use udev to that automatically and extract the value, and
pass it with the event for the eth0 device.

Kay

Re: [PATCH] Add firmware label support to iproute2

[Kay Sievers]

On Thu, Aug 26, 2010 at 17:38, Loke, Chetan <Chetan.Loke@netscout.com> wrote:
> Sorry, my bad.
> I agree w/ your recommendation then -
> 1) stuff it in ifalias(or create a new alias if net_device::ifalias is
> used for snmp etc).
>
> But how do we consume it?
>   1.1) spit it out via sysfs? But Doc/sysfsrules.txt says "Accessing
> /sys/class/net/eth0/device is a bug in the application"

You walk up the parent devices of the netdev until you find the one
you are looking for. The "device" link is broken by design and might
just point to the first parent device. The kernel is free to insert
devices into the device tree if needed, and then the device link
target may change.

/sys/class/net/eth0 is a symlink pointing to a location the
/sys/devices tree, like:
  ../../devices/pci0000:00/0000:00:19.0/net/eth0
One of the parent devices of the eth0 device has this attribute., but
you can never be sure that it's just the next parent (broken concept
of the device link), you have to walk up the chain of parents.

You can just use udev to that automatically and extract the value, and
pass it with the event for the eth0 device.

Kay