[RFC PATCH net-next 0/6] ethtool: track custom RSS contexts in the core

[RFC PATCH net-next 0/6] ethtool: track custom RSS contexts in the core

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Make the core responsible for tracking the set of custom RSS contexts,
 their IDs, indirection tables, hash keys, and hash functions; this
 lets us get rid of duplicative code in drivers, and will allow us to
 support netlink dumps later.

This series only moves the sfc EF10 driver over to the new API; if the
 design is approved of, I plan to add patches to convert the other
 drivers and remove the legacy API.  (However, I don't have hardware
 for the drivers besides sfc, so I won't be able to test those myself.)

Edward Cree (6):
  net: ethtool: attach an IDR of custom RSS contexts to a netdevice
  net: ethtool: record custom RSS contexts in the IDR
  net: ethtool: let the core choose RSS context IDs
  net: ethtool: pass ctx_priv and create into .set_rxfh_context
  net: ethtool: add a mutex protecting RSS contexts
  sfc: use new .set_rxfh_context API

 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |   2 +-
 .../marvell/octeontx2/nic/otx2_ethtool.c      |   2 +-
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  |   2 +-
 drivers/net/ethernet/sfc/ef10.c               |   2 +-
 drivers/net/ethernet/sfc/efx.c                |   2 +-
 drivers/net/ethernet/sfc/efx.h                |   2 +-
 drivers/net/ethernet/sfc/efx_common.c         |  10 +-
 drivers/net/ethernet/sfc/ethtool_common.c     |  96 ++++++-------
 drivers/net/ethernet/sfc/ethtool_common.h     |   4 +-
 drivers/net/ethernet/sfc/mcdi_filters.c       | 131 +++++++++---------
 drivers/net/ethernet/sfc/mcdi_filters.h       |   8 +-
 drivers/net/ethernet/sfc/net_driver.h         |  28 ++--
 drivers/net/ethernet/sfc/rx_common.c          |  64 ++-------
 drivers/net/ethernet/sfc/rx_common.h          |   8 +-
 drivers/net/ethernet/sfc/siena/ethtool.c      |   2 +-
 include/linux/ethtool.h                       |  66 ++++++++-
 include/linux/netdevice.h                     |   8 ++
 net/core/dev.c                                |  36 +++++
 net/ethtool/ioctl.c                           | 107 +++++++++++++-
 19 files changed, 359 insertions(+), 221 deletions(-)

[RFC PATCH net-next 1/6] net: ethtool: attach an IDR of custom RSS contexts to a netdevice

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Each context stores the RXFH settings (indir, key, and hfunc) as well
 as optionally some driver private data.
Delete any still-existing contexts at netdev unregister time.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 include/linux/ethtool.h   | 33 +++++++++++++++++++++++++++++++++
 include/linux/netdevice.h |  5 +++++
 net/core/dev.c            | 23 +++++++++++++++++++++++
 3 files changed, 61 insertions(+)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 798d35890118..1898f4446666 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -157,6 +157,39 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
 	return index % n_rx_rings;
 }
 
+/**
+ * struct ethtool_rxfh_context - a custom RSS context configuration
+ * @indir_size: Number of u32 entries in indirection table
+ * @key_size: Size of hash key, in bytes
+ * @hfunc: RSS hash function identifier.  One of the %ETH_RSS_HASH_*
+ * @priv_size: Size of driver private data, in bytes
+ */
+struct ethtool_rxfh_context {
+	u32 indir_size;
+	u32 key_size;
+	u8 hfunc;
+	u16 priv_size;
+	/* private: indirection table, hash key, and driver private data are
+	 * stored sequentially in @data area.  Use below helpers to access
+	 */
+	u8 data[];
+};
+
+static inline u32 *ethtool_rxfh_context_indir(struct ethtool_rxfh_context *ctx)
+{
+	return (u32 *)&ctx->data;
+}
+
+static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
+{
+	return (u8 *)(ethtool_rxfh_context_indir(ctx) + ctx->indir_size);
+}
+
+static inline void *ethtool_rxfh_context_priv(struct ethtool_rxfh_context *ctx)
+{
+	return ethtool_rxfh_context_key(ctx) + ctx->key_size;
+}
+
 /* declare a link mode bitmap */
 #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name)		\
 	DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a740be3bb911..91f7dad070bd 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2028,6 +2028,8 @@ enum netdev_ml_priv_type {
  *	@udp_tunnel_nic_info:	static structure describing the UDP tunnel
  *				offload capabilities of the device
  *	@udp_tunnel_nic:	UDP tunnel offload state
+ *	@rss_ctx_max_id:	maximum (exclusive) supported RSS context ID
+ *	@rss_ctx:	IDR storing custom RSS context state
  *	@xdp_state:		stores info on attached XDP BPF programs
  *
  *	@nested_level:	Used as a parameter of spin_lock_nested() of
@@ -2397,6 +2399,9 @@ struct net_device {
 	const struct udp_tunnel_nic_info	*udp_tunnel_nic_info;
 	struct udp_tunnel_nic	*udp_tunnel_nic;
 
+	u32			rss_ctx_max_id;
+	struct idr		rss_ctx;
+
 	/* protected by rtnl_lock */
 	struct bpf_xdp_entity	xdp_state[__MAX_XDP_MODE];
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 7ce5985be84b..d0a936d4e532 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9983,6 +9983,9 @@ int register_netdevice(struct net_device *dev)
 	if (ret)
 		return ret;
 
+	/* rss ctx ID 0 is reserved for the default context, start from 1 */
+	idr_init_base(&dev->rss_ctx, 1);
+
 	spin_lock_init(&dev->addr_list_lock);
 	netdev_set_addr_lockdep_class(dev);
 
@@ -10777,6 +10780,24 @@ void synchronize_net(void)
 }
 EXPORT_SYMBOL(synchronize_net);
 
+static void netdev_rss_contexts_free(struct net_device *dev)
+{
+	struct ethtool_rxfh_context *ctx;
+	u32 context;
+
+	if (!dev->ethtool_ops->set_rxfh_context)
+		return;
+	idr_for_each_entry(&dev->rss_ctx, ctx, context) {
+		u32 *indir = ethtool_rxfh_context_indir(ctx);
+		u8 *key = ethtool_rxfh_context_key(ctx);
+
+		idr_remove(&dev->rss_ctx, context);
+		dev->ethtool_ops->set_rxfh_context(dev, indir, key, ctx->hfunc,
+						   &context, true);
+		kfree(ctx);
+	}
+}
+
 /**
  *	unregister_netdevice_queue - remove device from the kernel
  *	@dev: device
@@ -10881,6 +10902,8 @@ void unregister_netdevice_many_notify(struct list_head *head,
 		netdev_name_node_alt_flush(dev);
 		netdev_name_node_free(dev->name_node);
 
+		netdev_rss_contexts_free(dev);
+
 		call_netdevice_notifiers(NETDEV_PRE_UNINIT, dev);
 
 		if (dev->netdev_ops->ndo_uninit)

[RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Since drivers are still choosing the context IDs, we have to force the
 IDR to use the ID they've chosen rather than picking one ourselves.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 include/linux/ethtool.h | 17 +++++++++++
 net/ethtool/ioctl.c     | 67 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 1898f4446666..a16580a8e9d7 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -163,12 +163,16 @@ static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
  * @key_size: Size of hash key, in bytes
  * @hfunc: RSS hash function identifier.  One of the %ETH_RSS_HASH_*
  * @priv_size: Size of driver private data, in bytes
+ * @indir_no_change: indir was not specified at create time
+ * @key_no_change: hkey was not specified at create time
  */
 struct ethtool_rxfh_context {
 	u32 indir_size;
 	u32 key_size;
 	u8 hfunc;
 	u16 priv_size;
+	u8 indir_no_change:1;
+	u8 key_no_change:1;
 	/* private: indirection table, hash key, and driver private data are
 	 * stored sequentially in @data area.  Use below helpers to access
 	 */
@@ -190,6 +194,16 @@ static inline void *ethtool_rxfh_context_priv(struct ethtool_rxfh_context *ctx)
 	return ethtool_rxfh_context_key(ctx) + ctx->key_size;
 }
 
+static inline size_t ethtool_rxfh_context_size(u32 indir_size, u32 key_size,
+					       u16 priv_size)
+{
+	size_t indir_bytes = array_size(indir_size, sizeof(u32));
+	size_t flex_len;
+
+	flex_len = size_add(size_add(indir_bytes, key_size), priv_size);
+	return struct_size((struct ethtool_rxfh_context *)0, data, flex_len);
+}
+
 /* declare a link mode bitmap */
 #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name)		\
 	DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)
@@ -727,6 +741,8 @@ struct ethtool_mm_stats {
  *	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_rxfh_priv_size: Get the size of the driver private data area the
+ *	core should allocate for an RSS context.
  * @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.
@@ -880,6 +896,7 @@ struct ethtool_ops {
 			    u8 *hfunc);
 	int	(*set_rxfh)(struct net_device *, const u32 *indir,
 			    const u8 *key, const u8 hfunc);
+	u16	(*get_rxfh_priv_size)(struct net_device *);
 	int	(*get_rxfh_context)(struct net_device *, u32 *indir, u8 *key,
 				    u8 *hfunc, u32 rss_context);
 	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 59adc4e6e9ee..c8f11ac343c9 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1248,14 +1248,15 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 {
 	int ret;
 	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct ethtool_rxfh_context *ctx;
 	struct ethtool_rxnfc rx_rings;
 	struct ethtool_rxfh rxfh;
-	u32 dev_indir_size = 0, dev_key_size = 0, i;
+	u32 dev_indir_size = 0, dev_key_size = 0, dev_priv_size = 0, i;
 	u32 *indir = NULL, indir_bytes = 0;
 	u8 *hkey = NULL;
 	u8 *rss_config;
 	u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]);
-	bool delete = false;
+	bool create = false, delete = false;
 
 	if (!ops->get_rxnfc || !ops->set_rxfh)
 		return -EOPNOTSUPP;
@@ -1274,6 +1275,9 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 	/* Most drivers don't handle rss_context, check it's 0 as well */
 	if (rxfh.rss_context && !ops->set_rxfh_context)
 		return -EOPNOTSUPP;
+	create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;
+	if (create && ops->get_rxfh_priv_size)
+		dev_priv_size = ops->get_rxfh_priv_size(dev);
 
 	/* If either indir, hash key or function is valid, proceed further.
 	 * Must request at least one change: indir size, hash key or function.
@@ -1331,6 +1335,31 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 		}
 	}
 
+	if (create) {
+		if (delete) {
+			ret = -EINVAL;
+			goto out;
+		}
+		ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
+							dev_key_size,
+							dev_priv_size),
+			      GFP_USER);
+		if (!ctx) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		ctx->indir_size = dev_indir_size;
+		ctx->key_size = dev_key_size;
+		ctx->hfunc = rxfh.hfunc;
+		ctx->priv_size = dev_priv_size;
+	} else if (rxfh.rss_context) {
+		ctx = idr_find(&dev->rss_ctx, rxfh.rss_context);
+		if (!ctx) {
+			ret = -ENOENT;
+			goto out;
+		}
+	}
+
 	if (rxfh.rss_context)
 		ret = ops->set_rxfh_context(dev, indir, hkey, rxfh.hfunc,
 					    &rxfh.rss_context, delete);
@@ -1350,6 +1379,40 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 		else if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
 			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 (WARN_ON(idr_find(&dev->rss_ctx, rxfh.rss_context)))
+			/* context ID reused, our tracking is screwed */
+			goto out;
+		/* Allocate the exact ID the driver gave us */
+		WARN_ON(idr_alloc(&dev->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->rss_ctx, rxfh.rss_context) != ctx);
+		kfree(ctx);
+	} else if (ctx) {
+		if (indir) {
+			for (i = 0; i < dev_indir_size; i++)
+				ethtool_rxfh_context_indir(ctx)[i] = indir[i];
+			ctx->indir_no_change = 0;
+		}
+		if (hkey) {
+			memcpy(ethtool_rxfh_context_key(ctx), hkey,
+			       dev_key_size);
+			ctx->key_no_change = 0;
+		}
+		if (rxfh.hfunc != ETH_RSS_HASH_NO_CHANGE)
+			ctx->hfunc = rxfh.hfunc;
+	}
 
 out:
 	kfree(rss_config);

[RFC PATCH net-next 3/6] net: ethtool: let the core choose RSS context IDs

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Rename the existing .set_rxfh_context API to .set_rxfh_context_old, and
 add a new .set_rxfh_context API that passes in the newly-chosen context
 ID (not as a pointer) rather than leaving the driver to choose it.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |  2 +-
 .../marvell/octeontx2/nic/otx2_ethtool.c      |  2 +-
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  |  2 +-
 drivers/net/ethernet/sfc/ef100_ethtool.c      |  2 +-
 drivers/net/ethernet/sfc/ethtool.c            |  2 +-
 drivers/net/ethernet/sfc/siena/ethtool.c      |  2 +-
 include/linux/ethtool.h                       |  8 ++-
 net/core/dev.c                                | 13 +++--
 net/ethtool/ioctl.c                           | 49 +++++++++++++------
 9 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index adc953611913..8d626c753f8e 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -5758,7 +5758,7 @@ static const struct ethtool_ops mvpp2_eth_tool_ops = {
 	.get_rxfh		= mvpp2_ethtool_get_rxfh,
 	.set_rxfh		= mvpp2_ethtool_set_rxfh,
 	.get_rxfh_context	= mvpp2_ethtool_get_rxfh_context,
-	.set_rxfh_context	= mvpp2_ethtool_set_rxfh_context,
+	.set_rxfh_context_old	= mvpp2_ethtool_set_rxfh_context,
 };
 
 /* Used for PPv2.1, or PPv2.2 with the old Device Tree binding that
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index 0f8d1a69139f..67310434cb18 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -1325,7 +1325,7 @@ static const struct ethtool_ops otx2_ethtool_ops = {
 	.get_rxfh		= otx2_get_rxfh,
 	.set_rxfh		= otx2_set_rxfh,
 	.get_rxfh_context	= otx2_get_rxfh_context,
-	.set_rxfh_context	= otx2_set_rxfh_context,
+	.set_rxfh_context_old	= otx2_set_rxfh_context,
 	.get_msglevel		= otx2_get_msglevel,
 	.set_msglevel		= otx2_set_msglevel,
 	.get_pauseparam		= otx2_get_pauseparam,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index 1f5a2110d31f..edf099d64fdb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -2425,7 +2425,7 @@ const struct ethtool_ops mlx5e_ethtool_ops = {
 	.get_rxfh          = mlx5e_get_rxfh,
 	.set_rxfh          = mlx5e_set_rxfh,
 	.get_rxfh_context  = mlx5e_get_rxfh_context,
-	.set_rxfh_context  = mlx5e_set_rxfh_context,
+	.set_rxfh_context_old = mlx5e_set_rxfh_context,
 	.get_rxnfc         = mlx5e_get_rxnfc,
 	.set_rxnfc         = mlx5e_set_rxnfc,
 	.get_tunable       = mlx5e_get_tunable,
diff --git a/drivers/net/ethernet/sfc/ef100_ethtool.c b/drivers/net/ethernet/sfc/ef100_ethtool.c
index 702abbe59b76..ec210ad77b21 100644
--- a/drivers/net/ethernet/sfc/ef100_ethtool.c
+++ b/drivers/net/ethernet/sfc/ef100_ethtool.c
@@ -61,7 +61,7 @@ const struct ethtool_ops ef100_ethtool_ops = {
 	.get_rxfh		= efx_ethtool_get_rxfh,
 	.set_rxfh		= efx_ethtool_set_rxfh,
 	.get_rxfh_context	= efx_ethtool_get_rxfh_context,
-	.set_rxfh_context	= efx_ethtool_set_rxfh_context,
+	.set_rxfh_context_old	= efx_ethtool_set_rxfh_context,
 
 	.get_module_info	= efx_ethtool_get_module_info,
 	.get_module_eeprom	= efx_ethtool_get_module_eeprom,
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 364323599f7b..6c421cb1a9cf 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -270,7 +270,7 @@ const struct ethtool_ops efx_ethtool_ops = {
 	.get_rxfh		= efx_ethtool_get_rxfh,
 	.set_rxfh		= efx_ethtool_set_rxfh,
 	.get_rxfh_context	= efx_ethtool_get_rxfh_context,
-	.set_rxfh_context	= efx_ethtool_set_rxfh_context,
+	.set_rxfh_context_old	= efx_ethtool_set_rxfh_context,
 	.get_ts_info		= efx_ethtool_get_ts_info,
 	.get_module_info	= efx_ethtool_get_module_info,
 	.get_module_eeprom	= efx_ethtool_get_module_eeprom,
diff --git a/drivers/net/ethernet/sfc/siena/ethtool.c b/drivers/net/ethernet/sfc/siena/ethtool.c
index e4ec589216c1..1378d1cfc5e2 100644
--- a/drivers/net/ethernet/sfc/siena/ethtool.c
+++ b/drivers/net/ethernet/sfc/siena/ethtool.c
@@ -270,7 +270,7 @@ const struct ethtool_ops efx_siena_ethtool_ops = {
 	.get_rxfh		= efx_siena_ethtool_get_rxfh,
 	.set_rxfh		= efx_siena_ethtool_set_rxfh,
 	.get_rxfh_context	= efx_siena_ethtool_get_rxfh_context,
-	.set_rxfh_context	= efx_siena_ethtool_set_rxfh_context,
+	.set_rxfh_context_old	= efx_siena_ethtool_set_rxfh_context,
 	.get_ts_info		= efx_ethtool_get_ts_info,
 	.get_module_info	= efx_siena_ethtool_get_module_info,
 	.get_module_eeprom	= efx_siena_ethtool_get_module_eeprom,
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index a16580a8e9d7..0c7df2e043b2 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -752,6 +752,9 @@ struct ethtool_mm_stats {
  *	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.
+ * @set_rxfh_context_old: Legacy version of @set_rxfh_context, where driver
+ *	chooses the new context ID in the %ETH_RXFH_CONTEXT_ALLOC case.
+ *	Arguments and return otherwise the same.
  * @get_channels: Get number of channels.
  * @set_channels: Set number of channels.  Returns a negative error code or
  *	zero.
@@ -901,7 +904,10 @@ struct ethtool_ops {
 				    u8 *hfunc, u32 rss_context);
 	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,
 				    const u8 *key, const u8 hfunc,
-				    u32 *rss_context, bool delete);
+				    u32 rss_context, bool delete);
+	int	(*set_rxfh_context_old)(struct net_device *, const u32 *indir,
+					const u8 *key, const u8 hfunc,
+					u32 *rss_context, bool delete);
 	void	(*get_channels)(struct net_device *, struct ethtool_channels *);
 	int	(*set_channels)(struct net_device *, struct ethtool_channels *);
 	int	(*get_dump_flag)(struct net_device *, struct ethtool_dump *);
diff --git a/net/core/dev.c b/net/core/dev.c
index d0a936d4e532..0600945a6810 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10785,15 +10785,22 @@ 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->set_rxfh_context &&
+	    !dev->ethtool_ops->set_rxfh_context_old)
 		return;
 	idr_for_each_entry(&dev->rss_ctx, ctx, context) {
 		u32 *indir = ethtool_rxfh_context_indir(ctx);
 		u8 *key = ethtool_rxfh_context_key(ctx);
 
 		idr_remove(&dev->rss_ctx, context);
-		dev->ethtool_ops->set_rxfh_context(dev, indir, key, ctx->hfunc,
-						   &context, true);
+		if (dev->ethtool_ops->set_rxfh_context)
+			dev->ethtool_ops->set_rxfh_context(dev, indir, key,
+							   ctx->hfunc, context,
+							   true);
+		else
+			dev->ethtool_ops->set_rxfh_context_old(dev, indir, key,
+							       ctx->hfunc,
+							       &context, true);
 		kfree(ctx);
 	}
 }
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index c8f11ac343c9..9e41dc9151d2 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->set_rxfh_context ||
+				  ops->set_rxfh_context_old))
 		return -EOPNOTSUPP;
 	create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;
 	if (create && ops->get_rxfh_priv_size)
@@ -1350,8 +1351,27 @@ 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 = dev_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->set_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->rss_ctx, ctx, 1,
+					   dev->rss_ctx_max_id, GFP_KERNEL);
+			/* 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->rss_ctx, rxfh.rss_context);
 		if (!ctx) {
@@ -1360,11 +1380,18 @@ 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->set_rxfh_context)
+			ret = ops->set_rxfh_context(dev, indir, hkey, rxfh.hfunc,
+						    rxfh.rss_context, delete);
+		else
+			ret = ops->set_rxfh_context_old(dev, indir, hkey,
+							rxfh.hfunc,
+							&rxfh.rss_context,
+							delete);
+	} else {
 		ret = ops->set_rxfh(dev, indir, hkey, rxfh.hfunc);
+	}
 	if (ret)
 		goto out;
 
@@ -1380,12 +1407,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->set_rxfh_context) {
+		/* driver uses old API, it chose context ID */
 		if (WARN_ON(idr_find(&dev->rss_ctx, rxfh.rss_context)))
 			/* context ID reused, our tracking is screwed */
 			goto out;
@@ -1393,8 +1416,6 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 		WARN_ON(idr_alloc(&dev->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->rss_ctx, rxfh.rss_context) != ctx);

[RFC PATCH net-next 4/6] net: ethtool: pass ctx_priv and create into .set_rxfh_context

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

Allows drivers to easily use the private data area in the ctx to store
 their hardware-specific state.
The create flag is needed to inform them that in this case ctx_priv
 has not already been populated.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 include/linux/ethtool.h | 12 ++++++++----
 net/core/dev.c          |  5 +++--
 net/ethtool/ioctl.c     |  6 ++++--
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 0c7df2e043b2..141b020a8855 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -750,11 +750,13 @@ struct ethtool_mm_stats {
  *	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.
+ *	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.
  * @set_rxfh_context_old: Legacy version of @set_rxfh_context, where driver
  *	chooses the new context ID in the %ETH_RXFH_CONTEXT_ALLOC case.
- *	Arguments and return otherwise the same.
  * @get_channels: Get number of channels.
  * @set_channels: Set number of channels.  Returns a negative error code or
  *	zero.
@@ -902,9 +904,11 @@ struct ethtool_ops {
 	u16	(*get_rxfh_priv_size)(struct net_device *);
 	int	(*get_rxfh_context)(struct net_device *, u32 *indir, u8 *key,
 				    u8 *hfunc, u32 rss_context);
-	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,
-				    const u8 *key, const u8 hfunc,
-				    u32 rss_context, bool delete);
+	int	(*set_rxfh_context)(struct net_device *,
+				    struct ethtool_rxfh_context *ctx,
+				    const u32 *indir, const u8 *key,
+				    const u8 hfunc, u32 rss_context,
+				    bool create, bool delete);
 	int	(*set_rxfh_context_old)(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 0600945a6810..b2cfc631761d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10794,8 +10794,9 @@ static void netdev_rss_contexts_free(struct net_device *dev)
 
 		idr_remove(&dev->rss_ctx, context);
 		if (dev->ethtool_ops->set_rxfh_context)
-			dev->ethtool_ops->set_rxfh_context(dev, indir, key,
-							   ctx->hfunc, context,
+			dev->ethtool_ops->set_rxfh_context(dev, ctx, indir,
+							   key, ctx->hfunc,
+							   context, false,
 							   true);
 		else
 			dev->ethtool_ops->set_rxfh_context_old(dev, indir, key,
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 9e41dc9151d2..fa0a3de1e9fb 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1382,8 +1382,10 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 
 	if (rxfh.rss_context) {
 		if (ops->set_rxfh_context)
-			ret = ops->set_rxfh_context(dev, indir, hkey, rxfh.hfunc,
-						    rxfh.rss_context, delete);
+			ret = ops->set_rxfh_context(dev, ctx, indir, hkey,
+						    rxfh.hfunc,
+						    rxfh.rss_context, create,
+						    delete);
 		else
 			ret = ops->set_rxfh_context_old(dev, indir, hkey,
 							rxfh.hfunc,

[RFC PATCH net-next 5/6] net: ethtool: add a mutex protecting RSS contexts

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

While this is not needed to serialise the ethtool entry points (which
 are all under RTNL), drivers may have cause to asynchronously access
 dev->rss_ctx; taking dev->rss_lock allows them to do this safely
 without needing to take the RTNL.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 include/linux/netdevice.h | 3 +++
 net/core/dev.c            | 5 +++++
 net/ethtool/ioctl.c       | 7 +++++++
 3 files changed, 15 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 91f7dad070bd..598bbffdcfd2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2030,6 +2030,8 @@ enum netdev_ml_priv_type {
  *	@udp_tunnel_nic:	UDP tunnel offload state
  *	@rss_ctx_max_id:	maximum (exclusive) supported RSS context ID
  *	@rss_ctx:	IDR storing custom RSS context state
+ *	@rss_lock:	Protects entries in @rss_ctx.  May be taken from
+ *			within RTNL.
  *	@xdp_state:		stores info on attached XDP BPF programs
  *
  *	@nested_level:	Used as a parameter of spin_lock_nested() of
@@ -2401,6 +2403,7 @@ struct net_device {
 
 	u32			rss_ctx_max_id;
 	struct idr		rss_ctx;
+	struct mutex		rss_lock;
 
 	/* protected by rtnl_lock */
 	struct bpf_xdp_entity	xdp_state[__MAX_XDP_MODE];
diff --git a/net/core/dev.c b/net/core/dev.c
index b2cfc631761d..8309178e9d1a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9987,6 +9987,7 @@ int register_netdevice(struct net_device *dev)
 	idr_init_base(&dev->rss_ctx, 1);
 
 	spin_lock_init(&dev->addr_list_lock);
+	mutex_init(&dev->rss_lock);
 	netdev_set_addr_lockdep_class(dev);
 
 	ret = dev_get_valid_name(net, dev, dev->name);
@@ -10788,6 +10789,7 @@ static void netdev_rss_contexts_free(struct net_device *dev)
 	if (!dev->ethtool_ops->set_rxfh_context &&
 	    !dev->ethtool_ops->set_rxfh_context_old)
 		return;
+	mutex_lock(&dev->rss_lock);
 	idr_for_each_entry(&dev->rss_ctx, ctx, context) {
 		u32 *indir = ethtool_rxfh_context_indir(ctx);
 		u8 *key = ethtool_rxfh_context_key(ctx);
@@ -10804,6 +10806,7 @@ static void netdev_rss_contexts_free(struct net_device *dev)
 							       &context, true);
 		kfree(ctx);
 	}
+	mutex_unlock(&dev->rss_lock);
 }
 
 /**
@@ -10917,6 +10920,8 @@ void unregister_netdevice_many_notify(struct list_head *head,
 		if (dev->netdev_ops->ndo_uninit)
 			dev->netdev_ops->ndo_uninit(dev);
 
+		mutex_destroy(&dev->rss_lock);
+
 		if (skb)
 			rtmsg_ifinfo_send(skb, dev, GFP_KERNEL, portid, nlh);
 
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index fa0a3de1e9fb..3d1190e3abb3 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1257,6 +1257,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 	u8 *rss_config;
 	u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]);
 	bool create = false, delete = false;
+	bool locked = false; /* dev->rss_lock taken */
 
 	if (!ops->get_rxnfc || !ops->set_rxfh)
 		return -EOPNOTSUPP;
@@ -1336,6 +1337,10 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 		}
 	}
 
+	if (rxfh.rss_context) {
+		mutex_lock(&dev->rss_lock);
+		locked = true;
+	}
 	if (create) {
 		if (delete) {
 			ret = -EINVAL;
@@ -1438,6 +1443,8 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 	}
 
 out:
+	if (locked)
+		mutex_unlock(&dev->rss_lock);
 	kfree(rss_config);
 	return ret;
 }

[RFC PATCH net-next 6/6] sfc: use new .set_rxfh_context API

[edward.cree]

From: Edward Cree <ecree.xilinx@gmail.com>

The core is now responsible for allocating IDs and a memory region for
 us to store our state (struct efx_rss_context_priv), so we no longer
 need efx_alloc_rss_context_entry() and friends.
Since the contexts are now maintained by the core, use the core's lock
 (net_dev->rss_lock), rather than our own mutex (efx->rss_lock), to
 serialise access against changes; and remove the now-unused
 efx->rss_lock from struct efx_nic.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/ef10.c           |   2 +-
 drivers/net/ethernet/sfc/ef100_ethtool.c  |   2 +-
 drivers/net/ethernet/sfc/efx.c            |   2 +-
 drivers/net/ethernet/sfc/efx.h            |   2 +-
 drivers/net/ethernet/sfc/efx_common.c     |  10 +-
 drivers/net/ethernet/sfc/ethtool.c        |   2 +-
 drivers/net/ethernet/sfc/ethtool_common.c |  96 +++++++---------
 drivers/net/ethernet/sfc/ethtool_common.h |   4 +-
 drivers/net/ethernet/sfc/mcdi_filters.c   | 131 +++++++++++-----------
 drivers/net/ethernet/sfc/mcdi_filters.h   |   8 +-
 drivers/net/ethernet/sfc/net_driver.h     |  28 ++---
 drivers/net/ethernet/sfc/rx_common.c      |  64 ++---------
 drivers/net/ethernet/sfc/rx_common.h      |   8 +-
 13 files changed, 150 insertions(+), 209 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index d30459dbfe8f..6f12fcee8247 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -1394,7 +1394,7 @@ static void efx_ef10_table_reset_mc_allocations(struct efx_nic *efx)
 	efx_mcdi_filter_table_reset_mc_allocations(efx);
 	nic_data->must_restore_piobufs = true;
 	efx_ef10_forget_old_piobufs(efx);
-	efx->rss_context.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+	efx->rss_context.priv.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
 
 	/* Driver-created vswitches and vports must be re-created */
 	nic_data->must_probe_vswitching = true;
diff --git a/drivers/net/ethernet/sfc/ef100_ethtool.c b/drivers/net/ethernet/sfc/ef100_ethtool.c
index ec210ad77b21..702abbe59b76 100644
--- a/drivers/net/ethernet/sfc/ef100_ethtool.c
+++ b/drivers/net/ethernet/sfc/ef100_ethtool.c
@@ -61,7 +61,7 @@ const struct ethtool_ops ef100_ethtool_ops = {
 	.get_rxfh		= efx_ethtool_get_rxfh,
 	.set_rxfh		= efx_ethtool_set_rxfh,
 	.get_rxfh_context	= efx_ethtool_get_rxfh_context,
-	.set_rxfh_context_old	= efx_ethtool_set_rxfh_context,
+	.set_rxfh_context	= efx_ethtool_set_rxfh_context,
 
 	.get_module_info	= efx_ethtool_get_module_info,
 	.get_module_eeprom	= efx_ethtool_get_module_eeprom,
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 746fd9164e30..1b2c281c1cc1 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -298,7 +298,7 @@ static int efx_probe_nic(struct efx_nic *efx)
 	if (efx->n_channels > 1)
 		netdev_rss_key_fill(efx->rss_context.rx_hash_key,
 				    sizeof(efx->rss_context.rx_hash_key));
-	efx_set_default_rx_indir_table(efx, &efx->rss_context);
+	efx_set_default_rx_indir_table(efx, efx->rss_context.rx_indir_table);
 
 	/* Initialise the interrupt moderation settings */
 	efx->irq_mod_step_us = DIV_ROUND_UP(efx->timer_quantum_ns, 1000);
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index 4239c7ece123..a077f648bbde 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -160,7 +160,7 @@ static inline s32 efx_filter_get_rx_ids(struct efx_nic *efx,
 }
 
 /* RSS contexts */
-static inline bool efx_rss_active(struct efx_rss_context *ctx)
+static inline bool efx_rss_active(struct efx_rss_context_priv *ctx)
 {
 	return ctx->context_id != EFX_MCDI_RSS_CONTEXT_INVALID;
 }
diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
index cc30524c2fe4..1412193a882c 100644
--- a/drivers/net/ethernet/sfc/efx_common.c
+++ b/drivers/net/ethernet/sfc/efx_common.c
@@ -717,7 +717,7 @@ void efx_reset_down(struct efx_nic *efx, enum reset_type method)
 
 	mutex_lock(&efx->mac_lock);
 	down_write(&efx->filter_sem);
-	mutex_lock(&efx->rss_lock);
+	mutex_lock(&efx->net_dev->rss_lock);
 	efx->type->fini(efx);
 }
 
@@ -780,7 +780,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
 
 	if (efx->type->rx_restore_rss_contexts)
 		efx->type->rx_restore_rss_contexts(efx);
-	mutex_unlock(&efx->rss_lock);
+	mutex_unlock(&efx->net_dev->rss_lock);
 	efx->type->filter_table_restore(efx);
 	up_write(&efx->filter_sem);
 	if (efx->type->sriov_reset)
@@ -798,7 +798,7 @@ int efx_reset_up(struct efx_nic *efx, enum reset_type method, bool ok)
 fail:
 	efx->port_initialized = false;
 
-	mutex_unlock(&efx->rss_lock);
+	mutex_unlock(&efx->net_dev->rss_lock);
 	up_write(&efx->filter_sem);
 	mutex_unlock(&efx->mac_lock);
 
@@ -1005,9 +1005,7 @@ int efx_init_struct(struct efx_nic *efx, struct pci_dev *pci_dev)
 		efx->type->rx_hash_offset - efx->type->rx_prefix_size;
 	efx->rx_packet_ts_offset =
 		efx->type->rx_ts_offset - efx->type->rx_prefix_size;
-	INIT_LIST_HEAD(&efx->rss_context.list);
-	efx->rss_context.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
-	mutex_init(&efx->rss_lock);
+	efx->rss_context.priv.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
 	efx->vport_id = EVB_PORT_ID_ASSIGNED;
 	spin_lock_init(&efx->stats_lock);
 	efx->vi_stride = EFX_DEFAULT_VI_STRIDE;
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 6c421cb1a9cf..364323599f7b 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -270,7 +270,7 @@ const struct ethtool_ops efx_ethtool_ops = {
 	.get_rxfh		= efx_ethtool_get_rxfh,
 	.set_rxfh		= efx_ethtool_set_rxfh,
 	.get_rxfh_context	= efx_ethtool_get_rxfh_context,
-	.set_rxfh_context_old	= efx_ethtool_set_rxfh_context,
+	.set_rxfh_context	= efx_ethtool_set_rxfh_context,
 	.get_ts_info		= efx_ethtool_get_ts_info,
 	.get_module_info	= efx_ethtool_get_module_info,
 	.get_module_eeprom	= efx_ethtool_get_module_eeprom,
diff --git a/drivers/net/ethernet/sfc/ethtool_common.c b/drivers/net/ethernet/sfc/ethtool_common.c
index a8cbceeb301b..dcf584c07bf7 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.c
+++ b/drivers/net/ethernet/sfc/ethtool_common.c
@@ -820,10 +820,10 @@ int efx_ethtool_get_rxnfc(struct net_device *net_dev,
 		return 0;
 
 	case ETHTOOL_GRXFH: {
-		struct efx_rss_context *ctx = &efx->rss_context;
+		struct efx_rss_context_priv *ctx = &efx->rss_context.priv;
 		__u64 data;
 
-		mutex_lock(&efx->rss_lock);
+		mutex_lock(&net_dev->rss_lock);
 		if (info->flow_type & FLOW_RSS && info->rss_context) {
 			ctx = efx_find_rss_context_entry(efx, info->rss_context);
 			if (!ctx) {
@@ -864,7 +864,7 @@ int efx_ethtool_get_rxnfc(struct net_device *net_dev,
 out_setdata_unlock:
 		info->data = data;
 out_unlock:
-		mutex_unlock(&efx->rss_lock);
+		mutex_unlock(&net_dev->rss_lock);
 		return rc;
 	}
 
@@ -1163,6 +1163,11 @@ u32 efx_ethtool_get_rxfh_key_size(struct net_device *net_dev)
 	return efx->type->rx_hash_key_size;
 }
 
+u16 efx_ethtool_get_rxfh_priv_size(struct net_device *net_dev)
+{
+	return sizeof(struct efx_rss_context_priv);
+}
+
 int efx_ethtool_get_rxfh(struct net_device *net_dev, u32 *indir, u8 *key,
 			 u8 *hfunc)
 {
@@ -1207,42 +1212,43 @@ int efx_ethtool_get_rxfh_context(struct net_device *net_dev, u32 *indir,
 				 u8 *key, u8 *hfunc, u32 rss_context)
 {
 	struct efx_nic *efx = efx_netdev_priv(net_dev);
-	struct efx_rss_context *ctx;
+	struct efx_rss_context_priv *ctx_priv;
+	struct efx_rss_context ctx;
 	int rc = 0;
 
 	if (!efx->type->rx_pull_rss_context_config)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&efx->rss_lock);
-	ctx = efx_find_rss_context_entry(efx, rss_context);
-	if (!ctx) {
+	mutex_lock(&net_dev->rss_lock);
+	ctx_priv = efx_find_rss_context_entry(efx, rss_context);
+	if (!ctx_priv) {
 		rc = -ENOENT;
 		goto out_unlock;
 	}
-	rc = efx->type->rx_pull_rss_context_config(efx, ctx);
+	ctx.priv = *ctx_priv;
+	rc = efx->type->rx_pull_rss_context_config(efx, &ctx);
 	if (rc)
 		goto out_unlock;
 
 	if (hfunc)
 		*hfunc = ETH_RSS_HASH_TOP;
 	if (indir)
-		memcpy(indir, ctx->rx_indir_table, sizeof(ctx->rx_indir_table));
+		memcpy(indir, ctx.rx_indir_table, sizeof(ctx.rx_indir_table));
 	if (key)
-		memcpy(key, ctx->rx_hash_key, efx->type->rx_hash_key_size);
+		memcpy(key, ctx.rx_hash_key, efx->type->rx_hash_key_size);
 out_unlock:
-	mutex_unlock(&efx->rss_lock);
+	mutex_unlock(&net_dev->rss_lock);
 	return rc;
 }
 
 int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
+				 struct ethtool_rxfh_context *ctx,
 				 const u32 *indir, const u8 *key,
-				 const u8 hfunc, u32 *rss_context,
+				 const u8 hfunc, u32 rss_context, bool create,
 				 bool delete)
 {
 	struct efx_nic *efx = efx_netdev_priv(net_dev);
-	struct efx_rss_context *ctx;
-	bool allocated = false;
-	int rc;
+	struct efx_rss_context_priv *priv;
 
 	if (!efx->type->rx_push_rss_context_config)
 		return -EOPNOTSUPP;
@@ -1250,53 +1256,33 @@ int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
 	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&efx->rss_lock);
+	priv = ethtool_rxfh_context_priv(ctx);
 
-	if (*rss_context == ETH_RXFH_CONTEXT_ALLOC) {
-		if (delete) {
+	if (create) {
+		if (delete)
 			/* alloc + delete == Nothing to do */
-			rc = -EINVAL;
-			goto out_unlock;
-		}
-		ctx = efx_alloc_rss_context_entry(efx);
-		if (!ctx) {
-			rc = -ENOMEM;
-			goto out_unlock;
-		}
-		ctx->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
-		/* Initialise indir table and key to defaults */
-		efx_set_default_rx_indir_table(efx, ctx);
-		netdev_rss_key_fill(ctx->rx_hash_key, sizeof(ctx->rx_hash_key));
-		allocated = true;
-	} else {
-		ctx = efx_find_rss_context_entry(efx, *rss_context);
-		if (!ctx) {
-			rc = -ENOENT;
-			goto out_unlock;
-		}
-	}
-
-	if (delete) {
-		/* delete this context */
-		rc = efx->type->rx_push_rss_context_config(efx, ctx, NULL, NULL);
-		if (!rc)
-			efx_free_rss_context_entry(ctx);
-		goto out_unlock;
+			return -EINVAL;
+		priv->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+		priv->rx_hash_udp_4tuple = false;
+		/* Generate default indir table and/or key if not specified.
+		 * We use ctx as a place to store these; this is fine because
+		 * we're doing a create, so if we fail then the ctx will just
+		 * be deleted.
+		 */
+		if (!indir)
+			efx_set_default_rx_indir_table(efx, ethtool_rxfh_context_indir(ctx));
+		if (!key)
+			netdev_rss_key_fill(ethtool_rxfh_context_key(ctx),
+					    ctx->key_size);
 	}
 
 	if (!key)
-		key = ctx->rx_hash_key;
+		key = ethtool_rxfh_context_key(ctx);
 	if (!indir)
-		indir = ctx->rx_indir_table;
+		indir = ethtool_rxfh_context_indir(ctx);
 
-	rc = efx->type->rx_push_rss_context_config(efx, ctx, indir, key);
-	if (rc && allocated)
-		efx_free_rss_context_entry(ctx);
-	else
-		*rss_context = ctx->user_id;
-out_unlock:
-	mutex_unlock(&efx->rss_lock);
-	return rc;
+	return efx->type->rx_push_rss_context_config(efx, priv, indir, key,
+						     delete);
 }
 
 int efx_ethtool_reset(struct net_device *net_dev, u32 *flags)
diff --git a/drivers/net/ethernet/sfc/ethtool_common.h b/drivers/net/ethernet/sfc/ethtool_common.h
index 659491932101..6280312417f1 100644
--- a/drivers/net/ethernet/sfc/ethtool_common.h
+++ b/drivers/net/ethernet/sfc/ethtool_common.h
@@ -44,6 +44,7 @@ int efx_ethtool_set_rxnfc(struct net_device *net_dev,
 			  struct ethtool_rxnfc *info);
 u32 efx_ethtool_get_rxfh_indir_size(struct net_device *net_dev);
 u32 efx_ethtool_get_rxfh_key_size(struct net_device *net_dev);
+u16 efx_ethtool_get_rxfh_priv_size(struct net_device *net_dev);
 int efx_ethtool_get_rxfh(struct net_device *net_dev, u32 *indir, u8 *key,
 			 u8 *hfunc);
 int efx_ethtool_set_rxfh(struct net_device *net_dev,
@@ -51,8 +52,9 @@ int efx_ethtool_set_rxfh(struct net_device *net_dev,
 int efx_ethtool_get_rxfh_context(struct net_device *net_dev, u32 *indir,
 				 u8 *key, u8 *hfunc, u32 rss_context);
 int efx_ethtool_set_rxfh_context(struct net_device *net_dev,
+				 struct ethtool_rxfh_context *ctx,
 				 const u32 *indir, const u8 *key,
-				 const u8 hfunc, u32 *rss_context,
+				 const u8 hfunc, u32 rss_context, bool create,
 				 bool delete);
 int efx_ethtool_reset(struct net_device *net_dev, u32 *flags);
 int efx_ethtool_get_module_eeprom(struct net_device *net_dev,
diff --git a/drivers/net/ethernet/sfc/mcdi_filters.c b/drivers/net/ethernet/sfc/mcdi_filters.c
index 4ff6586116ee..e6966f4d91a6 100644
--- a/drivers/net/ethernet/sfc/mcdi_filters.c
+++ b/drivers/net/ethernet/sfc/mcdi_filters.c
@@ -194,7 +194,7 @@ efx_mcdi_filter_push_prep_set_match_fields(struct efx_nic *efx,
 static void efx_mcdi_filter_push_prep(struct efx_nic *efx,
 				      const struct efx_filter_spec *spec,
 				      efx_dword_t *inbuf, u64 handle,
-				      struct efx_rss_context *ctx,
+				      struct efx_rss_context_priv *ctx,
 				      bool replacing)
 {
 	u32 flags = spec->flags;
@@ -245,7 +245,7 @@ static void efx_mcdi_filter_push_prep(struct efx_nic *efx,
 
 static int efx_mcdi_filter_push(struct efx_nic *efx,
 				const struct efx_filter_spec *spec, u64 *handle,
-				struct efx_rss_context *ctx, bool replacing)
+				struct efx_rss_context_priv *ctx, bool replacing)
 {
 	MCDI_DECLARE_BUF(inbuf, MC_CMD_FILTER_OP_EXT_IN_LEN);
 	MCDI_DECLARE_BUF(outbuf, MC_CMD_FILTER_OP_EXT_OUT_LEN);
@@ -345,9 +345,9 @@ static s32 efx_mcdi_filter_insert_locked(struct efx_nic *efx,
 					 bool replace_equal)
 {
 	DECLARE_BITMAP(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT);
+	struct efx_rss_context_priv *ctx = NULL;
 	struct efx_mcdi_filter_table *table;
 	struct efx_filter_spec *saved_spec;
-	struct efx_rss_context *ctx = NULL;
 	unsigned int match_pri, hash;
 	unsigned int priv_flags;
 	bool rss_locked = false;
@@ -380,12 +380,12 @@ static s32 efx_mcdi_filter_insert_locked(struct efx_nic *efx,
 		bitmap_zero(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT);
 
 	if (spec->flags & EFX_FILTER_FLAG_RX_RSS) {
-		mutex_lock(&efx->rss_lock);
+		mutex_lock(&efx->net_dev->rss_lock);
 		rss_locked = true;
 		if (spec->rss_context)
 			ctx = efx_find_rss_context_entry(efx, spec->rss_context);
 		else
-			ctx = &efx->rss_context;
+			ctx = &efx->rss_context.priv;
 		if (!ctx) {
 			rc = -ENOENT;
 			goto out_unlock;
@@ -548,7 +548,7 @@ static s32 efx_mcdi_filter_insert_locked(struct efx_nic *efx,
 
 out_unlock:
 	if (rss_locked)
-		mutex_unlock(&efx->rss_lock);
+		mutex_unlock(&efx->net_dev->rss_lock);
 	up_write(&table->lock);
 	return rc;
 }
@@ -611,13 +611,13 @@ static int efx_mcdi_filter_remove_internal(struct efx_nic *efx,
 
 		new_spec.priority = EFX_FILTER_PRI_AUTO;
 		new_spec.flags = (EFX_FILTER_FLAG_RX |
-				  (efx_rss_active(&efx->rss_context) ?
+				  (efx_rss_active(&efx->rss_context.priv) ?
 				   EFX_FILTER_FLAG_RX_RSS : 0));
 		new_spec.dmaq_id = 0;
 		new_spec.rss_context = 0;
 		rc = efx_mcdi_filter_push(efx, &new_spec,
 					  &table->entry[filter_idx].handle,
-					  &efx->rss_context,
+					  &efx->rss_context.priv,
 					  true);
 
 		if (rc == 0)
@@ -764,7 +764,7 @@ static int efx_mcdi_filter_insert_addr_list(struct efx_nic *efx,
 		ids = vlan->uc;
 	}
 
-	filter_flags = efx_rss_active(&efx->rss_context) ? EFX_FILTER_FLAG_RX_RSS : 0;
+	filter_flags = efx_rss_active(&efx->rss_context.priv) ? EFX_FILTER_FLAG_RX_RSS : 0;
 
 	/* Insert/renew filters */
 	for (i = 0; i < addr_count; i++) {
@@ -833,7 +833,7 @@ static int efx_mcdi_filter_insert_def(struct efx_nic *efx,
 	int rc;
 	u16 *id;
 
-	filter_flags = efx_rss_active(&efx->rss_context) ? EFX_FILTER_FLAG_RX_RSS : 0;
+	filter_flags = efx_rss_active(&efx->rss_context.priv) ? EFX_FILTER_FLAG_RX_RSS : 0;
 
 	efx_filter_init_rx(&spec, EFX_FILTER_PRI_AUTO, filter_flags, 0);
 
@@ -1375,8 +1375,8 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
 	struct efx_mcdi_filter_table *table = efx->filter_state;
 	unsigned int invalid_filters = 0, failed = 0;
 	struct efx_mcdi_filter_vlan *vlan;
+	struct efx_rss_context_priv *ctx;
 	struct efx_filter_spec *spec;
-	struct efx_rss_context *ctx;
 	unsigned int filter_idx;
 	u32 mcdi_flags;
 	int match_pri;
@@ -1388,7 +1388,7 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
 		return;
 
 	down_write(&table->lock);
-	mutex_lock(&efx->rss_lock);
+	mutex_lock(&efx->net_dev->rss_lock);
 
 	for (filter_idx = 0; filter_idx < EFX_MCDI_FILTER_TBL_ROWS; filter_idx++) {
 		spec = efx_mcdi_filter_entry_spec(table, filter_idx);
@@ -1407,7 +1407,7 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
 		if (spec->rss_context)
 			ctx = efx_find_rss_context_entry(efx, spec->rss_context);
 		else
-			ctx = &efx->rss_context;
+			ctx = &efx->rss_context.priv;
 		if (spec->flags & EFX_FILTER_FLAG_RX_RSS) {
 			if (!ctx) {
 				netif_warn(efx, drv, efx->net_dev,
@@ -1444,7 +1444,7 @@ void efx_mcdi_filter_table_restore(struct efx_nic *efx)
 		}
 	}
 
-	mutex_unlock(&efx->rss_lock);
+	mutex_unlock(&efx->net_dev->rss_lock);
 	up_write(&table->lock);
 
 	/*
@@ -1861,7 +1861,8 @@ bool efx_mcdi_filter_rfs_expire_one(struct efx_nic *efx, u32 flow_id,
 					 RSS_MODE_HASH_ADDRS << MC_CMD_RSS_CONTEXT_GET_FLAGS_OUT_UDP_IPV6_RSS_MODE_LBN |\
 					 RSS_MODE_HASH_ADDRS << MC_CMD_RSS_CONTEXT_GET_FLAGS_OUT_OTHER_IPV6_RSS_MODE_LBN)
 
-int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context, u32 *flags)
+static int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context,
+					  u32 *flags)
 {
 	/*
 	 * Firmware had a bug (sfc bug 61952) where it would not actually
@@ -1909,8 +1910,8 @@ int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context, u32 *flags)
  * Defaults are 4-tuple for TCP and 2-tuple for UDP and other-IP, so we
  * just need to set the UDP ports flags (for both IP versions).
  */
-void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
-				    struct efx_rss_context *ctx)
+static void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
+					   struct efx_rss_context_priv *ctx)
 {
 	MCDI_DECLARE_BUF(inbuf, MC_CMD_RSS_CONTEXT_SET_FLAGS_IN_LEN);
 	u32 flags;
@@ -1931,7 +1932,7 @@ void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
 }
 
 static int efx_mcdi_filter_alloc_rss_context(struct efx_nic *efx, bool exclusive,
-					     struct efx_rss_context *ctx,
+					     struct efx_rss_context_priv *ctx,
 					     unsigned *context_size)
 {
 	MCDI_DECLARE_BUF(inbuf, MC_CMD_RSS_CONTEXT_ALLOC_IN_LEN);
@@ -2032,25 +2033,26 @@ void efx_mcdi_rx_free_indir_table(struct efx_nic *efx)
 {
 	int rc;
 
-	if (efx->rss_context.context_id != EFX_MCDI_RSS_CONTEXT_INVALID) {
-		rc = efx_mcdi_filter_free_rss_context(efx, efx->rss_context.context_id);
+	if (efx->rss_context.priv.context_id != EFX_MCDI_RSS_CONTEXT_INVALID) {
+		rc = efx_mcdi_filter_free_rss_context(efx, efx->rss_context.priv.context_id);
 		WARN_ON(rc != 0);
 	}
-	efx->rss_context.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+	efx->rss_context.priv.context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
 }
 
 static int efx_mcdi_filter_rx_push_shared_rss_config(struct efx_nic *efx,
 					      unsigned *context_size)
 {
 	struct efx_mcdi_filter_table *table = efx->filter_state;
-	int rc = efx_mcdi_filter_alloc_rss_context(efx, false, &efx->rss_context,
-					    context_size);
+	int rc = efx_mcdi_filter_alloc_rss_context(efx, false,
+						   &efx->rss_context.priv,
+						   context_size);
 
 	if (rc != 0)
 		return rc;
 
 	table->rx_rss_context_exclusive = false;
-	efx_set_default_rx_indir_table(efx, &efx->rss_context);
+	efx_set_default_rx_indir_table(efx, efx->rss_context.rx_indir_table);
 	return 0;
 }
 
@@ -2058,26 +2060,27 @@ static int efx_mcdi_filter_rx_push_exclusive_rss_config(struct efx_nic *efx,
 						 const u32 *rx_indir_table,
 						 const u8 *key)
 {
+	u32 old_rx_rss_context = efx->rss_context.priv.context_id;
 	struct efx_mcdi_filter_table *table = efx->filter_state;
-	u32 old_rx_rss_context = efx->rss_context.context_id;
 	int rc;
 
-	if (efx->rss_context.context_id == EFX_MCDI_RSS_CONTEXT_INVALID ||
+	if (efx->rss_context.priv.context_id == EFX_MCDI_RSS_CONTEXT_INVALID ||
 	    !table->rx_rss_context_exclusive) {
-		rc = efx_mcdi_filter_alloc_rss_context(efx, true, &efx->rss_context,
-						NULL);
+		rc = efx_mcdi_filter_alloc_rss_context(efx, true,
+						       &efx->rss_context.priv,
+						       NULL);
 		if (rc == -EOPNOTSUPP)
 			return rc;
 		else if (rc != 0)
 			goto fail1;
 	}
 
-	rc = efx_mcdi_filter_populate_rss_table(efx, efx->rss_context.context_id,
+	rc = efx_mcdi_filter_populate_rss_table(efx, efx->rss_context.priv.context_id,
 					 rx_indir_table, key);
 	if (rc != 0)
 		goto fail2;
 
-	if (efx->rss_context.context_id != old_rx_rss_context &&
+	if (efx->rss_context.priv.context_id != old_rx_rss_context &&
 	    old_rx_rss_context != EFX_MCDI_RSS_CONTEXT_INVALID)
 		WARN_ON(efx_mcdi_filter_free_rss_context(efx, old_rx_rss_context) != 0);
 	table->rx_rss_context_exclusive = true;
@@ -2091,9 +2094,9 @@ static int efx_mcdi_filter_rx_push_exclusive_rss_config(struct efx_nic *efx,
 	return 0;
 
 fail2:
-	if (old_rx_rss_context != efx->rss_context.context_id) {
-		WARN_ON(efx_mcdi_filter_free_rss_context(efx, efx->rss_context.context_id) != 0);
-		efx->rss_context.context_id = old_rx_rss_context;
+	if (old_rx_rss_context != efx->rss_context.priv.context_id) {
+		WARN_ON(efx_mcdi_filter_free_rss_context(efx, efx->rss_context.priv.context_id) != 0);
+		efx->rss_context.priv.context_id = old_rx_rss_context;
 	}
 fail1:
 	netif_err(efx, hw, efx->net_dev, "%s: failed rc=%d\n", __func__, rc);
@@ -2101,33 +2104,28 @@ static int efx_mcdi_filter_rx_push_exclusive_rss_config(struct efx_nic *efx,
 }
 
 int efx_mcdi_rx_push_rss_context_config(struct efx_nic *efx,
-					struct efx_rss_context *ctx,
+					struct efx_rss_context_priv *ctx,
 					const u32 *rx_indir_table,
-					const u8 *key)
+					const u8 *key, bool delete)
 {
 	int rc;
 
-	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+	WARN_ON(!mutex_is_locked(&efx->net_dev->rss_lock));
 
 	if (ctx->context_id == EFX_MCDI_RSS_CONTEXT_INVALID) {
+		if (delete)
+			/* already wasn't in HW, nothing to do */
+			return 0;
 		rc = efx_mcdi_filter_alloc_rss_context(efx, true, ctx, NULL);
 		if (rc)
 			return rc;
 	}
 
-	if (!rx_indir_table) /* Delete this context */
+	if (delete) /* Delete this context */
 		return efx_mcdi_filter_free_rss_context(efx, ctx->context_id);
 
-	rc = efx_mcdi_filter_populate_rss_table(efx, ctx->context_id,
-					 rx_indir_table, key);
-	if (rc)
-		return rc;
-
-	memcpy(ctx->rx_indir_table, rx_indir_table,
-	       sizeof(efx->rss_context.rx_indir_table));
-	memcpy(ctx->rx_hash_key, key, efx->type->rx_hash_key_size);
-
-	return 0;
+	return efx_mcdi_filter_populate_rss_table(efx, ctx->context_id,
+						  rx_indir_table, key);
 }
 
 int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
@@ -2139,16 +2137,16 @@ int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
 	size_t outlen;
 	int rc, i;
 
-	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+	WARN_ON(!mutex_is_locked(&efx->net_dev->rss_lock));
 
 	BUILD_BUG_ON(MC_CMD_RSS_CONTEXT_GET_TABLE_IN_LEN !=
 		     MC_CMD_RSS_CONTEXT_GET_KEY_IN_LEN);
 
-	if (ctx->context_id == EFX_MCDI_RSS_CONTEXT_INVALID)
+	if (ctx->priv.context_id == EFX_MCDI_RSS_CONTEXT_INVALID)
 		return -ENOENT;
 
 	MCDI_SET_DWORD(inbuf, RSS_CONTEXT_GET_TABLE_IN_RSS_CONTEXT_ID,
-		       ctx->context_id);
+		       ctx->priv.context_id);
 	BUILD_BUG_ON(ARRAY_SIZE(ctx->rx_indir_table) !=
 		     MC_CMD_RSS_CONTEXT_GET_TABLE_OUT_INDIRECTION_TABLE_LEN);
 	rc = efx_mcdi_rpc(efx, MC_CMD_RSS_CONTEXT_GET_TABLE, inbuf, sizeof(inbuf),
@@ -2164,7 +2162,7 @@ int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
 				RSS_CONTEXT_GET_TABLE_OUT_INDIRECTION_TABLE)[i];
 
 	MCDI_SET_DWORD(inbuf, RSS_CONTEXT_GET_KEY_IN_RSS_CONTEXT_ID,
-		       ctx->context_id);
+		       ctx->priv.context_id);
 	BUILD_BUG_ON(ARRAY_SIZE(ctx->rx_hash_key) !=
 		     MC_CMD_RSS_CONTEXT_SET_KEY_IN_TOEPLITZ_KEY_LEN);
 	rc = efx_mcdi_rpc(efx, MC_CMD_RSS_CONTEXT_GET_KEY, inbuf, sizeof(inbuf),
@@ -2186,35 +2184,42 @@ int efx_mcdi_rx_pull_rss_config(struct efx_nic *efx)
 {
 	int rc;
 
-	mutex_lock(&efx->rss_lock);
+	mutex_lock(&efx->net_dev->rss_lock);
 	rc = efx_mcdi_rx_pull_rss_context_config(efx, &efx->rss_context);
-	mutex_unlock(&efx->rss_lock);
+	mutex_unlock(&efx->net_dev->rss_lock);
 	return rc;
 }
 
 void efx_mcdi_rx_restore_rss_contexts(struct efx_nic *efx)
 {
 	struct efx_mcdi_filter_table *table = efx->filter_state;
-	struct efx_rss_context *ctx;
+	struct ethtool_rxfh_context *ctx;
+	u32 context;
 	int rc;
 
-	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+	WARN_ON(!mutex_is_locked(&efx->net_dev->rss_lock));
 
 	if (!table->must_restore_rss_contexts)
 		return;
 
-	list_for_each_entry(ctx, &efx->rss_context.list, list) {
+	idr_for_each_entry(&efx->net_dev->rss_ctx, ctx, context) {
+		struct efx_rss_context_priv *priv;
+		u32 *indir;
+		u8 *key;
+
+		priv = ethtool_rxfh_context_priv(ctx);
 		/* previous NIC RSS context is gone */
-		ctx->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
+		priv->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
 		/* so try to allocate a new one */
-		rc = efx_mcdi_rx_push_rss_context_config(efx, ctx,
-							 ctx->rx_indir_table,
-							 ctx->rx_hash_key);
+		indir = ethtool_rxfh_context_indir(ctx);
+		key = ethtool_rxfh_context_key(ctx);
+		rc = efx_mcdi_rx_push_rss_context_config(efx, priv, indir, key,
+							 false);
 		if (rc)
 			netif_warn(efx, probe, efx->net_dev,
 				   "failed to restore RSS context %u, rc=%d"
 				   "; RSS filters may fail to be applied\n",
-				   ctx->user_id, rc);
+				   context, rc);
 	}
 	table->must_restore_rss_contexts = false;
 }
@@ -2276,7 +2281,7 @@ int efx_mcdi_vf_rx_push_rss_config(struct efx_nic *efx, bool user,
 {
 	if (user)
 		return -EOPNOTSUPP;
-	if (efx->rss_context.context_id != EFX_MCDI_RSS_CONTEXT_INVALID)
+	if (efx->rss_context.priv.context_id != EFX_MCDI_RSS_CONTEXT_INVALID)
 		return 0;
 	return efx_mcdi_filter_rx_push_shared_rss_config(efx, NULL);
 }
@@ -2295,7 +2300,7 @@ int efx_mcdi_push_default_indir_table(struct efx_nic *efx,
 
 	efx_mcdi_rx_free_indir_table(efx);
 	if (rss_spread > 1) {
-		efx_set_default_rx_indir_table(efx, &efx->rss_context);
+		efx_set_default_rx_indir_table(efx, efx->rss_context.rx_indir_table);
 		rc = efx->type->rx_push_rss_config(efx, false,
 				   efx->rss_context.rx_indir_table, NULL);
 	}
diff --git a/drivers/net/ethernet/sfc/mcdi_filters.h b/drivers/net/ethernet/sfc/mcdi_filters.h
index c0d6558b9fd2..11b9f87ed9e1 100644
--- a/drivers/net/ethernet/sfc/mcdi_filters.h
+++ b/drivers/net/ethernet/sfc/mcdi_filters.h
@@ -145,9 +145,9 @@ void efx_mcdi_filter_del_vlan(struct efx_nic *efx, u16 vid);
 
 void efx_mcdi_rx_free_indir_table(struct efx_nic *efx);
 int efx_mcdi_rx_push_rss_context_config(struct efx_nic *efx,
-					struct efx_rss_context *ctx,
+					struct efx_rss_context_priv *ctx,
 					const u32 *rx_indir_table,
-					const u8 *key);
+					const u8 *key, bool delete);
 int efx_mcdi_pf_rx_push_rss_config(struct efx_nic *efx, bool user,
 				   const u32 *rx_indir_table,
 				   const u8 *key);
@@ -161,10 +161,6 @@ int efx_mcdi_push_default_indir_table(struct efx_nic *efx,
 int efx_mcdi_rx_pull_rss_config(struct efx_nic *efx);
 int efx_mcdi_rx_pull_rss_context_config(struct efx_nic *efx,
 					struct efx_rss_context *ctx);
-int efx_mcdi_get_rss_context_flags(struct efx_nic *efx, u32 context,
-				   u32 *flags);
-void efx_mcdi_set_rss_context_flags(struct efx_nic *efx,
-				    struct efx_rss_context *ctx);
 void efx_mcdi_rx_restore_rss_contexts(struct efx_nic *efx);
 
 static inline void efx_mcdi_update_rx_scatter(struct efx_nic *efx)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index fcd51d3992fa..bceeada24d6c 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -770,21 +770,24 @@ struct vfdi_status;
 /* The reserved RSS context value */
 #define EFX_MCDI_RSS_CONTEXT_INVALID	0xffffffff
 /**
- * struct efx_rss_context - A user-defined RSS context for filtering
- * @list: node of linked list on which this struct is stored
+ * struct efx_rss_context_priv - driver private data for an RSS context
  * @context_id: the RSS_CONTEXT_ID returned by MC firmware, or
  *	%EFX_MCDI_RSS_CONTEXT_INVALID if this context is not present on the NIC.
- *	For Siena, 0 if RSS is active, else %EFX_MCDI_RSS_CONTEXT_INVALID.
- * @user_id: the rss_context ID exposed to userspace over ethtool.
  * @rx_hash_udp_4tuple: UDP 4-tuple hashing enabled
+ */
+struct efx_rss_context_priv {
+	u32 context_id;
+	bool rx_hash_udp_4tuple;
+};
+
+/**
+ * struct efx_rss_context - an RSS context
+ * @priv: hardware-specific state
  * @rx_hash_key: Toeplitz hash key for this RSS context
  * @indir_table: Indirection table for this RSS context
  */
 struct efx_rss_context {
-	struct list_head list;
-	u32 context_id;
-	u32 user_id;
-	bool rx_hash_udp_4tuple;
+	struct efx_rss_context_priv priv;
 	u8 rx_hash_key[40];
 	u32 rx_indir_table[128];
 };
@@ -917,9 +920,7 @@ struct efx_mae;
  * @rx_packet_ts_offset: Offset of timestamp from start of packet data
  *	(valid only if channel->sync_timestamps_enabled; always negative)
  * @rx_scatter: Scatter mode enabled for receives
- * @rss_context: Main RSS context.  Its @list member is the head of the list of
- *	RSS contexts created by user requests
- * @rss_lock: Protects custom RSS context software state in @rss_context.list
+ * @rss_context: Main RSS context.
  * @vport_id: The function's vport ID, only relevant for PFs
  * @int_error_count: Number of internal errors seen recently
  * @int_error_expire: Time at which error count will be expired
@@ -1090,7 +1091,6 @@ struct efx_nic {
 	int rx_packet_ts_offset;
 	bool rx_scatter;
 	struct efx_rss_context rss_context;
-	struct mutex rss_lock;
 	u32 vport_id;
 
 	unsigned int_error_count;
@@ -1462,9 +1462,9 @@ struct efx_nic_type {
 				  const u32 *rx_indir_table, const u8 *key);
 	int (*rx_pull_rss_config)(struct efx_nic *efx);
 	int (*rx_push_rss_context_config)(struct efx_nic *efx,
-					  struct efx_rss_context *ctx,
+					  struct efx_rss_context_priv *ctx,
 					  const u32 *rx_indir_table,
-					  const u8 *key);
+					  const u8 *key, bool delete);
 	int (*rx_pull_rss_context_config)(struct efx_nic *efx,
 					  struct efx_rss_context *ctx);
 	void (*rx_restore_rss_contexts)(struct efx_nic *efx);
diff --git a/drivers/net/ethernet/sfc/rx_common.c b/drivers/net/ethernet/sfc/rx_common.c
index d2f35ee15eff..ae440d510d4e 100644
--- a/drivers/net/ethernet/sfc/rx_common.c
+++ b/drivers/net/ethernet/sfc/rx_common.c
@@ -556,69 +556,25 @@ efx_rx_packet_gro(struct efx_channel *channel, struct efx_rx_buffer *rx_buf,
 	napi_gro_frags(napi);
 }
 
-/* RSS contexts.  We're using linked lists and crappy O(n) algorithms, because
- * (a) this is an infrequent control-plane operation and (b) n is small (max 64)
- */
-struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx)
+struct efx_rss_context_priv *efx_find_rss_context_entry(struct efx_nic *efx,
+							u32 id)
 {
-	struct list_head *head = &efx->rss_context.list;
-	struct efx_rss_context *ctx, *new;
-	u32 id = 1; /* Don't use zero, that refers to the master RSS context */
-
-	WARN_ON(!mutex_is_locked(&efx->rss_lock));
+	struct ethtool_rxfh_context *ctx;
 
-	/* Search for first gap in the numbering */
-	list_for_each_entry(ctx, head, list) {
-		if (ctx->user_id != id)
-			break;
-		id++;
-		/* Check for wrap.  If this happens, we have nearly 2^32
-		 * allocated RSS contexts, which seems unlikely.
-		 */
-		if (WARN_ON_ONCE(!id))
-			return NULL;
-	}
+	WARN_ON(!mutex_is_locked(&efx->net_dev->rss_lock));
 
-	/* Create the new entry */
-	new = kmalloc(sizeof(*new), GFP_KERNEL);
-	if (!new)
+	ctx = idr_find(&efx->net_dev->rss_ctx, id);
+	if (!ctx)
 		return NULL;
-	new->context_id = EFX_MCDI_RSS_CONTEXT_INVALID;
-	new->rx_hash_udp_4tuple = false;
-
-	/* Insert the new entry into the gap */
-	new->user_id = id;
-	list_add_tail(&new->list, &ctx->list);
-	return new;
-}
-
-struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id)
-{
-	struct list_head *head = &efx->rss_context.list;
-	struct efx_rss_context *ctx;
-
-	WARN_ON(!mutex_is_locked(&efx->rss_lock));
-
-	list_for_each_entry(ctx, head, list)
-		if (ctx->user_id == id)
-			return ctx;
-	return NULL;
-}
-
-void efx_free_rss_context_entry(struct efx_rss_context *ctx)
-{
-	list_del(&ctx->list);
-	kfree(ctx);
+	return ethtool_rxfh_context_priv(ctx);
 }
 
-void efx_set_default_rx_indir_table(struct efx_nic *efx,
-				    struct efx_rss_context *ctx)
+void efx_set_default_rx_indir_table(struct efx_nic *efx, u32 *indir)
 {
 	size_t i;
 
-	for (i = 0; i < ARRAY_SIZE(ctx->rx_indir_table); i++)
-		ctx->rx_indir_table[i] =
-			ethtool_rxfh_indir_default(i, efx->rss_spread);
+	for (i = 0; i < ARRAY_SIZE(efx->rss_context.rx_indir_table); i++)
+		indir[i] = ethtool_rxfh_indir_default(i, efx->rss_spread);
 }
 
 /**
diff --git a/drivers/net/ethernet/sfc/rx_common.h b/drivers/net/ethernet/sfc/rx_common.h
index fbd2769307f9..75fa84192362 100644
--- a/drivers/net/ethernet/sfc/rx_common.h
+++ b/drivers/net/ethernet/sfc/rx_common.h
@@ -84,11 +84,9 @@ void
 efx_rx_packet_gro(struct efx_channel *channel, struct efx_rx_buffer *rx_buf,
 		  unsigned int n_frags, u8 *eh, __wsum csum);
 
-struct efx_rss_context *efx_alloc_rss_context_entry(struct efx_nic *efx);
-struct efx_rss_context *efx_find_rss_context_entry(struct efx_nic *efx, u32 id);
-void efx_free_rss_context_entry(struct efx_rss_context *ctx);
-void efx_set_default_rx_indir_table(struct efx_nic *efx,
-				    struct efx_rss_context *ctx);
+struct efx_rss_context_priv *efx_find_rss_context_entry(struct efx_nic *efx,
+							u32 id);
+void efx_set_default_rx_indir_table(struct efx_nic *efx, u32 *indir);
 
 bool efx_filter_is_mc_recipient(const struct efx_filter_spec *spec);
 bool efx_filter_spec_equal(const struct efx_filter_spec *left,

Re: [RFC PATCH net-next 1/6] net: ethtool: attach an IDR of custom RSS contexts to a netdevice

[Jakub Kicinski]

On Mon, 3 Apr 2023 17:32:58 +0100 edward.cree@amd.com wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> Each context stores the RXFH settings (indir, key, and hfunc) as well
>  as optionally some driver private data.
> Delete any still-existing contexts at netdev unregister time.

> +/**
> + * struct ethtool_rxfh_context - a custom RSS context configuration
> + * @indir_size: Number of u32 entries in indirection table
> + * @key_size: Size of hash key, in bytes
> + * @hfunc: RSS hash function identifier.  One of the %ETH_RSS_HASH_*
> + * @priv_size: Size of driver private data, in bytes
> + */
> +struct ethtool_rxfh_context {
> +	u32 indir_size;
> +	u32 key_size;
> +	u8 hfunc;
> +	u16 priv_size;
> +	/* private: indirection table, hash key, and driver private data are
> +	 * stored sequentially in @data area.  Use below helpers to access
> +	 */
> +	u8 data[];

I think that something needs to get aligned here...
Driver priv needs to guarantee ulong alignment in case someone puts 
a pointer in it.

> +};
> +
> +static inline u32 *ethtool_rxfh_context_indir(struct ethtool_rxfh_context *ctx)
> +{
> +	return (u32 *)&ctx->data;
> +}
> +
> +static inline u8 *ethtool_rxfh_context_key(struct ethtool_rxfh_context *ctx)
> +{
> +	return (u8 *)(ethtool_rxfh_context_indir(ctx) + ctx->indir_size);
> +}
> +
> +static inline void *ethtool_rxfh_context_priv(struct ethtool_rxfh_context *ctx)
> +{
> +	return ethtool_rxfh_context_key(ctx) + ctx->key_size;

ALIGN_PTR() ... ?
Or align data[] and reorder..

> +}
> +
>  /* declare a link mode bitmap */
>  #define __ETHTOOL_DECLARE_LINK_MODE_MASK(name)		\
>  	DECLARE_BITMAP(name, __ETHTOOL_LINK_MODE_MASK_NBITS)

> +	u32			rss_ctx_max_id;
> +	struct idr		rss_ctx;

noob question, why not xarray?
Isn't IDR just a legacy wrapper around xarray anyway?

Re: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[Jakub Kicinski]

On Mon, 3 Apr 2023 17:32:59 +0100 edward.cree@amd.com wrote:
> @@ -880,6 +896,7 @@ struct ethtool_ops {
>  			    u8 *hfunc);
>  	int	(*set_rxfh)(struct net_device *, const u32 *indir,
>  			    const u8 *key, const u8 hfunc);
> +	u16	(*get_rxfh_priv_size)(struct net_device *);
>  	int	(*get_rxfh_context)(struct net_device *, u32 *indir, u8 *key,
>  				    u8 *hfunc, u32 rss_context);
>  	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,

Would a static value not do for most drivers?
We already have a handful of data fields in the "ops" structure.

> @@ -1331,6 +1335,31 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
>  		}
>  	}
>  
> +	if (create) {
> +		if (delete) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +		ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
> +							dev_key_size,
> +							dev_priv_size),
> +			      GFP_USER);

GFP_USER? Do you mean it for accounting? GFP_KERNEL_ACCOUNT?

Re: [RFC PATCH net-next 3/6] net: ethtool: let the core choose RSS context IDs

[Jakub Kicinski]

On Mon, 3 Apr 2023 17:33:00 +0100 edward.cree@amd.com wrote:
>  	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,
>  				    const u8 *key, const u8 hfunc,
> -				    u32 *rss_context, bool delete);
> +				    u32 rss_context, bool delete);

Would it be easier to pass struct ethtool_rxfh_context instead of
doing it field by field?  Otherwise Intel will need to add more
arguments and touch all drivers. Or are you thinking that they should
use a separate callback for the "RR RSS" or whatever their thing is?

And maybe separate op for create / change / delete?

And an extack on top... :)

Re: [RFC PATCH net-next 5/6] net: ethtool: add a mutex protecting RSS contexts

[Jakub Kicinski]

On Mon, 3 Apr 2023 17:33:02 +0100 edward.cree@amd.com wrote:
>  	u32			rss_ctx_max_id;
>  	struct idr		rss_ctx;
> +	struct mutex		rss_lock;

Argh, the mutex doubles the size of the state, and most drivers don't
implement this feature.  My thinking was to add a "ethtool state"
pointer to net_device which will be allocated by ethtool on demand
and can hold all ethtool related state.

For psychological reasons primarily (IOW I feel like people shy away
from storing state in the core because it feels expensive to add stuff
to net_device while it would not seem expensive to add it to struct
ethtool_netdev_state).

Maybe we can do the on-demand allocation later - but could we at least
wrap the ethtool-related fields in a separate struct to hold them
together?

Re: [RFC PATCH net-next 1/6] net: ethtool: attach an IDR of custom RSS contexts to a netdevice

[Edward Cree]

On 03/04/2023 22:43, Jakub Kicinski wrote:
> On Mon, 3 Apr 2023 17:32:58 +0100 edward.cree@amd.com wrote:
>> +	/* private: indirection table, hash key, and driver private data are
>> +	 * stored sequentially in @data area.  Use below helpers to access
>> +	 */
>> +	u8 data[];
> 
> I think that something needs to get aligned here...
> Driver priv needs to guarantee ulong alignment in case someone puts 
> a pointer in it.
> 
>> +static inline void *ethtool_rxfh_context_priv(struct ethtool_rxfh_context *ctx)
>> +{
>> +	return ethtool_rxfh_context_key(ctx) + ctx->key_size;
> 
> ALIGN_PTR() ... ?
> Or align data[] and reorder..

Very good points.  Indir also needs 4-byte alignment.
Will fix in next version.

>> +	u32			rss_ctx_max_id;
>> +	struct idr		rss_ctx;
> 
> noob question, why not xarray?

Because I know how to use the IDR API, but have never used
 xarray directly, so would need to learn it.

> Isn't IDR just a legacy wrapper around xarray anyway?

I see it as a *convenience* wrapper.  Is it deprecated?

Re: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[Edward Cree]

On 03/04/2023 22:48, Jakub Kicinski wrote:
> On Mon, 3 Apr 2023 17:32:59 +0100 edward.cree@amd.com wrote:
>> @@ -880,6 +896,7 @@ struct ethtool_ops {
>>  			    u8 *hfunc);
>>  	int	(*set_rxfh)(struct net_device *, const u32 *indir,
>>  			    const u8 *key, const u8 hfunc);
>> +	u16	(*get_rxfh_priv_size)(struct net_device *);
>>  	int	(*get_rxfh_context)(struct net_device *, u32 *indir, u8 *key,
>>  				    u8 *hfunc, u32 rss_context);
>>  	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,
> 
> Would a static value not do for most drivers?

Yes, it would.

> We already have a handful of data fields in the "ops" structure.

I didn't notice that / realise it was an option.  Will do.

>> @@ -1331,6 +1335,31 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
>>  		}
>>  	}
>>  
>> +	if (create) {
>> +		if (delete) {
>> +			ret = -EINVAL;
>> +			goto out;
>> +		}
>> +		ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
>> +							dev_key_size,
>> +							dev_priv_size),
>> +			      GFP_USER);
> 
> GFP_USER? Do you mean it for accounting? GFP_KERNEL_ACCOUNT?

It's an allocation triggerable by userland; I was under the
 impression that those were supposed to use GFP_USER for some
 reason; the rss_config alloc further up this function does.

Re: [RFC PATCH net-next 3/6] net: ethtool: let the core choose RSS context IDs

[Edward Cree]

On 03/04/2023 22:54, Jakub Kicinski wrote:
> On Mon, 3 Apr 2023 17:33:00 +0100 edward.cree@amd.com wrote:
>>  	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,
>>  				    const u8 *key, const u8 hfunc,
>> -				    u32 *rss_context, bool delete);
>> +				    u32 rss_context, bool delete);
> 
> Would it be easier to pass struct ethtool_rxfh_context instead of
> doing it field by field?  Otherwise Intel will need to add more
> arguments and touch all drivers. Or are you thinking that they should
> use a separate callback for the "RR RSS" or whatever their thing is?

Initially I tried to just pass in ctx with the new values already
 filled in.  But that breaks if the op fails; we have to leave the
 old values in ctx.  We maybe could create a second, ephemeral
 struct ethtool_rxfh_context to pass the new values in, but then
 we have to worry about which one's priv the driver uses.
(We can't e.g. just pass in the ephemeral one, and copy its priv
 across when we update the real ctx after the op returns, because
 what if the driver stores, say, a list_head in its priv?)

And if we did pass a struct wrapping indir, key and hfunc, then
 any patch adding more fields to it would need existing drivers
 to check the new fields were unused / set to NO_CHANGE.

So I think we just have to accept that new fields will mean
 changing all drivers.  (There's only half a dozen, anyway.)
And doing that through the op arguments means the compiler will
 catch any driver that hasn't been updated, rather than the
 driver potentially silently ignoring the new field.

> And maybe separate op for create / change / delete?

Good idea, that would also elide renaming the legacy op.

> And an extack on top... :)

Sure.

Re: [RFC PATCH net-next 5/6] net: ethtool: add a mutex protecting RSS contexts

[Edward Cree]

On 03/04/2023 23:03, Jakub Kicinski wrote:
> On Mon, 3 Apr 2023 17:33:02 +0100 edward.cree@amd.com wrote:
>>  	u32			rss_ctx_max_id;
>>  	struct idr		rss_ctx;
>> +	struct mutex		rss_lock;
> 
> Argh, the mutex doubles the size of the state, and most drivers don't
> implement this feature.  My thinking was to add a "ethtool state"> pointer to net_device which will be allocated by ethtool on demand
> and can hold all ethtool related state.

Would any other existing net_device fields go in this struct, or is
 it just the RSS stuff so far?

Re: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <57c0a5a7d41e1341e8a7b0256ca8ed6f3e3ea9c0.1680538846.git.ecree.xilinx@gmail.com>
References: <57c0a5a7d41e1341e8a7b0256ca8ed6f3e3ea9c0.1680538846.git.ecree.xilinx@gmail.com>
TO: edward.cree@amd.com

Hi,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/edward-cree-amd-com/net-ethtool-attach-an-IDR-of-custom-RSS-contexts-to-a-netdevice/20230404-003917
patch link:    https://lore.kernel.org/r/57c0a5a7d41e1341e8a7b0256ca8ed6f3e3ea9c0.1680538846.git.ecree.xilinx%40gmail.com
patch subject: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR
:::::: branch date: 25 hours ago
:::::: commit date: 25 hours ago
config: i386-randconfig-m021-20230403 (https://download.01.org/0day-ci/archive/20230405/202304050121.mBnnA2B8-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304050121.mBnnA2B8-lkp@intel.com/

New smatch warnings:
net/ethtool/ioctl.c:1402 ethtool_set_rxfh() error: uninitialized symbol 'ctx'.

Old smatch warnings:
net/ethtool/ioctl.c:1419 ethtool_set_rxfh() warn: possible memory leak of 'ctx'

vim +/ctx +1402 net/ethtool/ioctl.c

3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1245  
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1246  static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1247  					       void __user *useraddr)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1248  {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1249  	int ret;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1250  	const struct ethtool_ops *ops = dev->ethtool_ops;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1251  	struct ethtool_rxfh_context *ctx;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1252  	struct ethtool_rxnfc rx_rings;
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1253  	struct ethtool_rxfh rxfh;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1254  	u32 dev_indir_size = 0, dev_key_size = 0, dev_priv_size = 0, i;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1255  	u32 *indir = NULL, indir_bytes = 0;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1256  	u8 *hkey = NULL;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1257  	u8 *rss_config;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1258  	u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1259  	bool create = false, delete = false;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1260  
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1261  	if (!ops->get_rxnfc || !ops->set_rxfh)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1262  		return -EOPNOTSUPP;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1263  
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1264  	if (ops->get_rxfh_indir_size)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1265  		dev_indir_size = ops->get_rxfh_indir_size(dev);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1266  	if (ops->get_rxfh_key_size)
d340c862e76081 net/core/ethtool.c  Dan Carpenter   2015-02-20  1267  		dev_key_size = ops->get_rxfh_key_size(dev);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1268  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1269  	if (copy_from_user(&rxfh, useraddr, sizeof(rxfh)))
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1270  		return -EFAULT;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1271  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1272  	/* Check that reserved fields are 0 for now */
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1273  	if (rxfh.rsvd8[0] || rxfh.rsvd8[1] || rxfh.rsvd8[2] || rxfh.rsvd32)
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1274  		return -EINVAL;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1275  	/* Most drivers don't handle rss_context, check it's 0 as well */
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1276  	if (rxfh.rss_context && !ops->set_rxfh_context)
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1277  		return -EOPNOTSUPP;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1278  	create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1279  	if (create && ops->get_rxfh_priv_size)
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1280  		dev_priv_size = ops->get_rxfh_priv_size(dev);
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1281  
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1282  	/* If either indir, hash key or function is valid, proceed further.
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1283  	 * Must request at least one change: indir size, hash key or function.
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1284  	 */
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1285  	if ((rxfh.indir_size &&
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1286  	     rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE &&
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1287  	     rxfh.indir_size != dev_indir_size) ||
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1288  	    (rxfh.key_size && (rxfh.key_size != dev_key_size)) ||
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1289  	    (rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE &&
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1290  	     rxfh.key_size == 0 && rxfh.hfunc == ETH_RSS_HASH_NO_CHANGE))
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1291  		return -EINVAL;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1292  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1293  	if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1294  		indir_bytes = dev_indir_size * sizeof(indir[0]);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1295  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1296  	rss_config = kzalloc(indir_bytes + rxfh.key_size, GFP_USER);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1297  	if (!rss_config)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1298  		return -ENOMEM;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1299  
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1300  	rx_rings.cmd = ETHTOOL_GRXRINGS;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1301  	ret = ops->get_rxnfc(dev, &rx_rings, NULL);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1302  	if (ret)
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1303  		goto out;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1304  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1305  	/* rxfh.indir_size == 0 means reset the indir table to default (master
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1306  	 * context) or delete the context (other RSS contexts).
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1307  	 * rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE means leave it unchanged.
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1308  	 */
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1309  	if (rxfh.indir_size &&
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1310  	    rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1311  		indir = (u32 *)rss_config;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1312  		ret = ethtool_copy_validate_indir(indir,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1313  						  useraddr + rss_cfg_offset,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1314  						  &rx_rings,
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1315  						  rxfh.indir_size);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1316  		if (ret)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1317  			goto out;
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1318  	} else if (rxfh.indir_size == 0) {
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1319  		if (rxfh.rss_context == 0) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1320  			indir = (u32 *)rss_config;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1321  			for (i = 0; i < dev_indir_size; i++)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1322  				indir[i] = ethtool_rxfh_indir_default(i, rx_rings.data);
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1323  		} else {
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1324  			delete = true;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1325  		}
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1326  	}
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1327  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1328  	if (rxfh.key_size) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1329  		hkey = rss_config + indir_bytes;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1330  		if (copy_from_user(hkey,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1331  				   useraddr + rss_cfg_offset + indir_bytes,
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1332  				   rxfh.key_size)) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1333  			ret = -EFAULT;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1334  			goto out;
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1335  		}
278bc4296bd64f net/core/ethtool.c  Ben Hutchings   2011-12-15  1336  	}
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1337  
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1338  	if (create) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1339  		if (delete) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1340  			ret = -EINVAL;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1341  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1342  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1343  		ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1344  							dev_key_size,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1345  							dev_priv_size),
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1346  			      GFP_USER);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1347  		if (!ctx) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1348  			ret = -ENOMEM;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1349  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1350  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1351  		ctx->indir_size = dev_indir_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1352  		ctx->key_size = dev_key_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1353  		ctx->hfunc = rxfh.hfunc;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1354  		ctx->priv_size = dev_priv_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1355  	} else if (rxfh.rss_context) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1356  		ctx = idr_find(&dev->rss_ctx, rxfh.rss_context);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1357  		if (!ctx) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1358  			ret = -ENOENT;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1359  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1360  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1361  	}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1362  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1363  	if (rxfh.rss_context)
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1364  		ret = ops->set_rxfh_context(dev, indir, hkey, rxfh.hfunc,
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1365  					    &rxfh.rss_context, delete);
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1366  	else
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1367  		ret = ops->set_rxfh(dev, indir, hkey, rxfh.hfunc);
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1368  	if (ret)
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1369  		goto out;
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1370  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1371  	if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, rss_context),
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1372  			 &rxfh.rss_context, sizeof(rxfh.rss_context)))
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1373  		ret = -EFAULT;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1374  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1375  	if (!rxfh.rss_context) {
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1376  		/* indicate whether rxfh was set to default */
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1377  		if (rxfh.indir_size == 0)
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1378  			dev->priv_flags &= ~IFF_RXFH_CONFIGURED;
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1379  		else if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1380  			dev->priv_flags |= IFF_RXFH_CONFIGURED;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1381  	}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1382  	/* Update rss_ctx tracking */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1383  	if (create) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1384  		/* Ideally this should happen before calling the driver,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1385  		 * so that we can fail more cleanly; but we don't have the
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1386  		 * context ID until the driver picks it, so we have to
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1387  		 * wait until after.
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1388  		 */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1389  		if (WARN_ON(idr_find(&dev->rss_ctx, rxfh.rss_context)))
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1390  			/* context ID reused, our tracking is screwed */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1391  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1392  		/* Allocate the exact ID the driver gave us */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1393  		WARN_ON(idr_alloc(&dev->rss_ctx, ctx, rxfh.rss_context,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1394  				  rxfh.rss_context + 1, GFP_KERNEL) !=
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1395  			rxfh.rss_context);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1396  		ctx->indir_no_change = rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1397  		ctx->key_no_change = !rxfh.key_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1398  	}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1399  	if (delete) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1400  		WARN_ON(idr_remove(&dev->rss_ctx, rxfh.rss_context) != ctx);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1401  		kfree(ctx);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03 @1402  	} else if (ctx) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1403  		if (indir) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1404  			for (i = 0; i < dev_indir_size; i++)
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1405  				ethtool_rxfh_context_indir(ctx)[i] = indir[i];
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1406  			ctx->indir_no_change = 0;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1407  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1408  		if (hkey) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1409  			memcpy(ethtool_rxfh_context_key(ctx), hkey,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1410  			       dev_key_size);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1411  			ctx->key_no_change = 0;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1412  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1413  		if (rxfh.hfunc != ETH_RSS_HASH_NO_CHANGE)
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1414  			ctx->hfunc = rxfh.hfunc;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1415  	}
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1416  
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1417  out:
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1418  	kfree(rss_config);
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1419  	return ret;
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1420  }
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1421  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [RFC PATCH net-next 1/6] net: ethtool: attach an IDR of custom RSS contexts to a netdevice

[Jakub Kicinski]

On Tue, 4 Apr 2023 12:30:42 +0100 Edward Cree wrote:
> > Isn't IDR just a legacy wrapper around xarray anyway?  
> 
> I see it as a *convenience* wrapper.  Is it deprecated?

Hm, I'm not so sure what the relation between idr and xarray
is after glancing at the code.
I'm more used to seeing xarray these days but if you prefer 
to keep the IDR it's fine.

Re: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[Jakub Kicinski]

On Tue, 4 Apr 2023 12:49:18 +0100 Edward Cree wrote:
> > GFP_USER? Do you mean it for accounting? GFP_KERNEL_ACCOUNT?  
> 
> It's an allocation triggerable by userland; I was under the
>  impression that those were supposed to use GFP_USER for some
>  reason; the rss_config alloc further up this function does.

That's what I thought, too, and that it implies memory accounting.
But then someone from MM told me that that's not the case, and
that GFP_USER is supposed to be mmap()able. Or maybe the latter
part I got from the kdoc in gfp_types.h.

I think we should make sure the memory is accounted.

Re: [RFC PATCH net-next 3/6] net: ethtool: let the core choose RSS context IDs

[Jakub Kicinski]

On Tue, 4 Apr 2023 13:14:39 +0100 Edward Cree wrote:
> > Would it be easier to pass struct ethtool_rxfh_context instead of
> > doing it field by field?  Otherwise Intel will need to add more
> > arguments and touch all drivers. Or are you thinking that they should
> > use a separate callback for the "RR RSS" or whatever their thing is?  
> 
> Initially I tried to just pass in ctx with the new values already
>  filled in.  But that breaks if the op fails; we have to leave the
>  old values in ctx.  We maybe could create a second, ephemeral
>  struct ethtool_rxfh_context to pass the new values in, but then
>  we have to worry about which one's priv the driver uses.
> (We can't e.g. just pass in the ephemeral one, and copy its priv
>  across when we update the real ctx after the op returns, because
>  what if the driver stores, say, a list_head in its priv?)
> 
> And if we did pass a struct wrapping indir, key and hfunc, then
>  any patch adding more fields to it would need existing drivers
>  to check the new fields were unused / set to NO_CHANGE.
> 
> So I think we just have to accept that new fields will mean
>  changing all drivers.  (There's only half a dozen, anyway.)
> And doing that through the op arguments means the compiler will
>  catch any driver that hasn't been updated, rather than the
>  driver potentially silently ignoring the new field.

Fair point with needing to copy in case of error, okay :(

Re: [RFC PATCH net-next 5/6] net: ethtool: add a mutex protecting RSS contexts

[Jakub Kicinski]

On Tue, 4 Apr 2023 13:32:04 +0100 Edward Cree wrote:
> > Argh, the mutex doubles the size of the state, and most drivers don't
> > implement this feature.  My thinking was to add a "ethtool state">
> > pointer to net_device which will be allocated by ethtool on demand
> > and can hold all ethtool related state.  
> 
> Would any other existing net_device fields go in this struct, or is
>  it just the RSS stuff so far?

Only wol_enabled possibly, nothing else looks relevant.
But wol_enabled is one bit so up to you if you want to move it.

Re: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[Edward Cree]

On 05/04/2023 00:40, Jakub Kicinski wrote:
> On Tue, 4 Apr 2023 12:49:18 +0100 Edward Cree wrote:
>>> GFP_USER? Do you mean it for accounting? GFP_KERNEL_ACCOUNT?  
>>
>> It's an allocation triggerable by userland; I was under the
>>  impression that those were supposed to use GFP_USER for some
>>  reason; the rss_config alloc further up this function does.
> 
> That's what I thought, too, and that it implies memory accounting.
> But then someone from MM told me that that's not the case, and
> that GFP_USER is supposed to be mmap()able. Or maybe the latter
> part I got from the kdoc in gfp_types.h.
> 
> I think we should make sure the memory is accounted.

Okay.  Presumably this doesn't apply to `rss_config` because it's
 short-lived (freed on the way out of the function)?
(In which case I think `rss_config` should just be GFP_KERNEL;
 we don't try to map it to userspace, we just copy_{from,to}_user
 between it and the ioctl data.  But that's unrelated cleanup
 which we can worry about later, if at all.)

Re: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[Dan Carpenter]

Hi,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/edward-cree-amd-com/net-ethtool-attach-an-IDR-of-custom-RSS-contexts-to-a-netdevice/20230404-003917
patch link:    https://lore.kernel.org/r/57c0a5a7d41e1341e8a7b0256ca8ed6f3e3ea9c0.1680538846.git.ecree.xilinx%40gmail.com
patch subject: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR
config: i386-randconfig-m021-20230403 (https://download.01.org/0day-ci/archive/20230405/202304050121.mBnnA2B8-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202304050121.mBnnA2B8-lkp@intel.com/

New smatch warnings:
net/ethtool/ioctl.c:1402 ethtool_set_rxfh() error: uninitialized symbol 'ctx'.

Old smatch warnings:
net/ethtool/ioctl.c:1419 ethtool_set_rxfh() warn: possible memory leak of 'ctx'

vim +/ctx +1402 net/ethtool/ioctl.c

3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1246  static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1247  					       void __user *useraddr)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1248  {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1249  	int ret;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1250  	const struct ethtool_ops *ops = dev->ethtool_ops;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1251  	struct ethtool_rxfh_context *ctx;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1252  	struct ethtool_rxnfc rx_rings;
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1253  	struct ethtool_rxfh rxfh;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1254  	u32 dev_indir_size = 0, dev_key_size = 0, dev_priv_size = 0, i;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1255  	u32 *indir = NULL, indir_bytes = 0;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1256  	u8 *hkey = NULL;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1257  	u8 *rss_config;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1258  	u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1259  	bool create = false, delete = false;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1260  
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1261  	if (!ops->get_rxnfc || !ops->set_rxfh)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1262  		return -EOPNOTSUPP;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1263  
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1264  	if (ops->get_rxfh_indir_size)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1265  		dev_indir_size = ops->get_rxfh_indir_size(dev);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1266  	if (ops->get_rxfh_key_size)
d340c862e76081 net/core/ethtool.c  Dan Carpenter   2015-02-20  1267  		dev_key_size = ops->get_rxfh_key_size(dev);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1268  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1269  	if (copy_from_user(&rxfh, useraddr, sizeof(rxfh)))
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1270  		return -EFAULT;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1271  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1272  	/* Check that reserved fields are 0 for now */
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1273  	if (rxfh.rsvd8[0] || rxfh.rsvd8[1] || rxfh.rsvd8[2] || rxfh.rsvd32)
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1274  		return -EINVAL;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1275  	/* Most drivers don't handle rss_context, check it's 0 as well */
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1276  	if (rxfh.rss_context && !ops->set_rxfh_context)
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1277  		return -EOPNOTSUPP;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1278  	create = rxfh.rss_context == ETH_RXFH_CONTEXT_ALLOC;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1279  	if (create && ops->get_rxfh_priv_size)
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1280  		dev_priv_size = ops->get_rxfh_priv_size(dev);
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1281  
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1282  	/* If either indir, hash key or function is valid, proceed further.
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1283  	 * Must request at least one change: indir size, hash key or function.
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1284  	 */
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1285  	if ((rxfh.indir_size &&
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1286  	     rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE &&
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1287  	     rxfh.indir_size != dev_indir_size) ||
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1288  	    (rxfh.key_size && (rxfh.key_size != dev_key_size)) ||
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1289  	    (rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE &&
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1290  	     rxfh.key_size == 0 && rxfh.hfunc == ETH_RSS_HASH_NO_CHANGE))
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1291  		return -EINVAL;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1292  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1293  	if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1294  		indir_bytes = dev_indir_size * sizeof(indir[0]);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1295  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1296  	rss_config = kzalloc(indir_bytes + rxfh.key_size, GFP_USER);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1297  	if (!rss_config)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1298  		return -ENOMEM;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1299  
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1300  	rx_rings.cmd = ETHTOOL_GRXRINGS;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1301  	ret = ops->get_rxnfc(dev, &rx_rings, NULL);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1302  	if (ret)
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1303  		goto out;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1304  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1305  	/* rxfh.indir_size == 0 means reset the indir table to default (master
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1306  	 * context) or delete the context (other RSS contexts).
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1307  	 * rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE means leave it unchanged.
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1308  	 */
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1309  	if (rxfh.indir_size &&
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1310  	    rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1311  		indir = (u32 *)rss_config;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1312  		ret = ethtool_copy_validate_indir(indir,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1313  						  useraddr + rss_cfg_offset,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1314  						  &rx_rings,
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1315  						  rxfh.indir_size);
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1316  		if (ret)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1317  			goto out;
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1318  	} else if (rxfh.indir_size == 0) {
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1319  		if (rxfh.rss_context == 0) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1320  			indir = (u32 *)rss_config;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1321  			for (i = 0; i < dev_indir_size; i++)
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1322  				indir[i] = ethtool_rxfh_indir_default(i, rx_rings.data);
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1323  		} else {
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1324  			delete = true;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1325  		}
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1326  	}
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1327  
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1328  	if (rxfh.key_size) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1329  		hkey = rss_config + indir_bytes;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1330  		if (copy_from_user(hkey,
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1331  				   useraddr + rss_cfg_offset + indir_bytes,
f062a3844845d2 net/core/ethtool.c  Ben Hutchings   2014-05-15  1332  				   rxfh.key_size)) {
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1333  			ret = -EFAULT;
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1334  			goto out;
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1335  		}
278bc4296bd64f net/core/ethtool.c  Ben Hutchings   2011-12-15  1336  	}
7850f63f162051 net/core/ethtool.c  Ben Hutchings   2011-12-15  1337  
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1338  	if (create) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1339  		if (delete) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1340  			ret = -EINVAL;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1341  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1342  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1343  		ctx = kzalloc(ethtool_rxfh_context_size(dev_indir_size,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1344  							dev_key_size,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1345  							dev_priv_size),
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1346  			      GFP_USER);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1347  		if (!ctx) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1348  			ret = -ENOMEM;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1349  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1350  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1351  		ctx->indir_size = dev_indir_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1352  		ctx->key_size = dev_key_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1353  		ctx->hfunc = rxfh.hfunc;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1354  		ctx->priv_size = dev_priv_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1355  	} else if (rxfh.rss_context) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1356  		ctx = idr_find(&dev->rss_ctx, rxfh.rss_context);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1357  		if (!ctx) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1358  			ret = -ENOENT;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1359  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1360  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1361  	}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1362  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1363  	if (rxfh.rss_context)
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1364  		ret = ops->set_rxfh_context(dev, indir, hkey, rxfh.hfunc,
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1365  					    &rxfh.rss_context, delete);
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1366  	else
892311f66f2411 net/core/ethtool.c  Eyal Perry      2014-12-02  1367  		ret = ops->set_rxfh(dev, indir, hkey, rxfh.hfunc);
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1368  	if (ret)
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1369  		goto out;
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1370  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1371  	if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh, rss_context),
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1372  			 &rxfh.rss_context, sizeof(rxfh.rss_context)))
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1373  		ret = -EFAULT;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1374  
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1375  	if (!rxfh.rss_context) {
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1376  		/* indicate whether rxfh was set to default */
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1377  		if (rxfh.indir_size == 0)
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1378  			dev->priv_flags &= ~IFF_RXFH_CONFIGURED;
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1379  		else if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
d4ab4286276fcd net/core/ethtool.c  Keller, Jacob E 2016-02-08  1380  			dev->priv_flags |= IFF_RXFH_CONFIGURED;
84a1d9c4820080 net/core/ethtool.c  Edward Cree     2018-03-08  1381  	}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1382  	/* Update rss_ctx tracking */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1383  	if (create) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1384  		/* Ideally this should happen before calling the driver,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1385  		 * so that we can fail more cleanly; but we don't have the
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1386  		 * context ID until the driver picks it, so we have to
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1387  		 * wait until after.
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1388  		 */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1389  		if (WARN_ON(idr_find(&dev->rss_ctx, rxfh.rss_context)))
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1390  			/* context ID reused, our tracking is screwed */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1391  			goto out;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1392  		/* Allocate the exact ID the driver gave us */
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1393  		WARN_ON(idr_alloc(&dev->rss_ctx, ctx, rxfh.rss_context,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1394  				  rxfh.rss_context + 1, GFP_KERNEL) !=
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1395  			rxfh.rss_context);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1396  		ctx->indir_no_change = rxfh.indir_size == ETH_RXFH_INDIR_NO_CHANGE;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1397  		ctx->key_no_change = !rxfh.key_size;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1398  	}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1399  	if (delete) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1400  		WARN_ON(idr_remove(&dev->rss_ctx, rxfh.rss_context) != ctx);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1401  		kfree(ctx);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03 @1402  	} else if (ctx) {

"ctx" is either non-NULL or uninitialized so this condition doesn't
make sense.

59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1403  		if (indir) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1404  			for (i = 0; i < dev_indir_size; i++)
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1405  				ethtool_rxfh_context_indir(ctx)[i] = indir[i];
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1406  			ctx->indir_no_change = 0;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1407  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1408  		if (hkey) {
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1409  			memcpy(ethtool_rxfh_context_key(ctx), hkey,
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1410  			       dev_key_size);
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1411  			ctx->key_no_change = 0;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1412  		}
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1413  		if (rxfh.hfunc != ETH_RSS_HASH_NO_CHANGE)
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1414  			ctx->hfunc = rxfh.hfunc;
59b320b4d43d43 net/ethtool/ioctl.c Edward Cree     2023-04-03  1415  	}
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1416  
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1417  out:
3de0b592394d17 net/core/ethtool.c  Venkata Duvvuru 2014-04-21  1418  	kfree(rss_config);
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1419  	return ret;
a5b6ee291e39e2 net/core/ethtool.c  Ben Hutchings   2010-06-30  1420  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [RFC PATCH net-next 2/6] net: ethtool: record custom RSS contexts in the IDR

[Edward Cree]

On 06/04/2023 10:07, Dan Carpenter wrote:
> New smatch warnings:
> net/ethtool/ioctl.c:1402 ethtool_set_rxfh() error: uninitialized symbol 'ctx'.

Yup, it needs to be initialised to NULL for the default context case.
Thanks for catching.

Re: [RFC PATCH net-next 4/6] net: ethtool: pass ctx_priv and create into .set_rxfh_context

[kernel test robot]

Hi,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/edward-cree-amd-com/net-ethtool-attach-an-IDR-of-custom-RSS-contexts-to-a-netdevice/20230404-003917
patch link:    https://lore.kernel.org/r/2b0bb0b96c09cc79d39ef79ce6733fc0244c5548.1680538846.git.ecree.xilinx%40gmail.com
patch subject: [RFC PATCH net-next 4/6] net: ethtool: pass ctx_priv and create into .set_rxfh_context
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20230412/202304120334.ipYDVA8i-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/44c47024e39be318c99069a6e3ffc9ef18a29cfe
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review edward-cree-amd-com/net-ethtool-attach-an-IDR-of-custom-RSS-contexts-to-a-netdevice/20230404-003917
        git checkout 44c47024e39be318c99069a6e3ffc9ef18a29cfe
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpio/ drivers/mtd/spi-nor/ drivers/net/ethernet/marvell/octeontx2/nic/ drivers/net/ethernet/netronome/nfp/ drivers/net/wireless/mediatek/mt76/mt7996/ drivers/nvmem/ drivers/rtc/ drivers/spi/ drivers/staging/media/tegra-video/ net/ipv4/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304120334.ipYDVA8i-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1447:35: error: initialization of 'int (*)(struct net_device *, struct ethtool_rxfh_context *, const u32 *, const u8 *, const u8,  u32,  bool,  bool)' {aka 'int (*)(struct net_device *, struct ethtool_rxfh_context *, const unsigned int *, const unsigned char *, const unsigned char,  unsigned int,  _Bool,  _Bool)'} from incompatible pointer type 'int (*)(struct net_device *, const u32 *, const u8 *, const u8,  u32 *, bool)' {aka 'int (*)(struct net_device *, const unsigned int *, const unsigned char *, const unsigned char,  unsigned int *, _Bool)'} [-Werror=incompatible-pointer-types]
    1447 |         .set_rxfh_context       = otx2_set_rxfh_context,
         |                                   ^~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c:1447:35: note: (near initialization for 'otx2vf_ethtool_ops.set_rxfh_context')
   cc1: some warnings being treated as errors


vim +1447 drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c

1a50280c25ecfb Christina Jacob      2021-02-09  1426  
05c22b5431488d Tomasz Duszynski     2020-03-21  1427  static const struct ethtool_ops otx2vf_ethtool_ops = {
05c22b5431488d Tomasz Duszynski     2020-03-21  1428  	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
619c010a65391d Suman Ghosh          2022-06-12  1429  				     ETHTOOL_COALESCE_MAX_FRAMES |
619c010a65391d Suman Ghosh          2022-06-12  1430  				     ETHTOOL_COALESCE_USE_ADAPTIVE,
68258596cbc9a6 Subbaraya Sundeep    2022-02-23  1431  	.supported_ring_params  = ETHTOOL_RING_USE_RX_BUF_LEN |
68258596cbc9a6 Subbaraya Sundeep    2022-02-23  1432  				  ETHTOOL_RING_USE_CQE_SIZE,
05c22b5431488d Tomasz Duszynski     2020-03-21  1433  	.get_link		= otx2_get_link,
05c22b5431488d Tomasz Duszynski     2020-03-21  1434  	.get_drvinfo		= otx2vf_get_drvinfo,
05c22b5431488d Tomasz Duszynski     2020-03-21  1435  	.get_strings		= otx2vf_get_strings,
05c22b5431488d Tomasz Duszynski     2020-03-21  1436  	.get_ethtool_stats	= otx2vf_get_ethtool_stats,
05c22b5431488d Tomasz Duszynski     2020-03-21  1437  	.get_sset_count		= otx2vf_get_sset_count,
05c22b5431488d Tomasz Duszynski     2020-03-21  1438  	.set_channels		= otx2_set_channels,
05c22b5431488d Tomasz Duszynski     2020-03-21  1439  	.get_channels		= otx2_get_channels,
3cffaed2136cd6 Rakesh Babu          2021-08-17  1440  	.get_rxnfc		= otx2_get_rxnfc,
3cffaed2136cd6 Rakesh Babu          2021-08-17  1441  	.set_rxnfc              = otx2_set_rxnfc,
05c22b5431488d Tomasz Duszynski     2020-03-21  1442  	.get_rxfh_key_size	= otx2_get_rxfh_key_size,
05c22b5431488d Tomasz Duszynski     2020-03-21  1443  	.get_rxfh_indir_size	= otx2_get_rxfh_indir_size,
05c22b5431488d Tomasz Duszynski     2020-03-21  1444  	.get_rxfh		= otx2_get_rxfh,
05c22b5431488d Tomasz Duszynski     2020-03-21  1445  	.set_rxfh		= otx2_set_rxfh,
81a4362016e7d8 Geetha sowjanya      2021-01-04  1446  	.get_rxfh_context	= otx2_get_rxfh_context,
81a4362016e7d8 Geetha sowjanya      2021-01-04 @1447  	.set_rxfh_context	= otx2_set_rxfh_context,
05c22b5431488d Tomasz Duszynski     2020-03-21  1448  	.get_ringparam		= otx2_get_ringparam,
05c22b5431488d Tomasz Duszynski     2020-03-21  1449  	.set_ringparam		= otx2_set_ringparam,
05c22b5431488d Tomasz Duszynski     2020-03-21  1450  	.get_coalesce		= otx2_get_coalesce,
05c22b5431488d Tomasz Duszynski     2020-03-21  1451  	.set_coalesce		= otx2_set_coalesce,
05c22b5431488d Tomasz Duszynski     2020-03-21  1452  	.get_msglevel		= otx2_get_msglevel,
05c22b5431488d Tomasz Duszynski     2020-03-21  1453  	.set_msglevel		= otx2_set_msglevel,
05c22b5431488d Tomasz Duszynski     2020-03-21  1454  	.get_pauseparam		= otx2_get_pauseparam,
05c22b5431488d Tomasz Duszynski     2020-03-21  1455  	.set_pauseparam		= otx2_set_pauseparam,
1a50280c25ecfb Christina Jacob      2021-02-09  1456  	.get_link_ksettings     = otx2vf_get_link_ksettings,
43510ef4ddad39 Naveen Mamindlapalli 2021-09-28  1457  	.get_ts_info		= otx2_get_ts_info,
05c22b5431488d Tomasz Duszynski     2020-03-21  1458  };
05c22b5431488d Tomasz Duszynski     2020-03-21  1459  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests