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>
* 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"
int ring;
int i, j;
- ASSERT_RTNL();
+ netdev_assert_locked(netdev);
priv = netdev_priv(netdev);
num_tx_queues = gve_num_tx_queues(priv);
.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,
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_*.
* @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
*
* 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-
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 *);
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
};
/* 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)
{
#include <linux/phy_link_topology.h>
#include <linux/pm_runtime.h>
+#include "common.h"
#include "module_fw.h"
#include "netlink.h"
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;
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;
const struct ethnl_dump_ctx *ctx,
const struct genl_info *info)
{
+ bool need_rtnl;
void *ehdr;
int ret;
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);