From 8845484367dade6811bbc3c0c1d66a2a0721c3c0 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:01 -0700 Subject: [PATCH 01/12] net: ethtool: serialize broadcast notification sequence allocation ethnl_bcast_seq is a global counter stamped into the nlmsg_seq field of every multicast notification, allowing userspace to detect dropped messages. Today the ordering is achieved by using rtnl_lock(). Moving forward we will want ethtool ops to run under just the netdev instance lock so to establish ordering we need a separate lock for notifications. With the netdev instance locks operations on different devices may bypass each other but the expectation is that it should not matter. What we need to prevent is: - notification IDs getting out of order - operations on one device getting out of order For simplicity defer allocating the ID of the notification right before the notification is delivered. This removes the need for special handling in ethnl_rss_create_send_ntf(). Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/netlink.c | 28 +++++++++++++++++----------- net/ethtool/netlink.h | 1 - net/ethtool/rss.c | 1 - 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index 25e22c48060a..c4054a9795ff 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -13,6 +13,12 @@ static struct genl_family ethtool_genl_family; static bool ethnl_ok __read_mostly; + +/* Serializes broadcast notification sequence allocation with the multicast + * send, so that userspace observes nlmsg_seq monotonic in receive order + * regardless of which lock the caller holds (rtnl or instance lock). + */ +static DEFINE_MUTEX(ethnl_bcast_lock); static u32 ethnl_bcast_seq; #define ETHTOOL_FLAGS_BASIC (ETHTOOL_FLAG_COMPACT_BITSETS | \ @@ -82,12 +88,6 @@ static void ethnl_sock_priv_destroy(void *priv) } } -u32 ethnl_bcast_seq_next(void) -{ - ASSERT_RTNL(); - return ++ethnl_bcast_seq; -} - int ethnl_ops_begin(struct net_device *dev) { int ret; @@ -329,8 +329,7 @@ void *ethnl_dump_put(struct sk_buff *skb, struct netlink_callback *cb, u8 cmd) void *ethnl_bcastmsg_put(struct sk_buff *skb, u8 cmd) { - return genlmsg_put(skb, 0, ++ethnl_bcast_seq, ðtool_genl_family, 0, - cmd); + return genlmsg_put(skb, 0, 0, ðtool_genl_family, 0, cmd); } void *ethnl_unicast_put(struct sk_buff *skb, u32 portid, u32 seq, u8 cmd) @@ -340,8 +339,15 @@ void *ethnl_unicast_put(struct sk_buff *skb, u32 portid, u32 seq, u8 cmd) int ethnl_multicast(struct sk_buff *skb, struct net_device *dev) { - return genlmsg_multicast_netns(ðtool_genl_family, dev_net(dev), skb, - 0, ETHNL_MCGRP_MONITOR, GFP_KERNEL); + struct nlmsghdr *nlh = nlmsg_hdr(skb); + int ret; + + mutex_lock(ðnl_bcast_lock); + nlh->nlmsg_seq = ++ethnl_bcast_seq; + ret = genlmsg_multicast_netns(ðtool_genl_family, dev_net(dev), skb, + 0, ETHNL_MCGRP_MONITOR, GFP_KERNEL); + mutex_unlock(ðnl_bcast_lock); + return ret; } /* GET request helpers */ @@ -1081,7 +1087,7 @@ void ethnl_notify(struct net_device *dev, unsigned int cmd, { if (unlikely(!ethnl_ok)) return; - ASSERT_RTNL(); + netdev_assert_locked_ops_compat(dev); if (likely(cmd < ARRAY_SIZE(ethnl_notify_handlers) && ethnl_notify_handlers[cmd])) diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index 674c9c19529b..f94aaa66379c 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -10,7 +10,6 @@ struct ethnl_req_info; -u32 ethnl_bcast_seq_next(void); int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info, const struct nlattr *nest, struct net *net, struct netlink_ext_ack *extack, diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index 53792f53f922..65bad23d5c59 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -995,7 +995,6 @@ ethnl_rss_create_send_ntf(const struct sk_buff *rsp, struct net_device *dev) nlh = nlmsg_hdr(ntf); /* Convert the reply into a notification */ nlh->nlmsg_pid = 0; - nlh->nlmsg_seq = ethnl_bcast_seq_next(); genl_hdr = nlmsg_data(nlh); genl_hdr->cmd = ETHTOOL_MSG_RSS_CREATE_NTF; From ded86da4bbb78cad74cecc368fee3ae3a296e2ca Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:02 -0700 Subject: [PATCH 02/12] net: ethtool: relax ethnl_req_get_phydev() locking assertion phydev <> netdev linking and lifecycle depends on rtnl_lock. We want to switch to instance locks for most ethtool ops. Let's add an assert that ops locked devices don't use phydev today. If one does we can either opt the phy ops out of being purely ops locked, or do deeper surgery to make phy locking ops-compatible. I don't think there's any fundamental challenge to make that work. Reviewed-by: Nicolai Buchwitz Reviewed-by: Maxime Chevallier Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/phy/phy_link_topology.c | 8 ++++++++ include/linux/phy_link_topology.h | 5 +++++ net/ethtool/netlink.c | 6 ++++-- net/ethtool/netlink.h | 7 ++++--- net/ethtool/phy.c | 1 - 5 files changed, 21 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c index 1f1eb5d59b38..aed3b26c1674 100644 --- a/drivers/net/phy/phy_link_topology.c +++ b/drivers/net/phy/phy_link_topology.c @@ -10,6 +10,7 @@ #include #include #include +#include static int netdev_alloc_phy_link_topology(struct net_device *dev) { @@ -35,6 +36,13 @@ int phy_link_topo_add_phy(struct net_device *dev, struct phy_device_node *pdn; int ret; + /* ethtool ops may run without rtnl_lock, and rtnl_lock is what + * currently protects the PHY topology. No driver currently mixes + * the two, flag if someone tries. See also ethnl_req_get_phydev(). + */ + if (WARN_ON_ONCE(netdev_need_ops_lock(dev))) + return -EOPNOTSUPP; + if (!topo) { ret = netdev_alloc_phy_link_topology(dev); if (ret) diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h index 68a59e25821c..95575f68d5bc 100644 --- a/include/linux/phy_link_topology.h +++ b/include/linux/phy_link_topology.h @@ -36,6 +36,11 @@ struct phy_device_node { struct phy_device *phy; }; +static inline bool phy_link_topo_empty(struct net_device *dev) +{ + return !dev->link_topo; +} + #if IS_ENABLED(CONFIG_PHYLIB) int phy_link_topo_add_phy(struct net_device *dev, struct phy_device *phy, diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index c4054a9795ff..afafed738584 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -226,11 +226,13 @@ struct phy_device *ethnl_req_get_phydev(const struct ethnl_req_info *req_info, { struct phy_device *phydev; - ASSERT_RTNL(); - if (!req_info->dev) return NULL; + /* If there is no PHY in sight there's no need for assert locking */ + if (!phy_link_topo_empty(req_info->dev)) + ASSERT_RTNL(); + if (!req_info->phy_index) return req_info->dev->phydev; diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h index f94aaa66379c..4ca2eca2e94b 100644 --- a/net/ethtool/netlink.h +++ b/net/ethtool/netlink.h @@ -275,14 +275,15 @@ static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info) /** * ethnl_req_get_phydev() - Gets the phy_device targeted by this request, - * if any. Must be called under rntl_lock(). + * if any. * @req_info: The ethnl request to get the phy from. * @tb: The netlink attributes array, for error reporting. * @header: The netlink header index, used for error reporting. * @extack: The netlink extended ACK, for error reporting. * - * The caller must hold RTNL, until it's done interacting with the returned - * phy_device. + * If a phy_device is returned the caller must hold rtnl_lock when calling + * this function, and until it's done interacting with the returned phy_device. + * IOW caller must hold rtnl_lock unless they know netdev has no phy_device. * * Return: A phy_device pointer corresponding either to the passed phy_index * if one is provided. If not, the phy_device attached to the diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c index ddc6eab701ed..018b0412be86 100644 --- a/net/ethtool/phy.c +++ b/net/ethtool/phy.c @@ -78,7 +78,6 @@ static int phy_prepare_data(const struct ethnl_req_info *req_info, struct phy_device *phydev; int ret; - /* RTNL is held by the caller */ phydev = ethnl_req_get_phydev(req_info, tb, ETHTOOL_A_PHY_HEADER, info->extack); if (IS_ERR_OR_NULL(phydev)) From 97f51bf91b3afa8819fa10e9282e3f2328bb78e4 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:03 -0700 Subject: [PATCH 03/12] net: ethtool: make dev->hwprov ops-protected dev->hwprov tracks the active hwtstamp provider for the device. Make it ops protected (instance lock if the netdev driver opts into holding instance lock around callbacks, otherwise rtnl_lock). hwprov is written and read in: - drivers/net/phy/phy_device.c phydev and ops protection don't currently mix, add a comment - net/ethtool/ as of now holds both rtnl lock and ops lock, this one will soon only hold one lock or the other read in: - net/core/dev_ioctl.c holds both rtnl lock and ops lock - net/core/timestamping.c RCU reader The new netdev_ops_lock_dereference() helper does not have "compat" in the name. The name would be quite long and I think in this case it should be obvious that we need _a_ lock. netdev_lock_dereference() already exists and means dev->lock is always expected. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/phy/phy_device.c | 3 +++ drivers/net/phy/phy_link_topology.c | 4 +++- include/linux/netdevice.h | 3 +++ include/net/netdev_lock.h | 11 +++++++++++ net/core/dev_ioctl.c | 4 ++-- net/ethtool/tsconfig.c | 10 ++++++---- 6 files changed, 28 insertions(+), 7 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 3370eb822017..ea53e477465d 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1935,6 +1935,9 @@ void phy_detach(struct phy_device *phydev) if (dev) { struct hwtstamp_provider *hwprov; + /* hwprov may technically be protected by ops lock but + * not for devices with a phydev, see phy_link_topo_add_phy() + */ hwprov = rtnl_dereference(dev->hwprov); /* Disable timestamp if it is the one selected */ if (hwprov && hwprov->phydev == phydev) { diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c index aed3b26c1674..4134de7ae313 100644 --- a/drivers/net/phy/phy_link_topology.c +++ b/drivers/net/phy/phy_link_topology.c @@ -38,7 +38,9 @@ int phy_link_topo_add_phy(struct net_device *dev, /* ethtool ops may run without rtnl_lock, and rtnl_lock is what * currently protects the PHY topology. No driver currently mixes - * the two, flag if someone tries. See also ethnl_req_get_phydev(). + * the two, flag if someone tries. See also: + * - ethnl_req_get_phydev() + * - phy_detach() */ if (WARN_ON_ONCE(netdev_need_ops_lock(dev))) return -EOPNOTSUPP; diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 74507c006490..a8709d0cc8d4 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2583,6 +2583,9 @@ struct net_device { * Double protects: * @up, @moving_ns, @nd_net, @xdp_features * + * Ops protects: + * @hwprov + * * Double ops protects: * @real_num_rx_queues, @real_num_tx_queues * diff --git a/include/net/netdev_lock.h b/include/net/netdev_lock.h index d3daec4e93f3..9fb3e93857c3 100644 --- a/include/net/netdev_lock.h +++ b/include/net/netdev_lock.h @@ -102,6 +102,14 @@ static inline void netdev_unlock_ops_compat(struct net_device *dev) rtnl_unlock(); } +/* Matching "ops protected" category from netdevice.h */ +static inline int netdev_is_locked_ops_compat(const struct net_device *dev) +{ + if (netdev_need_ops_lock(dev)) + return lockdep_is_held(&dev->lock); + return lockdep_rtnl_is_held(); +} + static inline int netdev_lock_cmp_fn(const struct lockdep_map *a, const struct lockdep_map *b) { @@ -138,6 +146,9 @@ static inline int netdev_lock_cmp_fn(const struct lockdep_map *a, #define netdev_lock_dereference(p, dev) \ rcu_dereference_protected(p, lockdep_is_held(&(dev)->lock)) +#define netdev_ops_lock_dereference(p, dev) \ + rcu_dereference_protected(p, netdev_is_locked_ops_compat(dev)) + int netdev_debug_event(struct notifier_block *nb, unsigned long event, void *ptr); diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c index f3979b276090..a320e264eaaf 100644 --- a/net/core/dev_ioctl.c +++ b/net/core/dev_ioctl.c @@ -260,7 +260,7 @@ int dev_get_hwtstamp_phylib(struct net_device *dev, { struct hwtstamp_provider *hwprov; - hwprov = rtnl_dereference(dev->hwprov); + hwprov = netdev_ops_lock_dereference(dev->hwprov, dev); if (hwprov) { cfg->qualifier = hwprov->desc.qualifier; if (hwprov->source == HWTSTAMP_SOURCE_PHYLIB && @@ -337,7 +337,7 @@ int dev_set_hwtstamp_phylib(struct net_device *dev, bool phy_ts; int err; - hwprov = rtnl_dereference(dev->hwprov); + hwprov = netdev_ops_lock_dereference(dev->hwprov, dev); if (hwprov) { if (hwprov->source == HWTSTAMP_SOURCE_PHYLIB && hwprov->phydev) { diff --git a/net/ethtool/tsconfig.c b/net/ethtool/tsconfig.c index 664c3fe49b5b..24b64862011f 100644 --- a/net/ethtool/tsconfig.c +++ b/net/ethtool/tsconfig.c @@ -2,6 +2,7 @@ #include #include +#include #include "bitset.h" #include "common.h" @@ -57,7 +58,7 @@ static int tsconfig_prepare_data(const struct ethnl_req_info *req_base, data->hwtst_config.flags = cfg.flags; data->hwprov_desc.index = -1; - hwprov = rtnl_dereference(dev->hwprov); + hwprov = netdev_ops_lock_dereference(dev->hwprov, dev); if (hwprov) { data->hwprov_desc.index = hwprov->desc.index; data->hwprov_desc.qualifier = hwprov->desc.qualifier; @@ -213,7 +214,7 @@ static int tsconfig_send_reply(struct net_device *dev, struct genl_info *info) return -ENOMEM; } - ASSERT_RTNL(); + netdev_assert_locked_ops_compat(dev); reply_data->base.dev = dev; ret = tsconfig_prepare_data(&req_info->base, &reply_data->base, info); if (ret < 0) @@ -316,7 +317,7 @@ static int ethnl_set_tsconfig(struct ethnl_req_info *req_base, struct hwtstamp_provider_desc __hwprov_desc = {.index = -1}; struct hwtstamp_provider *__hwprov; - __hwprov = rtnl_dereference(dev->hwprov); + __hwprov = netdev_ops_lock_dereference(dev->hwprov, dev); if (__hwprov) { __hwprov_desc.index = __hwprov->desc.index; __hwprov_desc.qualifier = __hwprov->desc.qualifier; @@ -414,7 +415,8 @@ static int ethnl_set_tsconfig(struct ethnl_req_info *req_base, goto err_free_hwprov; /* Change the selected hwtstamp source */ - __hwprov = rcu_replace_pointer_rtnl(dev->hwprov, hwprov); + __hwprov = rcu_replace_pointer(dev->hwprov, hwprov, + netdev_is_locked_ops_compat(dev)); if (__hwprov) kfree_rcu(__hwprov, rcu_head); } From 45079e00133ee78fd216ccc4285534044ea69173 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:04 -0700 Subject: [PATCH 04/12] net: ethtool: optionally skip rtnl_lock on Netlink path for GET ops ethnl_default_doit() and ethnl_default_dump_one() are both used exclusively for GET callbacks (former to get info for a single device or get global strings). ops-locked devices don't need rtnl_lock for GET callbacks, stop taking it. Introduce an opt-out mechanism for devices which use phylink (fbnic) since phylink currently depends on rtnl_lock protection. Subsequent patches will add more exceptions, anyway. Practically the new helpers for judging if command needs rtnl_lock could also call netdev_need_ops_lock() but I find that it makes the code in the callers slightly less obvious. Add a helper for IOCTLs already, even tho it's unused so that we can keep them in sync as the series progresses. This is the first user-visible step of moving ethtool ops out from under rtnl. Subsequent patches do the same for SET ops, as well as the ioctl path. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-5-kuba@kernel.org Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/google/gve/gve_ethtool.c | 4 +- .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 2 + include/linux/ethtool.h | 18 +++++++- net/ethtool/common.h | 46 +++++++++++++++++++ net/ethtool/mm.c | 5 +- net/ethtool/netlink.c | 19 ++++++-- 6 files changed, 85 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c index dc2213b5ce24..7cd43e082b2a 100644 --- a/drivers/net/ethernet/google/gve/gve_ethtool.c +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c @@ -4,7 +4,7 @@ * Copyright (C) 2015-2024 Google LLC */ -#include +#include #include "gve.h" #include "gve_adminq.h" #include "gve_dqo.h" @@ -171,7 +171,7 @@ gve_get_ethtool_stats(struct net_device *netdev, int ring; int i, j; - ASSERT_RTNL(); + netdev_assert_locked(netdev); priv = netdev_priv(netdev); num_tx_queues = gve_num_tx_queues(priv); diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c index f14de2366854..a2c16d599389 100644 --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c @@ -2020,6 +2020,8 @@ static const struct ethtool_ops fbnic_ethtool_ops = { .supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT | ETHTOOL_RING_USE_HDS_THRS, .rxfh_max_num_contexts = FBNIC_RPC_RSS_TBL_COUNT, + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS | + ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM, .get_drvinfo = fbnic_get_drvinfo, .get_regs_len = fbnic_get_regs_len, .get_regs = fbnic_get_regs, diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index f51346a6a686..1da49161d36f 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -930,6 +930,13 @@ struct kernel_ethtool_ts_info { u32 rx_filters; }; +/* Bits for ethtool_ops::op_needs_rtnl + * LINKSETTINGS cover a number of commands, but in most cases we want to keep + * these bits separate, per GET and SET. GET is much easier to "unlock". + */ +#define ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS BIT(0) +#define ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM BIT(1) + /** * struct ethtool_ops - optional netdev operations * @supported_input_xfrm: supported types of input xfrm from %RXH_XFRM_*. @@ -956,6 +963,14 @@ struct kernel_ethtool_ts_info { * @supported_coalesce_params: supported types of interrupt coalescing. * @supported_ring_params: supported ring params. * @supported_hwtstamp_qualifiers: bitfield of supported hwtstamp qualifier. + * @op_needs_rtnl: mask of %ETHTOOL_OP_NEEDS_RTNL_* bits. + * For use with ops-locked drivers (ignored otherwise). Selects which + * ethtool callbacks driver needs to still be executed under rtnl_lock + * (in addition to the netdev instance lock). + * The following commonly used core APIs currently require rtnl_lock + * (this list may not be exhaustive): + * - phylink helpers (note that phydev is currently unsupported!) + * * @get_drvinfo: Report driver/device information. Modern drivers no * longer have to implement this callback. Most fields are * correctly filled in by the core using system information, or @@ -1155,7 +1170,7 @@ struct kernel_ethtool_ts_info { * * All operations are optional (i.e. the function pointer may be set * to %NULL) and callers must take this into account. Callers must - * hold the RTNL lock. + * hold the RTNL lock or netdev instance lock (see @op_needs_rtnl). * * See the structures used by these operations for further documentation. * Note that for all operations using a structure ending with a zero- @@ -1178,6 +1193,7 @@ struct ethtool_ops { u32 supported_coalesce_params; u32 supported_ring_params; u32 supported_hwtstamp_qualifiers; + u32 op_needs_rtnl; void (*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *); int (*get_regs_len)(struct net_device *); void (*get_regs)(struct net_device *, struct ethtool_regs *, void *); diff --git a/net/ethtool/common.h b/net/ethtool/common.h index 1609cf4e53eb..391c41ca56be 100644 --- a/net/ethtool/common.h +++ b/net/ethtool/common.h @@ -80,6 +80,52 @@ int ethtool_get_module_eeprom_call(struct net_device *dev, bool __ethtool_dev_mm_supported(struct net_device *dev); +/** + * ethtool_nl_msg_needs_rtnl() - does this Netlink cmd need rtnl_lock? + * @dev: target device + * @cmd: ETHTOOL_MSG_* Netlink command value + * + * Return: true if @cmd is a command for which @dev has opted-in to + * keeping rtnl_lock held across the call (via op_needs_rtnl). + */ +static inline bool +ethtool_nl_msg_needs_rtnl(const struct net_device *dev, u8 cmd) +{ + const struct ethtool_ops *ops = dev->ethtool_ops; + + switch (cmd) { + case ETHTOOL_MSG_LINKINFO_GET: + case ETHTOOL_MSG_LINKMODES_GET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS; + case ETHTOOL_MSG_PAUSE_GET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM; + } + return false; +} + +/** + * ethtool_ioctl_needs_rtnl() - does this legacy ioctl cmd need rtnl_lock? + * @dev: target device + * @ethcmd: ETHTOOL_* ioctl command value + * + * Return: true if @ethcmd is a command for which @dev has opted-in to + * keeping rtnl_lock held across the call (via op_needs_rtnl). + */ +static inline bool +ethtool_ioctl_needs_rtnl(const struct net_device *dev, u32 ethcmd) +{ + const struct ethtool_ops *ops = dev->ethtool_ops; + + switch (ethcmd) { + case ETHTOOL_GLINKSETTINGS: + case ETHTOOL_GSET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS; + case ETHTOOL_GPAUSEPARAM: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM; + } + return false; +} + #if IS_ENABLED(CONFIG_ETHTOOL_NETLINK) void ethtool_rss_notify(struct net_device *dev, u32 type, u32 rss_context); #else diff --git a/net/ethtool/mm.c b/net/ethtool/mm.c index 29bbbc149375..2d10e2a1393f 100644 --- a/net/ethtool/mm.c +++ b/net/ethtool/mm.c @@ -246,8 +246,9 @@ const struct ethnl_request_ops ethnl_mm_request_ops = { }; /* Returns whether a given device supports the MAC merge layer - * (has an eMAC and a pMAC). Must be called under rtnl_lock() and - * ethnl_ops_begin(). + * (has an eMAC and a pMAC). Must be called under whichever lock + * netdev_assert_locked_ops_compat() accepts (rtnl for traditional drivers, + * the netdev instance lock for ops-locked ones) and ethnl_ops_begin(). */ bool __ethtool_dev_mm_supported(struct net_device *dev) { diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index afafed738584..2c30eb1f4666 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -7,6 +7,7 @@ #include #include +#include "common.h" #include "module_fw.h" #include "netlink.h" @@ -509,6 +510,7 @@ static int ethnl_default_doit(struct sk_buff *skb, struct genl_info *info) struct ethnl_req_info *req_info = NULL; const u8 cmd = info->genlhdr->cmd; const struct ethnl_request_ops *ops; + bool need_rtnl = false; int hdr_len, reply_len; struct sk_buff *rskb; void *reply_payload; @@ -535,13 +537,17 @@ static int ethnl_default_doit(struct sk_buff *skb, struct genl_info *info) ethnl_init_reply_data(reply_data, ops, req_info->dev); if (req_info->dev) { - rtnl_lock(); + need_rtnl = !netdev_need_ops_lock(req_info->dev) || + ethtool_nl_msg_needs_rtnl(req_info->dev, cmd); + if (need_rtnl) + rtnl_lock(); netdev_lock_ops(req_info->dev); } ret = ops->prepare_data(req_info, reply_data, info); if (req_info->dev) { netdev_unlock_ops(req_info->dev); - rtnl_unlock(); + if (need_rtnl) + rtnl_unlock(); } if (ret < 0) goto err_dev; @@ -589,6 +595,7 @@ static int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev, const struct ethnl_dump_ctx *ctx, const struct genl_info *info) { + bool need_rtnl; void *ehdr; int ret; @@ -599,11 +606,15 @@ static int ethnl_default_dump_one(struct sk_buff *skb, struct net_device *dev, return -EMSGSIZE; ethnl_init_reply_data(ctx->reply_data, ctx->ops, dev); - rtnl_lock(); + need_rtnl = !netdev_need_ops_lock(dev) || + ethtool_nl_msg_needs_rtnl(dev, ctx->ops->request_cmd); + if (need_rtnl) + rtnl_lock(); netdev_lock_ops(dev); ret = ctx->ops->prepare_data(ctx->req_info, ctx->reply_data, info); netdev_unlock_ops(dev); - rtnl_unlock(); + if (need_rtnl) + rtnl_unlock(); if (ret < 0) goto out_cancel; ret = ethnl_fill_reply_header(skb, dev, ctx->ops->hdr_attr); From f9a3e05114b85d63452e7f9c172b53d6a1736fe0 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:05 -0700 Subject: [PATCH 05/12] net: ethtool: optionally skip rtnl_lock on Netlink path for SET ops Make ethtool not take rtnl_lock for SET commands when operation is performed on an ops-locked driver. cfg/cfg_pending are now ops-locked, since only ethtool modifies them. Some SET driver callbacks will still need rtnl_lock, most notably those which may end up calling netdev_update_features() or the qdisc layer (via netif_set_real_num_tx_queues()). Let drivers selectively opt back into the rtnl_lock with a new bitfield in ops. We need two helpers since Netlink and ioctl cmds have different values. Keep the helpers side by side in common.h to make sure they get updated together, even tho they will only get called from ioctl.c and netlink.c. SET commands which don't use ethnl_default_set_doit() are converted by subsequent commits. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-6-kuba@kernel.org Signed-off-by: Jakub Kicinski --- .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 4 +++ drivers/net/ethernet/google/gve/gve_ethtool.c | 2 ++ .../ethernet/mellanox/mlx5/core/en_ethtool.c | 3 ++ .../net/ethernet/mellanox/mlx5/core/en_rep.c | 2 ++ .../mellanox/mlx5/core/ipoib/ethtool.c | 2 ++ .../net/ethernet/meta/fbnic/fbnic_ethtool.c | 5 +++- .../ethernet/microsoft/mana/mana_ethtool.c | 2 ++ drivers/net/netdevsim/ethtool.c | 1 + include/linux/ethtool.h | 10 ++++++- include/linux/netdevice.h | 2 +- net/ethtool/common.h | 30 +++++++++++++++++++ net/ethtool/netlink.c | 9 ++++-- 12 files changed, 67 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c index edafa79f636c..56d74a3c24b7 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c @@ -5729,6 +5729,10 @@ const struct ethtool_ops bnxt_ethtool_ops = { .rxfh_max_num_contexts = BNXT_MAX_ETH_RSS_CTX + 1, .rxfh_indir_space = BNXT_MAX_RSS_TABLE_ENTRIES_P5, .rxfh_priv_size = sizeof(struct bnxt_rss_ctx), + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | + ETHTOOL_OP_NEEDS_RTNL_SCOALESCE | + ETHTOOL_OP_NEEDS_RTNL_RSS, .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_MAX_FRAMES | ETHTOOL_COALESCE_USECS_IRQ | diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c index 7cd43e082b2a..7cc22916852f 100644 --- a/drivers/net/ethernet/google/gve/gve_ethtool.c +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c @@ -983,6 +983,8 @@ const struct ethtool_ops gve_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS, .supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT | ETHTOOL_RING_USE_RX_BUF_LEN, + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, .get_drvinfo = gve_get_drvinfo, .get_strings = gve_get_strings, .get_sset_count = gve_get_sset_count, diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c index 61993485e451..2f5b626ba33f 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c @@ -2719,6 +2719,9 @@ const struct ethtool_ops mlx5e_ethtool_ops = { .rxfh_per_ctx_fields = true, .rxfh_per_ctx_key = true, .rxfh_max_num_contexts = MLX5E_MAX_NUM_RSS, + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM | + ETHTOOL_OP_NEEDS_RTNL_SPFLAGS, .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_MAX_FRAMES | ETHTOOL_COALESCE_USE_ADAPTIVE | diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c index ba6c0f38cc73..1a8a19f980d3 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c @@ -418,6 +418,8 @@ static const struct ethtool_ops mlx5e_rep_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_MAX_FRAMES | ETHTOOL_COALESCE_USE_ADAPTIVE, + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, .get_drvinfo = mlx5e_rep_get_drvinfo, .get_link = ethtool_op_get_link, .get_strings = mlx5e_rep_get_strings, diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c index 3b2f54ca30a8..9b3b32408c64 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c @@ -285,6 +285,8 @@ const struct ethtool_ops mlx5i_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_USECS | ETHTOOL_COALESCE_MAX_FRAMES | ETHTOOL_COALESCE_USE_ADAPTIVE, + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, .get_drvinfo = mlx5i_get_drvinfo, .get_strings = mlx5i_get_strings, .get_sset_count = mlx5i_get_sset_count, diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c index a2c16d599389..cb34fc166ef9 100644 --- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c @@ -2021,7 +2021,10 @@ static const struct ethtool_ops fbnic_ethtool_ops = { ETHTOOL_RING_USE_HDS_THRS, .rxfh_max_num_contexts = FBNIC_RPC_RSS_TBL_COUNT, .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS | - ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM, + ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM | + ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM | + ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, .get_drvinfo = fbnic_get_drvinfo, .get_regs_len = fbnic_get_regs_len, .get_regs = fbnic_get_regs, diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c index 04350973e19e..55df44d728a0 100644 --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c @@ -575,6 +575,8 @@ static int mana_get_link_ksettings(struct net_device *ndev, const struct ethtool_ops mana_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_RX_CQE_FRAMES, + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS | + ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM, .get_ethtool_stats = mana_get_ethtool_stats, .get_sset_count = mana_get_sset_count, .get_strings = mana_get_strings, diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c index 36a201533aae..9350ba48eb81 100644 --- a/drivers/net/netdevsim/ethtool.c +++ b/drivers/net/netdevsim/ethtool.c @@ -209,6 +209,7 @@ static const struct ethtool_ops nsim_ethtool_ops = { .supported_coalesce_params = ETHTOOL_COALESCE_ALL_PARAMS, .supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT | ETHTOOL_RING_USE_HDS_THRS, + .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_SCHANNELS, .get_pause_stats = nsim_get_pause_stats, .get_pauseparam = nsim_get_pauseparam, .set_pauseparam = nsim_set_pauseparam, diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 1da49161d36f..74c8109b0cf3 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -935,7 +935,13 @@ struct kernel_ethtool_ts_info { * these bits separate, per GET and SET. GET is much easier to "unlock". */ #define ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS BIT(0) -#define ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM BIT(1) +#define ETHTOOL_OP_NEEDS_RTNL_SPFLAGS BIT(1) +#define ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM BIT(2) +#define ETHTOOL_OP_NEEDS_RTNL_SCHANNELS BIT(3) +#define ETHTOOL_OP_NEEDS_RTNL_SCOALESCE BIT(4) +#define ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM BIT(5) +#define ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM BIT(6) +#define ETHTOOL_OP_NEEDS_RTNL_RSS BIT(7) /** * struct ethtool_ops - optional netdev operations @@ -970,6 +976,8 @@ struct kernel_ethtool_ts_info { * The following commonly used core APIs currently require rtnl_lock * (this list may not be exhaustive): * - phylink helpers (note that phydev is currently unsupported!) + * - netdev_update_features() + * - netif_set_real_num_tx_queues() * * @get_drvinfo: Report driver/device information. Modern drivers no * longer have to implement this callback. Most fields are diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index a8709d0cc8d4..403b6d1c67f8 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2584,7 +2584,7 @@ struct net_device { * @up, @moving_ns, @nd_net, @xdp_features * * Ops protects: - * @hwprov + * @cfg, @cfg_pending, @hwprov * * Double ops protects: * @real_num_rx_queues, @real_num_tx_queues diff --git a/net/ethtool/common.h b/net/ethtool/common.h index 391c41ca56be..e3052972f953 100644 --- a/net/ethtool/common.h +++ b/net/ethtool/common.h @@ -95,10 +95,24 @@ ethtool_nl_msg_needs_rtnl(const struct net_device *dev, u8 cmd) switch (cmd) { case ETHTOOL_MSG_LINKINFO_GET: + case ETHTOOL_MSG_LINKINFO_SET: case ETHTOOL_MSG_LINKMODES_GET: + case ETHTOOL_MSG_LINKMODES_SET: return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS; + case ETHTOOL_MSG_PRIVFLAGS_SET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SPFLAGS; + case ETHTOOL_MSG_RINGS_SET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM; + case ETHTOOL_MSG_CHANNELS_SET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SCHANNELS; + case ETHTOOL_MSG_COALESCE_SET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SCOALESCE; case ETHTOOL_MSG_PAUSE_GET: return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM; + case ETHTOOL_MSG_PAUSE_SET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM; + case ETHTOOL_MSG_RSS_SET: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_RSS; } return false; } @@ -119,9 +133,25 @@ ethtool_ioctl_needs_rtnl(const struct net_device *dev, u32 ethcmd) switch (ethcmd) { case ETHTOOL_GLINKSETTINGS: case ETHTOOL_GSET: + case ETHTOOL_SLINKSETTINGS: + case ETHTOOL_SSET: return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS; + case ETHTOOL_SPFLAGS: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SPFLAGS; + case ETHTOOL_SRINGPARAM: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM; + case ETHTOOL_SCHANNELS: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SCHANNELS; + case ETHTOOL_SCOALESCE: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SCOALESCE; case ETHTOOL_GPAUSEPARAM: return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_GPAUSEPARAM; + case ETHTOOL_SPAUSEPARAM: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_SPAUSEPARAM; + case ETHTOOL_SRSSH: + case ETHTOOL_SRXFH: + case ETHTOOL_SRXFHINDIR: + return ops->op_needs_rtnl & ETHTOOL_OP_NEEDS_RTNL_RSS; } return false; } diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c index 2c30eb1f4666..1af395b54330 100644 --- a/net/ethtool/netlink.c +++ b/net/ethtool/netlink.c @@ -903,6 +903,7 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info) const u8 cmd = info->genlhdr->cmd; struct ethnl_req_info *req_info; struct net_device *dev; + bool need_rtnl; int ret; ops = ethnl_default_requests[cmd]; @@ -927,8 +928,11 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info) } dev = req_info->dev; + need_rtnl = !netdev_need_ops_lock(dev) || + ethtool_nl_msg_needs_rtnl(dev, cmd); - rtnl_lock(); + if (need_rtnl) + rtnl_lock(); netdev_lock_ops(dev); dev->cfg_pending = kmemdup(dev->cfg, sizeof(*dev->cfg), GFP_KERNEL_ACCOUNT); @@ -958,7 +962,8 @@ static int ethnl_default_set_doit(struct sk_buff *skb, struct genl_info *info) out_tie_cfg: dev->cfg_pending = dev->cfg; netdev_unlock_ops(dev); - rtnl_unlock(); + if (need_rtnl) + rtnl_unlock(); out_dev: ethnl_parse_header_dev_put(req_info); out_free_req: From f16013fde46d38e1a79216976445e1cd79922a72 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:06 -0700 Subject: [PATCH 06/12] net: ethtool: optionally skip rtnl_lock in cable test handlers Skip rtnl_lock in cable test handlers. This is really a noop since no ops locked device supports these. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-7-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/cabletest.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/net/ethtool/cabletest.c b/net/ethtool/cabletest.c index 8d375dac2a40..9c22d4c767c6 100644 --- a/net/ethtool/cabletest.c +++ b/net/ethtool/cabletest.c @@ -73,8 +73,7 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info) dev = req_info.dev; - rtnl_lock(); - netdev_lock_ops(dev); + netdev_lock_ops_compat(dev); phydev = ethnl_req_get_phydev(&req_info, tb, ETHTOOL_A_CABLE_TEST_HEADER, info->extack); @@ -101,8 +100,7 @@ int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info) ethnl_cable_test_started(phydev, ETHTOOL_MSG_CABLE_TEST_NTF); out_unlock: - netdev_unlock_ops(dev); - rtnl_unlock(); + netdev_unlock_ops_compat(dev); ethnl_parse_header_dev_put(&req_info); return ret; } @@ -342,8 +340,7 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info) if (ret) goto out_dev_put; - rtnl_lock(); - netdev_lock_ops(dev); + netdev_lock_ops_compat(dev); phydev = ethnl_req_get_phydev(&req_info, tb, ETHTOOL_A_CABLE_TEST_TDR_HEADER, info->extack); @@ -371,8 +368,7 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info) ETHTOOL_MSG_CABLE_TEST_TDR_NTF); out_unlock: - netdev_unlock_ops(dev); - rtnl_unlock(); + netdev_unlock_ops_compat(dev); out_dev_put: ethnl_parse_header_dev_put(&req_info); return ret; From 0c334c21f5a599b593a3495031cfcefb9fc47b96 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:07 -0700 Subject: [PATCH 07/12] net: ethtool: optionally skip rtnl_lock in ethnl_tsinfo_dumpit() ethnl_tsinfo_dumpit() iterates netdevs and per-netdev PHY topology calling ops->get_ts_info(). Switch to the "ops compat locking" helpers which take either rtnl_lock or instance lock, depending on what the device needs. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-8-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/tsinfo.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/net/ethtool/tsinfo.c b/net/ethtool/tsinfo.c index 14bf01e3b55c..c9b680a9cc3f 100644 --- a/net/ethtool/tsinfo.c +++ b/net/ethtool/tsinfo.c @@ -6,6 +6,7 @@ #include #include +#include "../core/dev.h" #include "bitset.h" #include "common.h" #include "netlink.h" @@ -473,28 +474,25 @@ int ethnl_tsinfo_dumpit(struct sk_buff *skb, struct netlink_callback *cb) { struct ethnl_tsinfo_dump_ctx *ctx = (void *)cb->ctx; struct net *net = sock_net(skb->sk); - struct net_device *dev; int ret = 0; - rtnl_lock(); if (ctx->req_info->base.dev) { - dev = ctx->req_info->base.dev; - netdev_lock_ops(dev); + struct net_device *dev = ctx->req_info->base.dev; + + netdev_lock_ops_compat(dev); ret = ethnl_tsinfo_dump_one_net_topo(skb, dev, cb); - netdev_unlock_ops(dev); - } else { - for_each_netdev_dump(net, dev, ctx->pos_ifindex) { - netdev_lock_ops(dev); - ret = ethnl_tsinfo_dump_one_net_topo(skb, dev, cb); - netdev_unlock_ops(dev); - if (ret < 0 && ret != -EOPNOTSUPP) - break; - ctx->pos_phyindex = 0; - ctx->netdev_dump_done = false; - ctx->pos_phcqualifier = HWTSTAMP_PROVIDER_QUALIFIER_PRECISE; - } + netdev_unlock_ops_compat(dev); + return ret; + } + + for_each_netdev_lock_ops_compat_scoped(net, dev, ctx->pos_ifindex) { + ret = ethnl_tsinfo_dump_one_net_topo(skb, dev, cb); + if (ret < 0 && ret != -EOPNOTSUPP) + break; + ctx->pos_phyindex = 0; + ctx->netdev_dump_done = false; + ctx->pos_phcqualifier = HWTSTAMP_PROVIDER_QUALIFIER_PRECISE; } - rtnl_unlock(); return ret; } From 2526717624ab477fb7df2bf1af6e1f257c72ee0f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:08 -0700 Subject: [PATCH 08/12] net: ethtool: optionally skip rtnl_lock in ethnl_act_module_fw_flash() Module firmware flashing reads SFF-8024 identifier bytes via .get_module_eeprom_by_page(). Other than that it modifies a bit in the netdev->ethtool struct. Both should be ops-locked at this point. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-9-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/linux/netdevice.h | 2 +- net/ethtool/module.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 403b6d1c67f8..9b876cd930d7 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2584,7 +2584,7 @@ struct net_device { * @up, @moving_ns, @nd_net, @xdp_features * * Ops protects: - * @cfg, @cfg_pending, @hwprov + * @cfg, @cfg_pending, @ethtool, @hwprov * * Double ops protects: * @real_num_rx_queues, @real_num_tx_queues diff --git a/net/ethtool/module.c b/net/ethtool/module.c index c3388e6d7ec8..9cf670e089f2 100644 --- a/net/ethtool/module.c +++ b/net/ethtool/module.c @@ -429,8 +429,7 @@ int ethnl_act_module_fw_flash(struct sk_buff *skb, struct genl_info *info) return ret; dev = req_info.dev; - rtnl_lock(); - netdev_lock_ops(dev); + netdev_lock_ops_compat(dev); ret = ethnl_ops_begin(dev); if (ret < 0) goto out_unlock; @@ -445,8 +444,7 @@ int ethnl_act_module_fw_flash(struct sk_buff *skb, struct genl_info *info) ethnl_ops_complete(dev); out_unlock: - netdev_unlock_ops(dev); - rtnl_unlock(); + netdev_unlock_ops_compat(dev); ethnl_parse_header_dev_put(&req_info); return ret; } From 732fa5ced1cc82bbdae3d05e5b364c7b6ecf04c3 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:09 -0700 Subject: [PATCH 09/12] net: ethtool: optionally skip rtnl_lock in RSS context handlers Skip rtnl_lock in RSS context handlers if device is ops-locked. Fairly trivial conversion. bnxt needed rtnl_lock for changing the main context but looks like additional contexts are fine without it. Note (for review bots?) that ethnl_ops_begin() checks whether the device is still registered. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-10-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/rss.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c index 65bad23d5c59..d8adc78e3775 100644 --- a/net/ethtool/rss.c +++ b/net/ethtool/rss.c @@ -2,6 +2,7 @@ #include +#include "../core/dev.h" #include "common.h" #include "netlink.h" @@ -468,21 +469,16 @@ int ethnl_rss_dumpit(struct sk_buff *skb, struct netlink_callback *cb) { struct rss_nl_dump_ctx *ctx = rss_dump_ctx(cb); struct net *net = sock_net(skb->sk); - struct net_device *dev; int ret = 0; - rtnl_lock(); - for_each_netdev_dump(net, dev, ctx->ifindex) { + for_each_netdev_lock_ops_compat_scoped(net, dev, ctx->ifindex) { if (ctx->match_ifindex && ctx->match_ifindex != ctx->ifindex) break; - netdev_lock_ops(dev); ret = rss_dump_one_dev(skb, cb, dev); - netdev_unlock_ops(dev); if (ret) break; } - rtnl_unlock(); return ret; } @@ -1037,8 +1033,7 @@ int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info) if (ret) goto exit_free_dev; - rtnl_lock(); - netdev_lock_ops(dev); + netdev_lock_ops_compat(dev); ret = ethnl_ops_begin(dev); if (ret < 0) @@ -1125,8 +1120,7 @@ int ethnl_rss_create_doit(struct sk_buff *skb, struct genl_info *info) exit_ops: ethnl_ops_complete(dev); exit_dev_unlock: - netdev_unlock_ops(dev); - rtnl_unlock(); + netdev_unlock_ops_compat(dev); exit_free_dev: ethnl_parse_header_dev_put(&req.base); exit_free_rsp: @@ -1179,8 +1173,7 @@ int ethnl_rss_delete_doit(struct sk_buff *skb, struct genl_info *info) goto exit_free_dev; } - rtnl_lock(); - netdev_lock_ops(dev); + netdev_lock_ops_compat(dev); ret = ethnl_ops_begin(dev); if (ret < 0) @@ -1210,8 +1203,7 @@ int ethnl_rss_delete_doit(struct sk_buff *skb, struct genl_info *info) mutex_unlock(&dev->ethtool->rss_lock); ethnl_ops_complete(dev); exit_dev_unlock: - netdev_unlock_ops(dev); - rtnl_unlock(); + netdev_unlock_ops_compat(dev); exit_free_dev: ethnl_parse_header_dev_put(&req); return ret; From f58a40d07be39823abf2fa1f73b6a839580404f0 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:10 -0700 Subject: [PATCH 10/12] net: ethtool: ioctl: concentrate the locking Add another layer of helper functions to make upcoming locking changes easier. Otherwise we'd need a pretty complex goto structure. netdev instance lock is now taken slightly sooner but that should not be an issue since rtnl_lock is already held, anyway. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-11-kuba@kernel.org Signed-off-by: Jakub Kicinski --- net/ethtool/ioctl.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index a4b0cbae4063..c32c099845e3 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -3258,18 +3258,14 @@ static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr) /* The main entry point in this file. Called from net/core/dev_ioctl.c */ static int -__dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr, - u32 ethcmd, struct ethtool_devlink_compat *devlink_state) +dev_ethtool_locked(struct net *net, struct net_device *dev, + void __user *useraddr, + u32 ethcmd, struct ethtool_devlink_compat *devlink_state) { - struct net_device *dev; u32 sub_cmd; int rc; netdev_features_t old_features; - dev = __dev_get_by_name(net, ifr->ifr_name); - if (!dev) - return -ENODEV; - if (ethcmd == ETHTOOL_PERQUEUE) { if (copy_from_user(&sub_cmd, useraddr + sizeof(ethcmd), sizeof(sub_cmd))) return -EFAULT; @@ -3320,7 +3316,6 @@ __dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr, return -EPERM; } - netdev_lock_ops(dev); if (dev->dev.parent) pm_runtime_get_sync(dev->dev.parent); @@ -3560,7 +3555,29 @@ __dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr, out: if (dev->dev.parent) pm_runtime_put(dev->dev.parent); + + return rc; +} + +static int +__dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr, + u32 ethcmd, struct ethtool_devlink_compat *devlink_state) +{ + struct net_device *dev; + int rc; + + rtnl_lock(); + dev = __dev_get_by_name(net, ifr->ifr_name); + if (!dev) { + rc = -ENODEV; + goto exit_rtnl_unlock; + } + + netdev_lock_ops(dev); + rc = dev_ethtool_locked(net, dev, useraddr, ethcmd, devlink_state); netdev_unlock_ops(dev); +exit_rtnl_unlock: + rtnl_unlock(); return rc; } @@ -3588,9 +3605,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr) break; } - rtnl_lock(); rc = __dev_ethtool(net, ifr, useraddr, ethcmd, state); - rtnl_unlock(); if (rc) goto exit_free; From f994752b11273ded7ec6b49ae5c1d319eef21743 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:11 -0700 Subject: [PATCH 11/12] net: ethtool: optionally skip rtnl_lock on IOCTL path Convert the IOCTL path similarly to how we converted Netlink. The device lookup gets a little hairy. We could take rtnl_lock unconditionally and drop it before calling the driver (this would avoid the reference + liveness check). But I think being able to make progress even if rtnl is dead-locked is quite useful. First extra concern is handling features. List all the cmds which modify features and always take rtnl_lock. We could fold this list into ethtool_ioctl_needs_rtnl() but seems cleaner to keep ethtool_ioctl_needs_rtnl() driver-related. If a driver changed features and we were not holding rtnl_lock - warn about it. It can only happen on buggy ops locked drivers (buggy because they should have set appropriate "I need rtnl for op X" bit). Second wrinkle is the PHY ID hack which drops the locks while sleeping. Convert its static "busy" variable which used to be protected by rtnl_lock to a field in struct ethtool_netdev_state. This feature is about identifying an adapter or a port within a system, so being able to blink multiple LEDs at the same time is likely not very useful in practice. But it's the simplest fix, we can add a mutex if someone thinks a system should only be ID'ing one port at a time. Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-12-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/linux/ethtool.h | 2 + net/ethtool/ioctl.c | 106 ++++++++++++++++++++++++++++++---------- 2 files changed, 82 insertions(+), 26 deletions(-) diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 74c8109b0cf3..da29017c757f 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -1375,6 +1375,7 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev, * within RTNL. * @rss_indir_user_size: Number of user provided entries for the default * (context 0) indirection table. + * @phys_id_busy: Loop blinking the device LED is running. * @wol_enabled: Wake-on-LAN is enabled * @module_fw_flash_in_progress: Module firmware flashing is in progress. */ @@ -1382,6 +1383,7 @@ struct ethtool_netdev_state { struct xarray rss_ctx; struct mutex rss_lock; u32 rss_indir_user_size; + unsigned phys_id_busy:1; unsigned wol_enabled:1; unsigned module_fw_flash_in_progress:1; }; diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index c32c099845e3..a7bff829b758 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -544,7 +544,7 @@ static int ethtool_get_link_ksettings(struct net_device *dev, int err = 0; struct ethtool_link_ksettings link_ksettings; - ASSERT_RTNL(); + netdev_assert_locked_ops_compat(dev); if (!dev->ethtool_ops->get_link_ksettings) return -EOPNOTSUPP; @@ -601,7 +601,7 @@ static int ethtool_set_link_ksettings(struct net_device *dev, struct ethtool_link_ksettings link_ksettings = {}; int err; - ASSERT_RTNL(); + netdev_assert_locked_ops_compat(dev); if (!dev->ethtool_ops->set_link_ksettings) return -EOPNOTSUPP; @@ -675,7 +675,7 @@ static int ethtool_get_settings(struct net_device *dev, void __user *useraddr) struct ethtool_cmd cmd; int err; - ASSERT_RTNL(); + netdev_assert_locked_ops_compat(dev); if (!dev->ethtool_ops->get_link_ksettings) return -EOPNOTSUPP; @@ -711,7 +711,7 @@ static int ethtool_set_settings(struct net_device *dev, void __user *useraddr) struct ethtool_cmd cmd; int ret; - ASSERT_RTNL(); + netdev_assert_locked_ops_compat(dev); if (copy_from_user(&cmd, useraddr, sizeof(cmd))) return -EFAULT; @@ -2452,10 +2452,10 @@ void ethtool_puts(u8 **data, const char *str) } EXPORT_SYMBOL(ethtool_puts); -static int ethtool_phys_id(struct net_device *dev, void __user *useraddr) +static int ethtool_phys_id(struct net_device *dev, void __user *useraddr, + bool has_rtnl_lock) { struct ethtool_value id; - static bool busy; const struct ethtool_ops *ops = dev->ethtool_ops; netdevice_tracker dev_tracker; int rc; @@ -2463,7 +2463,7 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr) if (!ops->set_phys_id) return -EOPNOTSUPP; - if (busy) + if (dev->ethtool->phys_id_busy) return -EBUSY; if (copy_from_user(&id, useraddr, sizeof(id))) @@ -2473,13 +2473,14 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr) if (rc < 0) return rc; - /* Drop the RTNL lock while waiting, but prevent reentry or + /* Drop the locks while waiting, but prevent reentry or * removal of the device. */ - busy = true; + dev->ethtool->phys_id_busy = true; netdev_hold(dev, &dev_tracker, GFP_KERNEL); netdev_unlock_ops(dev); - rtnl_unlock(); + if (has_rtnl_lock) + rtnl_unlock(); if (rc == 0) { /* Driver will handle this itself */ @@ -2492,22 +2493,25 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr) u64 i = 0; do { - rtnl_lock(); + if (has_rtnl_lock) + rtnl_lock(); netdev_lock_ops(dev); rc = ops->set_phys_id(dev, (i++ & 1) ? ETHTOOL_ID_OFF : ETHTOOL_ID_ON); netdev_unlock_ops(dev); - rtnl_unlock(); + if (has_rtnl_lock) + rtnl_unlock(); if (rc) break; schedule_timeout_interruptible(interval); } while (!signal_pending(current) && (!id.data || i < count)); } - rtnl_lock(); + if (has_rtnl_lock) + rtnl_lock(); netdev_lock_ops(dev); netdev_put(dev, &dev_tracker); - busy = false; + dev->ethtool->phys_id_busy = false; (void) ops->set_phys_id(dev, ETHTOOL_ID_INACTIVE); return rc; @@ -3260,7 +3264,8 @@ static int ethtool_set_fecparam(struct net_device *dev, void __user *useraddr) static int dev_ethtool_locked(struct net *net, struct net_device *dev, void __user *useraddr, - u32 ethcmd, struct ethtool_devlink_compat *devlink_state) + u32 ethcmd, struct ethtool_devlink_compat *devlink_state, + bool has_rtnl_lock) { u32 sub_cmd; int rc; @@ -3316,6 +3321,8 @@ dev_ethtool_locked(struct net *net, struct net_device *dev, return -EPERM; } + netdev_assert_locked_ops_compat(dev); + if (dev->dev.parent) pm_runtime_get_sync(dev->dev.parent); @@ -3403,7 +3410,7 @@ dev_ethtool_locked(struct net *net, struct net_device *dev, rc = ethtool_get_strings(dev, useraddr); break; case ETHTOOL_PHYS_ID: - rc = ethtool_phys_id(dev, useraddr); + rc = ethtool_phys_id(dev, useraddr, has_rtnl_lock); break; case ETHTOOL_GSTATS: rc = ethtool_get_stats(dev, useraddr); @@ -3550,8 +3557,20 @@ dev_ethtool_locked(struct net *net, struct net_device *dev, if (dev->ethtool_ops->complete) dev->ethtool_ops->complete(dev); - if (old_features != dev->features) - netdev_features_change(dev); + switch (ethcmd) { + case ETHTOOL_PHYS_ID: + /* Don't check features if operation drops the locks. + * Someone else may have changed features in parallel. + */ + break; + default: + if (old_features != dev->features) { + if (has_rtnl_lock) + netdev_features_change(dev); + else + netdev_WARN(dev, "ethtool cmd %u changed features without rtnl_lock", ethcmd); + } + } out: if (dev->dev.parent) pm_runtime_put(dev->dev.parent); @@ -3559,25 +3578,60 @@ dev_ethtool_locked(struct net *net, struct net_device *dev, return rc; } +/* Commands that may toggle dev->features in net/ethtool/ioctl.c and so + * call into __netdev_update_features(), which still requires rtnl_lock. + * Driver-decided SET commands that may chain into rtnl-only helpers are + * covered by ethtool_ioctl_needs_rtnl()/ETHTOOL_OP_NEEDS_RTNL_*. + */ +static bool ethtool_cmd_changes_features(u32 ethcmd) +{ + switch (ethcmd) { + case ETHTOOL_SFEATURES: + case ETHTOOL_SFLAGS: + case ETHTOOL_STXCSUM: + case ETHTOOL_SRXCSUM: + case ETHTOOL_SSG: + case ETHTOOL_STSO: + case ETHTOOL_SGSO: + case ETHTOOL_SGRO: + return true; + } + return false; +} + static int __dev_ethtool(struct net *net, struct ifreq *ifr, void __user *useraddr, u32 ethcmd, struct ethtool_devlink_compat *devlink_state) { + netdevice_tracker dev_tracker; struct net_device *dev; + bool need_rtnl; int rc; - rtnl_lock(); - dev = __dev_get_by_name(net, ifr->ifr_name); - if (!dev) { + dev = netdev_get_by_name(net, ifr->ifr_name, &dev_tracker, GFP_KERNEL); + if (!dev) + return -ENODEV; + + need_rtnl = !netdev_need_ops_lock(dev) || + ethtool_cmd_changes_features(ethcmd) || + ethtool_ioctl_needs_rtnl(dev, ethcmd); + if (need_rtnl) + rtnl_lock(); + netdev_lock_ops(dev); + if (dev->reg_state > NETREG_REGISTERED || + dev->moving_ns || !net_eq(dev_net(dev), net)) { rc = -ENODEV; - goto exit_rtnl_unlock; + goto exit_ops_unlock; } - netdev_lock_ops(dev); - rc = dev_ethtool_locked(net, dev, useraddr, ethcmd, devlink_state); + rc = dev_ethtool_locked(net, dev, useraddr, ethcmd, devlink_state, + need_rtnl); + +exit_ops_unlock: netdev_unlock_ops(dev); -exit_rtnl_unlock: - rtnl_unlock(); + if (need_rtnl) + rtnl_unlock(); + netdev_put(dev, &dev_tracker); return rc; } From b5125db8d0205048a3b9dafec7f8e2b0a702101f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 4 Jun 2026 17:29:12 -0700 Subject: [PATCH 12/12] docs: net: ethtool: document ops-locked drivers and op_needs_rtnl Catch up various bits of documentation after the locking changes. Reviewed-by: Nicolai Buchwitz Reviewed-by: Eric Dumazet Acked-by: Stanislav Fomichev Reviewed-by: Jacob Keller Link: https://patch.msgid.link/20260605002912.3456868-13-kuba@kernel.org Signed-off-by: Jakub Kicinski --- Documentation/networking/netdev-features.rst | 7 +++++++ Documentation/networking/netdevices.rst | 17 ++++++++++------- include/linux/ethtool.h | 10 ++++++++-- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/Documentation/networking/netdev-features.rst b/Documentation/networking/netdev-features.rst index 6293d47e5b09..f9e1f3921f53 100644 --- a/Documentation/networking/netdev-features.rst +++ b/Documentation/networking/netdev-features.rst @@ -76,6 +76,13 @@ netdev instance lock, that lock must be held as well. This should not be done from ndo_*_features callbacks. netdev->features should not be modified by driver except by means of ndo_fix_features callback. +For "ops locked" drivers (see Documentation/networking/netdevices.rst), +ethtool callbacks that may end up invoking netdev_update_features() must +opt back into rtnl_lock by setting the matching ETHTOOL_OP_NEEDS_RTNL_* +bit in ``ethtool_ops::op_needs_rtnl``. The ethtool core then keeps +rtnl_lock held across those SET callbacks so the contract above still +holds. + ndo_features_check is called for each skb before that skb is passed to ndo_start_xmit. Driver may perform any non-trivial checks (e.g. exact header geometry / length) and withdraw features like HW_CSUM or TSO, diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst index 8fc96975b3bd..fde601acd1d2 100644 --- a/Documentation/networking/netdevices.rst +++ b/Documentation/networking/netdevices.rst @@ -351,10 +351,6 @@ virtual and the physical device. To prevent deadlocks, the virtual device's lock must always be acquired before the physical device's (see ``netdev_nl_queue_create_doit``). -In the future, there will be an option for individual -drivers to opt out of using ``rtnl_lock`` and instead perform their control -operations directly under the netdev instance lock. - Device drivers are encouraged to rely on the instance lock where possible. For the (mostly software) drivers that need to interact with the core stack, @@ -375,9 +371,16 @@ the instance lock. struct ethtool_ops ------------------ -Similarly to ``ndos`` the instance lock is only held for select drivers. -For "ops locked" drivers all ethtool ops without exceptions should -be called under the instance lock. +For non-"ops locked" drivers ethtool_ops are executed under ``rtnl_lock``. + +For "ops locked" drivers, ``ethtool_ops``, unlike ``ndos``, run under +the instance lock **only**. Drivers may request that ``rtnl_lock`` +is held around specific operations (both SET and GET) by setting +appropriate bits in ``ethtool_ops::op_needs_rtnl`` (if the necessary +``ETHTOOL_OP_NEEDS_RTNL_*`` bit doesn't exist, just add it). +Commonly used core helpers which force drivers to selectively opt-in to +``rtnl_lock`` protection include ``netdev_update_features()``, +``netif_set_real_num_tx_queues()``, and phylink helpers. struct netdev_stat_ops ---------------------- diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index da29017c757f..1b834e2a522e 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -1177,8 +1177,14 @@ struct kernel_ethtool_ts_info { * @get_mm_stats: Query the 802.3 MAC Merge layer statistics. * * All operations are optional (i.e. the function pointer may be set - * to %NULL) and callers must take this into account. Callers must - * hold the RTNL lock or netdev instance lock (see @op_needs_rtnl). + * to %NULL) and callers must take this into account. + * + * For traditional drivers callers hold ``rtnl_lock`` across the call. + * For "ops locked" drivers (see Documentation/networking/netdevices.rst) + * callers instead hold the netdev instance lock (``netdev_lock_ops``); + * ``rtnl_lock`` is additionally held only for callbacks for which + * the driver opts in via the matching ``ETHTOOL_OP_NEEDS_RTNL_*`` bit + * in @op_needs_rtnl. * * See the structures used by these operations for further documentation. * Note that for all operations using a structure ending with a zero-