Date: Sat, 16 Dec 2006 17:09:42 +0100 From: Max Laier <max@love2party.net> To: freebsd-pf@freebsd.org Cc: avatar@mmlab.cse.yzu.edu.tw, csjp@freebsd.org Subject: debug.mpsafenet=1 vs. user/group rules [Re: kern/106805: ...] Message-ID: <200612161709.48875.max@love2party.net> In-Reply-To: <200612161335.kBGDZkMj012022@freefall.freebsd.org> References: <200612161335.kBGDZkMj012022@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--nextPart2247468.qizqeL3thi Content-Type: multipart/mixed; boundary="Boundary-01=_IpBhF/ciB5YZr+C" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_IpBhF/ciB5YZr+C Content-Type: text/plain; charset="iso-8859-6" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Okay, spoken too quick ... I just had an idea (enlightment you might say -= =20 given the time of year), that might finally get us rid of this symptom=20 (not of the problem though). Short recap on why this is happening: Checking socket credentials on the=20 IP layer (where pf lives) is a layering violation. A useful one, you may=20 argue, but nontheless - it causes a lock order reversal: In order to=20 walk the pf rules we need to hold the pf lock, in order to walk the=20 socket hash we need to hold the "inp" lock. The attached diff circumvents the problem by **always** doing the=20 credential lookup *before* walking the pf rules. This has the benefit,=20 that it works (at least I think it should), but there is a price to pay. =20 Now we have to pay for the socket lookup for *every* tcp and udp packet=20 instead of just for those that really hit uid/gid rules. That's why I=20 decided to make is a config option "PF_MPFSAFE_UGID" which you can turn=20 on if you are running a setup that will benefit. The patch turns it on=20 for the module-built by default. A possible scenario that should benefit is a big iron SMP box running lot=20 of services that you want to filter using *stateful* uid/gid rules. For=20 this setup where a huge percentage of the packets that are not captured=20 by states eventually match a uid/gid rule, you will even get added=20 parallelism with this patch. On every other typical setup, it should be better to avoid user/group=20 rules or to disable mpsafenet. In order for this to hit the tree, I need tests confirming that it really=20 helps and possibly benchmarks that qualify the impact of it. Thanks. =2D-=20 /"\ Best regards, | mlaier@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mlaier@EFnet / \ ASCII Ribbon Campaign | Against HTML Mail and News --Boundary-01=_IpBhF/ciB5YZr+C Content-Type: text/x-diff; charset="iso-8859-6"; name="pf.ugid.take42.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="pf.ugid.take42.diff" Index: conf/options =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/conf/options,v retrieving revision 1.567 diff -u -r1.567 options =2D-- 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 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pf.c,v retrieving revision 1.42 diff -u -r1.42 pf.c =2D-- 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); } =20 +#if defined(__FreeBSD__) && defined(PF_MPSAFE_UGID) + PF_UNLOCK(); + lookup =3D pf_socket_lookup(&uid, &gid, direction, pd, inp); + PF_LOCK(); +#endif + r =3D TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_FILTER].active.ptr); =20 if (direction =3D=3D PF_OUT) { @@ -3428,6 +3434,12 @@ return (PF_DROP); } =20 +#if defined(__FreeBSD__) && defined(PF_MPSAFE_UGID) + PF_UNLOCK(); + lookup =3D pf_socket_lookup(&uid, &gid, direction, pd, inp); + PF_LOCK(); +#endif + r =3D TAILQ_FIRST(pf_main_ruleset.rules[PF_RULESET_FILTER].active.ptr); =20 if (direction =3D=3D PF_OUT) { Index: modules/pf/Makefile =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/modules/pf/Makefile,v retrieving revision 1.12 diff -u -r1.12 Makefile =2D-- 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 =20 =2DCFLAGS+=3D -I${.CURDIR}/../../contrib/pf +CFLAGS+=3D -I${.CURDIR}/../../contrib/pf -DPF_MPSAFE_UGID =20 .if !defined(KERNBUILDDIR) opt_inet.h: --Boundary-01=_IpBhF/ciB5YZr+C-- --nextPart2247468.qizqeL3thi Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (FreeBSD) iD8DBQBFhBpMXyyEoT62BG0RAivGAJ9hRgwyXIJtXsmpiI5t+Z94l5+qaACfTqsP DLlzi0gXLtJQsIi7CWbpiuQ= =y678 -----END PGP SIGNATURE----- --nextPart2247468.qizqeL3thi--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200612161709.48875.max>