Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 Jun 2005 14:16:21 -0700
From:      Brooks Davis <brooks@one-eyed-alien.net>
To:        Andrew Gallatin <gallatin@cs.duke.edu>
Cc:        cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject:   Re: cvs commit: src/share/man/man9 ifnet.9 src/sys/compat/ndis kern_ndis.c subr_ndis.c src/sys/contrib/altq/altq altq_rio.c src/sys/contrib/dev/oltr if_oltr.c if_oltr_pci.c if_oltrvar.h src/sys/contrib/pf/net if_pflog.c if_pflog.h if_pfsync.c ...
Message-ID:  <20050610211621.GA11303@odin.ac.hmc.edu>
In-Reply-To: <20050610170818.A32249@grasshopper.cs.duke.edu>
References:  <200506101649.j5AGnOPu077043@repoman.freebsd.org> <20050610170818.A32249@grasshopper.cs.duke.edu>

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

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

On Fri, Jun 10, 2005 at 05:08:18PM -0400, Andrew Gallatin wrote:
> Brooks Davis [brooks@FreeBSD.org] wrote:
> >   Stop embedding struct ifnet at the top of driver softcs. Instead the
> >   struct ifnet or the layer 2 common structure it was embedded in have
> >   been replaced with a struct ifnet pointer to be filled by a call to t=
he
> >   new function, if_alloc(). The layer 2 common structure is also alloca=
ted
> >   via if_alloc() based on the interface type. It is hung off the new
> >   struct ifnet member, if_l2com.
> >  =20
> >   This change removes the size of these structures from the kernel ABI =
and
> >   will allow us to better manage them as interfaces come and go.
>=20
> What is the appropriate way to detach an interface now?  I used to
> call just ether_ifdetach(ifp); Now I call both if_detach and if_free.
> Eg:
>=20
>     ether_ifdetach(ifp);
>     if_free(ifp);

This is correct.

> However, it looks like the ifp is getting leaked because I now
> see:
>     myri0: if_free_type: value was not if_alloced, skipping
>=20
>=20
> I think this is because if_detatch() will remove the device from
> the global ifnet list, so the check in if_free() will always fail:
>=20
>     if (ifp !=3D ifnet_byindex(ifp->if_index))

This is a bug.  I changed the checks in if_free too late in the game and
didn't check them closely enough. :(

The following should fix it.  I'll commit it once I've tested it.

-- Brooks

Index: if.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /home/ncvs/src/sys/net/if.c,v
retrieving revision 1.231
diff -u -p -r1.231 if.c
--- if.c	10 Jun 2005 16:49:18 -0000	1.231
+++ if.c	10 Jun 2005 21:15:32 -0000
@@ -426,6 +426,10 @@ if_free_type(struct ifnet *ifp, u_char t
 		return;
 	}
=20
+	ifnet_byindex(ifp->if_index) =3D NULL;
+	while (if_index > 0 && ifaddr_byindex(if_index) =3D=3D NULL)
+		if_index--;
+
 	if (if_com_free[type] !=3D NULL)
 		if_com_free[type](ifp->if_l2com, type);
=20
@@ -678,14 +682,10 @@ if_detach(struct ifnet *ifp)
 	 * Remove address from ifindex_table[] and maybe decrement if_index.
 	 * Clean up all addresses.
 	 */
-	ifnet_byindex(ifp->if_index) =3D NULL;
 	ifaddr_byindex(ifp->if_index) =3D NULL;
 	destroy_dev(ifdev_byindex(ifp->if_index));
 	ifdev_byindex(ifp->if_index) =3D NULL;
=20
-	while (if_index > 0 && ifaddr_byindex(if_index) =3D=3D NULL)
-		if_index--;
-
=20
 	/* We can now free link ifaddr. */
 	if (!TAILQ_EMPTY(&ifp->if_addrhead)) {

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

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

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

iD8DBQFCqgMlXY6L6fI4GtQRAnMGAJ9JyXZpCk0UukjVG8xPQ7dVYNrDSQCfdhvW
1gbcVHDPTxX+tSD1Gud2m2U=
=R6P4
-----END PGP SIGNATURE-----

--azLHFNyN32YCQGCU--



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