[PATCH 1/1] ipv6: corrects sended rtnetlink message

[PATCH 1/1] ipv6: corrects sended rtnetlink message

[Milan Kocian]

ipv6 sends a RTM_DELLINK netlink message on both events: NETDEV_DOWN,
NETDEV_UNREGISTER. Corrected by sending RTM_NEWLINK on NETDEV_DOWN event
and RTM_DELLINK on NETDEV_UNREGISTER event.
---

btw. I don't know why protocol sends a NL messages about iface state
changes. It comes from other sources. By contrast ipv6 doesn't send
no message on NETDEV_UP event !? IMO best possibility is deletion of
sending this message.


--- a/net/ipv6/addrconf.c	2007-08-13 11:35:23.481430029 +0200
+++ b/net/ipv6/addrconf.c	2007-08-13 12:47:01.825690662 +0200
@@ -2488,7 +2488,10 @@ static int addrconf_ifdown(struct net_de
 
 	/* Step 5: netlink notification of this interface */
 	idev->tstamp = jiffies;
-	inet6_ifinfo_notify(RTM_DELLINK, idev);
+	if (how)
+		inet6_ifinfo_notify(RTM_DELLINK, idev);
+	else 
+		inet6_ifinfo_notify(RTM_NEWLINK, idev);
 
 	/* Shot the device (if unregistered) */
 

Signed-off-by: Milan Kocian <milon@wq.cz>

Re: [PATCH 1/1] ipv6: corrects sended rtnetlink message

[David Miller]

From: Milan Kocian <milon@wq.cz>
Date: Wed, 15 Aug 2007 16:33:22 +0200

> ipv6 sends a RTM_DELLINK netlink message on both events: NETDEV_DOWN,
> NETDEV_UNREGISTER. Corrected by sending RTM_NEWLINK on NETDEV_DOWN event
> and RTM_DELLINK on NETDEV_UNREGISTER event.

Why would we indicate that a new device has appeared on NETDEV_DOWN?

I don't see any sense in saying "RTM_NEWLINK" for a removal, it's
for additions.

Re: [PATCH 1/1] ipv6: corrects sended rtnetlink message

[Milan Kocian]

On Tue, Aug 21, 2007 at 12:20:58AM -0700, David Miller wrote:
> From: Milan Kocian <milon@wq.cz>
> Date: Wed, 15 Aug 2007 16:33:22 +0200
> 
> > ipv6 sends a RTM_DELLINK netlink message on both events: NETDEV_DOWN,
> > NETDEV_UNREGISTER. Corrected by sending RTM_NEWLINK on NETDEV_DOWN event
> > and RTM_DELLINK on NETDEV_UNREGISTER event.
> 
> Why would we indicate that a new device has appeared on NETDEV_DOWN?
> 
> I don't see any sense in saying "RTM_NEWLINK" for a removal, it's
> for additions.
> 
Sorry for my late reply. I was out.

Because RTM_NEWLINK is used to notify about device status change
(as I see in net/core/rtnetlink.c) and RTM_DELLINK to inform about
NETDEV_UNREGISTER. Why should it be else in ipv6 subsystem ? And
userspace programs (quagga) suppose it. Now userspace get two rtnetlink's
'LINK' messages on 'ip l s down' event. First is RTM_NEWLINK from 
net/core/rtnetlink.c and second is RTM_DELLINK from ipv6.

quagga story:
On NEWLINK (flag IFF_UP is down) message quagga flushes all routes from RIB
but leaves ip adresses. On DELLINK message q. flushes routes and addresses.
On 'ip l s up' event ipv6 sends addresses and routes again but ipv4 not
(it sends only routes). Thus q. stays without knowledge about ipv4's addresses
after 'ip l s down/up' commands.

git change:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=979ad663125af4be120697263038bb06ddbb83b4

So from this point of view I tried to synchronize types of messages
on the same events.

IMHO second possibility is to remove rtnetlink notification about
NETDEV_DOWN/_UNREGISTER from ipv6 subsystem because it is duplicate message.

regards,

milon

Re: [PATCH 1/1] ipv6: corrects sended rtnetlink message

[Thomas Graf]

* Milan Kocian <milon@wq.cz> 2007-08-29 23:51
> Because RTM_NEWLINK is used to notify about device status change
> (as I see in net/core/rtnetlink.c) and RTM_DELLINK to inform about
> NETDEV_UNREGISTER. Why should it be else in ipv6 subsystem ? And
> userspace programs (quagga) suppose it. Now userspace get two rtnetlink's
> 'LINK' messages on 'ip l s down' event. First is RTM_NEWLINK from 
> net/core/rtnetlink.c and second is RTM_DELLINK from ipv6.

The logic behind is quite simple, we notify via RTM_NEWLINK whenever
a device changes any of its attributes.

> quagga story:
> On NEWLINK (flag IFF_UP is down) message quagga flushes all routes from RIB
> but leaves ip adresses. On DELLINK message q. flushes routes and addresses.
> On 'ip l s up' event ipv6 sends addresses and routes again but ipv4 not
> (it sends only routes). Thus q. stays without knowledge about ipv4's addresses
> after 'ip l s down/up' commands.
> 
> git change:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=979ad663125af4be120697263038bb06ddbb83b4

It is arguable whether this change is correct, the purpose is clearly
to notify that IPv6 is no longer available on this interface without
implying anything beyond that.

Fortunately, all unregister events have the ifi_change field set to ~0U
whereas the "IPv6 disabled" notification sets the ifi_chagne field to
0 making it trivial to differ between the two cases. So quagga can
simply ignore (RTM_DELLINK && !ifi_change) notifications.

> IMHO second possibility is to remove rtnetlink notification about
> NETDEV_DOWN/_UNREGISTER from ipv6 subsystem because it is duplicate message.

Please make sure nobody is relying on this notification, IPv6 includes
more information in link messages, someone may be relying on this. If
not, feel free to remove the notification.

Re: [PATCH 1/1] ipv6: corrects sended rtnetlink message

[Milan Kocian]

On Thu, Sep 06, 2007 at 12:47:11PM +0200, Thomas Graf wrote:
> * Milan Kocian <milon@wq.cz> 2007-08-29 23:51
> > Because RTM_NEWLINK is used to notify about device status change
> > (as I see in net/core/rtnetlink.c) and RTM_DELLINK to inform about
> > NETDEV_UNREGISTER. Why should it be else in ipv6 subsystem ? And
> > userspace programs (quagga) suppose it. Now userspace get two rtnetlink's
> > 'LINK' messages on 'ip l s down' event. First is RTM_NEWLINK from 
> > net/core/rtnetlink.c and second is RTM_DELLINK from ipv6.
> 
> The logic behind is quite simple, we notify via RTM_NEWLINK whenever
> a device changes any of its attributes.
> 
I agree but ipv6 sends on device change (NETDEV_DOWN) RTM_DELLINK message.
BTW when ipv6 send LINK message on NETDEV_UNREGISTER event, why doesn't 
send message on NETDEV_REGISTER event? No symmetry ?

> > quagga story:
> > On NEWLINK (flag IFF_UP is down) message quagga flushes all routes from RIB
> > but leaves ip adresses. On DELLINK message q. flushes routes and addresses.
> > On 'ip l s up' event ipv6 sends addresses and routes again but ipv4 not
> > (it sends only routes). Thus q. stays without knowledge about ipv4's addresses
> > after 'ip l s down/up' commands.
> > 
> > git change:
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=979ad663125af4be120697263038bb06ddbb83b4
> 
> It is arguable whether this change is correct, the purpose is clearly
> to notify that IPv6 is no longer available on this interface without
> implying anything beyond that.

ok. However, if I understand, LINK messages handle device changes not
protocol changes. Or not ?

> 
> Fortunately, all unregister events have the ifi_change field set to ~0U
> whereas the "IPv6 disabled" notification sets the ifi_chagne field to
> 0 making it trivial to differ between the two cases. So quagga can
> simply ignore (RTM_DELLINK && !ifi_change) notifications.

Now I ignore (RTM_DELLINK && ifi_family==AF_INET6) :-). But it's only workaround
till next change. 

> 
> > IMHO second possibility is to remove rtnetlink notification about
> > NETDEV_DOWN/_UNREGISTER from ipv6 subsystem because it is duplicate message.
> 
> Please make sure nobody is relying on this notification, IPv6 includes
> more information in link messages, someone may be relying on this. If
> not, feel free to remove the notification.
> 
Hard to find it. I can try to look at other routing sw (probably most using it)
about handling with RTM_DELLINK.
Howerer when was made change from RTM_NEWLINK to RTM_DELLINK without protests,
we can try remove message :-)

Regards,

-- 
Milan Kocian

Re: [PATCH 1/1] ipv6: corrects sended rtnetlink message

[Thomas Graf]

* Milan Kocian <milon@wq.cz> 2007-09-06 23:05
> I agree but ipv6 sends on device change (NETDEV_DOWN) RTM_DELLINK message.
> BTW when ipv6 send LINK message on NETDEV_UNREGISTER event, why doesn't 
> send message on NETDEV_REGISTER event? No symmetry ?

You should be seeing two RTM_DELLINK upon NETDEV_UNREGISTER if the
interface carried any IPv6 addresess. Once with ifi_change=~0
notifying you that the device is disappearing and once with
ifi_change=0 coming from the IPv6 protocol shutdown.

> ok. However, if I understand, LINK messages handle device changes not
> protocol changes. Or not ?

Yes, I personally think this behaviour is wrong but we can't remove it
unless we are sure it doesn't break anything.

> Now I ignore (RTM_DELLINK && ifi_family==AF_INET6) :-). But it's only workaround
> till next change. 

That's also correct, pure netdevice notification will always be sent
with the ifi_family set to AF_UNSPEC.

> Hard to find it. I can try to look at other routing sw (probably most using it)
> about handling with RTM_DELLINK.
> Howerer when was made change from RTM_NEWLINK to RTM_DELLINK without protests,
> we can try remove message :-)

I'd give the USAGI software tree a short peek, if there was a specific
reason for adding this notification in the first place, the software is
most probably found in that tree.

Re: [PATCH 1/1] ipv6: corrects sended rtnetlink message

[Milan Kocian]

> > Now I ignore (RTM_DELLINK && ifi_family==AF_INET6) :-). But it's only workaround
> > till next change. 
> 
> That's also correct, pure netdevice notification will always be sent
> with the ifi_family set to AF_UNSPEC.
> 
It's essential property, I missed. Thanks.

> > Hard to find it. I can try to look at other routing sw (probably most using it)
> > about handling with RTM_DELLINK.
> > Howerer when was made change from RTM_NEWLINK to RTM_DELLINK without protests,
> > we can try remove message :-)
> 
> I'd give the USAGI software tree a short peek, if there was a specific
> reason for adding this notification in the first place, the software is
> most probably found in that tree.

However I still think that this notitfication is redundant. I tried to look
at XORP, bird, USAGI , quagga and to see RTM_DELLINK handling. And imho
nobody depends on RTM_DELLINK message from ipv6.

regards,

-- 
Milan Kocian

Re: [PATCH 1/1] ipv6: corrects sended rtnetlink message

[Thomas Graf]

* Milan Kocian <milon@wq.cz> 2007-09-12 16:50
> However I still think that this notitfication is redundant. I tried to look
> at XORP, bird, USAGI , quagga and to see RTM_DELLINK handling. And imho
> nobody depends on RTM_DELLINK message from ipv6.

Send a patch to remove and we'll see if anyone complains.

[PATCH 1/1] ipv6: remove redundant RTM_DELLINK message

[Milan Kocian]

remove useless message. We get right message from other subsystem.
---

--- a/net/ipv6/addrconf.c	2007-09-13 11:22:31.087494976 +0200
+++ b/net/ipv6/addrconf.c	2007-09-13 11:25:56.056225711 +0200
@@ -2486,9 +2486,7 @@ static int addrconf_ifdown(struct net_de
 	else
 		ipv6_mc_down(idev);
 
-	/* Step 5: netlink notification of this interface */
 	idev->tstamp = jiffies;
-	inet6_ifinfo_notify(RTM_DELLINK, idev);
 
 	/* Shot the device (if unregistered) */
 
Signed-off-by: Milan Kocian <milon@wq.cz>

Re: [PATCH 1/1] ipv6: remove redundant RTM_DELLINK message

[David Miller]

From: Milan Kocian <milon@wq.cz>
Date: Thu, 13 Sep 2007 11:57:38 +0200

> remove useless message. We get right message from other subsystem.
 ...
> Signed-off-by: Milan Kocian <milon@wq.cz>

Applied, thanks Milan.