On Thu, 2008-09-04 at 21:58 -0400, Peter Memishian wrote:
> In the current code, I also don't see how the ipnet_populate_if()
> reference counting stuff works -- i.e., if ipnet_if_getby_index() returns
> non-NULL, then we hold a reference to an ipnetif_t, otherwise we call
> ipnet_create_if() and we don't hold a reference (though this would be
> addressed if ipnet_create_if() acquired a reference as suggested above).
Indeed, and I'm fixing this as part of my fix for 6745025, as the bug is
due to this inconsistency.
While doing this, I believe I've found a general problem with
ipnet_populate_if() in that it can race with incoming netinfo events to
create incorrect interfaces. Consider, for example the following
condenced ipnet_populate_if() code:
for (... phyif = net_phygetnext()...) {
net_getifname(...);
<---- A: NE_UNPLUMB
ipnet_create_if(...);
...
for (... lif = net_lifgetnext() ...) {
<---- B: NE_???
ipnet_create_ifaddr(...);
}
}
In case A, we end up with an interface that really doesn't really exist.
We ignored the NE_UNPLUMB event because the interface didn't exist in
the AVL tree at the time of the event.
In case B, we receive whatever event we get when an address if removed,
but we added the address to our list anyway.
Somehow, we need to make events queue up while we initialize our trees.
Could this simply be accomplished by sharing a common mutex lock between
this code and the event processing code?
-Seb
