--- a/sys/netgraph/eiface/ng_eiface.c +++ b/sys/netgraph/eiface/ng_eiface.c @@ -41,6 +41,8 @@ #include #include #include +#include +#include #include #include @@ -84,6 +86,9 @@ node_p node; /* Our netgraph node */ hook_p ether; /* Hook for ethernet stream */ struct private *next; /* When hung on the free list */ + int attach_state; /* async ether_ifattach state */ + int down_pending; /* shutdown seen while attaching */ + struct spinlock attach_spin; /* protects attach_state/down_pending */ }; typedef struct ng_eiface_private *priv_p; @@ -307,6 +312,55 @@ ************************************************************************/ /* + * Deferred interface attachment. + * + * Netgraph node constructors run in the control-message path, i.e. on a + * TD_TYPE_NETISR thread (the socket protocol thread). if_attach() may + * not run in netisr context: it takes the ifnet lock, which asserts + * against netisr, and it calls netmsg_service_sync(). So the + * ether_ifattach() is executed from a short-lived kernel thread. + */ +#define EIFACE_ATTACH_PENDING 0 /* thread not started yet */ +#define EIFACE_ATTACH_RUNNING 1 /* thread owns the ifnet */ +#define EIFACE_ATTACH_DONE 2 /* ifnet attached */ +#define EIFACE_ATTACH_CANCELLED 3 /* node died before attach */ + +static void +ng_eiface_attach_thread(void *xpriv) +{ + priv_p priv = xpriv; + int attach, down; + + spin_lock(&priv->attach_spin); + attach = (priv->attach_state == EIFACE_ATTACH_PENDING); + if (attach) + priv->attach_state = EIFACE_ATTACH_RUNNING; + spin_unlock(&priv->attach_spin); + + if (attach) + ether_ifattach(priv->ifp, priv->arpcom.ac_enaddr, NULL); + + spin_lock(&priv->attach_spin); + down = attach && priv->down_pending; + if (attach) + priv->attach_state = EIFACE_ATTACH_DONE; + spin_unlock(&priv->attach_spin); + + /* + * If the node was shut down while we were attaching, bring the + * interface down here; ng_eiface_rmnode() could not do it safely + * at that point. + */ + if (down) { + ifnet_serialize_all(priv->ifp); + priv->ifp->if_flags &= ~(IFF_UP | IFF_RUNNING); + ifq_clr_oactive(&priv->ifp->if_snd); + ifnet_deserialize_all(priv->ifp); + } + lwkt_exit(); +} + +/* * Constructor for a node */ static int @@ -352,8 +406,16 @@ ng_name_node(node, ifname); */ - /* Attach the interface */ - ether_ifattach(ifp, priv->arpcom.ac_enaddr, NULL); + /* Attach the interface (deferred; see ng_eiface_attach_thread) */ + spin_init(&priv->attach_spin, "ng_eiface_attach"); + priv->attach_state = EIFACE_ATTACH_PENDING; + if (kthread_create(ng_eiface_attach_thread, priv, NULL, + "ng_eiface_attach") != 0) { + node->private = NULL; + ng_unref(node); + kfree(priv, M_NETGRAPH); + return (ENOMEM); + } /* Done */ return (0); @@ -539,13 +601,31 @@ { const priv_p priv = node->private; struct ifnet *const ifp = priv->ifp; + int down_now; ng_cutlinks(node); node->flags &= ~NG_INVALID; - ifnet_serialize_all(ifp); - ifp->if_flags &= ~(IFF_UP | IFF_RUNNING); - ifq_clr_oactive(&ifp->if_snd); - ifnet_deserialize_all(ifp); + + spin_lock(&priv->attach_spin); + if (priv->attach_state == EIFACE_ATTACH_PENDING) { + /* attach thread has not touched the ifnet yet */ + priv->attach_state = EIFACE_ATTACH_CANCELLED; + down_now = 0; + } else if (priv->attach_state == EIFACE_ATTACH_RUNNING) { + /* defer the interface shutdown to the attach thread */ + priv->down_pending = 1; + down_now = 0; + } else { + down_now = 1; + } + spin_unlock(&priv->attach_spin); + + if (down_now) { + ifnet_serialize_all(ifp); + ifp->if_flags &= ~(IFF_UP | IFF_RUNNING); + ifq_clr_oactive(&ifp->if_snd); + ifnet_deserialize_all(ifp); + } return (0); } --- a/sys/netgraph/iface/ng_iface.c +++ b/sys/netgraph/iface/ng_iface.c @@ -65,6 +65,8 @@ #include #include #include +#include +#include #include #include @@ -102,6 +104,8 @@ int unit; /* Interface unit number */ node_p node; /* Our netgraph node */ hook_p hooks[NUM_FAMILIES]; /* Hook for each address family */ + int attach_state; /* async if_attach state */ + struct spinlock attach_spin; /* protects attach_state */ }; typedef struct ng_iface_private *priv_p; @@ -536,6 +540,77 @@ ************************************************************************/ /* + * Deferred interface attachment/detachment. + * + * Netgraph node constructors and shutdown methods run in the + * control-message path, i.e. on a TD_TYPE_NETISR thread (the socket + * protocol thread). if_attach()/if_detach() may not run in netisr + * context: they take the ifnet lock, which asserts against netisr, + * and if_attach() calls netmsg_service_sync(). So the ifnet side of + * the node lifecycle is executed from short-lived kernel threads. + * + * The attach_state machine transfers ownership of the ifnet (and, on + * teardown, of priv itself) atomically between the control path and + * the helper threads. + */ +#define IFACE_ATTACH_PENDING 0 /* thread not started yet */ +#define IFACE_ATTACH_RUNNING 1 /* attach thread owns the ifnet */ +#define IFACE_ATTACH_DONE 2 /* ifnet attached; rmnode owns it */ +#define IFACE_ATTACH_CANCELLED 3 /* rmnode owns it; never attached */ +#define IFACE_ATTACH_DYING 4 /* attach thread owns it + teardown */ +#define IFACE_ATTACH_DETACHING 5 /* detach thread owns it + teardown */ + +static void ng_iface_attach_thread(void *); +static void ng_iface_detach_thread(void *); +static void ng_iface_teardown(priv_p priv); + +static void +ng_iface_attach_thread(void *xpriv) +{ + priv_p priv = xpriv; + int attach; + + spin_lock(&priv->attach_spin); + attach = (priv->attach_state == IFACE_ATTACH_PENDING); + if (attach) + priv->attach_state = IFACE_ATTACH_RUNNING; + spin_unlock(&priv->attach_spin); + + if (attach) { + if_attach(priv->ifp, NULL); + bpfattach(priv->ifp, DLT_NULL, sizeof(uint32_t)); + + spin_lock(&priv->attach_spin); + if (priv->attach_state != IFACE_ATTACH_DYING) + priv->attach_state = IFACE_ATTACH_DONE; + attach = (priv->attach_state == IFACE_ATTACH_DYING); + spin_unlock(&priv->attach_spin); + + if (attach) + ng_iface_teardown(priv); + } + lwkt_exit(); +} + +static void +ng_iface_detach_thread(void *xpriv) +{ + ng_iface_teardown((priv_p)xpriv); + lwkt_exit(); +} + +static void +ng_iface_teardown(priv_p priv) +{ + bpfdetach(priv->ifp); + if_detach(priv->ifp); + kfree(priv->ifp, M_NETGRAPH); + priv->ifp = NULL; + ng_iface_free_unit(priv->unit); + kfree(priv, M_NETGRAPH); +} + +/* * Constructor for a node */ static int @@ -601,9 +676,20 @@ if (ng_name_node(node, ifname) != 0) log(LOG_WARNING, "%s: can't acquire netgraph name\n", ifname); - /* Attach the interface */ - if_attach(ifp, NULL); - bpfattach(ifp, DLT_NULL, sizeof(uint32_t)); + /* Attach the interface (deferred; see ng_iface_attach_thread) */ + spin_init(&priv->attach_spin, "ng_iface_attach"); + priv->attach_state = IFACE_ATTACH_PENDING; + error = kthread_create(ng_iface_attach_thread, priv, NULL, + "ng_iface_attach"); + if (error) { + ng_unname(node); + node->private = NULL; + ng_unref(node); + ng_iface_free_unit(priv->unit); + kfree(ifp, M_NETGRAPH); + kfree(priv, M_NETGRAPH); + return (error); + } /* Done */ return (0); @@ -794,15 +880,47 @@ ng_iface_rmnode(node_p node) { const priv_p priv = node->private; + int own_ifp; ng_cutlinks(node); ng_unname(node); - bpfdetach(priv->ifp); - if_detach(priv->ifp); - kfree(priv->ifp, M_NETGRAPH); - priv->ifp = NULL; - ng_iface_free_unit(priv->unit); - kfree(priv, M_NETGRAPH); + + spin_lock(&priv->attach_spin); + switch (priv->attach_state) { + case IFACE_ATTACH_PENDING: + /* never attached: inline cleanup is safe (no ifnet ops) */ + priv->attach_state = IFACE_ATTACH_CANCELLED; + own_ifp = 1; + break; + case IFACE_ATTACH_RUNNING: + /* attach thread owns it and will do the teardown */ + priv->attach_state = IFACE_ATTACH_DYING; + own_ifp = 0; + break; + case IFACE_ATTACH_DONE: + /* + * Attached: if_detach() must not run from the netisr + * control path; hand the teardown to a kernel thread. + */ + priv->attach_state = IFACE_ATTACH_DETACHING; + if (kthread_create(ng_iface_detach_thread, priv, NULL, + "ng_iface_detach") == 0) + own_ifp = 0; + else + own_ifp = 1; /* leak rather than panic */ + break; + default: + own_ifp = 0; + break; + } + spin_unlock(&priv->attach_spin); + + if (own_ifp) { + kfree(priv->ifp, M_NETGRAPH); + priv->ifp = NULL; + ng_iface_free_unit(priv->unit); + kfree(priv, M_NETGRAPH); + } node->private = NULL; ng_unref(node); return (0);