cvs commit: src/sys/netinet ip_icmp.c

[Andre Oppermann]

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