Defer vlan register/deregister operations in mlxen.

[mjoras]

Summary

This needs to be done to avoid a locking incompatibility with if_vlan's
rmlock(9).

Test Plan

ifconfig create/remove stress testing, internal vlan creation/removal tests.

[• hselasky]

Hi,

What code paths cannot sleep in the vlan filter callbacks?

This also affects mlx5en I believe. Would it be better to fix the caller than all of the clients?

--HPS

[mjoras]

In D5829#124568, @hselasky wrote:

What code paths cannot sleep in the vlan filter callbacks?

This also affects mlx5en I believe. Would it be better to fix the caller than all of the clients?

With the non-sleepable rmlock(9) in both if_lagg and if_vlan, any vlan_config/unconfig event handler cannot sleep. Further, even if both of those rmlocks were changed to sleepable locks, the if_lagg vlan_config (see lagg_register_vlan) caller happens under a shared lock, not an exclusive lock, so it can never sleep per rmlock(9). This might be fixable by making the locks sleepable and making all calls to the vlan_config/unconfig event handlers under exclusive locks, but that seemed like a heavy-handed solution.

I hadn't looked at mlx5en, but you're right, it does the same thing, so it would need to be fixed there as well.

[mjoras]

This comment was removed by mjoras.

[• hselasky]

Can this be fixed in the VLAN / LAGG code instead?

[mjoras]

In D5829#124731, @hselasky wrote:

Can this be fixed in the VLAN / LAGG code instead?

I believe it could be fixed there. The same thing could be done, except instead of defering a driver-specific action it would defer the event handler calls to vlan_config/vlan_unconfig. However what I'm imagining is more complex than the driver-specific patch. It doesn't seem unreasonable to fix this in the drivers that sleep while setting their filters, but it's possible that it would be worth it to address it generically in if_vlan so that the underlying drivers have the flexibility to sleep if they need to for some reason.

I've talked to rstone@ about this and he thinks that a wider discussion is probably warranted here, i.e. with -net or -current.