On 3/22/2022 2:58 PM, William McVicker wrote:
On 03/22/2022, Jeff Johnson wrote:
On 3/21/2022 1:07 PM, Johannes Berg wrote:
[..snip..]
I'm not an networking expert. So my main question is if I'm allowed to take
the RTNL lock inside the nl80211_vendor_cmd callbacks?
Evidently, you're not. It's interesting though, it used to be that we
called these with the RTNL held, now we don't, and the driver you're
using somehow "got fixed" to take it, but whoever fixed it didn't take
into account that this is not possible?
On this point I just want to remind that prior to the locking change that a
driver would specify on a per-vendor command basis whether or not it wanted
the rtnl_lock to be held via NL80211_FLAG_NEED_RTNL. I'm guessing for the
command in question the driver did not set this flag since the driver wanted
to explicitly take the lock itself, otherwise it would have deadlocked on
itself with the 5.10 kernel.
/jeff
On the 5.10 kernel, the core kernel sets NL80211_FLAG_NEED_RTNL as part of
the internal_flags for NL80211_CMD_VENDOR:
net/wireless/nl80211.c:
{
.cmd = NL80211_CMD_VENDOR,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = nl80211_vendor_cmd,
.dumpit = nl80211_vendor_cmd_dump,
.flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL |
NL80211_FLAG_CLEAR_SKB,
},
So the 5.10 version of this driver doesn't need to directly call rtnl_lock()
within the vendor command doit() functions since pre_doit() handles the RTNL
locking.
It would be nice if nl80211_vendor_cmd() could support taking the RTNL lock if
requested via the vendor flags. That would require moving the wiphy lock to
nl80211_vendor_cmds() so that it could take the RTNL and wiphy lock in the
correct order. Is that something you'd be open to Johannes?
--Will
Thanks for correcting my understanding. I concur that it would be useful
for vendor commands to be able to specify that a given command needs the
RTNL lock to be held.
