cvs commit: src/sys/net if_gif.c

[Brooks Davis]

On Sat, Apr 03, 2004 at 11:56:51AM +0300, Ruslan Ermilov wrote:
> On Wed, Mar 31, 2004 at 10:27:56AM +0300, Ruslan Ermilov wrote:
> > On Tue, Mar 30, 2004 at 03:25:17PM -0800, Brooks Davis wrote:
> > > On Wed, Mar 31, 2004 at 01:03:49AM +0300, Ruslan Ermilov wrote:
> > > > On Mon, Mar 22, 2004 at 06:24:26AM -0800, Robert Watson wrote:
> > > > > rwatson     2004/03/22 06:24:26 PST
> > > > > 
> > > > >   FreeBSD src repository
> > > > > 
> > > > >   Modified files:
> > > > >     sys/net              if_gif.c 
> > > > >   Log:
> > > > >   Move "called", a static function variable used to detect recursive
> > > > >   processing with gif interfaces, to a global variable named "gif_called".
> > > > >   Add an annotation that this approach will not work with a reentrant
> > > > >   network stack, and that we should instead use packet tags to detect
> > > > >   excessive recursive processing.
> > > > >   
> > > > >   Revision  Changes    Path
> > > > >   1.42      +11 -4     src/sys/net/if_gif.c
> > > > > 
> > > > Implemented this in the attached patch.  Note when testing: setting
> > > > net.link.gif.max_nesting too high (>20 on my system) and triggering
> > > > the recursion causes the kernel stack exhaustion.
> > > 
> > > Why not just do what OpenBSD does and do actual loop detection?  This
> > > gets rid of the nesting count hack which isn't really what you want to
> > > measure anyway.
> > > 
> > > http://www.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_gif.c.diff?r1=1.18&r2=1.19
> > > 
> > Good idea.  I will implement it and repost the updated patch here.
> > 
> Actually, just replacing nesting limiter with loop detection was a
> bad idea, so I didn't follow it.  It's a bad idea because you might
> have many nesting (but not looping) gif interfaces, and this will
> still cause panic by exhausting the kernel stack.  Instead, I have
> combined both checks.  Please review the attached patch.

Unless you can automaticly choose a valid value for max_gif_nesting, I
think it should be taken out and shot.  Unless you can do that, there's
nothing you can do to prevent the admin from making a configuration that
blows out the stack so why keep the extra annoyance of gif_max_nest
around?  It won't do anything to prevent the panic and will break things
in perfectly valid cases.  If we're really worried about the stack
issue, forcing a requeue instead of processing to completion any time
we're nested makes more sense to me.

-- Brooks

-- 
Any statement of the form "X is the one, true Y" is FALSE.
PGP fingerprint 655D 519C 26A7 82E7 2529  9BF0 5D8E 8BE9 F238 1AD4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20040403/fe64a0fa/attachment-0001.bin