Patch: link state notification for 2.5.41

Hi,

some months ago I've sent a patch to the linux kernel list that enabled notification of network driver link state changes over the netlink socket. Technically, this has been implemented by a kernel thread that mirrored dev->state:__LINK_STATE_NO_CARRIER into dev->flags:IFF_RUNNING and sent netlink messages. Note: The in-kernel-flag IFF_RUNNING is unused at least since 2.4, it is the LINK_STATE_NO_CARRIER bit that is seen as RUNNING from user mode.

This was followed by a discussion with Jamal (look at
http://marc.theaimsgroup.com/?t=101751064400001&r=1&w=2), however, it took ...ahem.. a while until the revised version I attach to this mail. Don't ask ;-)

I had quite positive feedback from LVS and zebra people, so I tried to remove most of the objections against the patch.

-Anyway, I wanted to keep the current semantics of netif_carrier_on()/_off(). The extended states that can be found RFC2863 did not seem to useful to me for linux, mainly because "notPresent" is already implemented with netif_device_detach()/_attach() and especially for ethernet drivers there is no real difference between "testing" and "down".
-Using Ingos workqueue implementation, I removed the need for the first versions' own kernel thread.
-I changed the simple approach of iterating over all network devices, now the handler only looks at devices that had an event. This change also increases compatibility to broken drivers that manipulate IFF_RUNNING directly, there is a good chance that these drivers will never generate state notification events. I kept the semantic not to create a list of events for one device, only the latest state change is forwarded when the queue is run.

Patch against 2.5.41 is attached for comments, of course with the aim of kernel inclusion. I have tested it on a UP machine, but hope that I've got the locking stuff right.

Cheers, Stefan

diff -uNrX dontdiff linux-2.5.41/include/linux/netdevice.h linux-2.5.41-stefan/include/linux/netdevice.h
@@ -207,7 +207,8 @@
 	__LINK_STATE_PRESENT,
 	__LINK_STATE_SCHED,
 	__LINK_STATE_NOCARRIER,
-	__LINK_STATE_RX_SCHED
+	__LINK_STATE_RX_SCHED,
+	__LINK_STATE_LINKWATCH_PENDING
 };
 
 

@@ -437,6 +438,9 @@
 	/* this will get initialized at each interface type init routine */
 	struct divert_blk	*divert;
 #endif /* CONFIG_NET_DIVERT */
+#ifdef CONFIG_LINKWATCH
+	struct net_device       *linkwatch_next;
+#endif
 };
 
 

@@ -480,6 +484,13 @@
 extern struct net_device	*__dev_get_by_index(int ifindex);
 extern int		dev_restart(struct net_device *dev);
 
+/* This gets called by netif_carrier_on()/_off() whenever
+ * state of an interface changes
+ */
+#ifdef CONFIG_LINKWATCH
+extern void linkwatch_fire_event(struct net_device *dev);
+#endif
+
 typedef int gifconf_func_t(struct net_device * dev, char * bufptr, int len);
 extern int		register_gifconf(unsigned int family, gifconf_func_t * gifconf);
 static inline int unregister_gifconf(unsigned int family)

@@ -640,14 +651,24 @@
 
 static inline void netif_carrier_on(struct net_device *dev)
 {
+#ifdef CONFIG_LINKWATCH
+	if (test_and_clear_bit(__LINK_STATE_NOCARRIER, &dev->state))
+		linkwatch_fire_event(dev);
+#else
 	clear_bit(__LINK_STATE_NOCARRIER, &dev->state);
+#endif
 	if (netif_running(dev))
 		__netdev_watchdog_up(dev);
 }
 
 static inline void netif_carrier_off(struct net_device *dev)
 {
+#ifdef CONFIG_LINKWATCH
+	if (!test_and_set_bit(__LINK_STATE_NOCARRIER, &dev->state))
+		linkwatch_fire_event(dev);
+#else
 	set_bit(__LINK_STATE_NOCARRIER, &dev->state);
+#endif
 }
 
 /* Hot-plugging. */

diff -uNrX dontdiff linux-2.5.41/net/Config.help linux-2.5.41-stefan/net/Config.help
@@ -472,6 +472,17 @@
   However, do not say Y here if you did not experience any serious
   problems.
 
+CONFIG_LINKWATCH
+  When this option is enabled, the kernel will forward changes in the
+  operative ("RUNNING") state of an interface via the netlink socket.
+  This is most useful when running linux as a router.
+
+  Note that currently not many drivers support this, compliant ones
+  can be found by watching the the RUNNING flag in ifconfig output
+  that should follow operative state.
+
+  If unsure, say 'N'.
+
 CONFIG_NET_SCHED
   When the kernel has several packets to send out over a network
   device, it has to decide which ones to send first, which ones to

diff -uNrX dontdiff linux-2.5.41/net/Config.in linux-2.5.41-stefan/net/Config.in
@@ -82,6 +82,7 @@
    tristate 'WAN router' CONFIG_WAN_ROUTER
    bool 'Fast switching (read help!)' CONFIG_NET_FASTROUTE
    bool 'Forwarding between high speed interfaces' CONFIG_NET_HW_FLOWCONTROL
+   bool 'Device link state notification (EXPERIMENTAL)' CONFIG_LINKWATCH
 fi
 
 mainmenu_option next_comment

diff -uNrX dontdiff linux-2.5.41/net/core/Makefile linux-2.5.41-stefan/net/core/Makefile
@@ -21,4 +21,6 @@
 # Ugly. I wish all wireless drivers were moved in drivers/net/wireless
 obj-$(CONFIG_NET_PCMCIA_RADIO) += wireless.o
 
+obj-$(CONFIG_LINKWATCH) += link_watch.o
+
 include $(TOPDIR)/Rules.make

diff -uNrX dontdiff linux-2.5.41/net/core/dev.c linux-2.5.41-stefan/net/core/dev.c
@@ -198,6 +198,9 @@
 int netdev_fastroute_obstacles;
 #endif
 
+#ifdef CONFIG_LINKWATCH
+void linkwatch_run_queue(void);
+#endif
 
 /*******************************************************************************
 

@@ -716,6 +719,13 @@
 		 *	Set the flags.
 		 */
 		dev->flags |= IFF_UP;
+#ifdef CONFIG_LINKWATCH
+		if (netif_carrier_ok(dev)) {
+			dev->flags |= IFF_RUNNING;
+		} else {
+			dev->flags &= ~IFF_RUNNING;
+		}
+#endif
 
 		/*
 		 *	Initialize multicasting status

@@ -2590,6 +2600,18 @@
 
 #ifdef CONFIG_NET_DIVERT
 	free_divert_blk(dev);
+#endif
+
+#ifdef CONFIG_LINKWATCH
+	if (test_bit(__LINK_STATE_LINKWATCH_PENDING, &dev->state)) {
+		/* We must not have linkwatch events pending
+		 * on unregister. If this happens, we simply
+		 * run the queue unscheduled, resulting in a
+		 * noop for this device
+		 */
+		linkwatch_run_queue();
+		BUG_ON(test_bit(__LINK_STATE_LINKWATCH_PENDING, &dev->state));
+	}
 #endif
 
 	if (dev->features & NETIF_F_DYNALLOC) {

diff -uNrX dontdiff linux-2.5.41/net/netsyms.c linux-2.5.41-stefan/net/netsyms.c
@@ -596,4 +596,8 @@
 EXPORT_SYMBOL(wireless_send_event);
 #endif	/* CONFIG_NET_RADIO || CONFIG_NET_PCMCIA_RADIO */
 
+#ifdef CONFIG_LINKWATCH
+EXPORT_SYMBOL(linkwatch_fire_event);
+#endif
+
 #endif  /* CONFIG_NET */

You didnt do the most important thing we discussed:
A global list of devices with work - not to pollute the net_device structure. This way it is more manageable. Imagine 10K devices (with software devices on top of physical devices this is not unusual). Devices register work to be done on this queue. If they register multiple times, then only the latest event if different from whats already on the queue gets propagated via netlink (this part you do correctly).

BTW, did you ping the USB people on notPresent? At least one of them i think participated in that discussion.

cheers,
jamal

[ CCs trimmed ]

That was likely me. :)

The "link active" issue for hotplug is perhaps just that while userspace sees /sbin/hotplug notifications when network interfaces are registered or unregistered, the right time to autoconfigure is actually when the link connects to a net ... or reconnects to a different one. So we really want (hotplug) events other than link register/unregister. (And some systems will want the events through a daemon instead.)

So long as there's a clean way to generate such events, and to use them to trigger autoconfiguration, my basic concern is addressed. Delivery issues should be a lot easier to resolve. I think I mentioned the notPresent bit purely as an RFC conformance issue, since I was trying to make sense of what the kernel code was expected/intended to do.

- Dave