Page MenuHomeFreeBSD
Files F167359871

D16808.id47069.diff
No OneTemporary
Actions

D16808.id47069.diff
View Options

Index: sys/net/if_vlan.c
===================================================================
--- sys/net/if_vlan.c
+++ sys/net/if_vlan.c
@@ -205,29 +205,22 @@
static eventhandler_tag iflladdr_tag;
/*
- * if_vlan uses two module-level locks to allow concurrent modification of vlan
- * interfaces and (mostly) allow for vlans to be destroyed while they are being
- * used for tx/rx. To accomplish this in a way that has acceptable performance
- * and cooperation with other parts of the network stack there is a
- * non-sleepable rmlock(9) and an sx(9). Both locks are exclusively acquired
- * when destroying a vlan interface, i.e. when the if_vlantrunk field of struct
- * ifnet is de-allocated and NULL'd. Thus a reader holding either lock has a
- * guarantee that the struct ifvlantrunk references a valid vlan trunk.
+ * if_vlan uses two module-level synchronizations primitives to allow concurrent
+ * modification of vlan interfaces and (mostly) allow for vlans to be destroyed
+ * while they are being used for tx/rx. To accomplish this in a way that has
+ * acceptable performance and cooperation with other parts of the network stack
+ * there is a non-sleepable epoch(9) and an sx(9).
*
- * The performance-sensitive paths that warrant using the rmlock(9) are
+ * The performance-sensitive paths that warrant using the epoch(9) are
* vlan_transmit and vlan_input. Both have to check for the vlan interface's
* existence using if_vlantrunk, and being in the network tx/rx paths the use
- * of an rmlock(9) gives a measureable improvement in performance.
+ * of an epoch(9) gives a measureable improvement in performance.
*
* The reason for having an sx(9) is mostly because there are still areas that
* must be sleepable and also have safe concurrent access to a vlan interface.
* Since the sx(9) exists, it is used by default in most paths unless sleeping
* is not permitted, or if it is not clear whether sleeping is permitted.
*
- * Note that despite these protections, there is still an inherent race in the
- * destruction of vlans since there's no guarantee that the ifnet hasn't been
- * freed/reused when the tx/rx functions are called by the stack. This can only
- * be fixed by addressing ifnet's lifetime issues.
*/
#define _VLAN_SX_ID ifv_sx
@@ -253,11 +246,8 @@
/*
- * We also have a per-trunk rmlock(9), that is locked shared on packet
- * processing and exclusive when configuration is changed. Note: This should
- * only be acquired while there is a shared lock on either of the global locks
- * via VLAN_SLOCK or VLAN_RLOCK. Thus, an exclusive lock on the global locks
- * makes a call to TRUNK_RLOCK/TRUNK_WLOCK technically superfluous.
+ * We also have a per-trunk mutex that should be acquired when changing
+ * its state.
*/
#define TRUNK_LOCK_INIT(trunk) mtx_init(&(trunk)->lock, vlanname, NULL, MTX_DEF)
#define TRUNK_LOCK_DESTROY(trunk) mtx_destroy(&(trunk)->lock)
@@ -370,7 +360,7 @@
int i, b;
struct ifvlan *ifv2;
- TRUNK_WLOCK_ASSERT(trunk);
+ VLAN_XLOCK_ASSERT();
KASSERT(trunk->hwidth > 0, ("%s: hwidth not positive", __func__));
b = 1 << trunk->hwidth;
@@ -400,7 +390,7 @@
int i, b;
struct ifvlan *ifv2;
- TRUNK_WLOCK_ASSERT(trunk);
+ VLAN_XLOCK_ASSERT();
KASSERT(trunk->hwidth > 0, ("%s: hwidth not positive", __func__));
b = 1 << trunk->hwidth;
@@ -428,7 +418,7 @@
struct ifvlanhead *hash2;
int hwidth2, i, j, n, n2;
- TRUNK_WLOCK_ASSERT(trunk);
+ VLAN_XLOCK_ASSERT();
KASSERT(trunk->hwidth > 0, ("%s: hwidth not positive", __func__));
if (howmuch == 0) {
@@ -444,8 +434,7 @@
if (hwidth2 < VLAN_DEF_HWIDTH)
return;
- /* M_NOWAIT because we're called with trunk mutex held */
- hash2 = malloc(sizeof(struct ifvlanhead) * n2, M_VLAN, M_NOWAIT);
+ hash2 = malloc(sizeof(struct ifvlanhead) * n2, M_VLAN, M_WAITOK);
if (hash2 == NULL) {
printf("%s: out of memory -- hash size not changed\n",
__func__);
@@ -571,10 +560,6 @@
struct vlan_mc_entry *mc;
int error;
- /*
- * XXX This stupidly needs the rmlock to avoid sleeping while holding
- * the in6_multi_mtx (see in6_mc_join_locked).
- */
VLAN_XLOCK_ASSERT();
/* Find the parent. */
@@ -711,7 +696,6 @@
if (ifp->if_type != IFT_L2VLAN)
return (NULL);
- /* Not clear if callers are sleepable, so acquire the rmlock. */
VLAN_RLOCK();
ifv = ifp->if_softc;
ifp = NULL;
@@ -792,7 +776,6 @@
struct ifvlantrunk *trunk;
struct ifvlan *ifv;
- /* Not clear if callers are sleepable, so acquire the rmlock. */
VLAN_RLOCK();
trunk = ifp->if_vlantrunk;
if (trunk == NULL) {
@@ -1121,6 +1104,7 @@
* ifvlan.
*/
taskqueue_drain(taskqueue_thread, &ifv->lladdr_task);
+ NET_EPOCH_WAIT();
if_free(ifp);
free(ifv, M_VLAN);
ifc_free_unit(ifc, unit);
@@ -1341,7 +1325,6 @@
if (ifv->ifv_trunk)
return (EBUSY);
- /* Acquire rmlock after the branch so we can M_WAITOK. */
VLAN_XLOCK();
if (p->if_vlantrunk == NULL) {
trunk = malloc(sizeof(struct ifvlantrunk),
@@ -1352,19 +1335,17 @@
p->if_vlantrunk = trunk;
trunk->parent = p;
if_ref(trunk->parent);
+ TRUNK_WUNLOCK(trunk);
} else {
trunk = p->if_vlantrunk;
- TRUNK_WLOCK(trunk);
}
ifv->ifv_vid = vid; /* must set this before vlan_inshash() */
ifv->ifv_pcp = 0; /* Default: best effort delivery. */
vlan_tag_recalculate(ifv);
error = vlan_inshash(trunk, ifv);
- if (error) {
- TRUNK_WUNLOCK(trunk);
+ if (error)
goto done;
- }
ifv->ifv_proto = ETHERTYPE_VLAN;
ifv->ifv_encaplen = ETHER_VLAN_ENCAP_LEN;
ifv->ifv_mintu = ETHERMIN;
@@ -1477,12 +1458,6 @@
parent = NULL;
if (trunk != NULL) {
- /*
- * Both vlan_transmit and vlan_input rely on the trunk fields
- * being NULL to determine whether to bail, so we need to get
- * an exclusive lock here to prevent them from using bad
- * ifvlans.
- */
parent = trunk->parent;
/*
@@ -1508,18 +1483,13 @@
error);
}
CK_SLIST_REMOVE_HEAD(&ifv->vlan_mc_listhead, mc_entries);
+ NET_EPOCH_WAIT();
free(mc, M_VLAN);
}
vlan_setflags(ifp, 0); /* clear special flags on parent */
- /*
- * The trunk lock isn't actually required here, but
- * vlan_remhash expects it.
- */
- TRUNK_WLOCK(trunk);
vlan_remhash(trunk, ifv);
- TRUNK_WUNLOCK(trunk);
ifv->ifv_trunk = NULL;
/*
@@ -1635,7 +1605,7 @@
u_long hwa = 0;
VLAN_SXLOCK_ASSERT();
- TRUNK_WLOCK_ASSERT(TRUNK(ifv));
+ TRUNK_RLOCK_ASSERT(TRUNK(ifv));
p = PARENT(ifv);
ifp = ifv->ifv_ifp;
@@ -1736,11 +1706,11 @@
VLAN_SUNLOCK();
return;
}
- TRUNK_WLOCK(trunk);
+ TRUNK_RLOCK(trunk);
VLAN_FOREACH(ifv, trunk) {
vlan_capabilities(ifv);
}
- TRUNK_WUNLOCK(trunk);
+ TRUNK_RUNLOCK(trunk);
VLAN_SUNLOCK();
}
@@ -1932,9 +1902,9 @@
ifv->ifv_capenable = ifr->ifr_reqcap;
trunk = TRUNK(ifv);
if (trunk != NULL) {
- TRUNK_WLOCK(trunk);
+ TRUNK_RLOCK(trunk);
vlan_capabilities(ifv);
- TRUNK_WUNLOCK(trunk);
+ TRUNK_RUNLOCK(trunk);
}
VLAN_SUNLOCK();
break;

File Metadata

Mime Type
text/plain
Expires
Sat, Aug 22, 7:25 AM (10 m, 23 s)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
37068194
Default Alt Text
D16808.id47069.diff (6 KB)

Event Timeline