Thread (42 messages) 42 messages, 5 authors, 2021-11-17

Re: [PATCH net-next] devlink: Require devlink lock during device reload

From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2021-11-15 12:54:11
Also in: lkml

On Mon, Nov 15, 2021 at 12:20:21PM +0100, Jiri Pirko wrote:
Sun, Nov 14, 2021 at 07:19:02AM CET, leon@kernel.org wrote:
quoted
On Fri, Nov 12, 2021 at 08:38:56AM +0100, Jiri Pirko wrote:
quoted
Thu, Nov 11, 2021 at 01:17:52PM CET, leon@kernel.org wrote:
quoted
On Thu, Nov 11, 2021 at 01:05:11PM +0100, Jiri Pirko wrote:
quoted
Tue, Nov 09, 2021 at 07:24:27PM CET, jgg@nvidia.com wrote:
quoted
On Tue, Nov 09, 2021 at 08:20:42AM -0800, Jakub Kicinski wrote:
quoted
On Tue, 9 Nov 2021 11:33:35 -0400 Jason Gunthorpe wrote:
quoted
quoted
quoted
I once sketched out fixing this by removing the need to hold the
per_net_rwsem just for list iteration, which in turn avoids holding it
over the devlink reload paths. It seemed like a reasonable step toward
finer grained locking.  
Seems to me the locking is just a symptom.  
My fear is this reload during net ns destruction is devlink uAPI now
and, yes it may be only a symptom, but the root cause may be unfixable
uAPI constraints.
If I'm reading this right it locks up 100% of the time, what is a uAPI
for? DoS? ;)

Hence my questions about the actual use cases.
Removing namespace support from devlink would solve the crasher. I
certainly didn't feel bold enough to suggest such a thing :)

If no other devlink driver cares about this it is probably the best
idea.
Devlink namespace support is not generic, not related to any driver.
What do you mean?

devlink_pernet_pre_exit() calls to devlink reload, which means that only
drivers that support reload care about it. The reload is driver thing.
However, Jason was talking about "namespace support removal from
devlink"..
The code that sparkles deadlocks is in devlink_pernet_pre_exit() and
this will be nice to remove. I just don't know if it is possible to do
without ripping whole namespace support from devlink.
As discussed offline, the non-standard mlx5/IB usage of network
namespaces requires non standard mlx5/IB workaround. Does not make any
sense to remove the devlink net namespace support removal.
Sorry, I don't agree that registering a net notifier in an aux device
probe function is non-standard or wrong.

This model must be supported sanely somehow in the netdev area and
cannot be worked around in leaf drivers.

Intel ice will have the same problem, as would broadcom if they ever
get their driver modernized.

Jason
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help