Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 03 Feb 2004 09:00:49 +0100
From:      Andre Oppermann <andre@freebsd.org>
To:        Brooks Davis <brooks@one-eyed-alien.net>
Cc:        cvs-all@freebsd.org
Subject:   Re: cvs commit: src/sys/netinet ip_icmp.c
Message-ID:  <401F5531.35474513@freebsd.org>
References:  <200402022253.i12MrGkO091411@repoman.freebsd.org> <20040202232614.GA24019@Odin.AC.HMC.Edu> <401EE109.D45B606@freebsd.org> <20040203000622.GA19568@Odin.AC.HMC.Edu>

next in thread | previous in thread | raw e-mail | index | archive | help
Brooks Davis wrote:
> 
> On Tue, Feb 03, 2004 at 12:45:13AM +0100, Andre Oppermann wrote:
> > Brooks Davis wrote:
> > >
> > > On Mon, Feb 02, 2004 at 02:53:16PM -0800, Andre Oppermann wrote:
> > > > andre       2004/02/02 14:53:16 PST
> > > >
> > > >   FreeBSD src repository
> > > >
> > > >   Modified files:
> > > >     sys/netinet          ip_icmp.c
> > > >   Log:
> > > >   Add sysctl net.inet.icmp.reply_src to specify the interface name
> > > >   used for the ICMP reply source in reponse to packets which are not
> > > >   directly addressed to us.  By default continue with with normal
> > > >   source selection.
> > >
> > > Please consider storing the interface index rather then the name.  It is
> > > much cheaper to go from index to ifp then name to ifp and the index will
> > > be invariant as long as the interface exists.  Sometime in the next week
> > > the name will no longer be invariant.
> >
> > I considered storing the ifp but the moment a stored ipf disappears
> > you get an instant panic.  There is no way to verify if the interface
> > pointer is still valid.  And with interface cloning I considered the
> > panic risk too high.  The only other way would be to check the ifp every
> > time a interface is manipulated and to purge the ifp if its interface is
> > gone.  I thought that was too intrusive.
> 
> I'm not sugguesting caching the ifp.  I'm suggesting storing the value
> of ifp->if_index.  You then use ifnet_byindex to get then the ifp when
> needed.  You have to check that it isn't NULL, but you should to be
> doing that for the ifunit() call as well since it will return NULL and
> cause a panic if the admin makes a typo.

Using the ifindex is a good way out.  The check for NULL is already
done, so a wrong interface name wont panic the box.

> As Juli commented, a SYSCTL_PROC to allow the administrator to get/set
> the interface by name is probably the right approach.

I'll write a modification to use the ifindex.  Since the ifname can
change now it will have to resolve the index to name when someone is
looking at the sysctl.  Thanks!

-- 
Andre



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?401F5531.35474513>