Subject: Re: [PATCH net] atlantic: fix deadlock at aq_nic_stop

From: Andrew Lunn <[email-masked]>
Date: Sat, 15 Oct 2022 17:09:34 +0200

> > Maybe the lock needs to be moved closer to what actually needs to be
> > protect? What is it protecting?
> 
> It's protecting the operations of aq_macsec_enable and aq_macsec_work.
> The locking was closer to them, but the idea of this patch is to move
> the locking to an earlier moment so, in the case we need to abort, do
> it before changing anything.

aq_check_txsa_expiration() seems to be one of the issues? At least,
the lock is taken before and released afterwards. So what in
aq_check_txsa_expiration() requires the lock?

I don't like the use of rtnl_trylock(). It suggests the basic design is
wrong, or overly complex, and so probably not working correctly.

https://blog.ffwll.ch/2022/07/locking-engineering.html

Please try to identify what is being protected. If it is driver
internal state, could it be replaced with a driver mutex, rather than
RTNL? Or is it network stack as a whole state, which really does
require RTNL? If so, how do other drivers deal with this problem? Is
it specific to MACSEC? Does MACSEC have a design problem?

   Andrew