From 251bc9db88fb351ea2dfbd976c4e6aaae6507a91 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:30 -0700 Subject: [PATCH 01/11] net: rename netdev_ops_assert_locked() Jakub suggests renaming the existing assert to match the netdev_lock_ops_compat() semantics. We want netdev_assert_locked_ops() to mean - if the driver is ops locked - check that it's holding the device lock. The existing helper check for either ops lock or rtnl_lock, which is the locking behavior of netdev_lock_ops_compat(). The reason for naming divergence is likely that netdev_ops_assert_locked() predated the _compat() helpers. Suggested-by: Jakub Sitnicki Reviewed-by: Nicolai Buchwitz Reviewed-by: Jakub Sitnicki Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20260603012840.2254293-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +- include/net/netdev_lock.h | 6 +++--- net/core/dev.c | 26 +++++++++++------------ net/core/dev_addr_lists.c | 2 +- net/core/link_watch.c | 2 +- net/core/lock_debug.c | 2 +- net/core/netdev_queues.c | 2 +- net/ethtool/netlink.c | 4 ++-- net/ipv6/addrconf.c | 2 +- net/xdp/xsk_buff_pool.c | 2 +- 10 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 3587f39202d2..2402cc865e76 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -16762,7 +16762,7 @@ int bnxt_restore_pf_fw_resources(struct bnxt *bp) { int rc; - netdev_ops_assert_locked(bp->dev); + netdev_assert_locked_ops_compat(bp->dev); bnxt_hwrm_func_qcaps(bp); if (netif_running(bp->dev)) diff --git a/include/net/netdev_lock.h b/include/net/netdev_lock.h index 3d3aef80beac..8e84d29b0bfb 100644 --- a/include/net/netdev_lock.h +++ b/include/net/netdev_lock.h @@ -64,7 +64,7 @@ static inline void netdev_unlock_full_to_ops(struct net_device *dev) netdev_unlock(dev); } -static inline void netdev_ops_assert_locked(const struct net_device *dev) +static inline void netdev_assert_locked_ops_compat(const struct net_device *dev) { if (netdev_need_ops_lock(dev)) lockdep_assert_held(&dev->lock); @@ -73,11 +73,11 @@ static inline void netdev_ops_assert_locked(const struct net_device *dev) } static inline void -netdev_ops_assert_locked_or_invisible(const struct net_device *dev) +netdev_assert_locked_ops_compat_or_invisible(const struct net_device *dev) { if (dev->reg_state == NETREG_REGISTERED || dev->reg_state == NETREG_UNREGISTERING) - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); } static inline void netdev_lock_ops_compat(struct net_device *dev) diff --git a/net/core/dev.c b/net/core/dev.c index 804e8ad25010..1ecd5691992e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1593,7 +1593,7 @@ EXPORT_SYMBOL(netdev_features_change); void netif_state_change(struct net_device *dev) { - netdev_ops_assert_locked_or_invisible(dev); + netdev_assert_locked_ops_compat_or_invisible(dev); if (dev->flags & IFF_UP) { struct netdev_notifier_change_info change_info = { @@ -1693,7 +1693,7 @@ static int __dev_open(struct net_device *dev, struct netlink_ext_ack *extack) set_bit(__LINK_STATE_START, &dev->state); - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); if (ops->ndo_validate_addr) ret = ops->ndo_validate_addr(dev); @@ -1770,7 +1770,7 @@ static void __dev_close_many(struct list_head *head) * event. */ - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); if (ops->ndo_stop) ops->ndo_stop(dev); @@ -3198,7 +3198,7 @@ int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq) if (dev->reg_state == NETREG_REGISTERED || dev->reg_state == NETREG_UNREGISTERING) { - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); rc = netdev_queue_update_kobjects(dev, dev->real_num_tx_queues, txq); @@ -3247,7 +3247,7 @@ int netif_set_real_num_rx_queues(struct net_device *dev, unsigned int rxq) return -EINVAL; if (dev->reg_state == NETREG_REGISTERED) { - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); rc = net_rx_queue_update_kobjects(dev, dev->real_num_rx_queues, rxq); @@ -7294,7 +7294,7 @@ void netif_queue_set_napi(struct net_device *dev, unsigned int queue_index, if (WARN_ON_ONCE(napi && !napi->dev)) return; - netdev_ops_assert_locked_or_invisible(dev); + netdev_assert_locked_ops_compat_or_invisible(dev); switch (type) { case NETDEV_QUEUE_TYPE_RX: @@ -9589,7 +9589,7 @@ int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify) kuid_t uid; kgid_t gid; - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); promiscuity = dev->promiscuity + inc; if (promiscuity == 0) { @@ -9648,7 +9648,7 @@ int netif_set_allmulti(struct net_device *dev, int inc, bool notify) unsigned int old_flags = dev->flags, old_gflags = dev->gflags; unsigned int allmulti, flags; - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); allmulti = dev->allmulti + inc; if (allmulti == 0) { @@ -9716,7 +9716,7 @@ int __dev_change_flags(struct net_device *dev, unsigned int flags, unsigned int old_flags = dev->flags; int ret; - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); /* * Set the flags on our device. @@ -9864,7 +9864,7 @@ int netif_set_mtu_ext(struct net_device *dev, int new_mtu, { int err, orig_mtu; - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); if (new_mtu == dev->mtu) return 0; @@ -10317,7 +10317,7 @@ static int dev_xdp_install(struct net_device *dev, enum bpf_xdp_mode mode, struct netdev_bpf xdp; int err; - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); if (dev->cfg->hds_config == ETHTOOL_TCP_DATA_SPLIT_ENABLED && prog && !prog->aux->xdp_has_frags) { @@ -10769,7 +10769,7 @@ u32 dev_get_min_mp_channel_count(const struct net_device *dev) { int i; - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); for (i = dev->real_num_rx_queues - 1; i >= 0; i--) if (dev->_rx[i].mp_params.mp_priv) @@ -10997,7 +10997,7 @@ int __netdev_update_features(struct net_device *dev) int err = -1; ASSERT_RTNL(); - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); features = netdev_get_wanted_features(dev); diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c index d73fcb0c6785..6b493af8dc8b 100644 --- a/net/core/dev_addr_lists.c +++ b/net/core/dev_addr_lists.c @@ -1260,7 +1260,7 @@ static void netif_rx_mode_run(struct net_device *dev) int err; might_sleep(); - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); __hw_addr_init(&uc_snap); __hw_addr_init(&mc_snap); diff --git a/net/core/link_watch.c b/net/core/link_watch.c index ff2c1d4538ef..9c35aac8b2e9 100644 --- a/net/core/link_watch.c +++ b/net/core/link_watch.c @@ -277,7 +277,7 @@ static bool linkwatch_clean_dev(struct net_device *dev) void __linkwatch_sync_dev(struct net_device *dev) { - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); if (linkwatch_clean_dev(dev)) { linkwatch_do_dev(dev); diff --git a/net/core/lock_debug.c b/net/core/lock_debug.c index 9e9fb25314b9..14fd8fcdcd56 100644 --- a/net/core/lock_debug.c +++ b/net/core/lock_debug.c @@ -24,7 +24,7 @@ int netdev_debug_event(struct notifier_block *nb, unsigned long event, case NETDEV_CHANGE: case NETDEV_REGISTER: case NETDEV_UP: - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); fallthrough; case NETDEV_DOWN: case NETDEV_REBOOT: diff --git a/net/core/netdev_queues.c b/net/core/netdev_queues.c index 73fb28087a93..4d6864bb4f6b 100644 --- a/net/core/netdev_queues.c +++ b/net/core/netdev_queues.c @@ -40,7 +40,7 @@ struct device *netdev_queue_get_dma_dev(struct net_device *dev, struct netdev_rx_queue *hw_rxq; struct device *dma_dev; - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); /* Only RX side supports queue leasing today. */ if (type != NETDEV_QUEUE_TYPE_RX || !netif_rxq_is_leased(dev, idx)) diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index 6cbd13b61bd1..25e22c48060a 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -98,7 +98,7 @@ int ethnl_ops_begin(struct net_device *dev) if (dev->dev.parent) pm_runtime_get_sync(dev->dev.parent); - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); if (!netif_device_present(dev) || dev->reg_state >= NETREG_UNREGISTERING) { @@ -1005,7 +1005,7 @@ static void ethnl_default_notify(struct net_device *dev, unsigned int cmd, ops->req_info_size - sizeof(*req_info)); } - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); ethnl_init_reply_data(reply_data, ops, dev); ret = ops->prepare_data(req_info, reply_data, &info); diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index f5a93155e7eb..af3a0e94773c 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -380,7 +380,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev) int err = -ENOMEM; ASSERT_RTNL(); - netdev_ops_assert_locked(dev); + netdev_assert_locked_ops_compat(dev); if (dev->mtu < IPV6_MIN_MTU && dev != blackhole_netdev) return ERR_PTR(-EINVAL); diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c index d981cfdd8535..1f28a9641571 100644 --- a/net/xdp/xsk_buff_pool.c +++ b/net/xdp/xsk_buff_pool.c @@ -239,7 +239,7 @@ int xp_assign_dev(struct xsk_buff_pool *pool, bpf.xsk.pool = pool; bpf.xsk.queue_id = queue_id; - netdev_ops_assert_locked(netdev); + netdev_assert_locked_ops_compat(netdev); err = netdev->netdev_ops->ndo_bpf(netdev, &bpf); if (err) goto err_unreg_pool; From 1d3964e29e53b02a7c61ed6d6e4aeccba3275ebc Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:31 -0700 Subject: [PATCH 02/11] net: ethtool: cmis_cdb: hold instance lock for ops locked devices FW module flashing was written so that the flashing happens without holding rtnl_lock. This allows flashing multiple modules at once. Current drivers can handle that well, but we should let drivers depend on the netdev instance lock. Instance lock is per netdev, and so is the module so we won't break parallel updates. Reviewed-by: Jakub Sitnicki Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20260603012840.2254293-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/net/netdev_lock.h | 6 ++++++ net/ethtool/cmis_cdb.c | 3 +++ net/ethtool/cmis_fw_update.c | 8 ++------ net/ethtool/module.c | 2 ++ 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/include/net/netdev_lock.h b/include/net/netdev_lock.h index 8e84d29b0bfb..d3daec4e93f3 100644 --- a/include/net/netdev_lock.h +++ b/include/net/netdev_lock.h @@ -80,6 +80,12 @@ netdev_assert_locked_ops_compat_or_invisible(const struct net_device *dev) netdev_assert_locked_ops_compat(dev); } +static inline void netdev_assert_locked_ops(const struct net_device *dev) +{ + if (netdev_need_ops_lock(dev)) + netdev_assert_locked(dev); +} + static inline void netdev_lock_ops_compat(struct net_device *dev) { if (netdev_need_ops_lock(dev)) diff --git a/net/ethtool/cmis_cdb.c b/net/ethtool/cmis_cdb.c index f3a53a984460..a4e8f4b3fb75 100644 --- a/net/ethtool/cmis_cdb.c +++ b/net/ethtool/cmis_cdb.c @@ -2,6 +2,7 @@ #include #include +#include #include "common.h" #include "module_fw.h" @@ -179,6 +180,7 @@ cmis_cdb_validate_password(struct ethtool_cmis_cdb *cdb, pe_pl = *((struct cmis_password_entry_pl *)page_data.data); pe_pl.password = params->password; + netdev_assert_locked_ops(dev); err = ops->set_module_eeprom_by_page(dev, &page_data, &extack); if (err < 0) { if (extack._msg) @@ -546,6 +548,7 @@ __ethtool_cmis_cdb_execute_cmd(struct net_device *dev, if (!page_data->data) return -ENOMEM; + netdev_assert_locked_ops(dev); err = ops->set_module_eeprom_by_page(dev, page_data, &extack); if (err < 0) { if (extack._msg) diff --git a/net/ethtool/cmis_fw_update.c b/net/ethtool/cmis_fw_update.c index 291d04d2776a..dff83807e975 100644 --- a/net/ethtool/cmis_fw_update.c +++ b/net/ethtool/cmis_fw_update.c @@ -435,13 +435,9 @@ cmis_fw_update_commit_image(struct ethtool_cmis_cdb *cdb, static int cmis_fw_update_reset(struct net_device *dev) { __u32 reset_data = ETH_RESET_PHY; - int ret; - netdev_lock_ops(dev); - ret = dev->ethtool_ops->reset(dev, &reset_data); - netdev_unlock_ops(dev); - - return ret; + netdev_assert_locked_ops(dev); + return dev->ethtool_ops->reset(dev, &reset_data); } void diff --git a/net/ethtool/module.c b/net/ethtool/module.c index ea4fb2a76650..c3388e6d7ec8 100644 --- a/net/ethtool/module.c +++ b/net/ethtool/module.c @@ -226,7 +226,9 @@ static void module_flash_fw_work(struct work_struct *work) module_fw = container_of(work, struct ethtool_module_fw_flash, work); dev = module_fw->fw_update.dev; + netdev_lock_ops(dev); ethtool_cmis_fw_update(&module_fw->fw_update); + netdev_unlock_ops(dev); module_flash_fw_work_list_del(&module_fw->list); From b8dfa196cb978815543a36bbdd077f4a161e160d Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:32 -0700 Subject: [PATCH 03/11] net: document NETDEV_CHANGENAME as ops locked NETDEV_CHANGENAME is only emitted from netif_change_name(). netif_change_name() has two callers both of which hold netdev_lock_ops() around the call site: - dev_change_name() - do_setlink() Document NETDEV_CHANGENAME as always ops locked. Reviewed-by: Jakub Sitnicki Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20260603012840.2254293-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- Documentation/networking/netdevices.rst | 1 + net/core/lock_debug.c | 1 + 2 files changed, 2 insertions(+) diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst index 60492d4df2ee..8fc96975b3bd 100644 --- a/Documentation/networking/netdevices.rst +++ b/Documentation/networking/netdevices.rst @@ -415,6 +415,7 @@ The following netdev notifiers are always run under the instance lock: For devices with locked ops, currently only the following notifiers are running under the lock: * ``NETDEV_CHANGE`` +* ``NETDEV_CHANGENAME`` * ``NETDEV_REGISTER`` * ``NETDEV_UP`` diff --git a/net/core/lock_debug.c b/net/core/lock_debug.c index 14fd8fcdcd56..8a81c5430705 100644 --- a/net/core/lock_debug.c +++ b/net/core/lock_debug.c @@ -65,6 +65,7 @@ int netdev_debug_event(struct notifier_block *nb, unsigned long event, break; case NETDEV_CHANGENAME: + netdev_assert_locked_ops(dev); ASSERT_RTNL_NET(net); break; } From f32fe1d79a18592b96342a0016cd62cf9601a054 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:33 -0700 Subject: [PATCH 04/11] net: ethtool: add netif_get_link_ksettings() for correct ops-locked use __ethtool_get_link_ksettings() is exported and called from sysfs and many drivers. It invokes ethtool_ops->get_link_ksettings so by our own docs it should be holding netdev lock for ops locked devices. Looks like commit 2bcf4772e45a ("net: ethtool: try to protect all callback with netdev instance lock") missed adding the ops lock here. There's a number of callers we need to fix up so let's add the netif_get_link_ksettings() helper first, without any actual locking changes (this commit is a nop). Not treating this as a fix because I don't think any driver cares at this point, but if we want to remove the rtnl_lock protection this will become critical. Reviewed-by: Maxime Chevallier Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20260603012840.2254293-5-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/linux/ethtool.h | 2 ++ net/ethtool/ioctl.c | 17 ++++++++++++++--- net/ethtool/linkinfo.c | 4 ++-- net/ethtool/linkmodes.c | 4 ++-- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 1cb0740ba331..f51346a6a686 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -325,6 +325,8 @@ struct ethtool_link_ksettings { extern int __ethtool_get_link_ksettings(struct net_device *dev, struct ethtool_link_ksettings *link_ksettings); +int netif_get_link_ksettings(struct net_device *dev, + struct ethtool_link_ksettings *link_ksettings); struct ethtool_keee { __ETHTOOL_DECLARE_LINK_MODE_MASK(supported); diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index bd97f9b9bf18..49da873b673d 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -436,10 +436,10 @@ struct ethtool_link_usettings { }; /* Internal kernel helper to query a device ethtool_link_settings. */ -int __ethtool_get_link_ksettings(struct net_device *dev, - struct ethtool_link_ksettings *link_ksettings) +int netif_get_link_ksettings(struct net_device *dev, + struct ethtool_link_ksettings *link_ksettings) { - ASSERT_RTNL(); + /* once callers fixed - assert ops locked */ if (!dev->ethtool_ops->get_link_ksettings) return -EOPNOTSUPP; @@ -450,6 +450,17 @@ int __ethtool_get_link_ksettings(struct net_device *dev, memset(link_ksettings, 0, sizeof(*link_ksettings)); return dev->ethtool_ops->get_link_ksettings(dev, link_ksettings); } +EXPORT_SYMBOL(netif_get_link_ksettings); + +/* Convenience helper for callers that hold only rtnl_lock(). */ +int __ethtool_get_link_ksettings(struct net_device *dev, + struct ethtool_link_ksettings *link_ksettings) +{ + ASSERT_RTNL(); + + /* once callers fixed - take the ops lock around this call */ + return netif_get_link_ksettings(dev, link_ksettings); +} EXPORT_SYMBOL(__ethtool_get_link_ksettings); /* convert ethtool_link_usettings in user space to a kernel internal diff --git a/net/ethtool/linkinfo.c b/net/ethtool/linkinfo.c index 244ff92e2ff9..d5b3dbc53c5f 100644 --- a/net/ethtool/linkinfo.c +++ b/net/ethtool/linkinfo.c @@ -34,7 +34,7 @@ static int linkinfo_prepare_data(const struct ethnl_req_info *req_base, ret = ethnl_ops_begin(dev); if (ret < 0) return ret; - ret = __ethtool_get_link_ksettings(dev, &data->ksettings); + ret = netif_get_link_ksettings(dev, &data->ksettings); if (ret < 0) GENL_SET_ERR_MSG(info, "failed to retrieve link settings"); ethnl_ops_complete(dev); @@ -104,7 +104,7 @@ ethnl_set_linkinfo(struct ethnl_req_info *req_info, struct genl_info *info) bool mod = false; int ret; - ret = __ethtool_get_link_ksettings(dev, &ksettings); + ret = netif_get_link_ksettings(dev, &ksettings); if (ret < 0) { GENL_SET_ERR_MSG(info, "failed to retrieve link settings"); return ret; diff --git a/net/ethtool/linkmodes.c b/net/ethtool/linkmodes.c index 30d703531652..a6d32f0d9fcc 100644 --- a/net/ethtool/linkmodes.c +++ b/net/ethtool/linkmodes.c @@ -39,7 +39,7 @@ static int linkmodes_prepare_data(const struct ethnl_req_info *req_base, if (ret < 0) return ret; - ret = __ethtool_get_link_ksettings(dev, &data->ksettings); + ret = netif_get_link_ksettings(dev, &data->ksettings); if (ret < 0) { GENL_SET_ERR_MSG(info, "failed to retrieve link settings"); goto out; @@ -324,7 +324,7 @@ ethnl_set_linkmodes(struct ethnl_req_info *req_info, struct genl_info *info) bool mod = false; int ret; - ret = __ethtool_get_link_ksettings(dev, &ksettings); + ret = netif_get_link_ksettings(dev, &ksettings); if (ret < 0) { GENL_SET_ERR_MSG(info, "failed to retrieve link settings"); return ret; From 107b097790e903651385a96dd96f9be158702730 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:34 -0700 Subject: [PATCH 05/11] net: bonding: don't recurse on the slave's netdev ops lock bond_update_speed_duplex() calls __ethtool_get_link_ksettings() on the slave, which will soon take the slave's ops lock. One of its callers already holds it and the other three don't, so the function would either deadlock or run unprotected depending on the path. Make the helper expect the slave's ops lock held and switch to netif_get_link_ksettings(). Wrap the three call sites that don't already hold it: * bond_enslave() (rtnl held; core drops the lower's ops lock around ->ndo_add_slave). * bond_miimon_commit() (rtnl_trylock'd from the mii workqueue). * bond_ethtool_get_link_ksettings() (rtnl held via ethtool layer, bond device itself is not ops locked). The call site which does already hold the ops lock is bond_slave_netdev_event() via NETDEV_UP / NETDEV_CHANGE notifiers, so it stays as-is. Reviewed-by: Nicolai Buchwitz Link: https://patch.msgid.link/20260603012840.2254293-6-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/bonding/bond_main.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 82e779f7916b..0bcab797e468 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -787,6 +787,10 @@ int bond_set_carrier(struct bonding *bond) * values are invalid, set speed and duplex to -1, * and return. Return 1 if speed or duplex settings are * UNKNOWN; 0 otherwise. + * + * Caller must hold the slave's netdev ops lock. The notifier path + * (bond_netdev_event NETDEV_CHANGE/UP) reaches us with the slave's ops + * lock held; other call sites take it explicitly. */ static int bond_update_speed_duplex(struct slave *slave) { @@ -794,7 +798,7 @@ static int bond_update_speed_duplex(struct slave *slave) struct ethtool_link_ksettings ecmd; int res; - res = __ethtool_get_link_ksettings(slave_dev, &ecmd); + res = netif_get_link_ksettings(slave_dev, &ecmd); if (res < 0) goto speed_duplex_unknown; if (ecmd.base.speed == 0 || ecmd.base.speed == ((__u32)-1)) @@ -2112,8 +2116,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev, new_slave->delay = 0; new_slave->link_failure_count = 0; - if (bond_update_speed_duplex(new_slave) && - bond_needs_speed_duplex(bond)) + netdev_lock_ops(slave_dev); + res = bond_update_speed_duplex(new_slave); + netdev_unlock_ops(slave_dev); + if (res && bond_needs_speed_duplex(bond)) new_slave->link = BOND_LINK_DOWN; new_slave->last_rx = jiffies - @@ -2780,6 +2786,7 @@ static void bond_miimon_commit(struct bonding *bond) struct slave *slave, *primary, *active; bool do_failover = false; struct list_head *iter; + int err; ASSERT_RTNL(); @@ -2798,8 +2805,10 @@ static void bond_miimon_commit(struct bonding *bond) continue; case BOND_LINK_UP: - if (bond_update_speed_duplex(slave) && - bond_needs_speed_duplex(bond)) { + netdev_lock_ops(slave->dev); + err = bond_update_speed_duplex(slave); + netdev_unlock_ops(slave->dev); + if (err && bond_needs_speed_duplex(bond)) { slave->link = BOND_LINK_DOWN; if (net_ratelimit()) slave_warn(bond->dev, slave->dev, @@ -5861,7 +5870,9 @@ static int bond_ethtool_get_link_ksettings(struct net_device *bond_dev, */ bond_for_each_slave(bond, slave, iter) { if (bond_slave_can_tx(slave)) { + netdev_lock_ops(slave->dev); bond_update_speed_duplex(slave); + netdev_unlock_ops(slave->dev); if (slave->speed != SPEED_UNKNOWN) { if (BOND_MODE(bond) == BOND_MODE_BROADCAST) speed = bond_mode_bcast_speed(slave, From a6882c1583262d5bd86a798198820842e6d32b62 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:35 -0700 Subject: [PATCH 06/11] net: team: don't recurse on the port's netdev ops lock __team_port_change_send() calls __ethtool_get_link_ksettings() on the port, which will soon take the port's ops lock. The notifier caller already holds it while the slave-add/del callers do not, so the function would either deadlock or run unprotected depending on the path. Make __team_port_change_send() expect the port's ops lock held and switch to netif_get_link_ksettings(). team_device_event()'s NETDEV_UP / NETDEV_CHANGE already arrive with the port's ops lock held. team_port_add() now take it explicitly. Note that NETDEV_DOWN and team_port_del() will pass false as @linkup so they will not execute netif_get_link_ksettings(). This is fortunate as NETDEV_DOWN has somewhat mixed locking right now. Link: https://patch.msgid.link/20260603012840.2254293-7-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/team/team_core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c index f51388d50307..feaa75fbf8fc 100644 --- a/drivers/net/team/team_core.c +++ b/drivers/net/team/team_core.c @@ -1375,7 +1375,9 @@ static int team_port_add(struct team *team, struct net_device *port_dev, list_add_tail_rcu(&port->list, &team->port_list); team_port_enable(team, port); netdev_compute_master_upper_features(dev, true); + netdev_lock_ops(port_dev); __team_port_change_port_added(port, !!netif_oper_up(port_dev)); + netdev_unlock_ops(port_dev); __team_options_change_check(team); netdev_info(dev, "Port device %s added\n", portname); @@ -3090,7 +3092,7 @@ static void __team_port_change_send(struct team_port *port, bool linkup) if (linkup) { struct ethtool_link_ksettings ecmd; - err = __ethtool_get_link_ksettings(port->dev, &ecmd); + err = netif_get_link_ksettings(port->dev, &ecmd); if (!err) { port->state.speed = ecmd.base.speed; port->state.duplex = ecmd.base.duplex; From 20398c41d0014ba08363c8e7ad98006e6e28747f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:36 -0700 Subject: [PATCH 07/11] net: bridge: don't recurse on the port's netdev ops lock port_cost() calls __ethtool_get_link_ksettings() on the port device, which will soon take the port's ops lock. br_port_carrier_check() is reached via the NETDEV_CHANGE notifier from linkwatch, which already holds the port's ops lock, so the call would deadlock. Make port_cost() expect the port's ops lock held and switch to netif_get_link_ksettings(). The only other caller is new_nbp(), make sure it takes the lock explicitly. Reviewed-by: Nicolai Buchwitz Acked-by: Nikolay Aleksandrov Reviewed-by: Ido Schimmel Link: https://patch.msgid.link/20260603012840.2254293-8-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/bridge/br_if.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index d39571e13744..049d1d25bc26 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -30,13 +31,13 @@ * Determine initial path cost based on speed. * using recommendations from 802.1d standard * - * Since driver might sleep need to not be holding any locks. + * Since driver might sleep, we need to not be holding any bridge spinlocks. */ static int port_cost(struct net_device *dev) { struct ethtool_link_ksettings ecmd; - if (!__ethtool_get_link_ksettings(dev, &ecmd)) { + if (!netif_get_link_ksettings(dev, &ecmd)) { switch (ecmd.base.speed) { case SPEED_10000: return 2; @@ -436,7 +437,9 @@ static struct net_bridge_port *new_nbp(struct net_bridge *br, p->br = br; netdev_hold(dev, &p->dev_tracker, GFP_KERNEL); p->dev = dev; + netdev_lock_ops(dev); p->path_cost = port_cost(dev); + netdev_unlock_ops(dev); p->priority = 0x8000 >> BR_PORT_BITS; p->port_no = index; p->flags = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD; From e50f6245be53e612dc3296b53c890a0d4a2d806a Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:37 -0700 Subject: [PATCH 08/11] net: sched: don't recurse on the netdev ops lock in qdiscs cbs_set_port_rate() and taprio_set_picos_per_byte() are reached from two paths and both already hold the device's ops lock: *_change(), via tc_modify_qdisc() which calls netdev_lock_ops(dev) before dispatching to the qdisc ops. *_dev_notifier() on NETDEV_UP / NETDEV_CHANGE, where caller holds the ops lock across the notifier chain. Switch to netif_get_link_ksettings() to avoid deadlock once __ethtool_get_link_ksettings() starts taking the netdev lock. Link: https://patch.msgid.link/20260603012840.2254293-9-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/sched/sch_cbs.c | 2 +- net/sched/sch_taprio.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c index b952353ec2a5..85b41ffc63ff 100644 --- a/net/sched/sch_cbs.c +++ b/net/sched/sch_cbs.c @@ -327,7 +327,7 @@ static void cbs_set_port_rate(struct net_device *dev, struct cbs_sched_data *q) s64 port_rate; int err; - err = __ethtool_get_link_ksettings(dev, &ecmd); + err = netif_get_link_ksettings(dev, &ecmd); if (err < 0) goto skip; diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c index d6b981e5df11..e83cbce62a54 100644 --- a/net/sched/sch_taprio.c +++ b/net/sched/sch_taprio.c @@ -1299,7 +1299,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev, int picos_per_byte; int err; - err = __ethtool_get_link_ksettings(dev, &ecmd); + err = netif_get_link_ksettings(dev, &ecmd); if (err < 0) goto skip; From ea8ea212d4154f8e445cf282e246c47649a18a59 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:38 -0700 Subject: [PATCH 09/11] leds: trigger: netdev: don't recurse on the netdev ops lock get_device_state() calls __ethtool_get_link_ksettings() on the trigger's netdev, which will soon take the dev's ops lock. Three of its callers already hold that lock and one doesn't, so the function would either deadlock or run unprotected depending on the path. Make get_device_state() expect the dev's ops lock held and switch to netif_get_link_ksettings(): * netdev_trig_notify() NETDEV_UP / NETDEV_CHANGE / NETDEV_CHANGENAME arrive with the dev's ops lock held (per netdevices.rst). * set_device_name() does not hold the lock, take it explicitly. Due to lock ordering we need to reshuffle the code in set_device_name() a little bit. We need to find the device earlier on, so that we can lock it before we take trigger_data->lock. Link: https://patch.msgid.link/20260603012840.2254293-10-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/leds/trigger/ledtrig-netdev.c | 37 +++++++++++++++------------ 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c index 12cb3311ea22..64c078e997f2 100644 --- a/drivers/leds/trigger/ledtrig-netdev.c +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "../leds.h" #define NETDEV_LED_DEFAULT_INTERVAL 50 @@ -228,7 +229,7 @@ static void get_device_state(struct led_netdev_data *trigger_data) trigger_data->carrier_link_up = netif_carrier_ok(trigger_data->net_dev); - if (__ethtool_get_link_ksettings(trigger_data->net_dev, &cmd)) + if (netif_get_link_ksettings(trigger_data->net_dev, &cmd)) return; if (trigger_data->carrier_link_up) { @@ -259,31 +260,33 @@ static ssize_t device_name_show(struct device *dev, static int set_device_name(struct led_netdev_data *trigger_data, const char *name, size_t size) { + struct net_device *new_dev = NULL; + char device_name[IFNAMSIZ]; + if (size >= IFNAMSIZ) return -EINVAL; cancel_delayed_work_sync(&trigger_data->work); + memcpy(device_name, name, size); + device_name[size] = 0; + if (size > 0 && device_name[size - 1] == '\n') + device_name[size - 1] = 0; + /* - * Take RTNL lock before trigger_data lock to prevent potential - * deadlock with netdev notifier registration. + * Lock order: rtnl_lock -> netdev instance lock -> trigger_data lock. */ rtnl_lock(); + if (device_name[0]) { + new_dev = dev_get_by_name(&init_net, device_name); + if (new_dev) + netdev_lock_ops(new_dev); + } mutex_lock(&trigger_data->lock); - if (trigger_data->net_dev) { - dev_put(trigger_data->net_dev); - trigger_data->net_dev = NULL; - } - - memcpy(trigger_data->device_name, name, size); - trigger_data->device_name[size] = 0; - if (size > 0 && trigger_data->device_name[size - 1] == '\n') - trigger_data->device_name[size - 1] = 0; - - if (trigger_data->device_name[0] != 0) - trigger_data->net_dev = - dev_get_by_name(&init_net, trigger_data->device_name); + dev_put(trigger_data->net_dev); + trigger_data->net_dev = new_dev; + strscpy(trigger_data->device_name, device_name); trigger_data->carrier_link_up = false; trigger_data->link_speed = SPEED_UNKNOWN; @@ -298,6 +301,8 @@ static int set_device_name(struct led_netdev_data *trigger_data, set_baseline_state(trigger_data); mutex_unlock(&trigger_data->lock); + if (new_dev) + netdev_unlock_ops(new_dev); rtnl_unlock(); return 0; From af1737dab07c861c3de4576f19dcfbd8f2329eb8 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:39 -0700 Subject: [PATCH 10/11] scsi: fcoe: don't recurse on the netdev's ops lock fcoe_link_speed_update() calls __ethtool_get_link_ksettings() on the lport's netdev, which will soon take the dev's ops lock. Some notifier callers already arrive with this lock held. Switch to netif_get_link_ksettings() and adjust the explicit call sites to take the netdev lock explicitly. Within fcoe_device_notification() try to only query the link speed from notifiers which announce link state change (UP / CHANGE), DOWN / GOING_DOWN notifiers are slightly sketchy when it comes to ops locking right now, and the code already special-cases those by maintaining the local link_possible variable. Also take the lock in bnx2fc_net_config(), even though I think that bnx2fc call sites are largely irrelevant since it's not an ops-locked driver. Link: https://patch.msgid.link/20260603012840.2254293-11-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 3 +++ drivers/scsi/fcoe/fcoe.c | 6 ++++-- drivers/scsi/fcoe/fcoe_transport.c | 4 +++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index 26e0ff380860..c95b084cad69 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -17,6 +17,7 @@ #include "bnx2fc.h" #include +#include static struct list_head adapter_list; static struct list_head if_list; @@ -815,7 +816,9 @@ static int bnx2fc_net_config(struct fc_lport *lport, struct net_device *netdev) port->fcoe_pending_queue_active = 0; timer_setup(&port->timer, fcoe_queue_timer, 0); + netdev_lock_ops(netdev); fcoe_link_speed_update(lport); + netdev_unlock_ops(netdev); if (!lport->vport) { if (fcoe_get_wwn(netdev, &wwnn, NETDEV_FCOE_WWNN)) diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c index 534596c6d76c..438ac7c3a9e3 100644 --- a/drivers/scsi/fcoe/fcoe.c +++ b/drivers/scsi/fcoe/fcoe.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -737,7 +738,9 @@ static int fcoe_netdev_config(struct fc_lport *lport, struct net_device *netdev) port->fcoe_pending_queue_active = 0; timer_setup(&port->timer, fcoe_queue_timer, 0); + netdev_lock_ops(netdev); fcoe_link_speed_update(lport); + netdev_unlock_ops(netdev); if (!lport->vport) { if (fcoe_get_wwn(netdev, &wwnn, NETDEV_FCOE_WWNN)) @@ -1841,6 +1844,7 @@ static int fcoe_device_notification(struct notifier_block *notifier, break; case NETDEV_UP: case NETDEV_CHANGE: + fcoe_link_speed_update(lport); break; case NETDEV_CHANGEMTU: if (netdev->fcoe_mtu) @@ -1871,8 +1875,6 @@ static int fcoe_device_notification(struct notifier_block *notifier, "from netdev netlink\n", event); } - fcoe_link_speed_update(lport); - cdev = fcoe_ctlr_to_ctlr_dev(ctlr); if (link_possible && !fcoe_link_ok(lport)) { diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c index 88d85fc9a52a..2bbb9a38e61d 100644 --- a/drivers/scsi/fcoe/fcoe_transport.c +++ b/drivers/scsi/fcoe/fcoe_transport.c @@ -111,6 +111,8 @@ static inline u32 eth2fc_speed(u32 eth_port_speed) * fcoe_link_speed_update() - Update the supported and actual link speeds * @lport: The local port to update speeds for * + * Caller must hold the netdev's ops lock. + * * Returns: 0 if the ethtool query was successful * -1 if the ethtool query failed */ @@ -119,7 +121,7 @@ int fcoe_link_speed_update(struct fc_lport *lport) struct net_device *netdev = fcoe_get_netdev(lport); struct ethtool_link_ksettings ecmd; - if (!__ethtool_get_link_ksettings(netdev, &ecmd)) { + if (!netif_get_link_ksettings(netdev, &ecmd)) { lport->link_supported_speeds &= ~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT | FC_PORTSPEED_20GBIT | From 9f275c2e9020a68b71a3c3bbe5d96e7c3546eee1 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Tue, 2 Jun 2026 18:28:40 -0700 Subject: [PATCH 11/11] net: ethtool: make sure __ethtool_get_link_ksettings() is ops-locked All drivers which may call *_get_link_ksettings() on ops-locked devices from paths already holding the ops lock are ready now. Make __ethtool_get_link_ksettings() take the ops lock, and assert that it's held in netif_get_link_ksettings(). Reviewed-by: Nicolai Buchwitz Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20260603012840.2254293-12-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/ioctl.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 49da873b673d..a4b0cbae4063 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -439,7 +439,7 @@ struct ethtool_link_usettings { int netif_get_link_ksettings(struct net_device *dev, struct ethtool_link_ksettings *link_ksettings) { - /* once callers fixed - assert ops locked */ + netdev_assert_locked_ops_compat(dev); if (!dev->ethtool_ops->get_link_ksettings) return -EOPNOTSUPP; @@ -456,10 +456,14 @@ EXPORT_SYMBOL(netif_get_link_ksettings); int __ethtool_get_link_ksettings(struct net_device *dev, struct ethtool_link_ksettings *link_ksettings) { + int ret; + ASSERT_RTNL(); - /* once callers fixed - take the ops lock around this call */ - return netif_get_link_ksettings(dev, link_ksettings); + netdev_lock_ops(dev); + ret = netif_get_link_ksettings(dev, link_ksettings); + netdev_unlock_ops(dev); + return ret; } EXPORT_SYMBOL(__ethtool_get_link_ksettings);