if_link_state_change() patch for review

[Gleb Smirnoff]

  Dear networkers,

we are working on fixing LORs in if_link_state_change() path, and
adding possibility to call if_link_state_change() pseudorecursively,
when link of interface depends on link of the other.

I'm posting this patch for wider review. An important point about it
is that, if several link events occur VERY quickly, only the last one
will be processed. I don't know of any software that will be broken by
such behavoir. If you know some, please tell me.

Thanks.

-- 
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
-------------- next part --------------
Index: if.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if.c,v
retrieving revision 1.226
diff -u -r1.226 if.c
--- if.c	15 Apr 2005 01:51:26 -0000	1.226
+++ if.c	18 Apr 2005 20:07:45 -0000
@@ -112,6 +112,7 @@
 static int	if_rtdel(struct radix_node *, void *);
 static int	ifhwioctl(u_long, struct ifnet *, caddr_t, struct thread *);
 static void	if_start_deferred(void *context, int pending);
+static void	do_link_state_change(void *, int);
 #ifdef INET6
 /*
  * XXX: declare here to avoid to include many inet6 related files..
@@ -385,6 +386,7 @@
 	struct ifaddr *ifa;
 
 	TASK_INIT(&ifp->if_starttask, 0, if_start_deferred, ifp);
+	TASK_INIT(&ifp->if_linktask, 0, do_link_state_change, ifp);
 	IF_AFDATA_LOCK_INIT(ifp);
 	ifp->if_afdata_initialized = 0;
 	IFNET_WLOCK();
@@ -542,6 +544,11 @@
  	struct ifnet *iter;
  	int found;
 
+	/*
+	 * Remove/wait for pending events.
+	 */
+	taskqueue_drain(taskqueue_swi, &ifp->if_linktask);
+
 	EVENTHANDLER_INVOKE(ifnet_departure_event, ifp);
 #ifdef DEV_CARP
 	/* Maybe hook to the generalized departure handler above?!? */
@@ -988,19 +995,30 @@
 void	(*vlan_link_state_p)(struct ifnet *, int);	/* XXX: private from if_vlan */
 
 /*
- * Handle a change in the interface link state.
+ * Handle a change in the interface link state. To avoid LORs
+ * between driver lock and upper layer locks, as well as possible
+ * recursions, we post event to taskqueue, and all job
+ * is done in static do_link_state_change().
  */
 void
 if_link_state_change(struct ifnet *ifp, int link_state)
 {
-	int link;
-
 	/* Return if state hasn't changed. */
 	if (ifp->if_link_state == link_state)
 		return;
 
 	ifp->if_link_state = link_state;
 
+	taskqueue_enqueue(taskqueue_swi, &ifp->if_linktask);
+}
+
+static void
+do_link_state_change(void *arg, int pending)
+{
+	struct ifnet *ifp = (struct ifnet *)arg;
+	int link_state = ifp->if_link_state;
+	int link;
+
 	/* Notify that the link state has changed. */
 	rt_ifmsg(ifp);
 	if (link_state == LINK_STATE_UP)
@@ -1020,6 +1038,8 @@
 	if (ifp->if_carp)
 		carp_carpdev_state(ifp->if_carp);
 #endif
+	if (pending > 1)
+		if_printf(ifp, "%d link states coalesced\n", pending);
 	if (log_link_state_change)
 		log(LOG_NOTICE, "%s: link state changed to %s\n", ifp->if_xname,
 		    (link_state == LINK_STATE_UP) ? "UP" : "DOWN" );
Index: if_var.h
===================================================================
RCS file: /home/ncvs/src/sys/net/if_var.h,v
retrieving revision 1.94
diff -u -r1.94 if_var.h
--- if_var.h	1 Mar 2005 10:59:14 -0000	1.94
+++ if_var.h	18 Apr 2005 17:49:38 -0000
@@ -194,6 +194,7 @@
 	int	if_afdata_initialized;
 	struct	mtx if_afdata_mtx;
 	struct	task if_starttask;	/* task for IFF_NEEDSGIANT */
+	struct	task if_linktask;	/* task for link change events */
 };
 
 typedef void if_init_f_t(void *);