iflib: Do not hold the ifnet lock across registration

[kbowling]

Summary

iflib_device_register() acquired IFNET_WLOCK to preserve lock order
when ether_ifattach() was called with the context lock held.  The context
lock is now released around ether_ifattach(), making registration-wide
ifnet serialization unnecessary.

Keeping IFNET_WLOCK across driver attachment also allows synchronous
interface event handlers to recurse on it.  The rtnetlink interface-group
dump does so through if_foreach_group() while handling the interface
attachment event.

Remove the outer lock and the corresponding failure-path unlock and
relock transitions.  Continue to drop the context lock around
ether_ifattach() and taskqueue drains, and preserve context-lock coverage
for driver attach and detach.

Validated under WITNESS on 82576 and I226 controllers.  Multiple VF
attach and detach cycles, netmap control operations, and every iflib
registration failure injection point completed without lock or cleanup
errors.

Reported by:    glebius
Fixes:          e0e12405285b ("netmap: fix LOR in iflib_netmap_register")
Fixes:          2f8f892ca344 ("rtnetlink: Add FreeBSD-specific IFLAF_GROUP support")
Fixes:          90e7dbe5e2ca ("iflib: Add registration failure injection points")
MFC after:      2 weeks

[kbowling]

Limit this change to the fe00ca2f8871 regression

[kbowling]

@glebius this does not resolve the WITNESS panic between e0e12405285b6 and 2f8f892ca344

[gallatin]

In D59294#1360356, @kbowling wrote:

Limit this change to the fe00ca2f8871 regression

Why? a52f23f4c49e4766fb9eb0bf460cc83c5f63f17d resolved the original reason that the IFNET_WLOCK was taken in the first place. It seems far, fare cleaner to just remove it.

Eg, I'd happily approve the first version of this patch

[glebius]

Did the original version of this review removed use of IFNET_WLOCK completely? If I understand phabricator correct, that would be URL https://reviews.freebsd.org/D59294?id=185524 I also would prefer this.

[kbowling]

In D59294#1360665, @glebius wrote:

Did the original version of this review removed use of IFNET_WLOCK completely? If I understand phabricator correct, that would be URL https://reviews.freebsd.org/D59294?id=185524 I also would prefer this.

It did but I read https://reviews.freebsd.org/D59289#1360317 and changed it to do what you requested. I stress tested the original version before seeing this comment. If you want me to send diff1 of this patch approve and say so, if you want me to send diff2 and you will solve the netmap integration elsewhere approve and say that.

[glebius]

I'd be happy if you go back to original version of this revision. Sorry for incorrectly reading the file and not understanding that both uses of IFNET_WLOCK were actually connected and both originating from the netmap change.

[kbowling]

Switch back to removal. I don't think this is merely a WITNESS ordering issue now, 2f8f892ca344 may trigger a timing sensitive deadlock with the existing registration lock and @netchild is hitting it.

[netchild]

This fixes the hang at boot I've seen.

[glebius]

I'm also hitting deadlock on boot on one of my desktops. And this patch helps. And of course it makes things correct.

P.S. The patch applied with offsets to main.