[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7a18d0588e8596ad9cc83234488bebe22ba3d328.1681236653.git.ecree.xilinx@gmail.com>
Date: Tue, 11 Apr 2023 19:26:12 +0100
From: <edward.cree@....com>
To: <linux-net-drivers@....com>, <davem@...emloft.net>,
<kuba@...nel.org>, <pabeni@...hat.com>, <edumazet@...gle.com>
CC: Edward Cree <ecree.xilinx@...il.com>, <netdev@...r.kernel.org>,
<habetsm.xilinx@...il.com>, <sudheer.mogilappagari@...el.com>
Subject: [RFC PATCH v2 net-next 4/7] net: ethtool: let the core choose RSS context IDs
From: Edward Cree <ecree.xilinx@...il.com>
Add a new API to create/modify/remove RSS contexts, that passes in the
newly-chosen context ID (not as a pointer) rather than leaving the
driver to choose it on create. Also pass in the ctx, allowing drivers
to easily use its private data area to store their hardware-specific
state.
Keep the existing .set_rxfh_context API for now as a fallback, but
deprecate it.
Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
---
Changes in v2:
* split the new API into create/modify/remove ops (kuba). Also means we
don't need to rename the old API and touch legacy drivers
* squash patch "net: ethtool: pass ctx_priv and create into .set_rxfh_context"
---
include/linux/ethtool.h | 40 ++++++++++++++++++++++--
net/core/dev.c | 11 +++++--
net/ethtool/ioctl.c | 67 ++++++++++++++++++++++++++++++++---------
3 files changed, 97 insertions(+), 21 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 710d6a985347..12ed3b79be68 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -747,10 +747,33 @@ struct ethtool_mm_stats {
* @get_rxfh_context: Get the contents of the RX flow hash indirection table,
* hash key, and/or hash function assiciated to the given rss context.
* Returns a negative error code or zero.
- * @set_rxfh_context: Create, remove and configure RSS contexts. Allows setting
+ * @create_rxfh_context: Create a new RSS context with the specified RX flow
+ * hash indirection table, hash key, and hash function.
+ * Arguments which are set to %NULL or zero will be populated to
+ * appropriate defaults by the driver.
+ * The &struct ethtool_rxfh_context for this context is passed in @ctx;
+ * note that the indir table, hkey and hfunc are not yet populated as
+ * of this call. The driver does not need to update these; the core
+ * will do so if this op succeeds.
+ * If the driver provides this method, it must also provide
+ * @modify_rxfh_context and @remove_rxfh_context.
+ * Returns a negative error code or zero.
+ * @modify_rxfh_context: Reconfigure the specified RSS context. Allows setting
* the contents of the RX flow hash indirection table, hash key, and/or
- * hash function associated to the given context. Arguments which are set
- * to %NULL or zero will remain unchanged.
+ * hash function associated with the given context.
+ * Arguments which are set to %NULL or zero will remain unchanged.
+ * The &struct ethtool_rxfh_context for this context is passed in @ctx;
+ * note that it will still contain the *old* settings. The driver does
+ * not need to update these; the core will do so if this op succeeds.
+ * Returns a negative error code or zero. An error code must be returned
+ * if at least one unsupported change was requested.
+ * @remove_rxfh_context: Remove the specified RSS context.
+ * The &struct ethtool_rxfh_context for this context is passed in @ctx.
+ * Returns a negative error code or zero.
+ * @set_rxfh_context: Deprecated API to create, remove and configure RSS
+ * contexts. Allows setting the contents of the RX flow hash indirection
+ * table, hash key, and/or hash function associated to the given context.
+ * Arguments which are set to %NULL or zero will remain unchanged.
* Returns a negative error code or zero. An error code must be returned
* if at least one unsupported change was requested.
* @get_channels: Get number of channels.
@@ -900,6 +923,17 @@ struct ethtool_ops {
const u8 *key, const u8 hfunc);
int (*get_rxfh_context)(struct net_device *, u32 *indir, u8 *key,
u8 *hfunc, u32 rss_context);
+ int (*create_rxfh_context)(struct net_device *,
+ struct ethtool_rxfh_context *ctx,
+ const u32 *indir, const u8 *key,
+ const u8 hfunc, u32 rss_context);
+ int (*modify_rxfh_context)(struct net_device *,
+ struct ethtool_rxfh_context *ctx,
+ const u32 *indir, const u8 *key,
+ const u8 hfunc, u32 rss_context);
+ int (*remove_rxfh_context)(struct net_device *,
+ struct ethtool_rxfh_context *ctx,
+ u32 rss_context);
int (*set_rxfh_context)(struct net_device *, const u32 *indir,
const u8 *key, const u8 hfunc,
u32 *rss_context, bool delete);
diff --git a/net/core/dev.c b/net/core/dev.c
index c9ed9f6ea695..4feb58b0beb3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10789,15 +10789,20 @@ static void netdev_rss_contexts_free(struct net_device *dev)
struct ethtool_rxfh_context *ctx;
u32 context;
- if (!dev->ethtool_ops->set_rxfh_context)
+ if (!dev->ethtool_ops->create_rxfh_context &&
+ !dev->ethtool_ops->set_rxfh_context)
return;
idr_for_each_entry(&dev->ethtool->rss_ctx, ctx, context) {
u32 *indir = ethtool_rxfh_context_indir(ctx);
u8 *key = ethtool_rxfh_context_key(ctx);
idr_remove(&dev->ethtool->rss_ctx, context);
- dev->ethtool_ops->set_rxfh_context(dev, indir, key, ctx->hfunc,
- &context, true);
+ if (dev->ethtool_ops->create_rxfh_context)
+ dev->ethtool_ops->remove_rxfh_context(dev, ctx, context);
+ else
+ dev->ethtool_ops->set_rxfh_context(dev, indir, key,
+ ctx->hfunc,
+ &context, true);
kfree(ctx);
}
}
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 9f9f8ba9c0f6..20154d6159a1 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1273,7 +1273,8 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
if (rxfh.rsvd8[0] || rxfh.rsvd8[1] || rxfh.rsvd8[2] || rxfh.rsvd32)
return -EINVAL;
/* Most drivers don't handle rss_context, check it's 0 as well */
- if (rxfh.rss_context && !ops->set_rxfh_context)
+ if (rxfh.rss_context && !(ops->create_rxfh_context ||
+ ops->set_rxfh_context))
return -EOPNOTSUPP;
create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;
@@ -1348,8 +1349,28 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
}
ctx->indir_size = dev_indir_size;
ctx->key_size = dev_key_size;
- ctx->hfunc = rxfh.hfunc;
ctx->priv_size = ops->rxfh_priv_size;
+ /* Initialise to an empty context */
+ ctx->indir_no_change = ctx->key_no_change = 1;
+ ctx->hfunc = ETH_RSS_HASH_NO_CHANGE;
+ if (ops->create_rxfh_context) {
+ int ctx_id;
+
+ /* driver uses new API, core allocates ID */
+ /* if rss_ctx_max_id is not specified (left as 0), it is
+ * treated as INT_MAX + 1 by idr_alloc
+ */
+ ctx_id = idr_alloc(&dev->ethtool->rss_ctx, ctx, 1,
+ dev->ethtool->rss_ctx_max_id,
+ GFP_KERNEL_ACCOUNT);
+ /* 0 is not allowed, so treat it like an error here */
+ if (ctx_id <= 0) {
+ kfree(ctx);
+ ret = -ENOMEM;
+ goto out;
+ }
+ rxfh.rss_context = ctx_id;
+ }
} else if (rxfh.rss_context) {
ctx = idr_find(&dev->ethtool->rss_ctx, rxfh.rss_context);
if (!ctx) {
@@ -1358,13 +1379,35 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
}
}
- if (rxfh.rss_context)
- ret = ops->set_rxfh_context(dev, indir, hkey, rxfh.hfunc,
- &rxfh.rss_context, delete);
- else
+ if (rxfh.rss_context) {
+ if (ops->create_rxfh_context) {
+ if (create)
+ ret = ops->create_rxfh_context(dev, ctx, indir,
+ hkey, rxfh.hfunc,
+ rxfh.rss_context);
+ else if (delete)
+ ret = ops->remove_rxfh_context(dev, ctx,
+ rxfh.rss_context);
+ else
+ ret = ops->modify_rxfh_context(dev, ctx, indir,
+ hkey, rxfh.hfunc,
+ rxfh.rss_context);
+ } else {
+ ret = ops->set_rxfh_context(dev, indir, hkey,
+ rxfh.hfunc,
+ &rxfh.rss_context, delete);
+ }
+ } else {
ret = ops->set_rxfh(dev, indir, hkey, rxfh.hfunc);
- if (ret)
+ }
+ if (ret) {
+ if (create && ops->create_rxfh_context) {
+ /* failed to create, discard our new tracking entry */
+ idr_remove(&dev->ethtool->rss_ctx, rxfh.rss_context);
+ kfree(ctx);
+ }
goto out;
+ }
if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, rss_context),
&rxfh.rss_context, sizeof(rxfh.rss_context)))
@@ -1378,12 +1421,8 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
dev->priv_flags |= IFF_RXFH_CONFIGURED;
}
/* Update rss_ctx tracking */
- if (create) {
- /* Ideally this should happen before calling the driver,
- * so that we can fail more cleanly; but we don't have the
- * context ID until the driver picks it, so we have to
- * wait until after.
- */
+ if (create && !ops->create_rxfh_context) {
+ /* driver uses old API, it chose context ID */
if (WARN_ON(idr_find(&dev->ethtool->rss_ctx, rxfh.rss_context)))
/* context ID reused, our tracking is screwed */
goto out;
@@ -1391,8 +1430,6 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
WARN_ON(idr_alloc(&dev->ethtool->rss_ctx, ctx, rxfh.rss_context,
rxfh.rss_context + 1, GFP_KERNEL) !=
rxfh.rss_context);
- ctx->indir_no_change = rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE;
- ctx->key_no_change = !rxfh.key_size;
}
if (delete) {
WARN_ON(idr_remove(&dev->ethtool->rss_ctx, rxfh.rss_context) != ctx);
Powered by blists - more mailing lists