defer routing table updates on link state changes (again)

2013-10-19 Thread Mike Belopuhov 
hi,

since mpi's if_index diff is now in, this should probably
go in as well.  it has received some testing in the meantime.

original description:

in order to make our life a bit easier and prevent rogue
accesses to the routing table from the hardware interrupt
context violating all kinds of spl assumptions we would
like if_link_state_change that is called by network device
drivers in their interrupt service routines to defer its
work to the process context or thereabouts.

please note that a token (an interface index) is passed
to the workq in order to make sure that if the interface
would be gone by the time syswq goes around to run the
task it would just fall through.

ok?

diff --git sys/net/if.c sys/net/if.c
index 6dafd0d..5b6800a 100644
--- sys/net/if.c
+++ sys/net/if.c
@@ -79,10 +79,11 @@
 #include sys/protosw.h
 #include sys/kernel.h
 #include sys/ioctl.h
 #include sys/domain.h
 #include sys/sysctl.h
+#include sys/workq.h
 
 #include net/if.h
 #include net/if_dl.h
 #include net/if_media.h
 #include net/if_types.h
@@ -151,10 +152,12 @@ int   if_clone_list(struct if_clonereq *);
 struct if_clone*if_clone_lookup(const char *, int *);
 
 void   if_congestion_clear(void *);
 intif_group_egress_build(void);
 
+void   if_link_state_change_task(void *, void *);
+
 intifai_cmp(struct ifaddr_item *,  struct ifaddr_item *);
 void   ifa_item_insert(struct sockaddr *, struct ifaddr *, struct ifnet *);
 void   ifa_item_remove(struct sockaddr *, struct ifaddr *, struct ifnet *);
 #ifndef SMALL_KERNEL
 void   ifa_print_rb(void);
@@ -1106,21 +1109,39 @@ if_up(struct ifnet *ifp)
 
m_clinitifp(ifp);
 }
 
 /*
- * Process a link state change.
- * NOTE: must be called at splsoftnet or equivalent.
+ * Schedule a link state change task.
  */
 void
 if_link_state_change(struct ifnet *ifp)
 {
-   rt_ifmsg(ifp);
+   /* try to put the routing table update task on syswq */
+   workq_add_task(NULL, 0, if_link_state_change_task,
+   (void *)((unsigned long)ifp-if_index), NULL);
+}
+
+/*
+ * Process a link state change.
+ */
+void
+if_link_state_change_task(void *arg, void *unused)
+{
+   unsigned int index = (unsigned long)arg;
+   struct ifnet *ifp;
+   int s;
+
+   s = splsoftnet();
+   if ((ifp = if_get(index)) != NULL) {
+   rt_ifmsg(ifp);
 #ifndef SMALL_KERNEL
-   rt_if_track(ifp);
+   rt_if_track(ifp);
 #endif
-   dohooks(ifp-if_linkstatehooks, 0);
+   dohooks(ifp-if_linkstatehooks, 0);
+   }
+   splx(s);
 }
 
 /*
  * Handle interface watchdog timer routines.  Called
  * from softclock, we decrement timers (if set) and

Re: defer routing table updates on link state changes (again)

2013-10-19 Thread Claudio Jeker 
On Sat, Oct 19, 2013 at 01:26:39PM +0200, Mike Belopuhov wrote:
 hi,
 
 since mpi's if_index diff is now in, this should probably
 go in as well.  it has received some testing in the meantime.
 
 original description:
 
 in order to make our life a bit easier and prevent rogue
 accesses to the routing table from the hardware interrupt
 context violating all kinds of spl assumptions we would
 like if_link_state_change that is called by network device
 drivers in their interrupt service routines to defer its
 work to the process context or thereabouts.
 
 please note that a token (an interface index) is passed
 to the workq in order to make sure that if the interface
 would be gone by the time syswq goes around to run the
 task it would just fall through.
 
 ok?

OK claudio@
 
 diff --git sys/net/if.c sys/net/if.c
 index 6dafd0d..5b6800a 100644
 --- sys/net/if.c
 +++ sys/net/if.c
 @@ -79,10 +79,11 @@
  #include sys/protosw.h
  #include sys/kernel.h
  #include sys/ioctl.h
  #include sys/domain.h
  #include sys/sysctl.h
 +#include sys/workq.h
  
  #include net/if.h
  #include net/if_dl.h
  #include net/if_media.h
  #include net/if_types.h
 @@ -151,10 +152,12 @@ int if_clone_list(struct if_clonereq *);
  struct if_clone  *if_clone_lookup(const char *, int *);
  
  void if_congestion_clear(void *);
  int  if_group_egress_build(void);
  
 +void if_link_state_change_task(void *, void *);
 +
  int  ifai_cmp(struct ifaddr_item *,  struct ifaddr_item *);
  void ifa_item_insert(struct sockaddr *, struct ifaddr *, struct ifnet *);
  void ifa_item_remove(struct sockaddr *, struct ifaddr *, struct ifnet *);
  #ifndef SMALL_KERNEL
  void ifa_print_rb(void);
 @@ -1106,21 +1109,39 @@ if_up(struct ifnet *ifp)
  
   m_clinitifp(ifp);
  }
  
  /*
 - * Process a link state change.
 - * NOTE: must be called at splsoftnet or equivalent.
 + * Schedule a link state change task.
   */
  void
  if_link_state_change(struct ifnet *ifp)
  {
 - rt_ifmsg(ifp);
 + /* try to put the routing table update task on syswq */
 + workq_add_task(NULL, 0, if_link_state_change_task,
 + (void *)((unsigned long)ifp-if_index), NULL);
 +}
 +
 +/*
 + * Process a link state change.
 + */
 +void
 +if_link_state_change_task(void *arg, void *unused)
 +{
 + unsigned int index = (unsigned long)arg;
 + struct ifnet *ifp;
 + int s;
 +
 + s = splsoftnet();
 + if ((ifp = if_get(index)) != NULL) {
 + rt_ifmsg(ifp);
  #ifndef SMALL_KERNEL
 - rt_if_track(ifp);
 + rt_if_track(ifp);
  #endif
 - dohooks(ifp-if_linkstatehooks, 0);
 + dohooks(ifp-if_linkstatehooks, 0);
 + }
 + splx(s);
  }
  
  /*
   * Handle interface watchdog timer routines.  Called
   * from softclock, we decrement timers (if set) and
 

-- 
:wq Claudio