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
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 *);
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to