[PATCH 2.6 NET] Device name changing via rtnetlink

[PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

Allows changing of device name via rtnetlink. Last bit needed to do full
link configuration via rtnetlink.

Signed-off-by: Thomas Graf <tgraf@suug.ch>


diff -Nru linux-2.6.9-rc1-bk15.orig/include/linux/netdevice.h linux-2.6.9-rc1-bk15/include/linux/netdevice.h
--- linux-2.6.9-rc1-bk15.orig/include/linux/netdevice.h	2004-09-08 18:32:05.000000000 +0200
+++ linux-2.6.9-rc1-bk15/include/linux/netdevice.h	2004-09-10 12:42:07.000000000 +0200
@@ -677,6 +677,7 @@
 extern int		dev_ethtool(struct ifreq *);
 extern unsigned		dev_get_flags(const struct net_device *);
 extern int		dev_change_flags(struct net_device *, unsigned);
+extern int		dev_change_name(struct net_device *, char *);
 extern int		dev_set_mtu(struct net_device *, int);
 extern void		dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev);
 
diff -Nru linux-2.6.9-rc1-bk15.orig/net/core/dev.c linux-2.6.9-rc1-bk15/net/core/dev.c
--- linux-2.6.9-rc1-bk15.orig/net/core/dev.c	2004-09-08 18:33:42.000000000 +0200
+++ linux-2.6.9-rc1-bk15/net/core/dev.c	2004-09-10 12:41:19.000000000 +0200
@@ -3347,6 +3347,7 @@
 EXPORT_SYMBOL(dev_set_allmulti);
 EXPORT_SYMBOL(dev_set_promiscuity);
 EXPORT_SYMBOL(dev_change_flags);
+EXPORT_SYMBOL(dev_change_name);
 EXPORT_SYMBOL(dev_set_mtu);
 EXPORT_SYMBOL(free_netdev);
 EXPORT_SYMBOL(netdev_boot_setup_check);
diff -Nru linux-2.6.9-rc1-bk15.orig/net/core/rtnetlink.c linux-2.6.9-rc1-bk15/net/core/rtnetlink.c
--- linux-2.6.9-rc1-bk15.orig/net/core/rtnetlink.c	2004-09-08 18:33:42.000000000 +0200
+++ linux-2.6.9-rc1-bk15/net/core/rtnetlink.c	2004-09-10 12:36:54.000000000 +0200
@@ -345,6 +345,23 @@
 		dev->weight = *((u32 *) RTA_DATA(ida[IFLA_WEIGHT - 1]));
 	}
 
+	if (ida[IFLA_IFNAME - 1]) {
+		char ifname[IFNAMSIZ];
+
+		if (ida[IFLA_IFNAME - 1]->rta_len > RTA_LENGTH(IFNAMSIZ))
+			goto out;
+
+		memset(ifname, 0, sizeof(ifname));
+		memcpy(ifname, RTA_DATA(ida[IFLA_IFNAME - 1]),
+			RTA_PAYLOAD(ida[IFLA_IFNAME - 1]));
+		ifname[IFNAMSIZ - 1] = '\0';
+
+		err = dev_change_name(dev, ifname);
+
+		if (err)
+			goto out;
+	}
+
 	err = 0;
 
 out:

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040910133637.GA20088@postel.suug.ch> (at Fri, 10 Sep 2004 15:36:37 +0200), Thomas Graf <tgraf@suug.ch> says:

> Allows changing of device name via rtnetlink. Last bit needed to do full
> link configuration via rtnetlink.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
:
> +		if (ida[IFLA_IFNAME - 1]->rta_len > RTA_LENGTH(IFNAMSIZ))
> +			goto out;

Please use sizeof(ifname) instead.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

* YOSHIFUJI Hideaki / ?$B5HF#1QL@ <20040910.230038.322907649.yoshfuji@linux-ipv6.org> 2004-09-10 23:00
> In article <20040910133637.GA20088@postel.suug.ch> (at Fri, 10 Sep 2004 15:36:37 +0200), Thomas Graf <tgraf@suug.ch> says:
> 
> > Allows changing of device name via rtnetlink. Last bit needed to do full
> > link configuration via rtnetlink.
> > 
> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
> :
> > +		if (ida[IFLA_IFNAME - 1]->rta_len > RTA_LENGTH(IFNAMSIZ))
> > +			goto out;
> 
> Please use sizeof(ifname) instead.

Thanks for the hint.

Allows changing of device name via rtnetlink. Last bit needed to do full
link configuration via rtnetlink.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

--- linux-2.6.9-rc1-bk15.orig/include/linux/netdevice.h	2004-09-08 18:32:05.000000000 +0200
+++ linux-2.6.9-rc1-bk15/include/linux/netdevice.h	2004-09-10 12:42:07.000000000 +0200
@@ -677,6 +677,7 @@
 extern int		dev_ethtool(struct ifreq *);
 extern unsigned		dev_get_flags(const struct net_device *);
 extern int		dev_change_flags(struct net_device *, unsigned);
+extern int		dev_change_name(struct net_device *, char *);
 extern int		dev_set_mtu(struct net_device *, int);
 extern void		dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev);
 
--- linux-2.6.9-rc1-bk15.orig/net/core/dev.c	2004-09-08 18:33:42.000000000 +0200
+++ linux-2.6.9-rc1-bk15/net/core/dev.c	2004-09-10 12:41:19.000000000 +0200
@@ -3347,6 +3347,7 @@
 EXPORT_SYMBOL(dev_set_allmulti);
 EXPORT_SYMBOL(dev_set_promiscuity);
 EXPORT_SYMBOL(dev_change_flags);
+EXPORT_SYMBOL(dev_change_name);
 EXPORT_SYMBOL(dev_set_mtu);
 EXPORT_SYMBOL(free_netdev);
 EXPORT_SYMBOL(netdev_boot_setup_check);
--- linux-2.6.9-rc1-bk15.orig/net/core/rtnetlink.c	2004-09-08 18:33:42.000000000 +0200
+++ linux-2.6.9-rc1-bk15/net/core/rtnetlink.c	2004-09-10 16:13:46.000000000 +0200
@@ -345,6 +345,23 @@
 		dev->weight = *((u32 *) RTA_DATA(ida[IFLA_WEIGHT - 1]));
 	}
 
+	if (ida[IFLA_IFNAME - 1]) {
+		char ifname[IFNAMSIZ];
+
+		if (ida[IFLA_IFNAME - 1]->rta_len > RTA_LENGTH(sizeof(ifname)))
+			goto out;
+
+		memset(ifname, 0, sizeof(ifname));
+		memcpy(ifname, RTA_DATA(ida[IFLA_IFNAME - 1]),
+			RTA_PAYLOAD(ida[IFLA_IFNAME - 1]));
+		ifname[IFNAMSIZ - 1] = '\0';
+
+		err = dev_change_name(dev, ifname);
+
+		if (err)
+			goto out;
+	}
+
 	err = 0;
 
 out:

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040910142804.GC20088@postel.suug.ch> (at Fri, 10 Sep 2004 16:28:04 +0200), Thomas Graf <tgraf@suug.ch> says:

> > Please use sizeof(ifname) instead.
> 
> Thanks for the hint.
> 
> Allows changing of device name via rtnetlink. Last bit needed to do full
> link configuration via rtnetlink.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

Thanks. Ack.
Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>

--yoshfuji

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Jean Tourrilhes]

Thomas Graf wrote :
> 
> Allows changing of device name via rtnetlink. Last bit needed to do full
> link configuration via rtnetlink.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>

	This does not work, because you don't return the new name to
user space. If the new name is a pattern, such as "eth%d" or "wlan%d",
you absolutely need to return the new instanciated device name to user
space so that userspace doesn't loose track of the device.

	Please look at this snipset from dev.c :

-----------------------------------------------------------------
		/*
		 *	These ioctl calls:
		 *	- require superuser power.
		 *	- require strict serialization.
		 *	- return a value
		 */
		case SIOCGMIIPHY:
		case SIOCGMIIREG:
		case SIOCSIFNAME:
			if (!capable(CAP_NET_ADMIN))
				return -EPERM;
			dev_load(ifr.ifr_name);
			rtnl_lock();
			ret = dev_ifsioc(&ifr, cmd);
			rtnl_unlock();
			if (!ret) {
				if (colon)
					*colon = ':';
				if (copy_to_user(arg, &ifr,
						 sizeof(struct ifreq)))
					ret = -EFAULT;
			}
			return ret;
-----------------------------------------------------------------

	I'm all for converting stuff to RtNetlink, but please don't
criple the functionality...

	Have fun...

	Jean

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

* Jean Tourrilhes <20040910195003.GA13912@bougret.hpl.hp.com> 2004-09-10 12:50
> Thomas Graf wrote :
> > 
> > Allows changing of device name via rtnetlink. Last bit needed to do full
> > link configuration via rtnetlink.
> > 
> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
> 
> 	This does not work, because you don't return the new name to
> user space. If the new name is a pattern, such as "eth%d" or "wlan%d",
> you absolutely need to return the new instanciated device name to user
> space so that userspace doesn't loose track of the device.

The ifindex stays the same, therefore the user space application can
simply dump the link list and fetch the new interface name. It would
theoretically be possible to provide the new name via an ACK but
this would break the RFC.

> 				if (colon)
> 					*colon = ':';

I ignored the colon handling on purpose, this is better done
in user space. I can change this if required.

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Jean Tourrilhes]

On Fri, Sep 10, 2004 at 10:06:44PM +0200, Thomas Graf wrote:
> * Jean Tourrilhes <20040910195003.GA13912@bougret.hpl.hp.com> 2004-09-10 12:50
> > Thomas Graf wrote :
> > > 
> > > Allows changing of device name via rtnetlink. Last bit needed to do full
> > > link configuration via rtnetlink.
> > > 
> > > Signed-off-by: Thomas Graf <tgraf@suug.ch>
> > 
> > 	This does not work, because you don't return the new name to
> > user space. If the new name is a pattern, such as "eth%d" or "wlan%d",
> > you absolutely need to return the new instanciated device name to user
> > space so that userspace doesn't loose track of the device.
> 
> The ifindex stays the same, therefore the user space application can
> simply dump the link list and fetch the new interface name.

	It's so simple to return the new name, so why not do it ?
There is no need to make applications more complex.

> It would
> theoretically be possible to provide the new name via an ACK but
> this would break the RFC.

	What do you mean, break the RFC ?

	Jean

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

* Jean Tourrilhes <20040910201302.GA16556@bougret.hpl.hp.com> 2004-09-10 13:13
> On Fri, Sep 10, 2004 at 10:06:44PM +0200, Thomas Graf wrote:
> > * Jean Tourrilhes <20040910195003.GA13912@bougret.hpl.hp.com> 2004-09-10 12:50
> > > Thomas Graf wrote :
> > > > 
> > > > Allows changing of device name via rtnetlink. Last bit needed to do full
> > > > link configuration via rtnetlink.
> > > > 
> > > > Signed-off-by: Thomas Graf <tgraf@suug.ch>
> > > 
> > > 	This does not work, because you don't return the new name to
> > > user space. If the new name is a pattern, such as "eth%d" or "wlan%d",
> > > you absolutely need to return the new instanciated device name to user
> > > space so that userspace doesn't loose track of the device.
> > 
> > The ifindex stays the same, therefore the user space application can
> > simply dump the link list and fetch the new interface name.
> 
> 	It's so simple to return the new name, so why not do it ?
> There is no need to make applications more complex.

Oh, I didn't realize that, can you enlighten me?

> > It would
> > theoretically be possible to provide the new name via an ACK but
> > this would break the RFC.
> 
> 	What do you mean, break the RFC ?

I can think of 2 ways implementing this:

1) Send an ACK and change IFLA_IFNAME in the original message. However,
   RFC 3549 specifies it to be the old message so user space
   applications can directly reuse it (forward, redirect, log, ...)

2) Implement NETDEV_CHANGE, I was actually about to implement this
   but it's not directly related to this change and requires the same
   effort by the user space application has refetching the link list.

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[jamal]

On Fri, 2004-09-10 at 16:22, Thomas Graf wrote:

> 2) Implement NETDEV_CHANGE, I was actually about to implement this
>    but it's not directly related to this change and requires the same
>    effort by the user space application has refetching the link list.

This is what he needs. Essentially NETDEV_CHANGE will generate a netlink
event.

cheers,
jamal

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Jean Tourrilhes]

On Fri, Sep 10, 2004 at 10:22:35PM +0200, Thomas Graf wrote:
> 
> 2) Implement NETDEV_CHANGE, I was actually about to implement this
>    but it's not directly related to this change and requires the same
>    effort by the user space application has refetching the link list.

	Do you have any info on NETDEV_CHANGE, I've got a potential
use for it.
	(By the way, there are other ways to do it, but if
NETDEV_CHANGE is the proposed standard, I might as well use it).

	Jean

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

* Jean Tourrilhes <20040910203203.GA17078@bougret.hpl.hp.com> 2004-09-10 13:32
> On Fri, Sep 10, 2004 at 10:22:35PM +0200, Thomas Graf wrote:
> > 
> > 2) Implement NETDEV_CHANGE, I was actually about to implement this
> >    but it's not directly related to this change and requires the same
> >    effort by the user space application has refetching the link list.
> 
> 	Do you have any info on NETDEV_CHANGE, I've got a potential
> use for it.
> 	(By the way, there are other ways to do it, but if
> NETDEV_CHANGE is the proposed standard, I might as well use it).

Currently call_netdevice_notifiers is called and sends out a
RTM_NEWLINK message with the new interface name to everyone
wanting to be notified. However, ifi_change is kept to 0
so far, which means you have to look what has changed yourself. I'm
not sure if this can be changed without too much troubles but
I will look into it.

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[jamal]

On Fri, 2004-09-10 at 16:43, Thomas Graf wrote:
> * Jean Tourrilhes <20040910203203.GA17078@bougret.hpl.hp.com> 2004-09-10 13:32
> > On Fri, Sep 10, 2004 at 10:22:35PM +0200, Thomas Graf wrote:
> > > 
> > > 2) Implement NETDEV_CHANGE, I was actually about to implement this
> > >    but it's not directly related to this change and requires the same
> > >    effort by the user space application has refetching the link list.
> > 
> > 	Do you have any info on NETDEV_CHANGE, I've got a potential
> > use for it.
> > 	(By the way, there are other ways to do it, but if
> > NETDEV_CHANGE is the proposed standard, I might as well use it).
> 
> Currently call_netdevice_notifiers is called and sends out a
> RTM_NEWLINK message with the new interface name to everyone
> wanting to be notified. However, ifi_change is kept to 0
> so far, which means you have to look what has changed yourself. I'm
> not sure if this can be changed without too much troubles but
> I will look into it.

I missed the RTM_NEWLINK part. It is easier to just keep it that way and
make sure it is well documented as so. It may not be trivial (as an
example all the inetdev name attributes change - look at inetdev_event).
So suggestion is to leave it to user space to find more details about
the event.

cheers,
jamal

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[David S. Miller]

On Fri, 10 Sep 2004 22:43:48 +0200
Thomas Graf <tgraf@suug.ch> wrote:

> * Jean Tourrilhes <20040910203203.GA17078@bougret.hpl.hp.com> 2004-09-10 13:32
> > On Fri, Sep 10, 2004 at 10:22:35PM +0200, Thomas Graf wrote:
> > > 
> > > 2) Implement NETDEV_CHANGE, I was actually about to implement this
> > >    but it's not directly related to this change and requires the same
> > >    effort by the user space application has refetching the link list.
> > 
> > 	Do you have any info on NETDEV_CHANGE, I've got a potential
> > use for it.
> > 	(By the way, there are other ways to do it, but if
> > NETDEV_CHANGE is the proposed standard, I might as well use it).
> 
> Currently call_netdevice_notifiers is called and sends out a
> RTM_NEWLINK message with the new interface name to everyone
> wanting to be notified. However, ifi_change is kept to 0
> so far, which means you have to look what has changed yourself. I'm
> not sure if this can be changed without too much troubles but
> I will look into it.

For now I'm going to apply Thomas's name changing patch
(with the sizeof() fix suggested by Yoshifuji), and we
can enhance it later.

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

* jamal <1094857082.1041.19.camel@jzny.localdomain> 2004-09-10 18:58
> On Fri, 2004-09-10 at 16:43, Thomas Graf wrote:
> > Currently call_netdevice_notifiers is called and sends out a
> > RTM_NEWLINK message with the new interface name to everyone
> > wanting to be notified. However, ifi_change is kept to 0
> > so far, which means you have to look what has changed yourself. I'm
> > not sure if this can be changed without too much troubles but
> > I will look into it.
> 
> I missed the RTM_NEWLINK part. It is easier to just keep it that way and
> make sure it is well documented as so. It may not be trivial (as an
> example all the inetdev name attributes change - look at inetdev_event).
> So suggestion is to leave it to user space to find more details about
> the event.

I agree, most applications will hold a cache of all links, like iproute2
is doing, and just update the cache.

There is one remaining problem, currently, changes via rtnetlink will
result in multiple notifies being sent out because NETDEV_CHANGEMTU
and NETDEV_CHANGENAME are invoked as well and result in a netlink
message. Changing them to not do anything in rtnetlink context
would solve it but we would lose the notify if the change was made
via ioctl.

The patch below would fixes the multiple notifies issue but prevents
notification upon ioctl via rtnetlink.. Any ideas how to work around
this other than not using dev_change_name and dev_set_mtu?

--- linux-2.6.9-rc1-bk15.orig/net/core/rtnetlink.c	2004-09-08 18:33:42.000000000 +0200
+++ linux-2.6.9-rc1-bk15/net/core/rtnetlink.c	2004-09-11 01:06:29.000000000 +0200
@@ -607,6 +626,8 @@
 		break;
 	case NETDEV_CHANGE:
 	case NETDEV_GOING_DOWN:
+	case NETDEV_CHANGEMTU:
+	case NETDEV_CHANGENAME:
 		break;
 	default:
 		rtmsg_ifinfo(RTM_NEWLINK, dev, 0);

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[jamal]

On Fri, 2004-09-10 at 19:17, Thomas Graf wrote:

> There is one remaining problem, currently, changes via rtnetlink will
> result in multiple notifies being sent out because NETDEV_CHANGEMTU
> and NETDEV_CHANGENAME are invoked as well and result in a netlink
> message. Changing them to not do anything in rtnetlink context
> would solve it but we would lose the notify if the change was made
> via ioctl.
> 

notify via ioctls is still needed.
Are you changing MTU and NAME at the same time?
Valid to receive the two updates i would think in that case.

cheers,
jamal

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

* jamal <1094868070.1042.77.camel@jzny.localdomain> 2004-09-10 22:01
> On Fri, 2004-09-10 at 19:17, Thomas Graf wrote:
> 
> > There is one remaining problem, currently, changes via rtnetlink will
> > result in multiple notifies being sent out because NETDEV_CHANGEMTU
> > and NETDEV_CHANGENAME are invoked as well and result in a netlink
> > message. Changing them to not do anything in rtnetlink context
> > would solve it but we would lose the notify if the change was made
> > via ioctl.
> > 
> Are you changing MTU and NAME at the same time?

Possibly, I change everything the user requests me to do.

> Valid to receive the two updates i would think in that case.

That's not the problem, the problem is that you may receive
more updates than requested changes. Let's assume you're changing
the MTU using IFLA_MTU and it succeeds. dev_set_mtu will send
out the first update, at the end the do_setlink will send
out another update.

Changing MTU and ifname would result in 3 updates:

 1) RTM_NEWLINK with updated mtu (old name)
 2) RTM_NEWLINK with mtu and name updated
 3) same again as 2)

I personally have no problem with the current behaviour but maybe
we should clean this up a bit.

Suggestion:
 Send out a notify for every change we make. This can result
 in 7 updates being sent out. It would allow the application
 to see how far the kernel was successful quite easly or
 do a revision control and switch back to older configurations.

Something like this:

--- linux-2.6.9-rc1-bk15.orig/net/core/rtnetlink.c	2004-09-11 15:39:06.000000000 +0200
+++ linux-2.6.9-rc1-bk15/net/core/rtnetlink.c	2004-09-11 15:39:37.000000000 +0200
@@ -295,6 +295,7 @@
 
 		if (err)
 			goto out;
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	}
 
 	if (ida[IFLA_ADDRESS - 1]) {
@@ -312,6 +313,7 @@
 		err = dev->set_mac_address(dev, RTA_DATA(ida[IFLA_ADDRESS - 1]));
 		if (err)
 			goto out;
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	}
 
 	if (ida[IFLA_BROADCAST - 1]) {
@@ -319,6 +321,7 @@
 			goto out;
 		memcpy(dev->broadcast, RTA_DATA(ida[IFLA_BROADCAST - 1]),
 		       dev->addr_len);
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	}
 
 	if (ida[IFLA_MTU - 1]) {
@@ -336,6 +339,7 @@
 			goto out;
 
 		dev->tx_queue_len = *((u32 *) RTA_DATA(ida[IFLA_TXQLEN - 1]));
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	}
 
 	if (ida[IFLA_WEIGHT - 1]) {
@@ -343,6 +347,7 @@
 			goto out;
 
 		dev->weight = *((u32 *) RTA_DATA(ida[IFLA_WEIGHT - 1]));
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 	}
 
 	if (ida[IFLA_IFNAME - 1]) {
@@ -365,8 +370,6 @@
 	err = 0;
 
 out:
-	if (!err)
-		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 
 	dev_put(dev);
 	return err;

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[jamal]

On Sat, 2004-09-11 at 09:44, Thomas Graf wrote:
> * jamal <1094868070.1042.77.camel@jzny.localdomain> 2004-09-10 22:01
> > On Fri, 2004-09-10 at 19:17, Thomas Graf wrote:

> Possibly, I change everything the user requests me to do.
> 
> > Valid to receive the two updates i would think in that case.
> 
> That's not the problem, the problem is that you may receive
> more updates than requested changes. Let's assume you're changing
> the MTU using IFLA_MTU and it succeeds. dev_set_mtu will send
> out the first update, at the end the do_setlink will send
> out another update.

Ok, this doesnt sound right. 

> Changing MTU and ifname would result in 3 updates:
> 
>  1) RTM_NEWLINK with updated mtu (old name)
>
>  2) RTM_NEWLINK with mtu and name updated
>  3) same again as 2)
> 
> I personally have no problem with the current behaviour but maybe
> we should clean this up a bit.
> 
> Suggestion:
>  Send out a notify for every change we make. This can result
>  in 7 updates being sent out. It would allow the application
>  to see how far the kernel was successful quite easly or
>  do a revision control and switch back to older configurations.
> 

Not necessary to send all those updates. 
do_setlink was doing the right thing before your patch. It was
announcing the change of addresses (broadcast, etc). You introduced new
features which while related to link level have nothing to do with
address changes. So my suggestion is making your code the exception.
i.e something along the lines of:

addrchg = 1;

if (mtu/weight/name related)
	addrchng = 0;
if (!err & addrchang)
	call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);

Also note, the semantics of netlink are all-or-nothing transaction. i.e
if one of the things requested for fails then you undo the rest. We can
probably loosely say that unles the ATOMIC flag is set you dont need to
undo that .. something to think about (summary is you probably dont want
to send progress update to user space unless the transaction is
successful; you however want to report progress of where failure happens
- as we are discussing at the moment in the error code thread).

cheers,
jamal

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[Thomas Graf]

* jamal <1094932793.2344.82.camel@jzny.localdomain> 2004-09-11 15:59
> So my suggestion is making your code the exception.
> i.e something along the lines of:
> 
> addrchg = 1;
> 
> if (mtu/weight/name related)
> 	addrchng = 0;
> if (!err & addrchang)
> 	call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);

OK, however I'd prefer to invert the concept. See patch
below. This would send out the following updates:

ififlags - NETDEV_(UP|DOWN) if IFF_UP has changed
IFLA_MAP - none
IFLA_ADDRESS|IFLA_BROADCAST - NETDEV_CHANGEADDR
IFLA_MTU - NETDEV_CHANGEMTU
IFLA_WEIGHT - none
IFLA_IFNAME - NETDEVCHANGENAME

PATCH:
Only send NETDEV_CHANGEADDR notifies for address and broadcast changes.
Notify is also sent out if only one of the 2 changes is successful.

Signed-off-by: Thomas Graf <tgraf@suug.ch>

--- linux-2.6.9-rc1-bk18.orig/net/core/rtnetlink.c	2004-09-11 16:30:30.000000000 +0200
+++ linux-2.6.9-rc1-bk18/net/core/rtnetlink.c	2004-09-11 23:30:05.000000000 +0200
@@ -265,7 +265,7 @@
 	struct ifinfomsg  *ifm = NLMSG_DATA(nlh);
 	struct rtattr    **ida = arg;
 	struct net_device *dev;
-	int err;
+	int err, send_addr_notify = 0;
 
 	dev = dev_get_by_index(ifm->ifi_index);
 	if (!dev)
@@ -312,6 +312,7 @@
 		err = dev->set_mac_address(dev, RTA_DATA(ida[IFLA_ADDRESS - 1]));
 		if (err)
 			goto out;
+		send_addr_notify = 1;
 	}
 
 	if (ida[IFLA_BROADCAST - 1]) {
@@ -319,6 +320,7 @@
 			goto out;
 		memcpy(dev->broadcast, RTA_DATA(ida[IFLA_BROADCAST - 1]),
 		       dev->addr_len);
+		send_addr_notify = 1;
 	}
 
 	if (ida[IFLA_MTU - 1]) {
@@ -365,7 +367,7 @@
 	err = 0;
 
 out:
-	if (!err)
+	if (send_addr_notify)
 		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
 
 	dev_put(dev);


> Also note, the semantics of netlink are all-or-nothing transaction. i.e
> if one of the things requested for fails then you undo the rest. We can
> probably loosely say that unles the ATOMIC flag is set you dont need to
> undo that .. something to think about (summary is you probably dont want
> to send progress update to user space unless the transaction is
> successful; you however want to report progress of where failure happens
> - as we are discussing at the moment in the error code thread).

Agreed, I think undo should happen from user space though. In the
example of do_setlink, you can see that while it would be quite easy to
do the sanity checks before actually doing something you can't catch
all errors if you don't want to implement the name/mtu/addr change
routines yourself. Keeping in mind that those sanity checks will
probably only fail during the development of a application it doesn't
really help. That's why I implemented it this way.

I'm experimenting with a commit/fallback netlink library which basically
fetches the subject of change and stores the netlink message to be used
in case of fallback. NLM_F_ATOMIC makes it even more useable but is
quite expensive. A netlink change daemon would probably solve the
problem better.

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[jamal]

On Sat, 2004-09-11 at 18:06, Thomas Graf wrote:
> * jamal <1094932793.2344.82.camel@jzny.localdomain> 2004-09-11 15:59
> > So my suggestion is making your code the exception.
> > i.e something along the lines of:
> > 
> > addrchg = 1;
> > 
> > if (mtu/weight/name related)
> > 	addrchng = 0;
> > if (!err & addrchang)
> > 	call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
> 
> OK, however I'd prefer to invert the concept. See patch
> below. This would send out the following updates:

Guess it wont make difference in number of lines of code, so fine.

> ififlags - NETDEV_(UP|DOWN) if IFF_UP has changed
> IFLA_MAP - none
> IFLA_ADDRESS|IFLA_BROADCAST - NETDEV_CHANGEADDR
> IFLA_MTU - NETDEV_CHANGEMTU
> IFLA_WEIGHT - none
> IFLA_IFNAME - NETDEVCHANGENAME
> 
> PATCH:
> Only send NETDEV_CHANGEADDR notifies for address and broadcast changes.
> Notify is also sent out if only one of the 2 changes is successful.

This one is sort of grey area. 
Sigh. In any case, i think that this is ok for now. So patch should go
in - we can revisit the whole atomicity where applicable in kernel at
some later point.
Dave please go ahead and apply the patch.

> 
> > Also note, the semantics of netlink are all-or-nothing transaction. i.e
> > if one of the things requested for fails then you undo the rest. We can
> > probably loosely say that unles the ATOMIC flag is set you dont need to
> > undo that .. something to think about (summary is you probably dont want
> > to send progress update to user space unless the transaction is
> > successful; you however want to report progress of where failure happens
> > - as we are discussing at the moment in the error code thread).
> 
> Agreed, I think undo should happen from user space though. In the
> example of do_setlink, you can see that while it would be quite easy to
> do the sanity checks before actually doing something you can't catch
> all errors if you don't want to implement the name/mtu/addr change
> routines yourself. Keeping in mind that those sanity checks will
> probably only fail during the development of a application it doesn't
> really help. That's why I implemented it this way.

sure. The hard part i think is you cant force every one wanting to make
changes to link parameters to go via setlink. If you could then it
should be doable to not make changes until after the attributes are all
known (and therefore make it easy to undo).

> I'm experimenting with a commit/fallback netlink library which basically
> fetches the subject of change and stores the netlink message to be used
> in case of fallback. NLM_F_ATOMIC makes it even more useable but is
> quite expensive. A netlink change daemon would probably solve the
> problem better.

Daemon seems to be the only way to do it. Lets discuss this at some
later point.

cheers,
jamal

Re: [PATCH 2.6 NET] Device name changing via rtnetlink

[David S. Miller]

On 12 Sep 2004 13:27:29 -0400
jamal <hadi@cyberus.ca> wrote:

> In any case, i think that this is ok for now. So patch should go
> in - we can revisit the whole atomicity where applicable in kernel at
> some later point.
> Dave please go ahead and apply the patch.

Done.