From owner-cvs-all@FreeBSD.ORG Tue Oct 5 23:37:11 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from green.homeunix.org (freefall.freebsd.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 15E0416A4CE; Tue, 5 Oct 2004 23:37:11 +0000 (GMT) Received: from green.homeunix.org (green@localhost [127.0.0.1]) by green.homeunix.org (8.13.1/8.13.1) with ESMTP id i95Nb8A4056688; Tue, 5 Oct 2004 19:37:08 -0400 (EDT) (envelope-from green@green.homeunix.org) Received: (from green@localhost) by green.homeunix.org (8.13.1/8.13.1/Submit) id i95Nb48O056687; Tue, 5 Oct 2004 19:37:04 -0400 (EDT) (envelope-from green) Date: Tue, 5 Oct 2004 19:37:04 -0400 From: Brian Fundakowski Feldman To: Max Laier Message-ID: <20041005233704.GF47017@green.homeunix.org> References: <200410052044.i95KiOVV072560@repoman.freebsd.org> <20041005212704.GB47017@green.homeunix.org> <200410060100.47991.max@love2party.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200410060100.47991.max@love2party.net> User-Agent: Mutt/1.5.6i cc: cvs-src@freebsd.org cc: Max Laier cc: src-committers@freebsd.org cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/contrib/pf/man pf.4 X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Oct 2004 23:37:11 -0000 On Wed, Oct 06, 2004 at 01:00:23AM +0200, Max Laier wrote: > On Tuesday 05 October 2004 23:27, Brian Fundakowski Feldman wrote: > > On Tue, Oct 05, 2004 at 08:44:24PM +0000, Max Laier wrote: > > > mlaier 2004-10-05 20:44:24 UTC > > > > > > FreeBSD src repository > > > > > > Modified files: (Branch: RELENG_5) > > > contrib/pf/man pf.4 > > > Log: > > > MFC: > > > PFIL_HOOKS in no longer an optional item. > > > > > > Submitted by: Anders Hanssen > > > > I have a bunch of questions regarding pf documentation... > > > > Do you think we should update pf(4)/pfctl(8) documentation to > > cross-reference IPFW at all? > > I fail to see that point, but I don't care much either way. Maybe I should add > pf to the firewall(7) "ADDITIONAL READING"? I'm being silly; I meant pf.conf(5), since it's the de facto documentation for ALTQ. > > Is it worth explaining in pfctl(8) what the default RED parameters for > > ALTQ are and how they relate to qlimit? > > Sure. pf.conf(5), right? That's the place you were thinking of - not pfctl(8)? > > > Isn't there an altq.4 somewhere? > > No. Feel free to write it. I agree that ALTQ documentation is suboptimal at > the moment. I had plans to evolve the configuration process, but didn't yet > find time to ... in the longrun it should no longer require dev/pf and all > that ... For now, How about just an MLINKS from pf.conf.5 to altq.4? > > Shouldn't pfctl(8) document what occurs when there is no memory to add > > an ALTQ tag? > > pf.conf(5)? Well, if you don't have memory for a tag you are in trouble > anyway. But what happens? The packet ends up in the default queue (I hope). Yeah, that's what happens, in both the classifiers that currently support it. I just think it's worth having in both manpages. > > P.S. Think we should MFC dc(4) ALTQ support? > > You know if it works or not, can't comment on that. If it does work, go for > it. Make sure to update altq(8) as well (or the TBD altq(4)) > > > P.P.S. Should we look again into changing the pfil locking to not > > fail-open? > > Feel free to make if fail-close. You must not sleep there, so it's either open > or close. In contrast to what I told you earlier - you can return EAGAIN or > ENOBUF so that applications don't get confused. I would presume to make it look something like this: Index: pfil.c =================================================================== RCS file: /usr/ncvs/src/sys/net/pfil.c,v retrieving revision 1.10 diff -u -r1.10 pfil.c --- pfil.c 29 Sep 2004 04:54:32 -0000 1.10 +++ pfil.c 5 Oct 2004 23:18:02 -0000 @@ -119,8 +119,15 @@ struct mbuf *m = *mp; int rv = 0; - if (ph->ph_busy_count == -1 || ph->ph_want_write) - return (0); + /* + * Prevent packet filtering from starving the modification of + * the packet filters. + */ + if (ph->ph_busy_count == -1 || ph->ph_want_write) { + m_freem(*mp); + *mp = NULL; + return (ENOBUFS); + } PFIL_RLOCK(ph); for (pfh = pfil_hook_get(dir, ph); pfh != NULL; > Other than that, I am still waiting for you to commit sxfast so that I can > redo the pfil locking with it. I am wondering, however, if you didn't try to > sleep there as well (which is not possible here). Since we're not worried about exclusive access starving shared access in this case, it's not so terribly important. The only other real benefit over what is there now is the ability to have WITNESS work, but that's not so important for a lock mechanism really only used in one file compared to more pervasively-accessed locks. It would take a peter or jhb to finish implementing sxfast locks, because I still don't understand how we're supposed to utilize the turnstile mechanism. -- Brian Fundakowski Feldman \'[ FreeBSD ]''''''''''\ <> green@FreeBSD.org \ The Power to Serve! \ Opinions expressed are my own. \,,,,,,,,,,,,,,,,,,,,,,\