]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
net: ethtool: optionally skip rtnl_lock on Netlink path for GET ops
authorJakub Kicinski <kuba@kernel.org>
Fri, 5 Jun 2026 00:29:04 +0000 (17:29 -0700)
committerJakub Kicinski <kuba@kernel.org>
Tue, 9 Jun 2026 17:13:04 +0000 (10:13 -0700)
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 <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-5-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/ethernet/google/gve/gve_ethtool.c
drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
include/linux/ethtool.h
net/ethtool/common.h
net/ethtool/mm.c
net/ethtool/netlink.c

index dc2213b5ce2464bc88e7b91dc6e21873bd50831c..7cd43e082b2a44b50129ad9ba6bb0e8c88abb00b 100644 (file)
@@ -4,7 +4,7 @@
  * Copyright (C) 2015-2024 Google LLC
  */
 
-#include <linux/rtnetlink.h>
+#include <net/netdev_lock.h>
 #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);
index f14de23668544d8f17a5557433e74f6c2b3040bd..a2c16d5993897d9b504e8c23ecb327c75b3e7d92 100644 (file)
@@ -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,
index f51346a6a6860130774b173224c491d2931c4abb..1da49161d36f1a3dd01e23ae99da945d596c777f 100644 (file)
@@ -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 *);
index 1609cf4e53ebbd74026cba4fc35dcb1e467e0ed9..391c41ca56beebaab1bd5d77d5fb7c89ba47f5cb 100644 (file)
@@ -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
index 29bbbc149375d467a6a40c3f26dd93892b3809e8..2d10e2a1393f7344e863c4e57b6cddea5d352101 100644 (file)
@@ -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)
 {
index afafed738584121c9d2bdf398622645553c1d23c..2c30eb1f466642c7fb1f655584f9e0d99b35bcde 100644 (file)
@@ -7,6 +7,7 @@
 #include <linux/phy_link_topology.h>
 #include <linux/pm_runtime.h>
 
+#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);