perry@piermont.com ("Perry E. Metzger") writes:

>net/if.h is defined by SuS and has a defined namespace, so it should
>probably NOT be exposing all the symbols in pfil.h in userland
>code. Should all that stuff in pfil.h be protected by #ifdef _KERNEL?

I would guess so, I haven't found anything in pfil.h that is used
in userland.

I would also suggest to get rid of the inline function.

The offending __inline is used exactly once, in net/pfil.c. Moving
that code into pfil_run_hooks() and dropping the pfil_hook_get
function doesn't break anything. It also avoids exposing the
internal hook pointers that are otherwise abstracted in the
pfil_add_hook/pfil_remove_hook/pfil_run_hooks interface.

Here is a patch for netbsd3 that does this.

Index: pfil.h
===================================================================
RCS file: /cvsroot/src/sys/net/pfil.h,v
retrieving revision 1.24
diff -u -r1.24 pfil.h
--- pfil.h	27 Jul 2004 12:22:59 -0000	1.24
+++ pfil.h	4 Jan 2006 12:50:33 -0000
@@ -98,22 +98,6 @@
 
 struct pfil_head *pfil_head_get(int, u_long);
 
-static __inline struct packet_filter_hook *
-pfil_hook_get(int dir, struct pfil_head *ph)
-{
-
-	if (dir == PFIL_IN)
-		return (TAILQ_FIRST(&ph->ph_in));
-	else if (dir == PFIL_OUT)
-		return (TAILQ_FIRST(&ph->ph_out));
-	else if (dir == PFIL_IFADDR)
-		return (TAILQ_FIRST(&ph->ph_ifaddr));
-	else if (dir == PFIL_IFNET)
-		return (TAILQ_FIRST(&ph->ph_ifnetevent));
-	else
-		return (NULL);
-}
-
 /* XXX */
 #if defined(_KERNEL_OPT)
 #include "ipfilter.h"
Index: pfil.c
===================================================================
RCS file: /cvsroot/src/sys/net/pfil.c,v
retrieving revision 1.23
diff -u -r1.23 pfil.c
--- pfil.c	27 Jul 2004 12:22:59 -0000	1.23
+++ pfil.c	4 Jan 2006 12:50:33 -0000
@@ -60,12 +60,30 @@
     int dir)
 {
 	struct packet_filter_hook *pfh;
+	pfil_list_t *pfl;
 	struct mbuf *m = NULL;
 	int rv = 0;
 
+	switch (dir) {
+	case PFIL_IN:	
+		pfl = &ph->ph_in;
+		break;
+	case PFIL_OUT:	
+		pfl = &ph->ph_out;
+		break;
+	case PFIL_IFADDR:	
+		pfl = &ph->ph_ifaddr;
+		break;
+	case PFIL_IFNET:	
+		pfl = &ph->ph_ifnetevent;
+		break;
+	default:
+		return (rv);
+	}
+
 	if ((dir & PFIL_ALL) && mp)
 		m = *mp;
-	for (pfh = pfil_hook_get(dir, ph); pfh != NULL;
+	for (pfh = TAILQ_FIRST(pfl);pfh != NULL;
 	     pfh = TAILQ_NEXT(pfh, pfil_link)) {
 		if (pfh->pfil_func != NULL) {
 			if (pfh->pfil_flags & PFIL_ALL) {