mirror of
https://github.com/torvalds/linux.git
synced 2026-10-12 21:31:03 +02:00
net: ethtool: keep rtnl_lock for the ioctl self test
An offline self test that brings the interface down and back up with
netif_close() / netif_open() requires rtnl_lock for both. Since the
ethtool IOCTL path became rtnl-optional for ops-locked drivers, the
ETHTOOL_TEST ioctl runs holding only the netdev instance lock, so on an
ops-locked driver the self test now tears the device down without
rtnl_lock.
With lockdep this reproduces deterministically on every offline self
test on such a driver; note the sole lock held is the instance lock, not
rtnl:
WARNING: suspicious RCU usage
net/core/netpoll.c:207 suspicious rcu_dereference_protected() usage!
1 lock held by ethtool/107:
#0: (&dev->lock){+.+.}, at: dev_ethtool
Call Trace:
netpoll_poll_disable
__dev_close_many
netif_close_many
netif_close
fbnic_self_test
dev_ethtool_locked
dev_ethtool
dev_ioctl
sock_ioctl
__x64_sys_ioctl
Without lockdep the same condition trips ASSERT_RTNL() in
__dev_close_many() / __dev_open(); that check only samples the global
rtnl state, so it can be masked by a concurrent rtnl holder, but the
device is still being reconfigured without the lock it requires.
The ethtool self_test is a legacy ioctl-only command, so an ETHTOOL_TEST
case is only needed on the ioctl path. Add an opt-in bit for drivers whose
self test needs rtnl_lock and set it on the ops-locked drivers whose
offline self test tears the interface down and up:
- fbnic (ops-locked via queue_mgmt_ops): fbnic_self_test() offline path
uses netif_close() / netif_open().
- bnxt (ops-locked via queue_mgmt_ops): bnxt_self_test() offline path
goes through bnxt_close_nic() / bnxt_half_open_nic() /
bnxt_half_close_nic() / bnxt_open_nic(), which close and reopen the
device.
Fixes: f994752b11 ("net: ethtool: optionally skip rtnl_lock on IOCTL path")
Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/178942019771.7700.338431553546884773.stgit@ahduyck-xeon-server.home.arpa
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
parent
95c4d54ed0
commit
1b82958f3f
|
|
@ -5733,7 +5733,8 @@ const struct ethtool_ops bnxt_ethtool_ops = {
|
|||
.op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS |
|
||||
ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM |
|
||||
ETHTOOL_OP_NEEDS_RTNL_SCOALESCE |
|
||||
ETHTOOL_OP_NEEDS_RTNL_RSS,
|
||||
ETHTOOL_OP_NEEDS_RTNL_RSS |
|
||||
ETHTOOL_OP_NEEDS_RTNL_TEST,
|
||||
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
|
||||
ETHTOOL_COALESCE_MAX_FRAMES |
|
||||
ETHTOOL_COALESCE_USECS_IRQ |
|
||||
|
|
|
|||
|
|
@ -2025,7 +2025,8 @@ static const struct ethtool_ops fbnic_ethtool_ops = {
|
|||
ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM |
|
||||
ETHTOOL_OP_NEEDS_RTNL_SCHANNELS |
|
||||
ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM |
|
||||
ETHTOOL_OP_NEEDS_RTNL_GLINK,
|
||||
ETHTOOL_OP_NEEDS_RTNL_GLINK |
|
||||
ETHTOOL_OP_NEEDS_RTNL_TEST,
|
||||
.get_drvinfo = fbnic_get_drvinfo,
|
||||
.get_regs_len = fbnic_get_regs_len,
|
||||
.get_regs = fbnic_get_regs,
|
||||
|
|
|
|||
|
|
@ -944,6 +944,7 @@ struct kernel_ethtool_ts_info {
|
|||
#define ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM BIT(6)
|
||||
#define ETHTOOL_OP_NEEDS_RTNL_RSS BIT(7)
|
||||
#define ETHTOOL_OP_NEEDS_RTNL_GLINK BIT(8)
|
||||
#define ETHTOOL_OP_NEEDS_RTNL_TEST BIT(9)
|
||||
|
||||
/**
|
||||
* struct ethtool_ops - optional netdev operations
|
||||
|
|
@ -981,6 +982,7 @@ struct kernel_ethtool_ts_info {
|
|||
* - netdev_update_features()
|
||||
* - netif_set_real_num_tx_queues()
|
||||
* - ethtool_op_get_link() (syncs link watch under rtnl_lock)
|
||||
* - netif_open() / netif_close() (used by @self_test)
|
||||
*
|
||||
* @get_drvinfo: Report driver/device information. Modern drivers no
|
||||
* longer have to implement this callback. Most fields are
|
||||
|
|
|
|||
|
|
@ -163,6 +163,8 @@ ethtool_ioctl_needs_rtnl(const struct net_device *dev, u32 ethcmd)
|
|||
return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_RSS;
|
||||
case ETHTOOL_GLINK:
|
||||
return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_GLINK;
|
||||
case ETHTOOL_TEST:
|
||||
return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_TEST;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user