> That isn't how we've defined the event NE_DOWN - it should only be > generated when there are no ipif's in the up state.
I had incorrectly assumed that NE_DOWN was to bring an address/ipif down. If it's instead to bring an ill_t down, then I agree with the semantics. > as a "this ipif was done" state comes through to ipif_down_tail() You mean "was down", right? As I mentioned before, that's easily fixed. > ill_ipif_up_count should still only be 0 once if you unplumb an interface > with 2 or more logical interfaces that are up, right? Yes, but you may still see a transition through zero as part of ipif_logical_down(), which is not a "true" down, but rather an implementation artifact of the way we deal with certain ipif/ill changes. In both ipif_down() and ipif_down_tail(), ill_ipif_up_count can be examined and an operation only performed if it's zero. So, I don't see what this has to do with the location of the pfhook. > What about the case when ipif_down() returns EINPROGRESS? > What are the circumstances in which ipif_down_tail() then gets called? In general, EINPROGRESS means that the operation currently executing inside the IPSQ cannot complete yet -- e.g., because it needs to wait for a response from another module, or it has to wait for a reference count to drop. In all cases, the operation is requeued via ipsq_pending_mp_add(), and since ipsq_current_ipif remains set, all future IPSQ operations will be queued (until the current operation completes, and then until the queue is drained). In this case, ipif_down() is waiting for the ipif_t to quiesce (as per the IPIF_DOWN flag passed to ipsq_pending_mp_add()). As the ill or ipif reference counts hit zero, ipif_ill_refrele_tail() will be called, and when the ipif finally quiesces, ipsq_pending_mp_get() will retrieve the pending operation and call ip_reprocess_ioctl() (either directly or indirectly, depending on whether the thread can immediately become exclusive on the IPSQ) to finish the ioctl. The ip_reprocess_ioctl() function will call the appropriate ioctl restart function -- e.g., for ip_sioctl_flags(), it will call ip_sioctl_flags_restart(), which will then call ipif_down_tail() to finish bringing the ipif (and perhaps ill) down. > There are many parts of our IP stack that can seem odd to > the casual observer :-) I don't consider myself a casual observer. I think having the names be something like NE_ILL_UP an NE_ILL_DOWN would be clearer. > Any chance of you posting a webrev (on grommit?) for this? I've asked for an account. The fix is trivial: set a new ipif->ipif_brought_down flag in ipif_down(), and check to see whether it's set in ipif_down_tail(). -- meem _______________________________________________ networking-discuss mailing list [email protected]
