Date: Tue, 19 Apr 2005 10:47:47 +0400 From: Gleb Smirnoff <glebius@FreeBSD.org> To: net@FreeBSD.org Subject: if_link_state_change() patch for review Message-ID: <20050419064747.GC734@cell.sick.ru>
next in thread | raw e-mail | index | archive | help
--qDbXVdCdHGoSgWSk
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
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
--qDbXVdCdHGoSgWSk
Content-Type: text/plain; charset=koi8-r
Content-Disposition: attachment; filename="if.link_state.2"
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 *);
--qDbXVdCdHGoSgWSk--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050419064747.GC734>