Resolve the confusion between the head_list and the hook list.

The linked list of pfil hooks is changed to "chain" and this term
is applied consistently.  The head_list remains with "list" term.

Add KASSERT to vnet_pfil_uninit().

Update and extend comments.

Reviewed by:	eri (previous version)

2 files changed, 48 insertions, 35 deletions

diff --git a/sys/net/pfil.c b/sys/net/pfil.c
index 51c22a1..45d8f4b 100644
--- a/sys/net/pfil.c
+++ b/sys/net/pfil.c
@@ -52,9 +52,9 @@ static struct mtx pfil_global_lock;
 MTX_SYSINIT(pfil_heads_lock, &pfil_global_lock, "pfil_head_list lock",
   MTX_DEF);
 
-static struct packet_filter_hook *pfil_hook_get(int, struct pfil_head *);
-static int pfil_list_add(pfil_list_t *, struct packet_filter_hook *, int);
-static int pfil_list_remove(pfil_list_t *, pfil_func_t, void *);
+static struct packet_filter_hook *pfil_chain_get(int, struct pfil_head *);
+static int pfil_chain_add(pfil_chain_t *, struct packet_filter_hook *, int);
+static int pfil_chain_remove(pfil_chain_t *, pfil_func_t, void *);
 
 LIST_HEAD(pfilheadhead, pfil_head);
 VNET_DEFINE(struct pfilheadhead, pfil_head_list);
@@ -63,7 +63,7 @@ VNET_DEFINE(struct rmlock, pfil_lock);
 #define	V_pfil_lock	VNET(pfil_lock)
 
 /*
- * pfil_run_hooks() runs the specified packet filter hooks.
+ * pfil_run_hooks() runs the specified packet filter hook chain.
  */
 int
 pfil_run_hooks(struct pfil_head *ph, struct mbuf **mp, struct ifnet *ifp,
@@ -76,8 +76,8 @@ pfil_run_hooks(struct pfil_head *ph, struct mbuf **mp, struct ifnet *ifp,
 
 	PFIL_RLOCK(ph, &rmpt);
 	KASSERT(ph->ph_nhooks >= 0, ("Pfil hook count dropped < 0"));
-	for (pfh = pfil_hook_get(dir, ph); pfh != NULL;
-	     pfh = TAILQ_NEXT(pfh, pfil_link)) {
+	for (pfh = pfil_chain_get(dir, ph); pfh != NULL;
+	     pfh = TAILQ_NEXT(pfh, pfil_chain)) {
 		if (pfh->pfil_func != NULL) {
 			rv = (*pfh->pfil_func)(pfh->pfil_arg, &m, ifp, dir,
 			    inp);
@@ -91,7 +91,7 @@ pfil_run_hooks(struct pfil_head *ph, struct mbuf **mp, struct ifnet *ifp,
 }
 
 static struct packet_filter_hook *
-pfil_hook_get(int dir, struct pfil_head *ph)
+pfil_chain_get(int dir, struct pfil_head *ph)
 {
 
 	if (dir == PFIL_IN)
@@ -163,6 +163,7 @@ pfil_wowned(struct pfil_head *ph)
 
 	return (PFIL_WOWNED(ph));
 }
+
 /*
  * pfil_head_register() registers a pfil_head with the packet filter hook
  * mechanism.
@@ -202,9 +203,9 @@ pfil_head_unregister(struct pfil_head *ph)
 	PFIL_LIST_LOCK();
 	LIST_REMOVE(ph, ph_list);
 	PFIL_LIST_UNLOCK();
-	TAILQ_FOREACH_SAFE(pfh, &ph->ph_in, pfil_link, pfnext)
+	TAILQ_FOREACH_SAFE(pfh, &ph->ph_in, pfil_chain, pfnext)
 		free(pfh, M_IFADDR);
-	TAILQ_FOREACH_SAFE(pfh, &ph->ph_out, pfil_link, pfnext)
+	TAILQ_FOREACH_SAFE(pfh, &ph->ph_out, pfil_chain, pfnext)
 		free(pfh, M_IFADDR);
 	PFIL_LOCK_DESTROY(ph);
 	return (0);
@@ -261,7 +262,7 @@ pfil_add_hook(pfil_func_t func, void *arg, int flags, struct pfil_head *ph)
 	if (flags & PFIL_IN) {
 		pfh1->pfil_func = func;
 		pfh1->pfil_arg = arg;
-		err = pfil_list_add(&ph->ph_in, pfh1, flags & ~PFIL_OUT);
+		err = pfil_chain_add(&ph->ph_in, pfh1, flags & ~PFIL_OUT);
 		if (err)
 			goto locked_error;
 		ph->ph_nhooks++;
@@ -269,10 +270,10 @@ pfil_add_hook(pfil_func_t func, void *arg, int flags, struct pfil_head *ph)
 	if (flags & PFIL_OUT) {
 		pfh2->pfil_func = func;
 		pfh2->pfil_arg = arg;
-		err = pfil_list_add(&ph->ph_out, pfh2, flags & ~PFIL_IN);
+		err = pfil_chain_add(&ph->ph_out, pfh2, flags & ~PFIL_IN);
 		if (err) {
 			if (flags & PFIL_IN)
-				pfil_list_remove(&ph->ph_in, func, arg);
+				pfil_chain_remove(&ph->ph_in, func, arg);
 			goto locked_error;
 		}
 		ph->ph_nhooks++;
@@ -291,7 +292,7 @@ error:
 
 /*
  * pfil_remove_hook removes a specific function from the packet filter hook
- * list.
+ * chain.
  */
 int
 pfil_remove_hook(pfil_func_t func, void *arg, int flags, struct pfil_head *ph)
@@ -300,12 +301,12 @@ pfil_remove_hook(pfil_func_t func, void *arg, int flags, struct pfil_head *ph)
 
 	PFIL_WLOCK(ph);
 	if (flags & PFIL_IN) {
-		err = pfil_list_remove(&ph->ph_in, func, arg);
+		err = pfil_chain_remove(&ph->ph_in, func, arg);
 		if (err == 0)
 			ph->ph_nhooks--;
 	}
 	if ((err == 0) && (flags & PFIL_OUT)) {
-		err = pfil_list_remove(&ph->ph_out, func, arg);
+		err = pfil_chain_remove(&ph->ph_out, func, arg);
 		if (err == 0)
 			ph->ph_nhooks--;
 	}
@@ -313,15 +314,18 @@ pfil_remove_hook(pfil_func_t func, void *arg, int flags, struct pfil_head *ph)
 	return (err);
 }
 
+/*
+ * Internal: Add a new pfil hook into a hook chain.
+ */
 static int
-pfil_list_add(pfil_list_t *list, struct packet_filter_hook *pfh1, int flags)
+pfil_chain_add(pfil_chain_t *chain, struct packet_filter_hook *pfh1, int flags)
 {
 	struct packet_filter_hook *pfh;
 
 	/*
 	 * First make sure the hook is not already there.
 	 */
-	TAILQ_FOREACH(pfh, list, pfil_link)
+	TAILQ_FOREACH(pfh, chain, pfil_chain)
 		if (pfh->pfil_func == pfh1->pfil_func &&
 		    pfh->pfil_arg == pfh1->pfil_arg)
 			return (EEXIST);
@@ -331,24 +335,23 @@ pfil_list_add(pfil_list_t *list, struct packet_filter_hook *pfh1, int flags)
 	 * the same path is followed in or out of the kernel.
 	 */
 	if (flags & PFIL_IN)
-		TAILQ_INSERT_HEAD(list, pfh1, pfil_link);
+		TAILQ_INSERT_HEAD(chain, pfh1, pfil_chain);
 	else
-		TAILQ_INSERT_TAIL(list, pfh1, pfil_link);
+		TAILQ_INSERT_TAIL(chain, pfh1, pfil_chain);
 	return (0);
 }
 
 /*
- * pfil_list_remove is an internal function that takes a function off the
- * specified list.
+ * Internal: Remove a pfil hook from a hook chain.
  */
 static int
-pfil_list_remove(pfil_list_t *list, pfil_func_t func, void *arg)
+pfil_chain_remove(pfil_chain_t *chain, pfil_func_t func, void *arg)
 {
 	struct packet_filter_hook *pfh;
 
-	TAILQ_FOREACH(pfh, list, pfil_link)
+	TAILQ_FOREACH(pfh, chain, pfil_chain)
 		if (pfh->pfil_func == func && pfh->pfil_arg == arg) {
-			TAILQ_REMOVE(list, pfh, pfil_link);
+			TAILQ_REMOVE(chain, pfh, pfil_chain);
 			free(pfh, M_IFADDR);
 			return (0);
 		}
@@ -375,7 +378,8 @@ static int
 vnet_pfil_uninit(const void *unused)
 {
 
-	/*  XXX should panic if list is not empty */
+	KASSERT(LIST_EMPTY(&V_pfil_head_list),
+	    ("%s: pfil_head_list %p not empty", __func__, &V_pfil_head_list));
 	PFIL_LOCK_DESTROY_REAL(&V_pfil_lock);
 	return (0);
 }
diff --git a/sys/net/pfil.h b/sys/net/pfil.h
index 5c763dc..6e4ccce6 100644
--- a/sys/net/pfil.h
+++ b/sys/net/pfil.h
@@ -48,10 +48,11 @@ typedef	int	(*pfil_func_t)(void *, struct mbuf **, struct ifnet *, int,
 
 /*
  * The packet filter hooks are designed for anything to call them to
- * possibly intercept the packet.
+ * possibly intercept the packet.  Multiple filter hooks are chained
+ * together and after each other in the specified order.
  */
 struct packet_filter_hook {
-        TAILQ_ENTRY(packet_filter_hook) pfil_link;
+        TAILQ_ENTRY(packet_filter_hook) pfil_chain;
 	pfil_func_t	pfil_func;
 	void	*pfil_arg;
 };
@@ -61,16 +62,20 @@ struct packet_filter_hook {
 #define PFIL_WAITOK	0x00000004
 #define PFIL_ALL	(PFIL_IN|PFIL_OUT)
 
-typedef	TAILQ_HEAD(pfil_list, packet_filter_hook) pfil_list_t;
+typedef	TAILQ_HEAD(pfil_chain, packet_filter_hook) pfil_chain_t;
 
 #define	PFIL_TYPE_AF		1	/* key is AF_* type */
 #define	PFIL_TYPE_IFNET		2	/* key is ifnet pointer */
 
 #define	PFIL_FLAG_PRIVATE_LOCK	0x01	/* Personal lock instead of global */
 
+/*
+ * A pfil head is created by each protocol or packet intercept point.
+ * For packet is then run through the hook chain for inspection.
+ */
 struct pfil_head {
-	pfil_list_t	ph_in;
-	pfil_list_t	ph_out;
+	pfil_chain_t	ph_in;
+	pfil_chain_t	ph_out;
 	int		ph_type;
 	int		ph_nhooks;
 #if defined( __linux__ ) || defined( _WIN32 )
@@ -89,11 +94,20 @@ struct pfil_head {
 	LIST_ENTRY(pfil_head) ph_list;
 };
 
+/* Public functions for pfil hook management by packet filters. */
+struct pfil_head *pfil_head_get(int, u_long);
 int	pfil_add_hook(pfil_func_t, void *, int, struct pfil_head *);
 int	pfil_remove_hook(pfil_func_t, void *, int, struct pfil_head *);
+
+/* Public functions to run the packet inspection by protocols. */
 int	pfil_run_hooks(struct pfil_head *, struct mbuf **, struct ifnet *,
 	    int, struct inpcb *inp);
 
+/* Public functions for pfil head management by protocols. */
+int	pfil_head_register(struct pfil_head *);
+int	pfil_head_unregister(struct pfil_head *);
+
+/* Internal pfil locking functions. */
 struct rm_priotracker;	/* Do not require including rmlock header */
 int pfil_try_rlock(struct pfil_head *, struct rm_priotracker *);
 void pfil_rlock(struct pfil_head *, struct rm_priotracker *);
@@ -102,11 +116,6 @@ void pfil_wlock(struct pfil_head *);
 void pfil_wunlock(struct pfil_head *);
 int pfil_wowned(struct pfil_head *ph);
 
-int	pfil_head_register(struct pfil_head *);
-int	pfil_head_unregister(struct pfil_head *);
-
-struct pfil_head *pfil_head_get(int, u_long);
-
 #define	PFIL_HOOKED(p) ((p)->ph_nhooks > 0)
 #define	PFIL_LOCK_INIT_REAL(l, t)	\
 	rm_init_flags(l, "PFil " t " rmlock", RM_RECURSE)