[PATCH net-next 00/11] ethtool: rss: support RSS_SET via Netlink

[PATCH net-next 00/11] ethtool: rss: support RSS_SET via Netlink

[Jakub Kicinski]

Support configuring RSS settings via Netlink.
Creating and removing contexts remains for the following series.

Jakub Kicinski (11):
  ethtool: rss: initial RSS_SET (indirection table handling)
  selftests: drv-net: rss_api: factor out checking min queue count
  tools: ynl: support packing binary arrays of scalars
  selftests: drv-net: rss_api: test setting indirection table via
    Netlink
  ethtool: rss: support setting hfunc via Netlink
  ethtool: rss: support setting hkey via Netlink
  selftests: drv-net: rss_api: test setting hashing key via Netlink
  netlink: specs: define input-xfrm enum in the spec
  ethtool: rss: support setting input-xfrm via Netlink
  ethtool: rss: support setting flow hashing fields
  selftests: drv-net: rss_api: test input-xfrm and hash fields

 Documentation/netlink/specs/ethtool.yaml      |  39 ++
 Documentation/networking/ethtool-netlink.rst  |  29 +-
 .../uapi/linux/ethtool_netlink_generated.h    |   1 +
 net/ethtool/common.h                          |   1 +
 net/ethtool/netlink.h                         |   1 +
 net/ethtool/common.c                          |  15 +
 net/ethtool/ioctl.c                           |   4 +-
 net/ethtool/netlink.c                         |   8 +
 net/ethtool/rss.c                             | 384 ++++++++++++++++++
 tools/net/ynl/pyynl/lib/ynl.py                |   7 +-
 .../selftests/drivers/net/hw/rss_api.py       | 278 ++++++++++++-
 11 files changed, 750 insertions(+), 17 deletions(-)

-- 
2.50.1

[PATCH net-next 01/11] ethtool: rss: initial RSS_SET (indirection table handling)

[Jakub Kicinski]

Add initial support for RSS_SET, for now only operations on
the indirection table are supported.

There are two special cases here:
 1) resetting the table to defaults;
 2) support for tables of different size.

For (1) I use an empty Netlink attribute (array of size 0).

(2) may require some background. AFAICT a lot of modern devices
allow allocating RSS tables of different sizes. mlx5 can upsize
its tables, bnxt has some "table size calculation", and Intel
folks asked about RSS table sizing in context of resource allocation
in the past. The ethtool IOCTL API has a concept of table size,
but right now the user is expected to provide a table exactly
the size the device requests. Some drivers may change the table
size at runtime (in response to queue count changes) but the
user is not in control of this. What's not great is that all
RSS contexts share the same table size. For example a device
with 128 queues enabled, 16 RSS contexts 8 queues in each will
likely have 256 entry tables for each of the 16 contexts,
while 32 would be more than enough given each context only has
8 queues. To address this the Netlink API should avoid enforcing
table size at the uAPI level, and should allow the user to express
the min table size they expect.

To fully solve (2) we will need more driver plumbing but
at the uAPI level this patch allows the user to specify
a table size smaller than what the device advertises. The device
table size must be a multiple of the user requested table size.
We then replicate the user-provided table to fill the full device
size table. This addresses the "allow the user to express the min
table size" objective, while not enforcing any fixed size.
From Netlink perspective .get_rxfh_indir_size() is now de facto
the "max" table size supported by the device.

We may choose to support table replication in ethtool, too,
when we actually plumb this thru the device APIs.

Initially I was considering moving full pattern generation
to the kernel (which queues to use, at which frequency and
what min sequence length). I don't think this complexity
would buy us much and most if not all devices have pow-2
table sizes, which simplifies the replication a lot.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/ethtool.yaml      |  12 ++
 Documentation/networking/ethtool-netlink.rst  |  26 ++-
 .../uapi/linux/ethtool_netlink_generated.h    |   1 +
 net/ethtool/netlink.h                         |   1 +
 net/ethtool/netlink.c                         |   8 +
 net/ethtool/rss.c                             | 192 ++++++++++++++++++
 6 files changed, 239 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index c38c03c624f0..1eca88a508a0 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -2643,6 +2643,18 @@ c-version-name: ethtool-genl-version
         attributes:
           - header
           - events
+    -
+      name: rss-set
+      doc: Set RSS params.
+
+      attribute-set: rss
+
+      do:
+        request:
+          attributes:
+            - header
+            - context
+            - indir
     -
       name: rss-ntf
       doc: |
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 248bc3d93da9..27db7540e60e 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -239,6 +239,7 @@ All constants identifying message types use ``ETHTOOL_CMD_`` prefix and suffix
   ``ETHTOOL_MSG_PHY_GET``               get Ethernet PHY information
   ``ETHTOOL_MSG_TSCONFIG_GET``          get hw timestamping configuration
   ``ETHTOOL_MSG_TSCONFIG_SET``          set hw timestamping configuration
+  ``ETHTOOL_MSG_RSS_SET``               set RSS settings
   ===================================== =================================
 
 Kernel to userspace:
@@ -292,6 +293,7 @@ All constants identifying message types use ``ETHTOOL_CMD_`` prefix and suffix
   ``ETHTOOL_MSG_TSCONFIG_GET_REPLY``       hw timestamping configuration
   ``ETHTOOL_MSG_TSCONFIG_SET_REPLY``       new hw timestamping configuration
   ``ETHTOOL_MSG_PSE_NTF``                  PSE events notification
+  ``ETHTOOL_MSG_RSS_NTF``                  RSS settings notification
   ======================================== =================================
 
 ``GET`` requests are sent by userspace applications to retrieve device
@@ -1989,6 +1991,28 @@ hfunc. Current supported options are symmetric-xor and symmetric-or-xor.
 ETHTOOL_A_RSS_FLOW_HASH carries per-flow type bitmask of which header
 fields are included in the hash calculation.
 
+RSS_SET
+=======
+
+Request contents:
+
+=====================================  ======  ==============================
+  ``ETHTOOL_A_RSS_HEADER``             nested  request header
+  ``ETHTOOL_A_RSS_CONTEXT``            u32     context number
+  ``ETHTOOL_A_RSS_INDIR``              binary  Indir table bytes
+=====================================  ======  ==============================
+
+``ETHTOOL_A_RSS_INDIR`` is the minimal RSS table the user expects. Kernel and
+the device driver may replicate the table if its smaller than smallest table
+size supported by the device. For example if user requests ``[0, 1]`` but the
+device needs at least 8 entries - the real table in use will end up being
+``[0, 1, 0, 1, 0, 1, 0, 1]``. Most devices require the table size to be power
+of 2, so tables which size is not a power of 2 will likely be rejected.
+Using table of size 0 will reset the indirection table to the default.
+
+Note that, at present, only a subset of RSS configuration can be accomplished
+over Netlink.
+
 PLCA_GET_CFG
 ============
 
@@ -2455,7 +2479,7 @@ are netlink only.
   ``ETHTOOL_GRXNTUPLE``               n/a
   ``ETHTOOL_GSSET_INFO``              ``ETHTOOL_MSG_STRSET_GET``
   ``ETHTOOL_GRXFHINDIR``              ``ETHTOOL_MSG_RSS_GET``
-  ``ETHTOOL_SRXFHINDIR``              n/a
+  ``ETHTOOL_SRXFHINDIR``              ``ETHTOOL_MSG_RSS_SET``
   ``ETHTOOL_GFEATURES``               ``ETHTOOL_MSG_FEATURES_GET``
   ``ETHTOOL_SFEATURES``               ``ETHTOOL_MSG_FEATURES_SET``
   ``ETHTOOL_GCHANNELS``               ``ETHTOOL_MSG_CHANNELS_GET``
diff --git a/include/uapi/linux/ethtool_netlink_generated.h b/include/uapi/linux/ethtool_netlink_generated.h
index 96027e26ffba..130bdf5c3516 100644
--- a/include/uapi/linux/ethtool_netlink_generated.h
+++ b/include/uapi/linux/ethtool_netlink_generated.h
@@ -840,6 +840,7 @@ enum {
 	ETHTOOL_MSG_PHY_GET,
 	ETHTOOL_MSG_TSCONFIG_GET,
 	ETHTOOL_MSG_TSCONFIG_SET,
+	ETHTOOL_MSG_RSS_SET,
 
 	__ETHTOOL_MSG_USER_CNT,
 	ETHTOOL_MSG_USER_MAX = (__ETHTOOL_MSG_USER_CNT - 1)
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 94a7eb402022..620dd1ab9b3b 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -484,6 +484,7 @@ extern const struct nla_policy ethnl_module_set_policy[ETHTOOL_A_MODULE_POWER_MO
 extern const struct nla_policy ethnl_pse_get_policy[ETHTOOL_A_PSE_HEADER + 1];
 extern const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1];
 extern const struct nla_policy ethnl_rss_get_policy[ETHTOOL_A_RSS_START_CONTEXT + 1];
+extern const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1];
 extern const struct nla_policy ethnl_plca_get_cfg_policy[ETHTOOL_A_PLCA_HEADER + 1];
 extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1];
 extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index b1f8999c1adc..0ae0d7a9667c 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -405,6 +405,7 @@ ethnl_default_requests[__ETHTOOL_MSG_USER_CNT] = {
 	[ETHTOOL_MSG_PSE_GET]		= &ethnl_pse_request_ops,
 	[ETHTOOL_MSG_PSE_SET]		= &ethnl_pse_request_ops,
 	[ETHTOOL_MSG_RSS_GET]		= &ethnl_rss_request_ops,
+	[ETHTOOL_MSG_RSS_SET]		= &ethnl_rss_request_ops,
 	[ETHTOOL_MSG_PLCA_GET_CFG]	= &ethnl_plca_cfg_request_ops,
 	[ETHTOOL_MSG_PLCA_SET_CFG]	= &ethnl_plca_cfg_request_ops,
 	[ETHTOOL_MSG_PLCA_GET_STATUS]	= &ethnl_plca_status_request_ops,
@@ -1504,6 +1505,13 @@ static const struct genl_ops ethtool_genl_ops[] = {
 		.policy = ethnl_tsconfig_set_policy,
 		.maxattr = ARRAY_SIZE(ethnl_tsconfig_set_policy) - 1,
 	},
+	{
+		.cmd	= ETHTOOL_MSG_RSS_SET,
+		.flags	= GENL_UNS_ADMIN_PERM,
+		.doit	= ethnl_default_set_doit,
+		.policy = ethnl_rss_set_policy,
+		.maxattr = ARRAY_SIZE(ethnl_rss_set_policy) - 1,
+	},
 };
 
 static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
index 41ab9fc67652..7167fc3c27a0 100644
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -218,6 +218,8 @@ rss_prepare(const struct rss_req_info *request, struct net_device *dev,
 {
 	rss_prepare_flow_hash(request, dev, data, info);
 
+	if (!dev->ethtool_ops->get_rxfh)
+		return 0;
 	if (request->rss_context)
 		return rss_prepare_ctx(request, dev, data, info);
 	return rss_prepare_get(request, dev, data, info);
@@ -466,6 +468,192 @@ void ethtool_rss_notify(struct net_device *dev, u32 rss_context)
 	ethnl_notify(dev, ETHTOOL_MSG_RSS_NTF, &req_info.base);
 }
 
+/* RSS_SET */
+
+const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1] = {
+	[ETHTOOL_A_RSS_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+	[ETHTOOL_A_RSS_CONTEXT] = { .type = NLA_U32, },
+	[ETHTOOL_A_RSS_INDIR] = { .type = NLA_BINARY, },
+};
+
+static int
+ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
+	struct rss_req_info *request = RSS_REQINFO(req_info);
+	struct nlattr **tb = info->attrs;
+	struct nlattr *bad_attr = NULL;
+
+	if (request->rss_context && !ops->create_rxfh_context)
+		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_CONTEXT];
+
+	if (bad_attr) {
+		NL_SET_BAD_ATTR(info->extack, bad_attr);
+		return -EOPNOTSUPP;
+	}
+
+	return 1;
+}
+
+static int
+rss_set_prep_indir(struct net_device *dev, struct genl_info *info,
+		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh,
+		   bool *reset, bool *mod)
+{
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	struct netlink_ext_ack *extack = info->extack;
+	struct nlattr **tb = info->attrs;
+	struct ethtool_rxnfc rx_rings;
+	size_t alloc_size;
+	u32 user_size;
+	int i, err;
+
+	if (!tb[ETHTOOL_A_RSS_INDIR])
+		return 0;
+	if (!data->indir_size)
+		return -EOPNOTSUPP;
+
+	rx_rings.cmd = ETHTOOL_GRXRINGS;
+	err = ops->get_rxnfc(dev, &rx_rings, NULL);
+	if (err)
+		return err;
+
+	if (nla_len(tb[ETHTOOL_A_RSS_INDIR]) % 4) {
+		NL_SET_BAD_ATTR(info->extack, tb[ETHTOOL_A_RSS_INDIR]);
+		return -EINVAL;
+	}
+	user_size = nla_len(tb[ETHTOOL_A_RSS_INDIR]) / 4;
+	if (!user_size) {
+		if (rxfh->rss_context) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_RSS_INDIR],
+					    "can't reset table for a context");
+			return -EINVAL;
+		}
+		*reset = true;
+	} else if (data->indir_size % user_size) {
+		NL_SET_ERR_MSG_ATTR_FMT(extack, tb[ETHTOOL_A_RSS_INDIR],
+					"size (%d) mismatch with device indir table (%d)",
+					user_size, data->indir_size);
+		return -EINVAL;
+	}
+
+	rxfh->indir_size = data->indir_size;
+	alloc_size = array_size(data->indir_size, sizeof(rxfh->indir[0]));
+	rxfh->indir = kzalloc(alloc_size, GFP_KERNEL);
+	if (!rxfh->indir)
+		return -ENOMEM;
+
+	nla_memcpy(rxfh->indir, tb[ETHTOOL_A_RSS_INDIR], alloc_size);
+	for (i = 0; i < user_size; i++) {
+		if (rxfh->indir[i] < rx_rings.data)
+			continue;
+
+		NL_SET_ERR_MSG_ATTR_FMT(extack, tb[ETHTOOL_A_RSS_INDIR],
+					"entry %d: queue out of range (%d)",
+					i, rxfh->indir[i]);
+		err = -EINVAL;
+		goto err_free;
+	}
+
+	if (user_size) {
+		/* Replicate the user-provided table to fill the device table */
+		for (i = user_size; i < data->indir_size; i++)
+			rxfh->indir[i] = rxfh->indir[i % user_size];
+	} else {
+		for (i = 0; i < data->indir_size; i++)
+			rxfh->indir[i] =
+				ethtool_rxfh_indir_default(i, rx_rings.data);
+	}
+
+	*mod |= memcmp(rxfh->indir, data->indir_table, data->indir_size);
+
+	return 0;
+
+err_free:
+	kfree(rxfh->indir);
+	rxfh->indir = NULL;
+	return err;
+}
+
+static void
+rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
+		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
+{
+	int i;
+
+	if (rxfh->indir) {
+		for (i = 0; i < data->indir_size; i++)
+			ethtool_rxfh_context_indir(ctx)[i] = rxfh->indir[i];
+		ctx->indir_configured = !!nla_len(tb[ETHTOOL_A_RSS_INDIR]);
+	}
+}
+
+static int
+ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+	struct rss_req_info *request = RSS_REQINFO(req_info);
+	bool indir_reset = false, indir_mod = false;
+	struct ethtool_rxfh_context *ctx = NULL;
+	struct net_device *dev = req_info->dev;
+	struct ethtool_rxfh_param rxfh = {};
+	struct nlattr **tb = info->attrs;
+	struct rss_reply_data data = {};
+	const struct ethtool_ops *ops;
+	bool mod = false;
+	int ret;
+
+	ops = dev->ethtool_ops;
+	data.base.dev = dev;
+
+	ret = rss_prepare(request, dev, &data, info);
+	if (ret)
+		return ret;
+
+	rxfh.rss_context = request->rss_context;
+
+	ret = rss_set_prep_indir(dev, info, &data, &rxfh,
+				 &indir_reset, &indir_mod);
+	if (ret)
+		goto exit_clean_data;
+	mod |= indir_mod;
+
+	rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
+	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
+
+	mutex_lock(&dev->ethtool->rss_lock);
+	if (request->rss_context) {
+		ctx = xa_load(&dev->ethtool->rss_ctx, request->rss_context);
+		if (!ctx) {
+			ret = -ENOENT;
+			goto exit_unlock;
+		}
+	}
+
+	if (!mod)
+		ret = 0; /* nothing to tell the driver */
+	else if (!rxfh.rss_context)
+		ret = ops->set_rxfh(dev, &rxfh, info->extack);
+	else
+		ret = ops->modify_rxfh_context(dev, ctx, &rxfh, info->extack);
+	if (ret)
+		goto exit_unlock;
+
+	if (ctx)
+		rss_set_ctx_update(ctx, tb, &data, &rxfh);
+	else if (indir_reset)
+		dev->priv_flags &= ~IFF_RXFH_CONFIGURED;
+	else if (indir_mod)
+		dev->priv_flags |= IFF_RXFH_CONFIGURED;
+
+exit_unlock:
+	mutex_unlock(&dev->ethtool->rss_lock);
+	kfree(rxfh.indir);
+exit_clean_data:
+	rss_cleanup_data(&data.base);
+
+	return ret ?: mod;
+}
+
 const struct ethnl_request_ops ethnl_rss_request_ops = {
 	.request_cmd		= ETHTOOL_MSG_RSS_GET,
 	.reply_cmd		= ETHTOOL_MSG_RSS_GET_REPLY,
@@ -478,4 +666,8 @@ const struct ethnl_request_ops ethnl_rss_request_ops = {
 	.reply_size		= rss_reply_size,
 	.fill_reply		= rss_fill_reply,
 	.cleanup_data		= rss_cleanup_data,
+
+	.set_validate		= ethnl_rss_set_validate,
+	.set			= ethnl_rss_set,
+	.set_ntf_cmd		= ETHTOOL_MSG_RSS_NTF,
 };
-- 
2.50.1

[PATCH net-next 02/11] selftests: drv-net: rss_api: factor out checking min queue count

[Jakub Kicinski]

Multiple tests check min queue count, create a helper.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../testing/selftests/drivers/net/hw/rss_api.py | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_api.py b/tools/testing/selftests/drivers/net/hw/rss_api.py
index 6ae908bed1a4..2c76fbdb2617 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_api.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_api.py
@@ -13,6 +13,13 @@ from lib.py import EthtoolFamily
 from lib.py import NetDrvEnv
 
 
+def _require_2qs(cfg):
+    qcnt = len(glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*"))
+    if qcnt < 2:
+        raise KsftSkipEx(f"Local has only {qcnt} queues")
+    return qcnt
+
+
 def _ethtool_create(cfg, act, opts):
     output = ethtool(f"{act} {cfg.ifname} {opts}").stdout
     # Output will be something like: "New RSS context is 1" or
@@ -57,10 +64,7 @@ from lib.py import NetDrvEnv
     Check that Netlink notifications are generated when RSS indirection
     table was modified.
     """
-
-    qcnt = len(glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*"))
-    if qcnt < 2:
-        raise KsftSkipEx(f"Local has only {qcnt} queues")
+    _require_2qs(cfg)
 
     ethnl = EthtoolFamily()
     ethnl.ntf_subscribe("monitor")
@@ -88,10 +92,7 @@ from lib.py import NetDrvEnv
     Check that Netlink notifications are generated when RSS indirection
     table was modified on an additional RSS context.
     """
-
-    qcnt = len(glob.glob(f"/sys/class/net/{cfg.ifname}/queues/rx-*"))
-    if qcnt < 2:
-        raise KsftSkipEx(f"Local has only {qcnt} queues")
+    _require_2qs(cfg)
 
     ctx_id = _ethtool_create(cfg, "-X", "context new")
     defer(ethtool, f"-X {cfg.ifname} context {ctx_id} delete")
-- 
2.50.1

[PATCH net-next 03/11] tools: ynl: support packing binary arrays of scalars

[Jakub Kicinski]

We support decoding a binary type with a scalar subtype already,
add support for sending such arrays to the kernel. While at it
also support using "None" to indicate that the binary attribute
should be empty. I couldn't decide whether empty binary should
be [] or None, but there should be no harm in supporting both.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: donald.hunter@gmail.com
---
 tools/net/ynl/pyynl/lib/ynl.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/net/ynl/pyynl/lib/ynl.py b/tools/net/ynl/pyynl/lib/ynl.py
index 7529bce174ff..13f1210d0010 100644
--- a/tools/net/ynl/pyynl/lib/ynl.py
+++ b/tools/net/ynl/pyynl/lib/ynl.py
@@ -575,7 +575,9 @@ genl_family_name_to_id = None
         elif attr["type"] == 'string':
             attr_payload = str(value).encode('ascii') + b'\x00'
         elif attr["type"] == 'binary':
-            if isinstance(value, bytes):
+            if value is None:
+                attr_payload = b''
+            elif isinstance(value, bytes):
                 attr_payload = value
             elif isinstance(value, str):
                 if attr.display_hint:
@@ -584,6 +586,9 @@ genl_family_name_to_id = None
                     attr_payload = bytes.fromhex(value)
             elif isinstance(value, dict) and attr.struct_name:
                 attr_payload = self._encode_struct(attr.struct_name, value)
+            elif isinstance(value, list) and attr.sub_type in NlAttr.type_formats:
+                format = NlAttr.get_format(attr.sub_type)
+                attr_payload = b''.join([format.pack(x) for x in value])
             else:
                 raise Exception(f'Unknown type for binary attribute, value: {value}')
         elif attr['type'] in NlAttr.type_formats or attr.is_auto_scalar:
-- 
2.50.1

[PATCH net-next 04/11] selftests: drv-net: rss_api: test setting indirection table via Netlink

[Jakub Kicinski]

Test setting indirection table via Netlink.

  # ./tools/testing/selftests/drivers/net/hw/rss_api.py
  TAP version 13
  1..6
  ok 1 rss_api.test_rxfh_nl_set_fail
  ok 2 rss_api.test_rxfh_nl_set_indir
  ok 3 rss_api.test_rxfh_nl_set_indir_ctx
  ok 4 rss_api.test_rxfh_indir_ntf
  ok 5 rss_api.test_rxfh_indir_ctx_ntf
  ok 6 rss_api.test_rxfh_fields
  # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../selftests/drivers/net/hw/rss_api.py       | 92 ++++++++++++++++++-
 1 file changed, 89 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_api.py b/tools/testing/selftests/drivers/net/hw/rss_api.py
index 2c76fbdb2617..e40a1b6730bb 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_api.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_api.py
@@ -6,10 +6,10 @@ API level tests for RSS (mostly Netlink vs IOCTL).
 """
 
 import glob
-from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_is, ksft_ne
+from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_is, ksft_ne, ksft_raises
 from lib.py import KsftSkipEx, KsftFailEx
-from lib.py import defer, ethtool
-from lib.py import EthtoolFamily
+from lib.py import defer, ethtool, CmdExitFailure
+from lib.py import EthtoolFamily, NlError
 from lib.py import NetDrvEnv
 
 
@@ -59,6 +59,91 @@ from lib.py import NetDrvEnv
     return ret
 
 
+def test_rxfh_nl_set_fail(cfg):
+    """
+    Test error path of Netlink SET.
+    """
+    _require_2qs(cfg)
+
+    ethnl = EthtoolFamily()
+    ethnl.ntf_subscribe("monitor")
+
+    with ksft_raises(NlError):
+        ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                       "indir": [100000]})
+    ntf = next(ethnl.poll_ntf(duration=0.2), None)
+    ksft_is(ntf, None)
+
+
+def test_rxfh_nl_set_indir(cfg):
+    """
+    Test setting indirection table via Netlink.
+    """
+    qcnt = _require_2qs(cfg)
+
+    # Test some SETs with a value
+    reset = defer(cfg.ethnl.rss_set,
+                  {"header": {"dev-index": cfg.ifindex}, "indir": None})
+    cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                       "indir": [1]})
+    rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    ksft_eq(set(rss.get("indir", [-1])), {1})
+
+    cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                       "indir": [0, 1]})
+    rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    ksft_eq(set(rss.get("indir", [-1])), {0, 1})
+
+    # Make sure we can't set the queue count below max queue used
+    with ksft_raises(CmdExitFailure):
+        ethtool(f"-L {cfg.ifname} combined 0 rx 1")
+    with ksft_raises(CmdExitFailure):
+        ethtool(f"-L {cfg.ifname} combined 1 rx 0")
+
+    # Test reset back to default
+    reset.exec()
+    rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    ksft_eq(set(rss.get("indir", [-1])), set(range(qcnt)))
+
+
+def test_rxfh_nl_set_indir_ctx(cfg):
+    """
+    Test setting indirection table via Netlink.
+    """
+    _require_2qs(cfg)
+
+    # Get setting for ctx 0, we'll make sure they don't get clobbered
+    dflt = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+
+    # Create context
+    ctx_id = _ethtool_create(cfg, "-X", "context new")
+    defer(ethtool, f"-X {cfg.ifname} context {ctx_id} delete")
+
+    cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                       "context": ctx_id, "indir": [1]})
+    rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex},
+                             "context": ctx_id})
+    ksft_eq(set(rss.get("indir", [-1])), {1})
+
+    ctx0 = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    ksft_eq(ctx0, dflt)
+
+    cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                       "context": ctx_id, "indir": [0, 1]})
+    rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex},
+                             "context": ctx_id})
+    ksft_eq(set(rss.get("indir", [-1])), {0, 1})
+
+    ctx0 = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    ksft_eq(ctx0, dflt)
+
+    # Make sure we can't set the queue count below max queue used
+    with ksft_raises(CmdExitFailure):
+        ethtool(f"-L {cfg.ifname} combined 0 rx 1")
+    with ksft_raises(CmdExitFailure):
+        ethtool(f"-L {cfg.ifname} combined 1 rx 0")
+
+
 def test_rxfh_indir_ntf(cfg):
     """
     Check that Netlink notifications are generated when RSS indirection
@@ -129,6 +214,7 @@ from lib.py import NetDrvEnv
     """ Ksft boiler plate main """
 
     with NetDrvEnv(__file__, nsim_test=False) as cfg:
+        cfg.ethnl = EthtoolFamily()
         ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg, ))
     ksft_exit()
 
-- 
2.50.1

[PATCH net-next 05/11] ethtool: rss: support setting hfunc via Netlink

[Jakub Kicinski]

Support setting RSS hash function / algo via ethtool Netlink.
Like IOCTL we don't validate that the function is within the
range known to the kernel. The drivers do a pretty good job
validating the inputs, and the IDs are technically "dynamically
queried" rather than part of uAPI.

Only change should be that in Netlink we don't support user
explicitly passing ETH_RSS_HASH_NO_CHANGE (0), if no change
is requested the attribute should be absent.

The ETH_RSS_HASH_NO_CHANGE is retained in driver-facing
API for consistency (not that I see a strong reason for it).

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/ethtool.yaml     |  1 +
 Documentation/networking/ethtool-netlink.rst |  1 +
 net/ethtool/rss.c                            | 12 +++++++++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 1eca88a508a0..0d02d8342e4c 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -2654,6 +2654,7 @@ c-version-name: ethtool-genl-version
           attributes:
             - header
             - context
+            - hfunc
             - indir
     -
       name: rss-ntf
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 27db7540e60e..f6e4439caa94 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1999,6 +1999,7 @@ RSS_SET
 =====================================  ======  ==============================
   ``ETHTOOL_A_RSS_HEADER``             nested  request header
   ``ETHTOOL_A_RSS_CONTEXT``            u32     context number
+  ``ETHTOOL_A_RSS_HFUNC``              u32     RSS hash func
   ``ETHTOOL_A_RSS_INDIR``              binary  Indir table bytes
 =====================================  ======  ==============================
 
diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
index 7167fc3c27a0..a11428889419 100644
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -473,6 +473,7 @@ void ethtool_rss_notify(struct net_device *dev, u32 rss_context)
 const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1] = {
 	[ETHTOOL_A_RSS_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
 	[ETHTOOL_A_RSS_CONTEXT] = { .type = NLA_U32, },
+	[ETHTOOL_A_RSS_HFUNC] = NLA_POLICY_MIN(NLA_U32, 1),
 	[ETHTOOL_A_RSS_INDIR] = { .type = NLA_BINARY, },
 };
 
@@ -487,6 +488,9 @@ ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
 	if (request->rss_context && !ops->create_rxfh_context)
 		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_CONTEXT];
 
+	if (request->rss_context && !ops->rxfh_per_ctx_key)
+		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_HFUNC];
+
 	if (bad_attr) {
 		NL_SET_BAD_ATTR(info->extack, bad_attr);
 		return -EOPNOTSUPP;
@@ -586,6 +590,8 @@ rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
 			ethtool_rxfh_context_indir(ctx)[i] = rxfh->indir[i];
 		ctx->indir_configured = !!nla_len(tb[ETHTOOL_A_RSS_INDIR]);
 	}
+	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE)
+		ctx->hfunc = rxfh->hfunc;
 }
 
 static int
@@ -617,7 +623,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 		goto exit_clean_data;
 	mod |= indir_mod;
 
-	rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
+	rxfh.hfunc = data.hfunc;
+	ethnl_update_u8(&rxfh.hfunc, tb[ETHTOOL_A_RSS_HFUNC], &mod);
+	if (rxfh.hfunc == data.hfunc)
+		rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
+
 	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
 
 	mutex_lock(&dev->ethtool->rss_lock);
-- 
2.50.1

[PATCH net-next 06/11] ethtool: rss: support setting hkey via Netlink

[Jakub Kicinski]

Support setting RSS hashing key via ethtool Netlink.
Use the Netlink policy to make sure user doesn't pass
an empty key, "resetting" the key is not a thing.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/ethtool.yaml     |  1 +
 Documentation/networking/ethtool-netlink.rst |  1 +
 net/ethtool/rss.c                            | 42 +++++++++++++++++++-
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 0d02d8342e4c..aa55fc9068e1 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -2656,6 +2656,7 @@ c-version-name: ethtool-genl-version
             - context
             - hfunc
             - indir
+            - hkey
     -
       name: rss-ntf
       doc: |
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index f6e4439caa94..1830354495ae 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -2001,6 +2001,7 @@ RSS_SET
   ``ETHTOOL_A_RSS_CONTEXT``            u32     context number
   ``ETHTOOL_A_RSS_HFUNC``              u32     RSS hash func
   ``ETHTOOL_A_RSS_INDIR``              binary  Indir table bytes
+  ``ETHTOOL_A_RSS_HKEY``               binary  Hash key bytes
 =====================================  ======  ==============================
 
 ``ETHTOOL_A_RSS_INDIR`` is the minimal RSS table the user expects. Kernel and
diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
index a11428889419..20c7122fdc99 100644
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -475,6 +475,7 @@ const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1] =
 	[ETHTOOL_A_RSS_CONTEXT] = { .type = NLA_U32, },
 	[ETHTOOL_A_RSS_HFUNC] = NLA_POLICY_MIN(NLA_U32, 1),
 	[ETHTOOL_A_RSS_INDIR] = { .type = NLA_BINARY, },
+	[ETHTOOL_A_RSS_HKEY] = NLA_POLICY_MIN(NLA_BINARY, 1),
 };
 
 static int
@@ -488,8 +489,10 @@ ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
 	if (request->rss_context && !ops->create_rxfh_context)
 		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_CONTEXT];
 
-	if (request->rss_context && !ops->rxfh_per_ctx_key)
+	if (request->rss_context && !ops->rxfh_per_ctx_key) {
 		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_HFUNC];
+		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_HKEY];
+	}
 
 	if (bad_attr) {
 		NL_SET_BAD_ATTR(info->extack, bad_attr);
@@ -579,6 +582,33 @@ rss_set_prep_indir(struct net_device *dev, struct genl_info *info,
 	return err;
 }
 
+static int
+rss_set_prep_hkey(struct net_device *dev, struct genl_info *info,
+		  struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh,
+		  bool *mod)
+{
+	struct nlattr **tb = info->attrs;
+
+	if (!tb[ETHTOOL_A_RSS_HKEY])
+		return 0;
+
+	if (nla_len(tb[ETHTOOL_A_RSS_HKEY]) != data->hkey_size) {
+		NL_SET_BAD_ATTR(info->extack, tb[ETHTOOL_A_RSS_HKEY]);
+		return -EINVAL;
+	}
+
+	rxfh->key_size = data->hkey_size;
+	rxfh->key = kzalloc(data->hkey_size, GFP_KERNEL);
+	if (!rxfh->key)
+		return -ENOMEM;
+
+	nla_memcpy(rxfh->key, tb[ETHTOOL_A_RSS_HKEY], rxfh->key_size);
+
+	*mod |= memcmp(rxfh->key, data->hkey, data->hkey_size);
+
+	return 0;
+}
+
 static void
 rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
 		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
@@ -590,6 +620,11 @@ rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
 			ethtool_rxfh_context_indir(ctx)[i] = rxfh->indir[i];
 		ctx->indir_configured = !!nla_len(tb[ETHTOOL_A_RSS_INDIR]);
 	}
+	if (rxfh->key) {
+		memcpy(ethtool_rxfh_context_key(ctx), rxfh->key,
+		       data->hkey_size);
+		ctx->key_configured = !!rxfh->key_size;
+	}
 	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE)
 		ctx->hfunc = rxfh->hfunc;
 }
@@ -628,6 +663,10 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 	if (rxfh.hfunc == data.hfunc)
 		rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
 
+	ret = rss_set_prep_hkey(dev, info, &data, &rxfh, &mod);
+	if (ret)
+		goto exit_clean_data;
+
 	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
 
 	mutex_lock(&dev->ethtool->rss_lock);
@@ -657,6 +696,7 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 
 exit_unlock:
 	mutex_unlock(&dev->ethtool->rss_lock);
+	kfree(rxfh.key);
 	kfree(rxfh.indir);
 exit_clean_data:
 	rss_cleanup_data(&data.base);
-- 
2.50.1

[PATCH net-next 07/11] selftests: drv-net: rss_api: test setting hashing key via Netlink

[Jakub Kicinski]

Test setting hashing key via Netlink.

  # ./tools/testing/selftests/drivers/net/hw/rss_api.py
  TAP version 13
  1..7
  ok 1 rss_api.test_rxfh_nl_set_fail
  ok 2 rss_api.test_rxfh_nl_set_indir
  ok 3 rss_api.test_rxfh_nl_set_indir_ctx
  ok 4 rss_api.test_rxfh_indir_ntf
  ok 5 rss_api.test_rxfh_indir_ctx_ntf
  ok 6 rss_api.test_rxfh_nl_set_key
  ok 7 rss_api.test_rxfh_fields
  # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../selftests/drivers/net/hw/rss_api.py       | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_api.py b/tools/testing/selftests/drivers/net/hw/rss_api.py
index e40a1b6730bb..a0f3f9937de8 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_api.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_api.py
@@ -6,6 +6,7 @@ API level tests for RSS (mostly Netlink vs IOCTL).
 """
 
 import glob
+import random
 from lib.py import ksft_run, ksft_exit, ksft_eq, ksft_is, ksft_ne, ksft_raises
 from lib.py import KsftSkipEx, KsftFailEx
 from lib.py import defer, ethtool, CmdExitFailure
@@ -195,6 +196,38 @@ from lib.py import NetDrvEnv
     ksft_eq(set(ntf["msg"]["indir"]), {1})
 
 
+def test_rxfh_nl_set_key(cfg):
+    """
+    Test setting hashing key via Netlink.
+    """
+
+    dflt = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    defer(cfg.ethnl.rss_set,
+          {"header": {"dev-index": cfg.ifindex},
+           "hkey": dflt["hkey"], "indir": None})
+
+    # Empty key should error out
+    with ksft_raises(NlError) as cm:
+        cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                           "hkey": None})
+    ksft_eq(cm.exception.nl_msg.extack['bad-attr'], '.hkey')
+
+    # Set key to random
+    mod = random.randbytes(len(dflt["hkey"]))
+    cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                       "hkey": mod})
+    rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    ksft_eq(rss.get("hkey", [-1]), mod)
+
+    # Set key to random and indir tbl to something at once
+    mod = random.randbytes(len(dflt["hkey"]))
+    cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                       "indir": [0, 1], "hkey": mod})
+    rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    ksft_eq(rss.get("hkey", [-1]), mod)
+    ksft_eq(set(rss.get("indir", [-1])), {0, 1})
+
+
 def test_rxfh_fields(cfg):
     """
     Test reading Rx Flow Hash over Netlink.
-- 
2.50.1

[PATCH net-next 08/11] netlink: specs: define input-xfrm enum in the spec

[Jakub Kicinski]

Help YNL decode the values for input-xfrm by defining
the possible values in the spec. Don't define "no change"
as it's an IOCTL artifact with no use in Netlink.

With this change on mlx5 input-xfrm gets decoded:

 # ynl --family ethtool --dump rss-get
 [{'header': {'dev-index': 2, 'dev-name': 'eth0'},
   'hfunc': 1,
   'hkey': b'V\xa8\xf9\x9 ...',
   'indir': [0, 1, ... ],
   'input-xfrm': 'sym-or-xor',                         <<<
   'flow-hash': {'ah4': {'ip-dst', 'ip-src'},
                 'ah6': {'ip-dst', 'ip-src'},
                 'esp4': {'ip-dst', 'ip-src'},
                 'esp6': {'ip-dst', 'ip-src'},
                 'ip4': {'ip-dst', 'ip-src'},
                 'ip6': {'ip-dst', 'ip-src'},
                 'tcp4': {'l4-b-0-1', 'ip-dst', 'l4-b-2-3', 'ip-src'},
                 'tcp6': {'l4-b-0-1', 'ip-dst', 'l4-b-2-3', 'ip-src'},
                 'udp4': {'l4-b-0-1', 'ip-dst', 'l4-b-2-3', 'ip-src'},
                 'udp6': {'l4-b-0-1', 'ip-dst', 'l4-b-2-3', 'ip-src'}}
 }]

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/ethtool.yaml | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index aa55fc9068e1..3a0453a92300 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -158,6 +158,28 @@ c-version-name: ethtool-genl-version
       -
         name: pse-event-sw-pw-control-error
         doc: PSE faced an error managing the power control from software
+  -
+    name: input-xfrm
+    doc: RSS hash function transformations.
+    type: enum
+    enum-name:
+    name-prefix: rxh-xfrm-
+    header: linux/ethtool.h
+    entries:
+      -
+        name: sym-xor
+        value: 1
+        doc: >-
+          XOR the corresponding source and destination fields of each specified
+          protocol. Both copies of the XOR'ed fields are fed into the RSS and
+          RXHASH calculation. Note that this XORing reduces the input set
+          entropy and could be exploited to reduce the RSS queue spread.
+      -
+        name: sym-or-xor
+        value: 2
+        doc: >-
+          Similar to SYM_XOR, except that one copy of the XOR'ed fields is
+          replaced by an OR of the same fields.
   -
     name: rxfh-fields
     name-prefix: rxh-
@@ -1621,6 +1643,7 @@ c-version-name: ethtool-genl-version
       -
         name: input-xfrm
         type: u32
+        enum: input-xfrm
       -
         name: start-context
         type: u32
-- 
2.50.1

[PATCH net-next 09/11] ethtool: rss: support setting input-xfrm via Netlink

[Jakub Kicinski]

Support configuring symmetric hashing via Netlink.
We have the flow field config prepared as part of SET handling,
so scan it for conflicts instead of querying the driver again.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/ethtool.yaml     |  1 +
 Documentation/networking/ethtool-netlink.rst |  4 +-
 net/ethtool/common.h                         |  1 +
 net/ethtool/common.c                         | 15 ++++++
 net/ethtool/ioctl.c                          |  4 +-
 net/ethtool/rss.c                            | 51 +++++++++++++++++++-
 6 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 3a0453a92300..15cc3f2184eb 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -2680,6 +2680,7 @@ c-version-name: ethtool-genl-version
             - hfunc
             - indir
             - hkey
+            - input-xfrm
     -
       name: rss-ntf
       doc: |
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 1830354495ae..2214d2ce346a 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -2002,6 +2002,7 @@ RSS_SET
   ``ETHTOOL_A_RSS_HFUNC``              u32     RSS hash func
   ``ETHTOOL_A_RSS_INDIR``              binary  Indir table bytes
   ``ETHTOOL_A_RSS_HKEY``               binary  Hash key bytes
+  ``ETHTOOL_A_RSS_INPUT_XFRM``         u32     RSS input data transformation
 =====================================  ======  ==============================
 
 ``ETHTOOL_A_RSS_INDIR`` is the minimal RSS table the user expects. Kernel and
@@ -2012,9 +2013,6 @@ device needs at least 8 entries - the real table in use will end up being
 of 2, so tables which size is not a power of 2 will likely be rejected.
 Using table of size 0 will reset the indirection table to the default.
 
-Note that, at present, only a subset of RSS configuration can be accomplished
-over Netlink.
-
 PLCA_GET_CFG
 ============
 
diff --git a/net/ethtool/common.h b/net/ethtool/common.h
index c41db1595621..b2718afe38b5 100644
--- a/net/ethtool/common.h
+++ b/net/ethtool/common.h
@@ -44,6 +44,7 @@ int ethtool_check_max_channel(struct net_device *dev,
 			      struct ethtool_channels channels,
 			      struct genl_info *info);
 int ethtool_check_rss_ctx_busy(struct net_device *dev, u32 rss_context);
+int ethtool_rxfh_config_is_sym(u64 rxfh);
 
 void ethtool_ringparam_get_cfg(struct net_device *dev,
 			       struct ethtool_ringparam *param,
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index d62dc56f2f5b..b02067882594 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -806,6 +806,21 @@ int ethtool_check_rss_ctx_busy(struct net_device *dev, u32 rss_context)
 	return rc;
 }
 
+/* Check if fields configured for flow hash are symmetric - if src is included
+ * so is dst and vice versa.
+ */
+int ethtool_rxfh_config_is_sym(u64 rxfh)
+{
+	bool sym;
+
+	sym = rxfh == (rxfh & (RXH_IP_SRC | RXH_IP_DST |
+			       RXH_L4_B_0_1 | RXH_L4_B_2_3));
+	sym &= !!(rxfh & RXH_IP_SRC)   == !!(rxfh & RXH_IP_DST);
+	sym &= !!(rxfh & RXH_L4_B_0_1) == !!(rxfh & RXH_L4_B_2_3);
+
+	return sym;
+}
+
 int ethtool_check_ops(const struct ethtool_ops *ops)
 {
 	if (WARN_ON(ops->set_coalesce && !ops->supported_coalesce_params))
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index cccb4694f5e1..d6c008b93fbb 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1027,9 +1027,7 @@ static int ethtool_check_xfrm_rxfh(u32 input_xfrm, u64 rxfh)
 	 */
 	if ((input_xfrm != RXH_XFRM_NO_CHANGE &&
 	     input_xfrm & (RXH_XFRM_SYM_XOR | RXH_XFRM_SYM_OR_XOR)) &&
-	    ((rxfh & ~(RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3)) ||
-	     (!!(rxfh & RXH_IP_SRC) ^ !!(rxfh & RXH_IP_DST)) ||
-	     (!!(rxfh & RXH_L4_B_0_1) ^ !!(rxfh & RXH_L4_B_2_3))))
+	    !ethtool_rxfh_config_is_sym(rxfh))
 		return -EINVAL;
 
 	return 0;
diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
index 20c7122fdc99..839cf65b35f8 100644
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -476,6 +476,8 @@ const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1] =
 	[ETHTOOL_A_RSS_HFUNC] = NLA_POLICY_MIN(NLA_U32, 1),
 	[ETHTOOL_A_RSS_INDIR] = { .type = NLA_BINARY, },
 	[ETHTOOL_A_RSS_HKEY] = NLA_POLICY_MIN(NLA_BINARY, 1),
+	[ETHTOOL_A_RSS_INPUT_XFRM] =
+		NLA_POLICY_MAX(NLA_U32, RXH_XFRM_SYM_OR_XOR),
 };
 
 static int
@@ -485,6 +487,7 @@ ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
 	struct rss_req_info *request = RSS_REQINFO(req_info);
 	struct nlattr **tb = info->attrs;
 	struct nlattr *bad_attr = NULL;
+	u32 input_xfrm;
 
 	if (request->rss_context && !ops->create_rxfh_context)
 		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_CONTEXT];
@@ -492,8 +495,13 @@ ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
 	if (request->rss_context && !ops->rxfh_per_ctx_key) {
 		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_HFUNC];
 		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_HKEY];
+		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_INPUT_XFRM];
 	}
 
+	input_xfrm = nla_get_u32_default(tb[ETHTOOL_A_RSS_INPUT_XFRM], 0);
+	if (input_xfrm & ~ops->supported_input_xfrm)
+		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_INPUT_XFRM];
+
 	if (bad_attr) {
 		NL_SET_BAD_ATTR(info->extack, bad_attr);
 		return -EOPNOTSUPP;
@@ -609,6 +617,33 @@ rss_set_prep_hkey(struct net_device *dev, struct genl_info *info,
 	return 0;
 }
 
+static int
+rss_check_rxfh_fields_sym(struct net_device *dev, struct genl_info *info,
+			  struct rss_reply_data *data, bool xfrm_sym)
+{
+	struct nlattr **tb = info->attrs;
+	int i;
+
+	if (!xfrm_sym)
+		return 0;
+	if (!data->has_flow_hash) {
+		NL_SET_ERR_MSG_ATTR(info->extack, tb[ETHTOOL_A_RSS_INPUT_XFRM],
+				    "hash field config not reported");
+		return -EINVAL;
+	}
+
+	for (i = 1; i < __ETHTOOL_A_FLOW_CNT; i++)
+		if (data->flow_hash[i] >= 0 &&
+		    !ethtool_rxfh_config_is_sym(data->flow_hash[i])) {
+			NL_SET_ERR_MSG_ATTR(info->extack,
+					    tb[ETHTOOL_A_RSS_INPUT_XFRM],
+					    "hash field config is not symmetric");
+			return -EINVAL;
+		}
+
+	return 0;
+}
+
 static void
 rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
 		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
@@ -627,13 +662,15 @@ rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
 	}
 	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE)
 		ctx->hfunc = rxfh->hfunc;
+	if (rxfh->input_xfrm != RXH_XFRM_NO_CHANGE)
+		ctx->input_xfrm = rxfh->input_xfrm;
 }
 
 static int
 ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 {
+	bool indir_reset = false, indir_mod = false, xfrm_sym = false;
 	struct rss_req_info *request = RSS_REQINFO(req_info);
-	bool indir_reset = false, indir_mod = false;
 	struct ethtool_rxfh_context *ctx = NULL;
 	struct net_device *dev = req_info->dev;
 	struct ethtool_rxfh_param rxfh = {};
@@ -667,7 +704,17 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 	if (ret)
 		goto exit_clean_data;
 
-	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
+	rxfh.input_xfrm = data.input_xfrm;
+	ethnl_update_u8(&rxfh.input_xfrm, tb[ETHTOOL_A_RSS_INPUT_XFRM], &mod);
+	/* xfrm_input is NO_CHANGE AKA 0xff if per-context not supported */
+	if (!request->rss_context || ops->rxfh_per_ctx_key)
+		xfrm_sym = !!rxfh.input_xfrm;
+	if (rxfh.input_xfrm == data.input_xfrm)
+		rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
+
+	ret = rss_check_rxfh_fields_sym(dev, info, &data, xfrm_sym);
+	if (ret)
+		goto exit_clean_data;
 
 	mutex_lock(&dev->ethtool->rss_lock);
 	if (request->rss_context) {
-- 
2.50.1

[PATCH net-next 10/11] ethtool: rss: support setting flow hashing fields

[Jakub Kicinski]

Add support for ETHTOOL_SRXFH (setting hashing fields) in RSS_SET.

The tricky part is dealing with symmetric hashing, user can change
the hashing fields and symmetric hash in one request. Since fields
and hash function config are separate driver callback changes to
the two are not atomic. Keep things simple and validate the settings
against both pre- and post- change ones. Meaning that we will reject
the config request if user tries to correct the flow fields and set
input_xfrm in one request, or disables input_xfrm and makes flow
fields non-symmetric.

We can adjust it later if there's a real need. Starting simple feels
right, and potentially partially applying the settings isn't nice,
either.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/netlink/specs/ethtool.yaml     |   1 +
 Documentation/networking/ethtool-netlink.rst |   3 +-
 net/ethtool/netlink.h                        |   2 +-
 net/ethtool/rss.c                            | 111 +++++++++++++++++--
 4 files changed, 107 insertions(+), 10 deletions(-)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 15cc3f2184eb..c9e95d96945e 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -2681,6 +2681,7 @@ c-version-name: ethtool-genl-version
             - indir
             - hkey
             - input-xfrm
+            - flow-hash
     -
       name: rss-ntf
       doc: |
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 2214d2ce346a..056832c77ffd 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -2003,6 +2003,7 @@ RSS_SET
   ``ETHTOOL_A_RSS_INDIR``              binary  Indir table bytes
   ``ETHTOOL_A_RSS_HKEY``               binary  Hash key bytes
   ``ETHTOOL_A_RSS_INPUT_XFRM``         u32     RSS input data transformation
+  ``ETHTOOL_A_RSS_FLOW_HASH``          nested  Header fields included in hash
 =====================================  ======  ==============================
 
 ``ETHTOOL_A_RSS_INDIR`` is the minimal RSS table the user expects. Kernel and
@@ -2464,7 +2465,7 @@ are netlink only.
   ``ETHTOOL_GPFLAGS``                 ``ETHTOOL_MSG_PRIVFLAGS_GET``
   ``ETHTOOL_SPFLAGS``                 ``ETHTOOL_MSG_PRIVFLAGS_SET``
   ``ETHTOOL_GRXFH``                   ``ETHTOOL_MSG_RSS_GET``
-  ``ETHTOOL_SRXFH``                   n/a
+  ``ETHTOOL_SRXFH``                   ``ETHTOOL_MSG_RSS_SET``
   ``ETHTOOL_GGRO``                    ``ETHTOOL_MSG_FEATURES_GET``
   ``ETHTOOL_SGRO``                    ``ETHTOOL_MSG_FEATURES_SET``
   ``ETHTOOL_GRXRINGS``                n/a
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 620dd1ab9b3b..ddb2fb00f929 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -484,7 +484,7 @@ extern const struct nla_policy ethnl_module_set_policy[ETHTOOL_A_MODULE_POWER_MO
 extern const struct nla_policy ethnl_pse_get_policy[ETHTOOL_A_PSE_HEADER + 1];
 extern const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1];
 extern const struct nla_policy ethnl_rss_get_policy[ETHTOOL_A_RSS_START_CONTEXT + 1];
-extern const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1];
+extern const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_FLOW_HASH + 1];
 extern const struct nla_policy ethnl_plca_get_cfg_policy[ETHTOOL_A_PLCA_HEADER + 1];
 extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1];
 extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
index 839cf65b35f8..c999133dbb9e 100644
--- a/net/ethtool/rss.c
+++ b/net/ethtool/rss.c
@@ -470,7 +470,41 @@ void ethtool_rss_notify(struct net_device *dev, u32 rss_context)
 
 /* RSS_SET */
 
-const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1] = {
+#define RFH_MASK (RXH_L2DA | RXH_VLAN | RXH_IP_SRC | RXH_IP_DST | \
+		  RXH_L3_PROTO | RXH_L4_B_0_1 | RXH_L4_B_2_3 |	  \
+		  RXH_GTP_TEID | RXH_DISCARD)
+
+static const struct nla_policy ethnl_rss_flows_policy[] = {
+	[ETHTOOL_A_FLOW_ETHER]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_IP4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_IP6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_TCP4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_UDP4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_SCTP4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_AH_ESP4]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_TCP6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_UDP6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_SCTP6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_AH_ESP6]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_AH4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_ESP4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_AH6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_ESP6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPC4]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPC6]		= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPC_TEID4]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPC_TEID6]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU_EH4]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU_EH6]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU_UL4]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU_UL6]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU_DL4]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+	[ETHTOOL_A_FLOW_GTPU_DL6]	= NLA_POLICY_MASK(NLA_UINT, RFH_MASK),
+};
+
+const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_FLOW_HASH + 1] = {
 	[ETHTOOL_A_RSS_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
 	[ETHTOOL_A_RSS_CONTEXT] = { .type = NLA_U32, },
 	[ETHTOOL_A_RSS_HFUNC] = NLA_POLICY_MIN(NLA_U32, 1),
@@ -478,6 +512,7 @@ const struct nla_policy ethnl_rss_set_policy[ETHTOOL_A_RSS_START_CONTEXT + 1] =
 	[ETHTOOL_A_RSS_HKEY] = NLA_POLICY_MIN(NLA_BINARY, 1),
 	[ETHTOOL_A_RSS_INPUT_XFRM] =
 		NLA_POLICY_MAX(NLA_U32, RXH_XFRM_SYM_OR_XOR),
+	[ETHTOOL_A_RSS_FLOW_HASH] = NLA_POLICY_NESTED(ethnl_rss_flows_policy),
 };
 
 static int
@@ -502,6 +537,12 @@ ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
 	if (input_xfrm & ~ops->supported_input_xfrm)
 		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_INPUT_XFRM];
 
+	if (tb[ETHTOOL_A_RSS_FLOW_HASH] && !ops->set_rxfh_fields)
+		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_FLOW_HASH];
+	if (request->rss_context &&
+	    tb[ETHTOOL_A_RSS_FLOW_HASH] && !ops->rxfh_per_ctx_fields)
+		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_FLOW_HASH];
+
 	if (bad_attr) {
 		NL_SET_BAD_ATTR(info->extack, bad_attr);
 		return -EOPNOTSUPP;
@@ -644,6 +685,59 @@ rss_check_rxfh_fields_sym(struct net_device *dev, struct genl_info *info,
 	return 0;
 }
 
+static int
+ethnl_set_rss_fields(struct net_device *dev, struct genl_info *info,
+		     u32 rss_context, struct rss_reply_data *data,
+		     bool xfrm_sym, bool *mod)
+{
+	struct nlattr *flow_nest = info->attrs[ETHTOOL_A_RSS_FLOW_HASH];
+	struct nlattr *flows[ETHTOOL_A_FLOW_MAX + 1];
+	const struct ethtool_ops *ops;
+	int i, ret;
+
+	ops = dev->ethtool_ops;
+
+	ret = rss_check_rxfh_fields_sym(dev, info, data, xfrm_sym);
+	if (ret)
+		return ret;
+
+	if (!flow_nest)
+		return 0;
+
+	ret = nla_parse_nested(flows, ARRAY_SIZE(ethnl_rss_flows_policy) - 1,
+			       flow_nest, ethnl_rss_flows_policy, info->extack);
+	if (ret < 0)
+		return ret;
+
+	for (i = 1; i < __ETHTOOL_A_FLOW_CNT; i++) {
+		struct ethtool_rxfh_fields fields = {
+			.flow_type	= ethtool_rxfh_ft_nl2ioctl[i],
+			.rss_context	= rss_context,
+		};
+
+		if (!flows[i])
+			continue;
+
+		fields.data = nla_get_u32(flows[i]);
+		if (data->has_flow_hash && data->flow_hash[i] == fields.data)
+			continue;
+
+		if (xfrm_sym && !ethtool_rxfh_config_is_sym(fields.data)) {
+			NL_SET_ERR_MSG_ATTR(info->extack, flows[i],
+					    "conflict with xfrm-input");
+			return -EINVAL;
+		}
+
+		ret = ops->set_rxfh_fields(dev, &fields, info->extack);
+		if (ret)
+			return ret;
+
+		*mod = true;
+	}
+
+	return 0;
+}
+
 static void
 rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
 		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
@@ -673,11 +767,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 	struct rss_req_info *request = RSS_REQINFO(req_info);
 	struct ethtool_rxfh_context *ctx = NULL;
 	struct net_device *dev = req_info->dev;
+	bool mod = false, fields_mod = false;
 	struct ethtool_rxfh_param rxfh = {};
 	struct nlattr **tb = info->attrs;
 	struct rss_reply_data data = {};
 	const struct ethtool_ops *ops;
-	bool mod = false;
 	int ret;
 
 	ops = dev->ethtool_ops;
@@ -708,14 +802,10 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 	ethnl_update_u8(&rxfh.input_xfrm, tb[ETHTOOL_A_RSS_INPUT_XFRM], &mod);
 	/* xfrm_input is NO_CHANGE AKA 0xff if per-context not supported */
 	if (!request->rss_context || ops->rxfh_per_ctx_key)
-		xfrm_sym = !!rxfh.input_xfrm;
+		xfrm_sym = rxfh.input_xfrm || data.input_xfrm;
 	if (rxfh.input_xfrm == data.input_xfrm)
 		rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
 
-	ret = rss_check_rxfh_fields_sym(dev, info, &data, xfrm_sym);
-	if (ret)
-		goto exit_clean_data;
-
 	mutex_lock(&dev->ethtool->rss_lock);
 	if (request->rss_context) {
 		ctx = xa_load(&dev->ethtool->rss_ctx, request->rss_context);
@@ -725,6 +815,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 		}
 	}
 
+	ret = ethnl_set_rss_fields(dev, info, request->rss_context,
+				   &data, xfrm_sym, &fields_mod);
+	if (ret)
+		goto exit_unlock;
+
 	if (!mod)
 		ret = 0; /* nothing to tell the driver */
 	else if (!rxfh.rss_context)
@@ -748,7 +843,7 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
 exit_clean_data:
 	rss_cleanup_data(&data.base);
 
-	return ret ?: mod;
+	return ret ?: mod || fields_mod;
 }
 
 const struct ethnl_request_ops ethnl_rss_request_ops = {
-- 
2.50.1

[PATCH net-next 11/11] selftests: drv-net: rss_api: test input-xfrm and hash fields

[Jakub Kicinski]

Test configuring input-xfrm and hash fields with all the limitations.
Tested on mlx5 (CX6):

  # ./ksft-net-drv/drivers/net/hw/rss_api.py
  TAP version 13
  1..10
  ok 1 rss_api.test_rxfh_nl_set_fail
  ok 2 rss_api.test_rxfh_nl_set_indir
  ok 3 rss_api.test_rxfh_nl_set_indir_ctx
  ok 4 rss_api.test_rxfh_indir_ntf
  ok 5 rss_api.test_rxfh_indir_ctx_ntf
  ok 6 rss_api.test_rxfh_nl_set_key
  ok 7 rss_api.test_rxfh_fields
  ok 8 rss_api.test_rxfh_fields_set
  ok 9 rss_api.test_rxfh_fields_set_xfrm
  ok 10 rss_api.test_rxfh_fields_ntf
  # Totals: pass:10 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 .../selftests/drivers/net/hw/rss_api.py       | 136 ++++++++++++++++++
 1 file changed, 136 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/hw/rss_api.py b/tools/testing/selftests/drivers/net/hw/rss_api.py
index a0f3f9937de8..c15690043dfd 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_api.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_api.py
@@ -243,6 +243,142 @@ from lib.py import NetDrvEnv
                 comment="Config for " + fl_type)
 
 
+def test_rxfh_fields_set(cfg):
+    """ Test configuring Rx Flow Hash over Netlink. """
+
+    flow_types = ["tcp4", "tcp6", "udp4", "udp6"]
+
+    # Collect current settings
+    cfg_old = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    defer(cfg.ethnl.rss_set,
+          {
+              "header": {"dev-index": cfg.ifindex},
+              "input-xfrm": cfg_old.get("input-xfrm", 0),
+              "flow-hash": cfg_old["flow-hash"],
+          }
+    )
+
+    # symmetric hashing prevents some of the configs below
+    if cfg_old.get("input-xfrm", None):
+        cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                           "input-xfrm": 0})
+
+    for fl_type in flow_types:
+        cur = _ethtool_get_cfg(cfg, fl_type)
+        if cur == "sdfn":
+            change_nl = {"ip-src", "ip-dst"}
+            change_ic = "sd"
+        else:
+            change_nl = {"l4-b-0-1", "l4-b-2-3", "ip-src", "ip-dst"}
+            change_ic = "sdfn"
+
+        cfg.ethnl.rss_set({
+            "header": {"dev-index": cfg.ifindex},
+            "flow-hash": {fl_type: change_nl}
+        })
+        reset = defer(ethtool, f"--disable-netlink -N {cfg.ifname} "
+                      f"rx-flow-hash {fl_type} {cur}")
+
+        cfg_nl = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+        ksft_eq(change_nl, cfg_nl["flow-hash"][fl_type],
+                comment=f"Config for {fl_type} over Netlink")
+        cfg_ic = _ethtool_get_cfg(cfg, fl_type)
+        ksft_eq(change_ic, cfg_ic,
+                comment=f"Config for {fl_type} over IOCTL")
+
+        reset.exec()
+        cfg_nl = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+        ksft_eq(cfg_old["flow-hash"][fl_type], cfg_nl["flow-hash"][fl_type],
+                comment=f"Un-config for {fl_type} over Netlink")
+        cfg_ic = _ethtool_get_cfg(cfg, fl_type)
+        ksft_eq(cur, cfg_ic, comment=f"Un-config for {fl_type} over IOCTL")
+
+    # Try to set multiple at once, the defer was already installed at the start
+    change = {"ip-src"}
+    if change == cfg_old["flow-hash"]["tcp4"]:
+        change = {"ip-dst"}
+    cfg.ethnl.rss_set({
+        "header": {"dev-index": cfg.ifindex},
+        "flow-hash": {x: change for x in flow_types}
+    })
+
+    cfg_nl = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    for fl_type in flow_types:
+        ksft_eq(change, cfg_nl["flow-hash"][fl_type],
+                comment=f"multi-config for {fl_type} over Netlink")
+
+
+def test_rxfh_fields_set_xfrm(cfg):
+    """ Test changing Rx Flow Hash vs xfrm_input at once.  """
+
+    def set_rss(cfg, xfrm, fh):
+        cfg.ethnl.rss_set({"header": {"dev-index": cfg.ifindex},
+                           "input-xfrm": xfrm, "flow-hash": fh})
+
+    # Install the reset handler
+    cfg_old = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    defer(set_rss, cfg, cfg_old.get("input-xfrm", 0),
+          cfg_old.get("flow-hash", {}))
+
+    # Make sure we start with input-xfrm off, and tcp4 config non-sym
+    set_rss(cfg, 0, {})
+    set_rss(cfg, 0, {"tcp4": {"ip-src"}})
+
+    # Setting sym and fixing tcp4 config not expected to pass right now
+    with ksft_raises(NlError):
+        set_rss(cfg, "sym-xor", {"tcp4": {"ip-src", "ip-dst"}})
+    # One at a time should work, hopefully
+    set_rss(cfg, 0, {"tcp4": {"ip-src", "ip-dst"}})
+    no_support = False
+    try:
+        set_rss(cfg, "sym-xor", {})
+    except NlError:
+        try:
+            set_rss(cfg, "sym-or-xor", {})
+        except NlError:
+            no_support = True
+    if no_support:
+        raise KsftSkipEx("no input-xfrm supported")
+    # Disabling two at once should not work either without kernel changes
+    with ksft_raises(NlError):
+        set_rss(cfg, 0, {"tcp4": {"ip-src"}})
+
+
+def test_rxfh_fields_ntf(cfg):
+    """ Test Rx Flow Hash notifications. """
+
+    cur = _ethtool_get_cfg(cfg, "tcp4")
+    if cur == "sdfn":
+        change = {"ip-src", "ip-dst"}
+    else:
+        change = {"l4-b-0-1", "l4-b-2-3", "ip-src", "ip-dst"}
+
+    ethnl = EthtoolFamily()
+    ethnl.ntf_subscribe("monitor")
+
+    ethnl.rss_set({
+        "header": {"dev-index": cfg.ifindex},
+        "flow-hash": {"tcp4": change}
+    })
+    reset = defer(ethtool,
+                  f"--disable-netlink -N {cfg.ifname} rx-flow-hash tcp4 {cur}")
+
+    ntf = next(ethnl.poll_ntf(duration=0.2), None)
+    if ntf is None:
+        raise KsftFailEx("No notification received after IOCTL change")
+    ksft_eq(ntf["name"], "rss-ntf")
+    ksft_eq(ntf["msg"]["flow-hash"]["tcp4"], change)
+    ksft_eq(next(ethnl.poll_ntf(duration=0.01), None), None)
+
+    reset.exec()
+    ntf = next(ethnl.poll_ntf(duration=0.2), None)
+    if ntf is None:
+        raise KsftFailEx("No notification received after Netlink change")
+    ksft_eq(ntf["name"], "rss-ntf")
+    ksft_ne(ntf["msg"]["flow-hash"]["tcp4"], change)
+    ksft_eq(next(ethnl.poll_ntf(duration=0.01), None), None)
+
+
 def main() -> None:
     """ Ksft boiler plate main """
 
-- 
2.50.1

Re: [PATCH net-next 00/11] ethtool: rss: support RSS_SET via Netlink

[Gal Pressman]

On 11/07/2025 4:52, Jakub Kicinski wrote:
> Support configuring RSS settings via Netlink.
> Creating and removing contexts remains for the following series.

I was also working on this, but admittedly your version looks better.

Given the fact that this is not "feature complete" compared to the ioctl
interface, isn't it considered a degradation from the user's perspective?

New userspace ethtool will choose the netlink path and some of the
functionality will be lost. I assume rss_ctx.py fails?

Re: [PATCH net-next 01/11] ethtool: rss: initial RSS_SET (indirection table handling)

[Gal Pressman]

On 11/07/2025 4:52, Jakub Kicinski wrote:
> Add initial support for RSS_SET, for now only operations on
> the indirection table are supported.
> 
> There are two special cases here:
>  1) resetting the table to defaults;
>  2) support for tables of different size.
> 
> For (1) I use an empty Netlink attribute (array of size 0).

Makes sense.

>  static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
> diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
> index 41ab9fc67652..7167fc3c27a0 100644
> --- a/net/ethtool/rss.c
> +++ b/net/ethtool/rss.c
> @@ -218,6 +218,8 @@ rss_prepare(const struct rss_req_info *request, struct net_device *dev,
>  {
>  	rss_prepare_flow_hash(request, dev, data, info);
>  
> +	if (!dev->ethtool_ops->get_rxfh)
> +		return 0;

What is this for?

>  	if (request->rss_context)
>  		return rss_prepare_ctx(request, dev, data, info);
>  	return rss_prepare_get(request, dev, data, info);
> @@ -466,6 +468,192 @@ void ethtool_rss_notify(struct net_device *dev, u32 rss_context)
>  	ethnl_notify(dev, ETHTOOL_MSG_RSS_NTF, &req_info.base);
>  }
>  
> +static int
> +ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
> +{
> +	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
> +	struct rss_req_info *request = RSS_REQINFO(req_info);
> +	struct nlattr **tb = info->attrs;
> +	struct nlattr *bad_attr = NULL;
> +
> +	if (request->rss_context && !ops->create_rxfh_context)
> +		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_CONTEXT];

If we wish to be consistent with the ioctl flow, we should also check
that "at least one change was requested".

i.e., if (!tb[ETHTOOL_A_RSS_INDIR]) return err?

> +
> +	if (bad_attr) {
> +		NL_SET_BAD_ATTR(info->extack, bad_attr);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 1;
> +}
> +
> +static int
> +rss_set_prep_indir(struct net_device *dev, struct genl_info *info,
> +		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh,
> +		   bool *reset, bool *mod)
> +{
> +	const struct ethtool_ops *ops = dev->ethtool_ops;
> +	struct netlink_ext_ack *extack = info->extack;
> +	struct nlattr **tb = info->attrs;
> +	struct ethtool_rxnfc rx_rings;
> +	size_t alloc_size;
> +	u32 user_size;
> +	int i, err;
> +
> +	if (!tb[ETHTOOL_A_RSS_INDIR])
> +		return 0;
> +	if (!data->indir_size)
> +		return -EOPNOTSUPP;
> +
> +	rx_rings.cmd = ETHTOOL_GRXRINGS;
> +	err = ops->get_rxnfc(dev, &rx_rings, NULL);
> +	if (err)
> +		return err;
> +
> +	if (nla_len(tb[ETHTOOL_A_RSS_INDIR]) % 4) {
> +		NL_SET_BAD_ATTR(info->extack, tb[ETHTOOL_A_RSS_INDIR]);
> +		return -EINVAL;
> +	}
> +	user_size = nla_len(tb[ETHTOOL_A_RSS_INDIR]) / 4;
> +	if (!user_size) {
> +		if (rxfh->rss_context) {
> +			NL_SET_ERR_MSG_ATTR(extack, tb[ETHTOOL_A_RSS_INDIR],
> +					    "can't reset table for a context");
> +			return -EINVAL;
> +		}
> +		*reset = true;
> +	} else if (data->indir_size % user_size) {
> +		NL_SET_ERR_MSG_ATTR_FMT(extack, tb[ETHTOOL_A_RSS_INDIR],
> +					"size (%d) mismatch with device indir table (%d)",
> +					user_size, data->indir_size);
> +		return -EINVAL;
> +	}
> +
> +	rxfh->indir_size = data->indir_size;
> +	alloc_size = array_size(data->indir_size, sizeof(rxfh->indir[0]));
> +	rxfh->indir = kzalloc(alloc_size, GFP_KERNEL);
> +	if (!rxfh->indir)
> +		return -ENOMEM;
> +
> +	nla_memcpy(rxfh->indir, tb[ETHTOOL_A_RSS_INDIR], alloc_size);

ethnl_update_binary() will take care of the explicit memcmp down the line?

> +	for (i = 0; i < user_size; i++) {
> +		if (rxfh->indir[i] < rx_rings.data)
> +			continue;
> +
> +		NL_SET_ERR_MSG_ATTR_FMT(extack, tb[ETHTOOL_A_RSS_INDIR],
> +					"entry %d: queue out of range (%d)",
> +					i, rxfh->indir[i]);
> +		err = -EINVAL;
> +		goto err_free;
> +	}
> +
> +	if (user_size) {
> +		/* Replicate the user-provided table to fill the device table */
> +		for (i = user_size; i < data->indir_size; i++)
> +			rxfh->indir[i] = rxfh->indir[i % user_size];
> +	} else {
> +		for (i = 0; i < data->indir_size; i++)
> +			rxfh->indir[i] =
> +				ethtool_rxfh_indir_default(i, rx_rings.data);

Unless you wanted the mcmp to also take care of this case?

> +	}
> +
> +	*mod |= memcmp(rxfh->indir, data->indir_table, data->indir_size);
> +
> +	return 0;
> +
> +err_free:
> +	kfree(rxfh->indir);
> +	rxfh->indir = NULL;
> +	return err;
> +}
> +
> +static int
> +ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
> +{
> +	struct rss_req_info *request = RSS_REQINFO(req_info);
> +	bool indir_reset = false, indir_mod = false;
> +	struct ethtool_rxfh_context *ctx = NULL;
> +	struct net_device *dev = req_info->dev;
> +	struct ethtool_rxfh_param rxfh = {};
> +	struct nlattr **tb = info->attrs;
> +	struct rss_reply_data data = {};
> +	const struct ethtool_ops *ops;
> +	bool mod = false;
> +	int ret;
> +
> +	ops = dev->ethtool_ops;
> +	data.base.dev = dev;
> +
> +	ret = rss_prepare(request, dev, &data, info);
> +	if (ret)
> +		return ret;
> +
> +	rxfh.rss_context = request->rss_context;
> +
> +	ret = rss_set_prep_indir(dev, info, &data, &rxfh,
> +				 &indir_reset, &indir_mod);
> +	if (ret)
> +		goto exit_clean_data;
> +	mod |= indir_mod;
> +
> +	rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
> +	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
> +
> +	mutex_lock(&dev->ethtool->rss_lock);
> +	if (request->rss_context) {
> +		ctx = xa_load(&dev->ethtool->rss_ctx, request->rss_context);
> +		if (!ctx) {
> +			ret = -ENOENT;
> +			goto exit_unlock;
> +		}
> +	}
> +
> +	if (!mod)
> +		ret = 0; /* nothing to tell the driver */
> +	else if (!rxfh.rss_context)
> +		ret = ops->set_rxfh(dev, &rxfh, info->extack);
> +	else
> +		ret = ops->modify_rxfh_context(dev, ctx, &rxfh, info->extack);
> +	if (ret)
> +		goto exit_unlock;
> +
> +	if (ctx)
> +		rss_set_ctx_update(ctx, tb, &data, &rxfh);
> +	else if (indir_reset)
> +		dev->priv_flags &= ~IFF_RXFH_CONFIGURED;
> +	else if (indir_mod)
> +		dev->priv_flags |= IFF_RXFH_CONFIGURED;

One can argue that IFF_RXFH_CONFIGURED should be set even if the
requested table is equal to the default one.

> +
> +exit_unlock:
> +	mutex_unlock(&dev->ethtool->rss_lock);
> +	kfree(rxfh.indir);
> +exit_clean_data:
> +	rss_cleanup_data(&data.base);
> +
> +	return ret ?: mod;
> +}

Re: [PATCH net-next 05/11] ethtool: rss: support setting hfunc via Netlink

[Gal Pressman]

On 11/07/2025 4:52, Jakub Kicinski wrote:
> Support setting RSS hash function / algo via ethtool Netlink.
> Like IOCTL we don't validate that the function is within the
> range known to the kernel. The drivers do a pretty good job
> validating the inputs, and the IDs are technically "dynamically
> queried" rather than part of uAPI.
> 
> Only change should be that in Netlink we don't support user
> explicitly passing ETH_RSS_HASH_NO_CHANGE (0), if no change
> is requested the attribute should be absent.

Makes sense.

> @@ -487,6 +488,9 @@ ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
>  	if (request->rss_context && !ops->create_rxfh_context)
>  		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_CONTEXT];
>  
> +	if (request->rss_context && !ops->rxfh_per_ctx_key)
> +		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_HFUNC];

Clever, I expected an 'if (tb[ETHTOOL_A_RSS_HFUNC])'.

> +
>  	if (bad_attr) {
>  		NL_SET_BAD_ATTR(info->extack, bad_attr);
>  		return -EOPNOTSUPP;
> @@ -586,6 +590,8 @@ rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
>  			ethtool_rxfh_context_indir(ctx)[i] = rxfh->indir[i];
>  		ctx->indir_configured = !!nla_len(tb[ETHTOOL_A_RSS_INDIR]);
>  	}
> +	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE)
> +		ctx->hfunc = rxfh->hfunc;
>  }
>  
>  static int
> @@ -617,7 +623,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  		goto exit_clean_data;
>  	mod |= indir_mod;
>  
> -	rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
> +	rxfh.hfunc = data.hfunc;

What is this for?

> +	ethnl_update_u8(&rxfh.hfunc, tb[ETHTOOL_A_RSS_HFUNC], &mod);
> +	if (rxfh.hfunc == data.hfunc)
> +		rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;

I think that this is a distinction that we don't currently make in the
drivers/ioctl flow.

NO_CHANGE was specifically used for cases where the user didn't specify
a parameter, not for cases where the request is equal to the configured one.
mlx5 for example, performs this check internally because it can't rely
on NO_CHANGE for requested == configured.

> +
>  	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
>  
>  	mutex_lock(&dev->ethtool->rss_lock);

Re: [PATCH net-next 06/11] ethtool: rss: support setting hkey via Netlink

[Gal Pressman]

On 11/07/2025 4:52, Jakub Kicinski wrote:
> Support setting RSS hashing key via ethtool Netlink.
> Use the Netlink policy to make sure user doesn't pass
> an empty key, "resetting" the key is not a thing.

Makes sense.

> +static int
> +rss_set_prep_hkey(struct net_device *dev, struct genl_info *info,
> +		  struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh,
> +		  bool *mod)
> +{
> +	struct nlattr **tb = info->attrs;
> +
> +	if (!tb[ETHTOOL_A_RSS_HKEY])
> +		return 0;
> +
> +	if (nla_len(tb[ETHTOOL_A_RSS_HKEY]) != data->hkey_size) {
> +		NL_SET_BAD_ATTR(info->extack, tb[ETHTOOL_A_RSS_HKEY]);
> +		return -EINVAL;
> +	}
> +
> +	rxfh->key_size = data->hkey_size;
> +	rxfh->key = kzalloc(data->hkey_size, GFP_KERNEL);
> +	if (!rxfh->key)
> +		return -ENOMEM;
> +
> +	nla_memcpy(rxfh->key, tb[ETHTOOL_A_RSS_HKEY], rxfh->key_size);
> +
> +	*mod |= memcmp(rxfh->key, data->hkey, data->hkey_size);

ethnl_update_binary()?

> +
> +	return 0;
> +}

Re: [PATCH net-next 09/11] ethtool: rss: support setting input-xfrm via Netlink

[Gal Pressman]

On 11/07/2025 4:53, Jakub Kicinski wrote:
> Support configuring symmetric hashing via Netlink.
> We have the flow field config prepared as part of SET handling,
> so scan it for conflicts instead of querying the driver again.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  static int
>  ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  {
> +	bool indir_reset = false, indir_mod = false, xfrm_sym = false;
>  	struct rss_req_info *request = RSS_REQINFO(req_info);
> -	bool indir_reset = false, indir_mod = false;
>  	struct ethtool_rxfh_context *ctx = NULL;
>  	struct net_device *dev = req_info->dev;
>  	struct ethtool_rxfh_param rxfh = {};
> @@ -667,7 +704,17 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  	if (ret)
>  		goto exit_clean_data;
>  
> -	rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
> +	rxfh.input_xfrm = data.input_xfrm;
> +	ethnl_update_u8(&rxfh.input_xfrm, tb[ETHTOOL_A_RSS_INPUT_XFRM], &mod);
> +	/* xfrm_input is NO_CHANGE AKA 0xff if per-context not supported */

Can you please explain this comment? Shouldn't we fail in this case?
Nit: xfrm_input -> input_xfrm.

> +	if (!request->rss_context || ops->rxfh_per_ctx_key)
> +		xfrm_sym = !!rxfh.input_xfrm;
> +	if (rxfh.input_xfrm == data.input_xfrm)
> +		rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
> +
> +	ret = rss_check_rxfh_fields_sym(dev, info, &data, xfrm_sym);
> +	if (ret)
> +		goto exit_clean_data;
>  
>  	mutex_lock(&dev->ethtool->rss_lock);
>  	if (request->rss_context) {

Re: [PATCH net-next 10/11] ethtool: rss: support setting flow hashing fields

[Gal Pressman]

On 11/07/2025 4:53, Jakub Kicinski wrote:
> Add support for ETHTOOL_SRXFH (setting hashing fields) in RSS_SET.
> 
> The tricky part is dealing with symmetric hashing, user can change
> the hashing fields and symmetric hash in one request. Since fields
> and hash function config are separate driver callback changes to
> the two are not atomic. Keep things simple and validate the settings
> against both pre- and post- change ones. Meaning that we will reject
> the config request if user tries to correct the flow fields and set
> input_xfrm in one request, or disables input_xfrm and makes flow
> fields non-symmetric.

How is it different than what we have in ioctl?

> 
> We can adjust it later if there's a real need. Starting simple feels
> right, and potentially partially applying the settings isn't nice,
> either.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  static void
>  rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
>  		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
> @@ -673,11 +767,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  	struct rss_req_info *request = RSS_REQINFO(req_info);
>  	struct ethtool_rxfh_context *ctx = NULL;
>  	struct net_device *dev = req_info->dev;
> +	bool mod = false, fields_mod = false;

Why not use mod?

>  	struct ethtool_rxfh_param rxfh = {};
>  	struct nlattr **tb = info->attrs;
>  	struct rss_reply_data data = {};
>  	const struct ethtool_ops *ops;
> -	bool mod = false;
>  	int ret;
>  
>  	ops = dev->ethtool_ops;
> @@ -708,14 +802,10 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  	ethnl_update_u8(&rxfh.input_xfrm, tb[ETHTOOL_A_RSS_INPUT_XFRM], &mod);
>  	/* xfrm_input is NO_CHANGE AKA 0xff if per-context not supported */
>  	if (!request->rss_context || ops->rxfh_per_ctx_key)
> -		xfrm_sym = !!rxfh.input_xfrm;
> +		xfrm_sym = rxfh.input_xfrm || data.input_xfrm;
>  	if (rxfh.input_xfrm == data.input_xfrm)
>  		rxfh.input_xfrm = RXH_XFRM_NO_CHANGE;
>  
> -	ret = rss_check_rxfh_fields_sym(dev, info, &data, xfrm_sym);
> -	if (ret)
> -		goto exit_clean_data;
> -
>  	mutex_lock(&dev->ethtool->rss_lock);
>  	if (request->rss_context) {
>  		ctx = xa_load(&dev->ethtool->rss_ctx, request->rss_context);
> @@ -725,6 +815,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  		}
>  	}
>  
> +	ret = ethnl_set_rss_fields(dev, info, request->rss_context,
> +				   &data, xfrm_sym, &fields_mod);
> +	if (ret)
> +		goto exit_unlock;
> +
>  	if (!mod)
>  		ret = 0; /* nothing to tell the driver */
>  	else if (!rxfh.rss_context)
> @@ -748,7 +843,7 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>  exit_clean_data:
>  	rss_cleanup_data(&data.base);
>  
> -	return ret ?: mod;
> +	return ret ?: mod || fields_mod;
>  }

Re: [PATCH net-next 08/11] netlink: specs: define input-xfrm enum in the spec

[Gal Pressman]

On 11/07/2025 4:53, Jakub Kicinski wrote:
> Help YNL decode the values for input-xfrm by defining
> the possible values in the spec. Don't define "no change"
> as it's an IOCTL artifact with no use in Netlink.
> 
> With this change on mlx5 input-xfrm gets decoded:
> 
>  # ynl --family ethtool --dump rss-get
>  [{'header': {'dev-index': 2, 'dev-name': 'eth0'},
>    'hfunc': 1,
>    'hkey': b'V\xa8\xf9\x9 ...',
>    'indir': [0, 1, ... ],
>    'input-xfrm': 'sym-or-xor',                         <<<

I think this breaks rss_input_xfrm.py:

    input_xfrm = cfg.ethnl.rss_get(
        {'header': {'dev-name': cfg.ifname}}).get('input-xfrm')

    # Check for symmetric xor/or-xor
    if not input_xfrm or (input_xfrm != 1 and input_xfrm != 2):
        raise KsftSkipEx("Symmetric RSS hash not requested")

input_xfrm is now a string, the test is always skipped.

Re: [PATCH net-next 11/11] selftests: drv-net: rss_api: test input-xfrm and hash fields

[Gal Pressman]

On 11/07/2025 4:53, Jakub Kicinski wrote:
> Test configuring input-xfrm and hash fields with all the limitations.
> Tested on mlx5 (CX6):
> 
>   # ./ksft-net-drv/drivers/net/hw/rss_api.py
>   TAP version 13
>   1..10
>   ok 1 rss_api.test_rxfh_nl_set_fail
>   ok 2 rss_api.test_rxfh_nl_set_indir
>   ok 3 rss_api.test_rxfh_nl_set_indir_ctx
>   ok 4 rss_api.test_rxfh_indir_ntf
>   ok 5 rss_api.test_rxfh_indir_ctx_ntf
>   ok 6 rss_api.test_rxfh_nl_set_key
>   ok 7 rss_api.test_rxfh_fields
>   ok 8 rss_api.test_rxfh_fields_set
>   ok 9 rss_api.test_rxfh_fields_set_xfrm
>   ok 10 rss_api.test_rxfh_fields_ntf
>   # Totals: pass:10 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Hmm, I'm running on the same device and getting:

TAP version 13
1..10
ok 1 rss_api.test_rxfh_nl_set_fail
ok 2 rss_api.test_rxfh_nl_set_indir
ok 3 rss_api.test_rxfh_nl_set_indir_ctx
ok 4 rss_api.test_rxfh_indir_ntf
ok 5 rss_api.test_rxfh_indir_ctx_ntf
ok 6 rss_api.test_rxfh_nl_set_key
ok 7 rss_api.test_rxfh_fields
# Exception while handling defer / cleanup (callback 1 of 1)!
# Defer Exception| Traceback (most recent call last):
# Defer Exception|   File
"/root/devel/linux/tools/testing/selftests/net/lib/py/ksft.py", line
154, in ksft_flush_defer
# Defer Exception|     entry.exec_only()
# Defer Exception|   File
"/root/devel/linux/tools/testing/selftests/net/lib/py/utils.py", line
157, in exec_only
# Defer Exception|     self.func(*self.args, **self.kwargs)
# Defer Exception|   File
"/root/devel/linux/tools/net/ynl/pyynl/lib/ynl.py", line 1106, in _op
# Defer Exception|     return self._ops(ops)[0]
# Defer Exception|            ^^^^^^^^^^^^^^
# Defer Exception|   File
"/root/devel/linux/tools/net/ynl/pyynl/lib/ynl.py", line 1062, in _ops
# Defer Exception|     raise NlError(nl_msg)
# Defer Exception| net.ynl.pyynl.lib.ynl.NlError: Netlink error: Invalid
argument
# Defer Exception| nl_len = 84 (68) nl_flags = 0x300 nl_type = 2
# Defer Exception| 	error: -22
# Defer Exception| 	extack: {'msg': 'hash field config is not
symmetric', 'bad-attr': '.input-xfrm'}
not ok 8 rss_api.test_rxfh_fields_set
ok 9 rss_api.test_rxfh_fields_set_xfrm # SKIP no input-xfrm supported
ok 10 rss_api.test_rxfh_fields_ntf
# Totals: pass:8 fail:1 xfail:0 xpass:0 skip:1 error:0

Also, after the test runs I see inconsistency in the rxfh values:

$ cli.py --family ethtool --dump rss-get
                'tcp4': {'ip-src', 'ip-dst', 'l4-b-0-1', 'l4-b-2-3'},

$ ethtool -n eth2 rx-flow-hash tcp4
TCP over IPV4 flows use these fields for computing Hash flow key:
IP SA

Re: [PATCH net-next 01/11] ethtool: rss: initial RSS_SET (indirection table handling)

[Jakub Kicinski]

On Sun, 13 Jul 2025 14:08:48 +0300 Gal Pressman wrote:
> >  static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
> > diff --git a/net/ethtool/rss.c b/net/ethtool/rss.c
> > index 41ab9fc67652..7167fc3c27a0 100644
> > --- a/net/ethtool/rss.c
> > +++ b/net/ethtool/rss.c
> > @@ -218,6 +218,8 @@ rss_prepare(const struct rss_req_info *request, struct net_device *dev,
> >  {
> >  	rss_prepare_flow_hash(request, dev, data, info);
> >  
> > +	if (!dev->ethtool_ops->get_rxfh)
> > +		return 0;  
> 
> What is this for?

Silly drivers which support selecting which fields are hashed, 
but don't have a callback for basic RSS config. I'll add a comment.

> >  	if (request->rss_context)
> >  		return rss_prepare_ctx(request, dev, data, info);
> >  	return rss_prepare_get(request, dev, data, info);
> > @@ -466,6 +468,192 @@ void ethtool_rss_notify(struct net_device *dev, u32 rss_context)
> >  	ethnl_notify(dev, ETHTOOL_MSG_RSS_NTF, &req_info.base);
> >  }
> >  
> > +static int
> > +ethnl_rss_set_validate(struct ethnl_req_info *req_info, struct genl_info *info)
> > +{
> > +	const struct ethtool_ops *ops = req_info->dev->ethtool_ops;
> > +	struct rss_req_info *request = RSS_REQINFO(req_info);
> > +	struct nlattr **tb = info->attrs;
> > +	struct nlattr *bad_attr = NULL;
> > +
> > +	if (request->rss_context && !ops->create_rxfh_context)
> > +		bad_attr = bad_attr ?: tb[ETHTOOL_A_RSS_CONTEXT];  
> 
> If we wish to be consistent with the ioctl flow, we should also check
> that "at least one change was requested".
> 
> i.e., if (!tb[ETHTOOL_A_RSS_INDIR]) return err?

I was wondering about that, netlink ethtool doesn't have such checks
for other ops. I think consistency within the netlink family trumps 
the random check in the ioctl code.

> > +	rxfh->indir_size = data->indir_size;
> > +	alloc_size = array_size(data->indir_size, sizeof(rxfh->indir[0]));
> > +	rxfh->indir = kzalloc(alloc_size, GFP_KERNEL);
> > +	if (!rxfh->indir)
> > +		return -ENOMEM;
> > +
> > +	nla_memcpy(rxfh->indir, tb[ETHTOOL_A_RSS_INDIR], alloc_size);  
> 
> ethnl_update_binary() will take care of the explicit memcmp down the line?
> 
> > +	for (i = 0; i < user_size; i++) {
> > +		if (rxfh->indir[i] < rx_rings.data)
> > +			continue;
> > +
> > +		NL_SET_ERR_MSG_ATTR_FMT(extack, tb[ETHTOOL_A_RSS_INDIR],
> > +					"entry %d: queue out of range (%d)",
> > +					i, rxfh->indir[i]);
> > +		err = -EINVAL;
> > +		goto err_free;
> > +	}
> > +
> > +	if (user_size) {
> > +		/* Replicate the user-provided table to fill the device table */
> > +		for (i = user_size; i < data->indir_size; i++)
> > +			rxfh->indir[i] = rxfh->indir[i % user_size];
> > +	} else {
> > +		for (i = 0; i < data->indir_size; i++)
> > +			rxfh->indir[i] =
> > +				ethtool_rxfh_indir_default(i, rx_rings.data);  
> 
> Unless you wanted the mcmp to also take care of this case?

Yes, and I think the case of upsizing could also result in false
negatives if we don't memcmp() the whole array.

> > +	if (ctx)
> > +		rss_set_ctx_update(ctx, tb, &data, &rxfh);
> > +	else if (indir_reset)
> > +		dev->priv_flags &= ~IFF_RXFH_CONFIGURED;
> > +	else if (indir_mod)
> > +		dev->priv_flags |= IFF_RXFH_CONFIGURED;  
> 
> One can argue that IFF_RXFH_CONFIGURED should be set even if the
> requested table is equal to the default one.

Good catch.

Re: [PATCH net-next 05/11] ethtool: rss: support setting hfunc via Netlink

[Jakub Kicinski]

On Sun, 13 Jul 2025 14:10:20 +0300 Gal Pressman wrote:
> > @@ -617,7 +623,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
> >  		goto exit_clean_data;
> >  	mod |= indir_mod;
> >  
> > -	rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
> > +	rxfh.hfunc = data.hfunc;  
> 
> What is this for?

WDYM? data is filled in by the GET handler. So we init rxfh.hfunc
to what driver returned from GET.

> > +	ethnl_update_u8(&rxfh.hfunc, tb[ETHTOOL_A_RSS_HFUNC], &mod);
> > +	if (rxfh.hfunc == data.hfunc)
> > +		rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;  
> 
> I think that this is a distinction that we don't currently make in the
> drivers/ioctl flow.
> 
> NO_CHANGE was specifically used for cases where the user didn't specify
> a parameter, not for cases where the request is equal to the configured one.
> mlx5 for example, performs this check internally because it can't rely
> on NO_CHANGE for requested == configured.

Yeah, no strong preference. We have to live with the ioctl path so 
the drivers will need to keep handling all corner cases. In this case 
I chose behaving somewhat consistently with the ioctl behavior (assuming
user space is well behaved). Otherwise if we move ethtool CLI to netlink
new drivers may forget that NO_CHANGE is a thing.

Re: [PATCH net-next 09/11] ethtool: rss: support setting input-xfrm via Netlink

[Jakub Kicinski]

On Sun, 13 Jul 2025 14:11:34 +0300 Gal Pressman wrote:
> > +	/* xfrm_input is NO_CHANGE AKA 0xff if per-context not supported */  
> 
> Can you please explain this comment? Shouldn't we fail in this case?
> Nit: xfrm_input -> input_xfrm.
> 
> > +	if (!request->rss_context || ops->rxfh_per_ctx_key)
> > +		xfrm_sym = !!rxfh.input_xfrm;

xfrm_sym is supposed to indicate whether input_xfrm is set to symmetric.
Normally !!input_xfrm will tell us, but if we're dealing with a request
for an RSS context and driver doesn't support per ctx key input_xfrm
will be 0xff so we'd false positive xfrm_sym and most likely fail the
SET request if the field config is not symmeric. I'll rewrite this.

BTW I have a check at netdev reg time to make sure all drivers which
support xfrm_sym and contexts also support per-context xfrm_sym, but 
it will conflict with your fix so I didn't include it, yet.

Re: [PATCH net-next 10/11] ethtool: rss: support setting flow hashing fields

[Jakub Kicinski]

On Sun, 13 Jul 2025 14:12:33 +0300 Gal Pressman wrote:
> On 11/07/2025 4:53, Jakub Kicinski wrote:
> > Add support for ETHTOOL_SRXFH (setting hashing fields) in RSS_SET.
> > 
> > The tricky part is dealing with symmetric hashing, user can change
> > the hashing fields and symmetric hash in one request. Since fields
> > and hash function config are separate driver callback changes to
> > the two are not atomic. Keep things simple and validate the settings
> > against both pre- and post- change ones. Meaning that we will reject
> > the config request if user tries to correct the flow fields and set
> > input_xfrm in one request, or disables input_xfrm and makes flow
> > fields non-symmetric.  
> 
> How is it different than what we have in ioctl?

Because:

  user can change the hashing fields and symmetric hash in one request

IOCTL has two separate calls for this so there's no way to even try
to change both at once. I'll add "unlike IOCTL which has separate
calls" ?

> > We can adjust it later if there's a real need. Starting simple feels
> > right, and potentially partially applying the settings isn't nice,
> > either.
> > 
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > ---
> >  static void
> >  rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
> >  		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
> > @@ -673,11 +767,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
> >  	struct rss_req_info *request = RSS_REQINFO(req_info);
> >  	struct ethtool_rxfh_context *ctx = NULL;
> >  	struct net_device *dev = req_info->dev;
> > +	bool mod = false, fields_mod = false;  
> 
> Why not use mod?

Because it's a difference driver-facing op.

Re: [PATCH net-next 11/11] selftests: drv-net: rss_api: test input-xfrm and hash fields

[Jakub Kicinski]

On Sun, 13 Jul 2025 17:05:12 +0300 Gal Pressman wrote:
> # Exception while handling defer / cleanup (callback 1 of 1)!
> # Defer Exception| Traceback (most recent call last):
> # Defer Exception|   File
> "/root/devel/linux/tools/testing/selftests/net/lib/py/ksft.py", line
> 154, in ksft_flush_defer
> # Defer Exception|     entry.exec_only()
> # Defer Exception|   File
> "/root/devel/linux/tools/testing/selftests/net/lib/py/utils.py", line
> 157, in exec_only
> # Defer Exception|     self.func(*self.args, **self.kwargs)
> # Defer Exception|   File
> "/root/devel/linux/tools/net/ynl/pyynl/lib/ynl.py", line 1106, in _op
> # Defer Exception|     return self._ops(ops)[0]
> # Defer Exception|            ^^^^^^^^^^^^^^
> # Defer Exception|   File
> "/root/devel/linux/tools/net/ynl/pyynl/lib/ynl.py", line 1062, in _ops
> # Defer Exception|     raise NlError(nl_msg)
> # Defer Exception| net.ynl.pyynl.lib.ynl.NlError: Netlink error: Invalid
> argument
> # Defer Exception| nl_len = 84 (68) nl_flags = 0x300 nl_type = 2
> # Defer Exception| 	error: -22
> # Defer Exception| 	extack: {'msg': 'hash field config is not
> symmetric', 'bad-attr': '.input-xfrm'}
> not ok 8 rss_api.test_rxfh_fields_set
> ok 9 rss_api.test_rxfh_fields_set_xfrm # SKIP no input-xfrm supported
> ok 10 rss_api.test_rxfh_fields_ntf
> # Totals: pass:8 fail:1 xfail:0 xpass:0 skip:1 error:0
> 
> Also, after the test runs I see inconsistency in the rxfh values:
> 
> $ cli.py --family ethtool --dump rss-get
>                 'tcp4': {'ip-src', 'ip-dst', 'l4-b-0-1', 'l4-b-2-3'},
> 
> $ ethtool -n eth2 rx-flow-hash tcp4
> TCP over IPV4 flows use these fields for computing Hash flow key:
> IP SA

Hmm.. I think what happens here is that the test changes the config 
so the subsequent runs will pass? Will fix.

Re: [PATCH net-next 00/11] ethtool: rss: support RSS_SET via Netlink

[Jakub Kicinski]

On Sun, 13 Jul 2025 14:06:25 +0300 Gal Pressman wrote:
> On 11/07/2025 4:52, Jakub Kicinski wrote:
> > Support configuring RSS settings via Netlink.
> > Creating and removing contexts remains for the following series.  
> 
> I was also working on this, but admittedly your version looks better.

Oh, sorry :S

> Given the fact that this is not "feature complete" compared to the ioctl
> interface, isn't it considered a degradation from the user's perspective?
> 
> New userspace ethtool will choose the netlink path and some of the
> functionality will be lost. I assume rss_ctx.py fails?

I'm planning / hoping to get all of the functionality implemented
before the merge window.

Re: [PATCH net-next 04/11] selftests: drv-net: rss_api: test setting indirection table via Netlink

[Edward Cree]

On 11/07/2025 02:52, Jakub Kicinski wrote:
> Test setting indirection table via Netlink.
> 
>   # ./tools/testing/selftests/drivers/net/hw/rss_api.py
>   TAP version 13
>   1..6
>   ok 1 rss_api.test_rxfh_nl_set_fail
>   ok 2 rss_api.test_rxfh_nl_set_indir
>   ok 3 rss_api.test_rxfh_nl_set_indir_ctx
>   ok 4 rss_api.test_rxfh_indir_ntf
>   ok 5 rss_api.test_rxfh_indir_ctx_ntf
>   ok 6 rss_api.test_rxfh_fields
>   # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

...

> +def test_rxfh_nl_set_indir_ctx(cfg):
> +    """
> +    Test setting indirection table via Netlink.

"... for custom context via Netlink"?

Apart from that LGTM.
Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>

Re: [PATCH net-next 04/11] selftests: drv-net: rss_api: test setting indirection table via Netlink

[Jakub Kicinski]

On Mon, 14 Jul 2025 23:19:21 +0100 Edward Cree wrote:
> > +def test_rxfh_nl_set_indir_ctx(cfg):
> > +    """
> > +    Test setting indirection table via Netlink.  
> 
> "... for custom context via Netlink"?
> 
> Apart from that LGTM.
> Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>

Oops, just posted v2..

Re: [PATCH net-next 05/11] ethtool: rss: support setting hfunc via Netlink

[Gal Pressman]

On 14/07/2025 19:21, Jakub Kicinski wrote:
> On Sun, 13 Jul 2025 14:10:20 +0300 Gal Pressman wrote:
>>> @@ -617,7 +623,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>>>  		goto exit_clean_data;
>>>  	mod |= indir_mod;
>>>  
>>> -	rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;
>>> +	rxfh.hfunc = data.hfunc;  
>>
>> What is this for?
> 
> WDYM? data is filled in by the GET handler. So we init rxfh.hfunc
> to what driver returned from GET.
> 
>>> +	ethnl_update_u8(&rxfh.hfunc, tb[ETHTOOL_A_RSS_HFUNC], &mod);
>>> +	if (rxfh.hfunc == data.hfunc)
>>> +		rxfh.hfunc = ETH_RSS_HASH_NO_CHANGE;  
>>
>> I think that this is a distinction that we don't currently make in the
>> drivers/ioctl flow.
>>
>> NO_CHANGE was specifically used for cases where the user didn't specify
>> a parameter, not for cases where the request is equal to the configured one.
>> mlx5 for example, performs this check internally because it can't rely
>> on NO_CHANGE for requested == configured.
> 
> Yeah, no strong preference. We have to live with the ioctl path so 
> the drivers will need to keep handling all corner cases. In this case 
> I chose behaving somewhat consistently with the ioctl behavior (assuming
> user space is well behaved). 

There's no harm in passing NO_CHANGE in such cases, but it is not
something that the driver developers will be able to rely on. There's a
certain risk of someone assuming ioctl will behave the same.

Re: [PATCH net-next 00/11] ethtool: rss: support RSS_SET via Netlink

[Gal Pressman]

On 14/07/2025 19:35, Jakub Kicinski wrote:
> On Sun, 13 Jul 2025 14:06:25 +0300 Gal Pressman wrote:
>> On 11/07/2025 4:52, Jakub Kicinski wrote:
>>> Support configuring RSS settings via Netlink.
>>> Creating and removing contexts remains for the following series.  
>>
>> I was also working on this, but admittedly your version looks better.
> 
> Oh, sorry :S
> 
>> Given the fact that this is not "feature complete" compared to the ioctl
>> interface, isn't it considered a degradation from the user's perspective?
>>
>> New userspace ethtool will choose the netlink path and some of the
>> functionality will be lost. I assume rss_ctx.py fails?
> 
> I'm planning / hoping to get all of the functionality implemented
> before the merge window.

Are you also working on the userspace part?

Re: [PATCH net-next 10/11] ethtool: rss: support setting flow hashing fields

[Gal Pressman]

On 14/07/2025 19:29, Jakub Kicinski wrote:
> On Sun, 13 Jul 2025 14:12:33 +0300 Gal Pressman wrote:
>> On 11/07/2025 4:53, Jakub Kicinski wrote:
>>> Add support for ETHTOOL_SRXFH (setting hashing fields) in RSS_SET.
>>>
>>> The tricky part is dealing with symmetric hashing, user can change
>>> the hashing fields and symmetric hash in one request. Since fields
>>> and hash function config are separate driver callback changes to
>>> the two are not atomic. Keep things simple and validate the settings
>>> against both pre- and post- change ones. Meaning that we will reject
>>> the config request if user tries to correct the flow fields and set
>>> input_xfrm in one request, or disables input_xfrm and makes flow
>>> fields non-symmetric.  
>>
>> How is it different than what we have in ioctl?
> 
> Because:
> 
>   user can change the hashing fields and symmetric hash in one request
> 
> IOCTL has two separate calls for this so there's no way to even try
> to change both at once. I'll add "unlike IOCTL which has separate
> calls" ?

So it's different because you can use netlink directly to change both,
but from userspace ethtool perspective there's no difference, right?
It's still two commands.

Makes sense, thanks for the explanation.

> 
>>> We can adjust it later if there's a real need. Starting simple feels
>>> right, and potentially partially applying the settings isn't nice,
>>> either.
>>>
>>> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>>> ---
>>>  static void
>>>  rss_set_ctx_update(struct ethtool_rxfh_context *ctx, struct nlattr **tb,
>>>  		   struct rss_reply_data *data, struct ethtool_rxfh_param *rxfh)
>>> @@ -673,11 +767,11 @@ ethnl_rss_set(struct ethnl_req_info *req_info, struct genl_info *info)
>>>  	struct rss_req_info *request = RSS_REQINFO(req_info);
>>>  	struct ethtool_rxfh_context *ctx = NULL;
>>>  	struct net_device *dev = req_info->dev;
>>> +	bool mod = false, fields_mod = false;  
>>
>> Why not use mod?
> 
> Because it's a difference driver-facing op.

Why do we need to differentiate where the mod originated? We have a
single return value.

Re: [PATCH net-next 10/11] ethtool: rss: support setting flow hashing fields

[Jakub Kicinski]

On Tue, 15 Jul 2025 13:27:51 +0300 Gal Pressman wrote:
> > IOCTL has two separate calls for this so there's no way to even try
> > to change both at once. I'll add "unlike IOCTL which has separate
> > calls" ?  
> 
> So it's different because you can use netlink directly to change both,
> but from userspace ethtool perspective there's no difference, right?
> It's still two commands.

Yup! The existing CLI will likely stay as is.

> Makes sense, thanks for the explanation.
> 
> >> Why not use mod?  
> > 
> > Because it's a difference driver-facing op.  
> 
> Why do we need to differentiate where the mod originated? We have a
> single return value.

Right, but if we only modified the indirection table there's no need 
to call the driver to update the hashing fields and vice versa.
To be clear this is just "an optimization", doesn't affect correctness
in any way.

Re: [PATCH net-next 00/11] ethtool: rss: support RSS_SET via Netlink

[Jakub Kicinski]

On Tue, 15 Jul 2025 09:33:38 +0300 Gal Pressman wrote:
> > I'm planning / hoping to get all of the functionality implemented
> > before the merge window.  
> 
> Are you also working on the userspace part?

No, TBH. My target is container management daemons.