Okay, spoken too quick ... I just had an idea (enlightment you might say - 
given the time of year), that might finally get us rid of this symptom 
(not of the problem though).

Short recap on why this is happening:  Checking socket credentials on the 
IP layer (where pf lives) is a layering violation.  A useful one, you may 
argue, but nontheless - it causes a lock order reversal:  In order to 
walk the pf rules we need to hold the pf lock, in order to walk the 
socket hash we need to hold the "inp" lock.

The attached diff circumvents the problem by **always** doing the 
credential lookup *before* walking the pf rules.  This has the benefit, 
that it works (at least I think it should), but there is a price to pay.  
Now we have to pay for the socket lookup for *every* tcp and udp packet 
instead of just for those that really hit uid/gid rules.  That's why I 
decided to make is a config option "PF_MPFSAFE_UGID" which you can turn 
on if you are running a setup that will benefit.  The patch turns it on 
for the module-built by default.

A possible scenario that should benefit is a big iron SMP box running lot 
of services that you want to filter using *stateful* uid/gid rules.  For 
this setup where a huge percentage of the packets that are not captured 
by states eventually match a uid/gid rule, you will even get added 
parallelism with this patch.

On every other typical setup, it should be better to avoid user/group 
rules or to disable mpsafenet.

In order for this to hit the tree, I need tests confirming that it really 
helps and possibly benchmarks that qualify the impact of it.  Thanks.

-- 
/"\  Best regards,                      | [EMAIL PROTECTED]
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | [EMAIL PROTECTED]
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News
Index: conf/options
===================================================================
RCS file: /usr/store/mlaier/fcvs/src/sys/conf/options,v
retrieving revision 1.567
diff -u -r1.567 options
--- conf/options	10 Dec 2006 04:23:23 -0000	1.567
+++ conf/options	16 Dec 2006 15:36:08 -0000
@@ -349,6 +349,7 @@
 DEV_PF			opt_pf.h
 DEV_PFLOG		opt_pf.h
 DEV_PFSYNC		opt_pf.h
+PF_MPSAFE_UGID		opt_pf.h
 ETHER_II		opt_ef.h
 ETHER_8023		opt_ef.h
 ETHER_8022		opt_ef.h
Index: contrib/pf/net/pf.c
===================================================================
RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pf.c,v
retrieving revision 1.42
diff -u -r1.42 pf.c
--- contrib/pf/net/pf.c	22 Oct 2006 11:52:11 -0000	1.42
+++ contrib/pf/net/pf.c	16 Dec 2006 15:34:52 -0000
@@ -3032,6 +3032,12 @@
 		return (PF_DROP);
 	}
 
+#if defined(__FreeBSD__) && defined(PF_MPSAFE_UGID)
+	PF_UNLOCK();
+	lookup = pf_socket_lookup(&uid, &gid, direction, pd, inp);
+	PF_LOCK();
+#endif
+
 	r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_FILTER].active.ptr);
 
 	if (direction == PF_OUT) {
@@ -3428,6 +3434,12 @@
 		return (PF_DROP);
 	}
 
+#if defined(__FreeBSD__) && defined(PF_MPSAFE_UGID)
+	PF_UNLOCK();
+	lookup = pf_socket_lookup(&uid, &gid, direction, pd, inp);
+	PF_LOCK();
+#endif
+
 	r = TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_FILTER].active.ptr);
 
 	if (direction == PF_OUT) {
Index: modules/pf/Makefile
===================================================================
RCS file: /usr/store/mlaier/fcvs/src/sys/modules/pf/Makefile,v
retrieving revision 1.12
diff -u -r1.12 Makefile
--- modules/pf/Makefile	12 Sep 2006 04:25:12 -0000	1.12
+++ modules/pf/Makefile	16 Dec 2006 15:41:00 -0000
@@ -10,7 +10,7 @@
 	in4_cksum.c \
 	opt_pf.h opt_inet.h opt_inet6.h opt_bpf.h opt_mac.h
 
-CFLAGS+=  -I${.CURDIR}/../../contrib/pf
+CFLAGS+=  -I${.CURDIR}/../../contrib/pf -DPF_MPSAFE_UGID
 
 .if !defined(KERNBUILDDIR)
 opt_inet.h:

Attachment: pgpOlvaxWUhql.pgp
Description: PGP signature

Reply via email to