Date: Tue, 5 Oct 2004 19:37:04 -0400 From: Brian Fundakowski Feldman <green@freebsd.org> To: Max Laier <max@love2party.net> Cc: cvs-all@freebsd.org Subject: Re: cvs commit: src/contrib/pf/man pf.4 Message-ID: <20041005233704.GF47017@green.homeunix.org> In-Reply-To: <200410060100.47991.max@love2party.net> References: <200410052044.i95KiOVV072560@repoman.freebsd.org> <20041005212704.GB47017@green.homeunix.org> <200410060100.47991.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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. \,,,,,,,,,,,,,,,,,,,,,,\
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041005233704.GF47017>