cvs commit: src/sys/netinet ip_icmp.c
andre at freebsd.org
Tue Feb 3 00:00:56 PST 2004
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!
More information about the cvs-src