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
[-- Attachment #1 --]
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 the
> > new function, if_alloc(). The layer 2 common structure is also allocated
> > via if_alloc() based on the interface type. It is hung off the new
> > struct ifnet member, if_l2com.
> >
> > 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.
>
> 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:
>
> 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
>
>
> 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:
>
> if (ifp != 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
===================================================================
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;
}
+ ifnet_byindex(ifp->if_index) = NULL;
+ while (if_index > 0 && ifaddr_byindex(if_index) == NULL)
+ if_index--;
+
if (if_com_free[type] != NULL)
if_com_free[type](ifp->if_l2com, type);
@@ -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) = NULL;
ifaddr_byindex(ifp->if_index) = NULL;
destroy_dev(ifdev_byindex(ifp->if_index));
ifdev_byindex(ifp->if_index) = NULL;
- while (if_index > 0 && ifaddr_byindex(if_index) == NULL)
- if_index--;
-
/* We can now free link ifaddr. */
if (!TAILQ_EMPTY(&ifp->if_addrhead)) {
--
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
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)
iD8DBQFCqgMlXY6L6fI4GtQRAnMGAJ9JyXZpCk0UukjVG8xPQ7dVYNrDSQCfdhvW
1gbcVHDPTxX+tSD1Gud2m2U=
=R6P4
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050610211621.GA11303>