[PATCH net 0/5] ethtool: more RSS fixes

[PATCH net 0/5] ethtool: more RSS fixes

[Jakub Kicinski]

More fixes for RSS setting. First two patches fix my own bugs
in bnxt conversion to the new API. The third patch fixes
what seems to be a 10 year old issue (present since the Linux
RSS API was created). Fourth patch fixes an issue with
the XArray state being out of sync. And then a small test.

Jakub Kicinski (5):
  eth: bnxt: reject unsupported hash functions
  eth: bnxt: populate defaults in the RSS context struct
  ethtool: fix setting key and resetting indir at once
  ethtool: fix the state of additional contexts with old API
  selftests: drv-net: rss_ctx: check for all-zero keys

 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 14 +++++-
 net/ethtool/ioctl.c                           | 43 ++++++++++++++-----
 .../selftests/drivers/net/hw/rss_ctx.py       | 37 ++++++++++++++--
 3 files changed, 79 insertions(+), 15 deletions(-)

-- 
2.45.2

[PATCH net 1/5] eth: bnxt: reject unsupported hash functions

[Jakub Kicinski]

In commit under Fixes I split the bnxt_set_rxfh_context() function,
and attached the appropriate chunks to new ops. I missed that
bnxt_set_rxfh_context() gets called after some initial checks
in bnxt_set_rxfh(), namely that the hash function is Toeplitz.

Fixes: 5c466b4d4e75 ("eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index d00ef0063820..0425a54eca98 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1863,8 +1863,14 @@ static void bnxt_modify_rss(struct bnxt *bp, struct ethtool_rxfh_context *ctx,
 }
 
 static int bnxt_rxfh_context_check(struct bnxt *bp,
+				   const struct ethtool_rxfh_param *rxfh,
 				   struct netlink_ext_ack *extack)
 {
+	if (rxfh->hfunc && rxfh->hfunc != ETH_RSS_HASH_TOP) {
+		NL_SET_ERR_MSG_MOD(extack, "RSS hash function not supported");
+		return -EOPNOTSUPP;
+	}
+
 	if (!BNXT_SUPPORTS_MULTI_RSS_CTX(bp)) {
 		NL_SET_ERR_MSG_MOD(extack, "RSS contexts not supported");
 		return -EOPNOTSUPP;
@@ -1888,7 +1894,7 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
 	struct bnxt_vnic_info *vnic;
 	int rc;
 
-	rc = bnxt_rxfh_context_check(bp, extack);
+	rc = bnxt_rxfh_context_check(bp, rxfh, extack);
 	if (rc)
 		return rc;
 
@@ -1953,7 +1959,7 @@ static int bnxt_modify_rxfh_context(struct net_device *dev,
 	struct bnxt_rss_ctx *rss_ctx;
 	int rc;
 
-	rc = bnxt_rxfh_context_check(bp, extack);
+	rc = bnxt_rxfh_context_check(bp, rxfh, extack);
 	if (rc)
 		return rc;
 
-- 
2.45.2

Re: [PATCH net 1/5] eth: bnxt: reject unsupported hash functions

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 2309 bytes --]

On Fri, Jul 26, 2024 at 3:54 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> In commit under Fixes I split the bnxt_set_rxfh_context() function,
> and attached the appropriate chunks to new ops. I missed that
> bnxt_set_rxfh_context() gets called after some initial checks
> in bnxt_set_rxfh(), namely that the hash function is Toeplitz.
>
> Fixes: 5c466b4d4e75 ("eth: bnxt: move from .set_rxfh to .create_rxfh_context and friends")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index d00ef0063820..0425a54eca98 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -1863,8 +1863,14 @@ static void bnxt_modify_rss(struct bnxt *bp, struct ethtool_rxfh_context *ctx,
>  }
>
>  static int bnxt_rxfh_context_check(struct bnxt *bp,
> +                                  const struct ethtool_rxfh_param *rxfh,
>                                    struct netlink_ext_ack *extack)
>  {
> +       if (rxfh->hfunc && rxfh->hfunc != ETH_RSS_HASH_TOP) {
> +               NL_SET_ERR_MSG_MOD(extack, "RSS hash function not supported");
> +               return -EOPNOTSUPP;
> +       }
> +
>         if (!BNXT_SUPPORTS_MULTI_RSS_CTX(bp)) {
>                 NL_SET_ERR_MSG_MOD(extack, "RSS contexts not supported");
>                 return -EOPNOTSUPP;
> @@ -1888,7 +1894,7 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
>         struct bnxt_vnic_info *vnic;
>         int rc;
>
> -       rc = bnxt_rxfh_context_check(bp, extack);
> +       rc = bnxt_rxfh_context_check(bp, rxfh, extack);
>         if (rc)
>                 return rc;
>
> @@ -1953,7 +1959,7 @@ static int bnxt_modify_rxfh_context(struct net_device *dev,
>         struct bnxt_rss_ctx *rss_ctx;
>         int rc;
>
> -       rc = bnxt_rxfh_context_check(bp, extack);
> +       rc = bnxt_rxfh_context_check(bp, rxfh, extack);
>         if (rc)
>                 return rc;
>
> --
> 2.45.2
>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

Thank you.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

[PATCH net 2/5] eth: bnxt: populate defaults in the RSS context struct

[Jakub Kicinski]

As described in the kdoc for .create_rxfh_context we are responsible
for populating the defaults. The core will not call .get_rxfh
for non-0 context.

The problem can be easily observed since Netlink doesn't currently
use the cache. Using netlink ethtool:

  $ ethtool -x eth0 context 1
  [...]
  RSS hash key:
  13:60:cd:60:14:d3:55:36:86:df:90:f2:96:14:e2:21:05:57:a8:8f:a5:12:5e:54:62:7f:fd:3c:15:7e:76:05:71:42:a2:9a:73:80:09:9c
  RSS hash function:
      toeplitz: on
      xor: off
      crc32: off

But using IOCTL ethtool shows:

  $ ./ethtool-old -x eth0 context 1
  [...]
  RSS hash key:
  00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00
  RSS hash function:
      Operation not supported

Fixes: 7964e7884643 ("net: ethtool: use the tracking array for get_rxfh on custom RSS contexts")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 0425a54eca98..ab8e3f197e7b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1921,8 +1921,12 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
 	if (rc)
 		goto out;
 
+	/* Populate defaults in the context */
 	bnxt_set_dflt_rss_indir_tbl(bp, ctx);
+	ctx->hfunc = ETH_RSS_HASH_TOP;
 	memcpy(vnic->rss_hash_key, bp->rss_hash_key, HW_HASH_KEY_SIZE);
+	memcpy(ethtool_rxfh_context_key(ctx),
+	       bp->rss_hash_key, HW_HASH_KEY_SIZE);
 
 	rc = bnxt_hwrm_vnic_alloc(bp, vnic, 0, bp->rx_nr_rings);
 	if (rc) {
-- 
2.45.2

Re: [PATCH net 2/5] eth: bnxt: populate defaults in the RSS context struct

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 2096 bytes --]

On Fri, Jul 26, 2024 at 3:54 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> As described in the kdoc for .create_rxfh_context we are responsible
> for populating the defaults. The core will not call .get_rxfh
> for non-0 context.
>
> The problem can be easily observed since Netlink doesn't currently
> use the cache. Using netlink ethtool:
>
>   $ ethtool -x eth0 context 1
>   [...]
>   RSS hash key:
>   13:60:cd:60:14:d3:55:36:86:df:90:f2:96:14:e2:21:05:57:a8:8f:a5:12:5e:54:62:7f:fd:3c:15:7e:76:05:71:42:a2:9a:73:80:09:9c
>   RSS hash function:
>       toeplitz: on
>       xor: off
>       crc32: off
>
> But using IOCTL ethtool shows:
>
>   $ ./ethtool-old -x eth0 context 1
>   [...]
>   RSS hash key:
>   00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00
>   RSS hash function:
>       Operation not supported
>
> Fixes: 7964e7884643 ("net: ethtool: use the tracking array for get_rxfh on custom RSS contexts")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index 0425a54eca98..ab8e3f197e7b 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -1921,8 +1921,12 @@ static int bnxt_create_rxfh_context(struct net_device *dev,
>         if (rc)
>                 goto out;
>
> +       /* Populate defaults in the context */
>         bnxt_set_dflt_rss_indir_tbl(bp, ctx);
> +       ctx->hfunc = ETH_RSS_HASH_TOP;
>         memcpy(vnic->rss_hash_key, bp->rss_hash_key, HW_HASH_KEY_SIZE);
> +       memcpy(ethtool_rxfh_context_key(ctx),
> +              bp->rss_hash_key, HW_HASH_KEY_SIZE);
>
>         rc = bnxt_hwrm_vnic_alloc(bp, vnic, 0, bp->rx_nr_rings);
>         if (rc) {
> --
> 2.45.2
>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>

Thank you.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

[PATCH net 3/5] ethtool: fix setting key and resetting indir at once

[Jakub Kicinski]

The indirection table and the key follow struct ethtool_rxfh
in user memory.

To reset the indirection table user space calls SET_RXFH with
table of size 0 (OTOH to say "no change" it should use -1 / ~0).
The logic for calculating the offset where they key sits is
incorrect in this case, as kernel would still offset by the full
table length, while for the reset there is no indir table and
key is immediately after the struct.

  $ ethtool -X eth0 default hkey 01:02:03...
  $ ethtool -x eth0
  [...]
  RSS hash key:
00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00:00
  [...]

Fixes: 3de0b592394d ("ethtool: Support for configurable RSS hash key")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ethtool/ioctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 983fee76f5cf..a37ba113610a 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1331,13 +1331,13 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 	u32 rss_cfg_offset = offsetof(struct ethtool_rxfh, rss_config[0]);
 	const struct ethtool_ops *ops = dev->ethtool_ops;
 	u32 dev_indir_size = 0, dev_key_size = 0, i;
+	u32 user_indir_len = 0, indir_bytes = 0;
 	struct ethtool_rxfh_param rxfh_dev = {};
 	struct ethtool_rxfh_context *ctx = NULL;
 	struct netlink_ext_ack *extack = NULL;
 	struct ethtool_rxnfc rx_rings;
 	struct ethtool_rxfh rxfh;
 	bool locked = false; /* dev->ethtool->rss_lock taken */
-	u32 indir_bytes = 0;
 	bool create = false;
 	u8 *rss_config;
 	int ret;
@@ -1400,6 +1400,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 	 */
 	if (rxfh.indir_size &&
 	    rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE) {
+		user_indir_len = indir_bytes;
 		rxfh_dev.indir = (u32 *)rss_config;
 		rxfh_dev.indir_size = dev_indir_size;
 		ret = ethtool_copy_validate_indir(rxfh_dev.indir,
@@ -1426,7 +1427,7 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 		rxfh_dev.key_size = dev_key_size;
 		rxfh_dev.key = rss_config + indir_bytes;
 		if (copy_from_user(rxfh_dev.key,
-				   useraddr + rss_cfg_offset + indir_bytes,
+				   useraddr + rss_cfg_offset + user_indir_len,
 				   rxfh.key_size)) {
 			ret = -EFAULT;
 			goto out;
-- 
2.45.2

[PATCH net 4/5] ethtool: fix the state of additional contexts with old API

[Jakub Kicinski]

We expect drivers implementing the new create/modify/destroy
API to populate the defaults in struct ethtool_rxfh_context.
In legacy API ctx isn't even passed, and rxfh.indir / rxfh.key
are NULL so drivers can't give us defaults even if they want to.
Call get_rxfh() to fetch the values. We can reuse rxfh_dev
for the get_rxfh(), rxfh stores the input from the user.

This fixes IOCTL reporting 0s instead of the default key /
indir table for drivers using legacy API.

Add a check to try to catch drivers using the new API
but not populating the key.

Fixes: 7964e7884643 ("net: ethtool: use the tracking array for get_rxfh on custom RSS contexts")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/ethtool/ioctl.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index a37ba113610a..8ca13208d240 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -1382,10 +1382,9 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 	     rxfh.input_xfrm == RXH_XFRM_NO_CHANGE))
 		return -EINVAL;
 
-	if (rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE)
-		indir_bytes = dev_indir_size * sizeof(rxfh_dev.indir[0]);
+	indir_bytes = dev_indir_size * sizeof(rxfh_dev.indir[0]);
 
-	rss_config = kzalloc(indir_bytes + rxfh.key_size, GFP_USER);
+	rss_config = kzalloc(indir_bytes + dev_key_size, GFP_USER);
 	if (!rss_config)
 		return -ENOMEM;
 
@@ -1475,16 +1474,21 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 	rxfh_dev.input_xfrm = rxfh.input_xfrm;
 
 	if (rxfh.rss_context && ops->create_rxfh_context) {
-		if (create)
+		if (create) {
 			ret = ops->create_rxfh_context(dev, ctx, &rxfh_dev,
 						       extack);
-		else if (rxfh_dev.rss_delete)
+			/* Make sure driver populates defaults */
+			WARN_ON_ONCE(!ret && !rxfh_dev.key &&
+				     !memchr_inv(ethtool_rxfh_context_key(ctx),
+						 0, ctx->key_size));
+		} else if (rxfh_dev.rss_delete) {
 			ret = ops->remove_rxfh_context(dev, ctx,
 						       rxfh.rss_context,
 						       extack);
-		else
+		} else {
 			ret = ops->modify_rxfh_context(dev, ctx, &rxfh_dev,
 						       extack);
+		}
 	} else {
 		ret = ops->set_rxfh(dev, &rxfh_dev, extack);
 	}
@@ -1523,6 +1527,22 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 			kfree(ctx);
 			goto out;
 		}
+
+		/* Fetch the defaults for the old API, in the new API drivers
+		 * should write defaults into ctx themselves.
+		 */
+		rxfh_dev.indir = (u32 *)rss_config;
+		rxfh_dev.indir_size = dev_indir_size;
+
+		rxfh_dev.key = rss_config + indir_bytes;
+		rxfh_dev.key_size = dev_key_size;
+
+		ret = ops->get_rxfh(dev, &rxfh_dev);
+		if (WARN_ON(ret)) {
+			xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context);
+			kfree(ctx);
+			goto out;
+		}
 	}
 	if (rxfh_dev.rss_delete) {
 		WARN_ON(xa_erase(&dev->ethtool->rss_ctx, rxfh.rss_context) != ctx);
@@ -1531,12 +1551,14 @@ static noinline_for_stack int ethtool_set_rxfh(struct net_device *dev,
 		if (rxfh_dev.indir) {
 			for (i = 0; i < dev_indir_size; i++)
 				ethtool_rxfh_context_indir(ctx)[i] = rxfh_dev.indir[i];
-			ctx->indir_configured = 1;
+			ctx->indir_configured =
+				rxfh.indir_size &&
+				rxfh.indir_size != ETH_RXFH_INDIR_NO_CHANGE;
 		}
 		if (rxfh_dev.key) {
 			memcpy(ethtool_rxfh_context_key(ctx), rxfh_dev.key,
 			       dev_key_size);
-			ctx->key_configured = 1;
+			ctx->key_configured = !!rxfh.key_size;
 		}
 		if (rxfh_dev.hfunc != ETH_RSS_HASH_NO_CHANGE)
 			ctx->hfunc = rxfh_dev.hfunc;
-- 
2.45.2

Re: [PATCH net 4/5] ethtool: fix the state of additional contexts with old API

[Edward Cree]

On 25/07/2024 23:23, Jakub Kicinski wrote:
> We expect drivers implementing the new create/modify/destroy
> API to populate the defaults in struct ethtool_rxfh_context.
> In legacy API ctx isn't even passed, and rxfh.indir / rxfh.key
> are NULL so drivers can't give us defaults even if they want to.
> Call get_rxfh() to fetch the values. We can reuse rxfh_dev
> for the get_rxfh(), rxfh stores the input from the user.
> 
> This fixes IOCTL reporting 0s instead of the default key /
> indir table for drivers using legacy API.
> 
> Add a check to try to catch drivers using the new API
> but not populating the key.
> 
> Fixes: 7964e7884643 ("net: ethtool: use the tracking array for get_rxfh on custom RSS contexts")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>

[PATCH net 5/5] selftests: drv-net: rss_ctx: check for all-zero keys

[Jakub Kicinski]

We had a handful of bugs relating to key being either all 0
or just reported incorrectly as all 0. Check for this in
the tests.

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

diff --git a/tools/testing/selftests/drivers/net/hw/rss_ctx.py b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
index 931dbc36ca43..011508ca604b 100755
--- a/tools/testing/selftests/drivers/net/hw/rss_ctx.py
+++ b/tools/testing/selftests/drivers/net/hw/rss_ctx.py
@@ -19,6 +19,15 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     return [random.randint(0, 255) for _ in range(length)]
 
 
+def _rss_key_check(cfg, data=None, context=0):
+    if data is None:
+        data = get_rss(cfg, context=context)
+    if 'rss-hash-key' not in data:
+        return
+    non_zero = [x for x in data['rss-hash-key'] if x != 0]
+    ksft_eq(bool(non_zero), True, comment=f"RSS key is all zero {data['rss-hash-key']}")
+
+
 def get_rss(cfg, context=0):
     return ethtool(f"-x {cfg.ifname} context {context}", json=True)[0]
 
@@ -90,8 +99,9 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
 def test_rss_key_indir(cfg):
     """Test basics like updating the main RSS key and indirection table."""
 
-    if len(_get_rx_cnts(cfg)) < 2:
-        KsftSkipEx("Device has only one queue (or doesn't support queue stats)")
+    qcnt = len(_get_rx_cnts(cfg))
+    if qcnt < 3:
+        KsftSkipEx("Device has fewer than 3 queues (or doesn't support queue stats)")
 
     data = get_rss(cfg)
     want_keys = ['rss-hash-key', 'rss-hash-function', 'rss-indirection-table']
@@ -101,6 +111,7 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
         if not data[k]:
             raise KsftFailEx(f"ethtool results empty for '{k}': {data[k]}")
 
+    _rss_key_check(cfg, data=data)
     key_len = len(data['rss-hash-key'])
 
     # Set the key
@@ -110,9 +121,26 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
     data = get_rss(cfg)
     ksft_eq(key, data['rss-hash-key'])
 
+    # Set the indirection table and the key together
+    key = _rss_key_rand(key_len)
+    ethtool(f"-X {cfg.ifname} equal 3 hkey " + _rss_key_str(key))
+    reset_indir = defer(ethtool, f"-X {cfg.ifname} default")
+
+    data = get_rss(cfg)
+    _rss_key_check(cfg, data=data)
+    ksft_eq(0, min(data['rss-indirection-table']))
+    ksft_eq(2, max(data['rss-indirection-table']))
+
+    # Reset indirection table and set the key
+    key = _rss_key_rand(key_len)
+    ethtool(f"-X {cfg.ifname} default hkey " + _rss_key_str(key))
+    data = get_rss(cfg)
+    _rss_key_check(cfg, data=data)
+    ksft_eq(0, min(data['rss-indirection-table']))
+    ksft_eq(qcnt - 1, max(data['rss-indirection-table']))
+
     # Set the indirection table
     ethtool(f"-X {cfg.ifname} equal 2")
-    reset_indir = defer(ethtool, f"-X {cfg.ifname} default")
     data = get_rss(cfg)
     ksft_eq(0, min(data['rss-indirection-table']))
     ksft_eq(1, max(data['rss-indirection-table']))
@@ -317,8 +345,11 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
             ctx_cnt = i
             break
 
+        _rss_key_check(cfg, context=ctx_id)
+
         if not create_with_cfg:
             ethtool(f"-X {cfg.ifname} context {ctx_id} {want_cfg}")
+            _rss_key_check(cfg, context=ctx_id)
 
         # Sanity check the context we just created
         data = get_rss(cfg, ctx_id)
-- 
2.45.2

Re: [PATCH net 5/5] selftests: drv-net: rss_ctx: check for all-zero keys

[Petr Machata]

Jakub Kicinski <kuba@kernel.org> writes:

> We had a handful of bugs relating to key being either all 0
> or just reported incorrectly as all 0. Check for this in
> the tests.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Petr Machata <petrm@nvidia.com>

> +def _rss_key_check(cfg, data=None, context=0):
> +    if data is None:
> +        data = get_rss(cfg, context=context)
> +    if 'rss-hash-key' not in data:
> +        return
> +    non_zero = [x for x in data['rss-hash-key'] if x != 0]
> +    ksft_eq(bool(non_zero), True, comment=f"RSS key is all zero {data['rss-hash-key']}")

If there's a v2, consider changing this to len(non_zero) > 0, or
non_zero != [].

Re: [PATCH net 0/5] ethtool: more RSS fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 25 Jul 2024 15:23:48 -0700 you wrote:
> More fixes for RSS setting. First two patches fix my own bugs
> in bnxt conversion to the new API. The third patch fixes
> what seems to be a 10 year old issue (present since the Linux
> RSS API was created). Fourth patch fixes an issue with
> the XArray state being out of sync. And then a small test.
> 
> Jakub Kicinski (5):
>   eth: bnxt: reject unsupported hash functions
>   eth: bnxt: populate defaults in the RSS context struct
>   ethtool: fix setting key and resetting indir at once
>   ethtool: fix the state of additional contexts with old API
>   selftests: drv-net: rss_ctx: check for all-zero keys
> 
> [...]

Here is the summary with links:
  - [net,1/5] eth: bnxt: reject unsupported hash functions
    https://git.kernel.org/netdev/net/c/daefd348a593
  - [net,2/5] eth: bnxt: populate defaults in the RSS context struct
    https://git.kernel.org/netdev/net/c/9dbad38336a9
  - [net,3/5] ethtool: fix setting key and resetting indir at once
    https://git.kernel.org/netdev/net/c/7195f0ef7f5b
  - [net,4/5] ethtool: fix the state of additional contexts with old API
    https://git.kernel.org/netdev/net/c/dc9755370e1c
  - [net,5/5] selftests: drv-net: rss_ctx: check for all-zero keys
    https://git.kernel.org/netdev/net/c/0d6ccfe6b319

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html