cvs commit: src/sys/netinet ip_icmp.c

[Brooks Davis]

On Tue, Feb 03, 2004 at 09:00:49AM +0100, Andre Oppermann wrote:
> 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.

Ah, the check is done, but the check was done without explicit
compatison to NULL so I spaced and missed it.  That's probalby worth
changing when you do the rest since style(9) suggests not trating
pointers and bools.

> > 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!

Thanks.

-- 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/20040203/2a0fa21a/attachment-0001.bin