I don't like this change. The problem comes from the fact that we are allowing to run administrative detach in parallel with any other ifioctl(). I believe there are much more issues here except this one. This is just a can of worms as long as we keep these two things to run in parallel. Back when resolving all possible races in carp(4) I just have created a single sx lock that serialized any possible CARP reconfigurations. And this isn't a performance critical path - nobody suffers from this serialization. I'm inclined that we need same approach at interface wide level. Not necessarily a global sx, a per-interface sx should work. And at the point where ifioctl() today does ifunit_ref() it should lock the sx. Same thing should if_clone_destroy() do instead of taking reference.

We once discussed this issue with John back when I was working on projects/ifnet branch in subversion. Of course now with epoch in place and if_detach() waiting on all callbacks the situation is very different to what we had in 2015.

This kind of change definitely doesn't need your time wasted on publishing a review.

I second Mark's latest suggestion.

Remove refcounting and rely on taskqueue_drain().

Drain the task. I'm not sure this is required as we reference the ifp, but
better safe than sorry.