From 0839aa5c04d5621c42577421c978c044d73d6af8 Mon Sep 17 00:00:00 2001 From: Gleb Smirnoff Date: Tue, 29 Oct 2019 17:36:06 +0000 Subject: [PATCH] There is a long standing problem with multicast programming for NICs and IPv6. With IPv6 we may call if_addmulti() in context of processing of an incoming packet. Usually this is interrupt context. While most of the NIC drivers are able to reprogram multicast filters without sleeping, some of them can't. An example is e1000 family of drivers. With iflib conversion the problem was somewhat hidden. Iflib processes packets in private taskqueue, so going to sleep doesn't trigger an assertion. However, the sleep would block operation of the driver and following incoming packets would fill the ring and eventually would start being dropped. Enabling epoch for the full time of a packet processing again started to trigger assertions for e1000. Fix this problem once and for all using a general taskqueue to call if_ioctl() method in all cases when if_addmulti() is called in a non sleeping context. Note that nobody cares about returned value. Reviewed by: hselasky, kib Differential Revision: https://reviews.freebsd.org/D22154 --- sys/net/if.c | 21 ++++++++++++++++++++- sys/net/if_var.h | 1 + 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/sys/net/if.c b/sys/net/if.c index 31bd54b5805d..507b758f9be6 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -271,6 +271,7 @@ static int if_getgroupmembers(struct ifgroupreq *); static void if_delgroups(struct ifnet *); static void if_attach_internal(struct ifnet *, int, struct if_clone *); static int if_detach_internal(struct ifnet *, int, struct if_clone **); +static void if_siocaddmulti(void *, int); #ifdef VIMAGE static void if_vmove(struct ifnet *, struct vnet *); #endif @@ -556,6 +557,7 @@ if_alloc_domain(u_char type, int numa_domain) IF_ADDR_LOCK_INIT(ifp); TASK_INIT(&ifp->if_linktask, 0, do_link_state_change, ifp); + TASK_INIT(&ifp->if_addmultitask, 0, if_siocaddmulti, ifp); ifp->if_afdata_initialized = 0; IF_AFDATA_LOCK_INIT(ifp); CK_STAILQ_INIT(&ifp->if_addrhead); @@ -1131,6 +1133,7 @@ if_detach_internal(struct ifnet *ifp, int vmove, struct if_clone **ifcp) if_delgroups(ifp); taskqueue_drain(taskqueue_swi, &ifp->if_linktask); + taskqueue_drain(taskqueue_swi, &ifp->if_addmultitask); /* * Check if this is a cloned interface or not. Must do even if @@ -3538,7 +3541,10 @@ if_addmulti(struct ifnet *ifp, struct sockaddr *sa, * interface to let them know about it. */ if (ifp->if_ioctl != NULL) { - (void) (*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0); + if (THREAD_CAN_SLEEP()) + (void )(*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0); + else + taskqueue_enqueue(taskqueue_swi, &ifp->if_addmultitask); } if ((llsa != NULL) && (llsa != (struct sockaddr *)&sdl)) @@ -3555,6 +3561,19 @@ if_addmulti(struct ifnet *ifp, struct sockaddr *sa, return (error); } +static void +if_siocaddmulti(void *arg, int pending) +{ + struct ifnet *ifp; + + ifp = arg; +#ifdef DIAGNOSTIC + if (pending > 1) + if_printf(ifp, "%d SIOCADDMULTI coalesced\n", pending); +#endif + (void )(*ifp->if_ioctl)(ifp, SIOCADDMULTI, 0); +} + /* * Delete a multicast group membership by network-layer group address. * diff --git a/sys/net/if_var.h b/sys/net/if_var.h index 1a78765a8c29..3b869346eb17 100644 --- a/sys/net/if_var.h +++ b/sys/net/if_var.h @@ -317,6 +317,7 @@ struct ifnet { struct ifaltq if_snd; /* output queue (includes altq) */ struct task if_linktask; /* task for link change events */ + struct task if_addmultitask; /* task for SIOCADDMULTI */ /* Addresses of different protocol families assigned to this if. */ struct mtx if_addr_lock; /* lock to protect address lists */