Reformulate use of linked lists in 'struct bpf_d' and 'struct bpf_if'

to use queue(3) list macros rather than hand-crafted lists.  While
here, move to doubly linked lists to eliminate iterating lists in
order to remove entries.  This change simplifies and clarifies the
list logic in the BPF descriptor code as a first step towards revising
the locking strategy.

RELENG_5 candidate.

Reviewed by:	fenner

2 files changed, 24 insertions, 38 deletions

diff --git a/sys/net/bpf.c b/sys/net/bpf.c
index 5877b73..856abbb 100644
--- a/sys/net/bpf.c
+++ b/sys/net/bpf.c
@@ -91,7 +91,7 @@ SYSCTL_INT(_debug, OID_AUTO, bpf_maxbufsize, CTLFLAG_RW,
 /*
  *  bpf_iflist is the list of interfaces; each corresponds to an ifnet
  */
-static struct bpf_if	*bpf_iflist;
+static LIST_HEAD(, bpf_if)	bpf_iflist;
 static struct mtx	bpf_mtx;		/* bpf global lock */
 
 static int	bpf_allocbufs(struct bpf_d *);
@@ -256,8 +256,7 @@ bpf_attachd(d, bp)
 	 */
 	BPFIF_LOCK(bp);
 	d->bd_bif = bp;
-	d->bd_next = bp->bif_dlist;
-	bp->bif_dlist = d;
+	LIST_INSERT_HEAD(&bp->bif_dlist, d, bd_next);
 
 	*bp->bif_driverp = bp;
 	BPFIF_UNLOCK(bp);
@@ -271,7 +270,6 @@ bpf_detachd(d)
 	struct bpf_d *d;
 {
 	int error;
-	struct bpf_d **p;
 	struct bpf_if *bp;
 
 	/* XXX locking */
@@ -297,14 +295,8 @@ bpf_detachd(d)
 	}
 	/* Remove d from the interface's descriptor list. */
 	BPFIF_LOCK(bp);
-	p = &bp->bif_dlist;
-	while (*p != d) {
-		p = &(*p)->bd_next;
-		if (*p == NULL)
-			panic("bpf_detachd: descriptor not in list");
-	}
-	*p = (*p)->bd_next;
-	if (bp->bif_dlist == NULL)
+	LIST_REMOVE(d, bd_next);
+	if (LIST_EMPTY(&bp->bif_dlist))
 		/*
 		 * Let the driver know that there are no more listeners.
 		 */
@@ -993,7 +985,7 @@ bpf_setif(d, ifr)
 	 * Look through attached interfaces for the named one.
 	 */
 	mtx_lock(&bpf_mtx);
-	for (bp = bpf_iflist; bp != NULL; bp = bp->bif_next) {
+	LIST_FOREACH(bp, &bpf_iflist, bif_next) {
 		struct ifnet *ifp = bp->bif_ifp;
 
 		if (ifp == NULL || ifp != theywant)
@@ -1149,11 +1141,11 @@ bpf_tap(bp, pkt, pktlen)
 	 * Lockless read to avoid cost of locking the interface if there are
 	 * no descriptors attached.
 	 */
-	if (bp->bif_dlist == NULL)
+	if (LIST_EMPTY(&bp->bif_dlist))
 		return;
 
 	BPFIF_LOCK(bp);
-	for (d = bp->bif_dlist; d != NULL; d = d->bd_next) {
+	LIST_FOREACH(d, &bp->bif_dlist, bd_next) {
 		BPFD_LOCK(d);
 		++d->bd_rcount;
 		slen = bpf_filter(d->bd_filter, pkt, pktlen, pktlen);
@@ -1210,7 +1202,7 @@ bpf_mtap(bp, m)
 	 * Lockless read to avoid cost of locking the interface if there are
 	 * no descriptors attached.
 	 */
-	if (bp->bif_dlist == NULL)
+	if (LIST_EMPTY(&bp->bif_dlist))
 		return;
 
 	pktlen = m_length(m, NULL);
@@ -1220,7 +1212,7 @@ bpf_mtap(bp, m)
 	}
 
 	BPFIF_LOCK(bp);
-	for (d = bp->bif_dlist; d != NULL; d = d->bd_next) {
+	LIST_FOREACH(d, &bp->bif_dlist, bd_next) {
 		if (!d->bd_seesent && (m->m_pkthdr.rcvif == NULL))
 			continue;
 		BPFD_LOCK(d);
@@ -1256,7 +1248,7 @@ bpf_mtap2(bp, data, dlen, m)
 	 * Lockless read to avoid cost of locking the interface if there are
 	 * no descriptors attached.
 	 */
-	if (bp->bif_dlist == NULL)
+	if (LIST_EMPTY(&bp->bif_dlist))
 		return;
 
 	pktlen = m_length(m, NULL);
@@ -1271,7 +1263,7 @@ bpf_mtap2(bp, data, dlen, m)
 	pktlen += dlen;
 
 	BPFIF_LOCK(bp);
-	for (d = bp->bif_dlist; d != NULL; d = d->bd_next) {
+	LIST_FOREACH(d, &bp->bif_dlist, bd_next) {
 		if (!d->bd_seesent && (m->m_pkthdr.rcvif == NULL))
 			continue;
 		BPFD_LOCK(d);
@@ -1436,15 +1428,14 @@ bpfattach2(ifp, dlt, hdrlen, driverp)
 	if (bp == NULL)
 		panic("bpfattach");
 
-	bp->bif_dlist = NULL;
+	LIST_INIT(&bp->bif_dlist);
 	bp->bif_driverp = driverp;
 	bp->bif_ifp = ifp;
 	bp->bif_dlt = dlt;
 	mtx_init(&bp->bif_mtx, "bpf interface lock", NULL, MTX_DEF);
 
 	mtx_lock(&bpf_mtx);
-	bp->bif_next = bpf_iflist;
-	bpf_iflist = bp;
+	LIST_INSERT_HEAD(&bpf_iflist, bp, bif_next);
 	mtx_unlock(&bpf_mtx);
 
 	*bp->bif_driverp = NULL;
@@ -1471,17 +1462,14 @@ void
 bpfdetach(ifp)
 	struct ifnet *ifp;
 {
-	struct bpf_if	*bp, *bp_prev;
+	struct bpf_if	*bp;
 	struct bpf_d	*d;
 
 	/* Locate BPF interface information */
-	bp_prev = NULL;
-
 	mtx_lock(&bpf_mtx);
-	for (bp = bpf_iflist; bp != NULL; bp = bp->bif_next) {
+	LIST_FOREACH(bp, &bpf_iflist, bif_next) {
 		if (ifp == bp->bif_ifp)
 			break;
-		bp_prev = bp;
 	}
 
 	/* Interface wasn't attached */
@@ -1491,14 +1479,10 @@ bpfdetach(ifp)
 		return;
 	}
 
-	if (bp_prev) {
-		bp_prev->bif_next = bp->bif_next;
-	} else {
-		bpf_iflist = bp->bif_next;
-	}
+	LIST_REMOVE(bp, bif_next);
 	mtx_unlock(&bpf_mtx);
 
-	while ((d = bp->bif_dlist) != NULL) {
+	while ((d = LIST_FIRST(&bp->bif_dlist)) != NULL) {
 		bpf_detachd(d);
 		BPFD_LOCK(d);
 		bpf_wakeup(d);
@@ -1525,7 +1509,7 @@ bpf_getdltlist(d, bfl)
 	n = 0;
 	error = 0;
 	mtx_lock(&bpf_mtx);
-	for (bp = bpf_iflist; bp != NULL; bp = bp->bif_next) {
+	LIST_FOREACH(bp, &bpf_iflist, bif_next) {
 		if (bp->bif_ifp != ifp)
 			continue;
 		if (bfl->bfl_list != NULL) {
@@ -1559,7 +1543,7 @@ bpf_setdlt(d, dlt)
 		return (0);
 	ifp = d->bd_bif->bif_ifp;
 	mtx_lock(&bpf_mtx);
-	for (bp = bpf_iflist; bp != NULL; bp = bp->bif_next) {
+	LIST_FOREACH(bp, &bpf_iflist, bif_next) {
 		if (bp->bif_ifp == ifp && bp->bif_dlt == dlt)
 			break;
 	}
@@ -1613,6 +1597,7 @@ bpf_drvinit(unused)
 {
 
 	mtx_init(&bpf_mtx, "bpf global lock", NULL, MTX_DEF);
+	LIST_INIT(&bpf_iflist);
 	EVENTHANDLER_REGISTER(dev_clone, bpf_clone, 0, 1000);
 }
 
diff --git a/sys/net/bpfdesc.h b/sys/net/bpfdesc.h
index ef03b70..3561927 100644
--- a/sys/net/bpfdesc.h
+++ b/sys/net/bpfdesc.h
@@ -41,12 +41,13 @@
 
 #include <sys/callout.h>
 #include <sys/selinfo.h>
+#include <sys/queue.h>
 
 /*
  * Descriptor associated with each open bpf file.
  */
 struct bpf_d {
-	struct bpf_d	*bd_next;	/* Linked list of descriptors */
+	LIST_ENTRY(bpf_d) bd_next;	/* Linked list of descriptors */
 	/*
 	 * Buffer slots: two mbuf clusters buffer the incoming packets.
 	 *   The model has three slots.  Sbuf is always occupied.
@@ -113,8 +114,8 @@ struct bpf_d {
  * Descriptor associated with each attached hardware interface.
  */
 struct bpf_if {
-	struct bpf_if *bif_next;	/* list of all interfaces */
-	struct bpf_d *bif_dlist;	/* descriptor list */
+	LIST_ENTRY(bpf_if)	bif_next;	/* list of all interfaces */
+	LIST_HEAD(, bpf_d)	bif_dlist;	/* descriptor list */
 	struct bpf_if **bif_driverp;	/* pointer into softc */
 	u_int bif_dlt;			/* link layer type */
 	u_int bif_hdrlen;		/* length of header (with padding) */