netdev - [net-queue bugfix RFC] i40e: Clear IFF_RXFH

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <1665701671-6353-1-git-send-email-jdamato@fastly.com>
Date:   Thu, 13 Oct 2022 15:54:31 -0700
From:   Joe Damato <jdamato@...tly.com>
To:     intel-wired-lan@...ts.osuosl.org
Cc:     netdev@...r.kernel.org, kuba@...nel.org, davem@...emloft.net,
        anthony.l.nguyen@...el.com, jesse.brandeburg@...el.com,
        Joe Damato <jdamato@...tly.com>
Subject: [net-queue bugfix RFC] i40e: Clear IFF_RXFH_CONFIGURED when RSS is reset

Sending this first as an RFC to ensure that this is the correct and desired
behavior when changing queue counts and flowhashes in i40e.

If this is approved, I can send an official "v1".

Before this change, reconfiguring the queue count using ethtool doesn't
always work, even for queue counts that were previously accepted because
the IFF_RXFH_CONFIGURED bit was not cleared when the flow indirection hash
is cleared by the driver.

For example:

$ sudo ethtool -x eth0
RX flow hash indirection table for eth0 with 34 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:     16    17    18    19    20    21    22    23
   24:     24    25    26    27    28    29    30    31
   32:     32    33     0     1     2     3     4     5
[...snip...]

As you can see, the flow indirection hash distributes flows to 34 queues.

Increasing the number of queues from 34 to 64 works, and the flow
indirection hash is reset automatically:

$ sudo ethtool -L eth0 combined 64
$ sudo ethtool -x eth0
RX flow hash indirection table for eth0 with 64 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:     16    17    18    19    20    21    22    23
   24:     24    25    26    27    28    29    30    31
   32:     32    33    34    35    36    37    38    39
   40:     40    41    42    43    44    45    46    47
   48:     48    49    50    51    52    53    54    55
   56:     56    57    58    59    60    61    62    63

However, reducing the queue count back to 34 (which previously worked)
fails:

$ sudo ethtool -L eth0 combined 34
Cannot set device channel parameters: Invalid argument

This happens because the kernel thinks that the user configured the flow
hash (since the IFF_RXFH_CONFIGURED bit is not cleared by the driver when
the driver reset it) and thus returns -EINVAL, presumably to prevent the
driver from resizing the queues and resetting the user-defined flowhash.

With this patch applied, the queue count can be reduced to fewer queues
than the flow indirection hash is set to distribute flows to if the flow
indirection hash was not modified by the user.

For example, with the patch applied:

$ sudo ethtool -L eth0 combined 32
$ sudo ethtool -x eth0
RX flow hash indirection table for eth0 with 32 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:     16    17    18    19    20    21    22    23
   24:     24    25    26    27    28    29    30    31
[..snip..]

I can now reduce the queue count to below 32 without error (unlike earlier):

$ sudo ethtool -L eth0 combined 24
$ sudo ethtool -x eth0
RX flow hash indirection table for eth0 with 24 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:     16    17    18    19    20    21    22    23

This works because I was using the default flow hash, so the driver discards
it and regenerates it.

However, if I manually set the flow hash to some user defined value:

$ sudo ethtool -X eth0 equal 20
$ sudo ethtool -x eth0
RX flow hash indirection table for eth0 with 24 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:     16    17    18    19     0     1     2     3
[..snip..]

I will now not be able to shrink the queue count again:

$ sudo ethtool -L eth0 combined 16
Cannot set device channel parameters: Invalid argument

But, I can increase the queue count and the flow hash is preserved:

$ sudo ethtool -L eth0 combined 64
$ sudo ethtool -x eth0
RX flow hash indirection table for eth0 with 64 RX ring(s):
    0:      0     1     2     3     4     5     6     7
    8:      8     9    10    11    12    13    14    15
   16:     16    17    18    19     0     1     2     3

Fixes: 28c5869f2bc4 ("i40e: add new fields to store user configuration")
Signed-off-by: Joe Damato <jdamato@...tly.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index feabd26..0e8dca7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11522,6 +11522,8 @@ static void i40e_clear_rss_config_user(struct i40e_vsi *vsi)
 
 	kfree(vsi->rss_lut_user);
 	vsi->rss_lut_user = NULL;
+
+	vsi->netdev->priv_flags &= ~IFF_RXFH_CONFIGURED;
 }
 
 /**
-- 
2.7.4