On Mon, Aug 26, 2013 at 13:36 +0200, Mike Belopuhov wrote:
> hi,
>
> 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.
>
> i did some testing here, but wouldn't mind if someone
> tries this diff in gre/vlan/ospf/anything-weird setups.
> making sure that hot-plugging/unplugging usb interfaces
> doesn't produce any undesirable effects would be superb
> as well.
>
> 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?
>
i've got an ok from mpi, anyone else would like to test
and/or comment?
> 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
