On Thu, 27 Jul 2006, Sam Leffler wrote:
---------------------------------------
@@ -209,6 +211,9 @@
*/
if (ic->ic_reset == NULL)
ic->ic_reset = ieee80211_default_reset;
+
+ KASSERT(ifp->if_spare2 == NULL, ("oops, hosed"));
+ ifp->if_spare2 = ic; /* XXX temp backpointer */
}
---------------------------------------
Please don't use spare pointers without properly allocating them (i.e.,
renaming them). I ran into this this morning when merging these changes
into the rwatson_ifnet branch, where if_spare2 was renamed to if_startmbuf.
However, if the above change is MFC'd as is, people may well find the
problem through run-time corruption when the network stack tries to execute
the ic contents thinking that it's a non-NULL if_startmbuf function
pointer, which is a lot harder to debug. The point of spare fields is that
they be spare -- if they are no longer spare, they should not be left
marked as spare, or they will get reused with unfortunate ABI consequences.
I don't mind if this spare is allocated (if_softc2, if_driver, or the
like), as we have one more spare pointer I can use in 6.x, but the current
use of the spare field is problematic and should not be MFC'd unless
cleaned up.
This was intended as a stopgap until the vap changes were in place and will
never be mfc'd. I understand about reuse; hence the assertion. Since you
renamed the field didn't code just stop compiling? I assumed these were
going to be treated like the M_PROTOx flags in mbuf.h that are kinda spare
but used within layers for their own purpose.
That wasn't the intent, but it's not clear that the intent was ever documented
(other than in the commit message). The original intent was to provide
pointers that could be reused as part of ifnet reworking without breaking the
ABI in 6.x. I'm happy if we want to designate one of these pointers for use
by the driver, but if we want to do that, we should just rename it to
if_driver or if_softc2 or the like to make sure its use is clear. How about
we rename if_spare* to if_unused*, and rename if_spare2 to if_driver?
The ABI worry is what happens with loadable modules -- that you load a 6.x
module from 6.2 on 6.3 and discover that things are unhappy because suddenly
the ifnet framework is invoking the function pointer that is otherwise NULL
(which may be a slight stretch).
Robert N M Watson
Computer Laboratory
University of Cambridge
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
