On Mon, Nov 08, 2021 at 10:46:08AM -0800, Jakub Kicinski wrote:
On Mon, 8 Nov 2021 20:24:37 +0200 Leon Romanovsky wrote:
quoted
quoted
I prefer my version. I think I asked you to show how the changes make
drivers simpler, which you failed to do.
Why did I fail? My version requires **zero** changes to the drivers.
Everything works without them changing anything. You can't ask for more.
For the last time.
"Your version" does require driver changes, but for better or worse
we have already committed them to the tree. All the re-ordering to make
sure devlink is registered last and more work is done at alloc,
remember?
It fixed access to devlink before driver is ready. Also it fixed devlink
reload of simple drivers (without net namespaces support). So yes, at
least for now, we have a workaround to devlink reload bugs. We rmmod
mlx5_ib before reload and after. Everything thanks to reordering.
The goal is to make the upstream drivers simpler. You failed to show
how your code does that.
Maybe you don't see the benefit because upstream simplifications are
hard to depend on in out-of-tree drivers?
I don't care about out-of-tree drivers, mlx5 is fully upstream.
quoted
quoted
I already told you how this is going to go, don't expect me to comment
too much.
quoted
However for net namespace aware drivers it still stays DOA.
As you can see, devlink reload holds pernet_ops_rwsem, which drivers should
take too in order to unregister_netdevice_notifier.
So for me, the difference between netdevsim and real device (mlx5) is
too huge to really invest time into netdevsim-centric API, because it
won't solve any of real world problems.
Did we not already go over this? Sorry, it feels like you're repeating
arguments which I replied to before. This is exhausting.
I don't enjoy it either.
quoted
nfp will benefit from the simplified locking as well, and so will bnxt,
although I'm not sure the maintainers will opt for using devlink framework
due to the downstream requirements.
Exactly why devlink should be fixed first.
If by "fixed first" you mean it needs 5 locks to be added and to remove
any guarantees on sub-object lifetime then no thanks.
How do you plan to fix pernet_ops_rwsem lock? By exposing devlink state
to the drivers? By providing unlocked version of unregister_netdevice_notifier?
This simple scenario has deadlocks:
sudo ip netns add n1
sudo devlink dev reload pci/0000:00:09.0 netns n1
sudo ip netns del n1
https://lore.kernel.org/netdev/20211108104608.378c106e@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/T/#m94b5c173f134c7d19daf455e3f6bad5ba6afd90d (local)