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

[Brooks Davis]

On Fri, Jun 10, 2005 at 05:08:18PM -0400, Andrew Gallatin wrote:
> Brooks Davis [brooks at 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
-------------- 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/20050610/75616d7b/attachment.bin