Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 3 Feb 2004 08:04:40 -0800
From:      Brooks Davis <brooks@one-eyed-alien.net>
To:        Andre Oppermann <andre@freebsd.org>
Cc:        cvs-all@freebsd.org
Subject:   Re: cvs commit: src/sys/netinet ip_icmp.c
Message-ID:  <20040203160440.GB27244@Odin.AC.HMC.Edu>
In-Reply-To: <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> <401F5531.35474513@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--b5gNqxB1S1yM7hjW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 03, 2004 at 09:00:49AM +0100, Andre Oppermann wrote:
> Brooks Davis wrote:
> >=20
> > 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 ev=
ery
> > > time a interface is manipulated and to purge the ifp if its interface=
 is
> > > gone.  I thought that was too intrusive.
> >=20
> > 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.
>=20
> 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.
>=20
> 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

--=20
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

--b5gNqxB1S1yM7hjW
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQFAH8aUXY6L6fI4GtQRAjOzAJ9WPu40b5OowV6x4TW/QHXvIVcuEgCff2xq
2DtpuzZGFkHS4YmsjpoWZf8=
=UlFu
-----END PGP SIGNATURE-----

--b5gNqxB1S1yM7hjW--



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