[PATCH v2 net-next 0/4] net: ethtool: make netdev_rss_key_fill() spread flows over all queues

[PATCH v2 net-next 0/4] net: ethtool: make netdev_rss_key_fill() spread flows over all queues

[Eric Dumazet]

netdev_rss_key_fill() hands drivers a uniformly random key. Because the
Toeplitz hash is linear over GF(2), a random key is singular for a given
header field and queue count with probability 1/2: flows differing only
in the low order bits of that field (such as a burst of consecutive
ephemeral ports) then cannot reach all RX queues. On one affected 16-queue
host, only 4 queues received traffic until the key was replaced.

Patch 1 synchronizes proc_do_rss_key() with netdev_rss_key_fill() using an
smp_wmb()/smp_rmb() pair and a boolean flag so readers of
/proc/sys/net/core/netdev_rss_key print zero bytes until the key is fully
populated.

Patch 2 keeps the key random but constrains 1008 of its 2048 bits so that,
at every 16-bit aligned position and for every power-of-two queue count up
to 256, consecutive values of a field ending there visit every queue once.
Applying the fixup on the 16-bit grid covers standard 2-tuple/4-tuple
hashes as well as encapsulated or bitmap-selected layouts (such as PSP
inner TCP ports at byte 78) without enumerating them in the core. The
fixup is a bijection onto the set of valid keys, and redraws in the ~1 in
5 case where two 32-bit windows a multiple of 8 bits apart alias.

Patch 3 adds RSS key, indirection table, and flow-hash reporting to
netdevsim so the selftest can exercise both the host key and ethtool
netlink RSS reporting in a virtual machine without hardware.

Patch 4 adds a selftest under tools/testing/selftests/drivers/net/rss_key.py
checking /proc/sys/net/core/netdev_rss_key and the live RSS key,
indirection table, and flow-hash layouts reported by a device via ethtool
netlink.

v2:
 - Fix reverse xmas tree in proc_do_rss_key() (Jakub)
 - Move netdev_rss_key and netdev_rss_key_fill() to net/ethtool/common.c (Jakub)
 - Drop the KUnit test patch (Jakub)
 - Add RSS reporting to netdevsim and fold the selftests into a single
   self-contained tools/testing/selftests/drivers/net/rss_key.py using
   ethtool netlink flow-hash reporting (Jakub)

Assisted-by: LLM

Eric Dumazet (4):
  net: synchronize proc_do_rss_key() with netdev_rss_key_fill()
  net: ethtool: generate RSS keys that spread flows over all queues
  netdevsim: support reporting RSS key, indirection table, and flow hash
    fields
  selftests: drivers: net: check the host and device RSS keys

 Documentation/networking/scaling.rst          |   9 +
 drivers/net/netdevsim/ethtool.c               |  53 +++
 drivers/net/netdevsim/netdevsim.h             |   3 +
 include/linux/netdevice.h                     |   1 -
 net/core/dev.h                                |   2 +
 net/core/sysctl_net_core.c                    |   7 +-
 net/ethtool/common.c                          | 174 +++++++++
 net/ethtool/ioctl.c                           |   9 -
 tools/testing/selftests/drivers/net/Makefile  |   1 +
 .../testing/selftests/drivers/net/rss_key.py  | 343 ++++++++++++++++++
 10 files changed, 591 insertions(+), 11 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/rss_key.py

-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH v2 net-next 1/4] net: synchronize proc_do_rss_key() with netdev_rss_key_fill()

[Eric Dumazet]

proc_do_rss_key() reads netdev_rss_key[] without synchronization, so a
concurrent reader of /proc/sys/net/core/netdev_rss_key can observe a
partially populated key while netdev_rss_key_fill() is initializing it.

Publish a boolean flag after an smp_wmb() once netdev_rss_key_fill() has
populated netdev_rss_key[], and pair it with an smp_rmb() in
proc_do_rss_key(), printing zero bytes until the flag is set.

Fixes: 960fb622f851 ("net: provide a per host RSS key generic infrastructure")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev.h             | 1 +
 net/core/sysctl_net_core.c | 7 ++++++-
 net/ethtool/ioctl.c        | 7 +++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.h b/net/core/dev.h
index b757faead4d1a3e445e54d2f468c38c9e09b6762..0127b4d03e5251e0ad695aa649d0c52c2248bc8c 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -95,6 +95,7 @@ extern int		netdev_unregister_timeout_secs;
 extern int		weight_p;
 extern int		dev_weight_rx_bias;
 extern int		dev_weight_tx_bias;
+extern bool		netdev_rss_key_initialized;
 
 extern struct rw_semaphore dev_addr_sem;
 
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 23310581f494553e3ad8c4e010c7799e04f976c0..473c162736d6928d13d22e9902262a43705196d9 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -337,10 +337,15 @@ static int proc_do_rss_key(const struct ctl_table *table, int write,
 {
 	char buf[NETDEV_RSS_KEY_LEN * 3];
 	struct ctl_table fake_table;
+	bool initialized;
 	char *pos = buf;
 
+	initialized = READ_ONCE(netdev_rss_key_initialized);
+	/* Pair with smp_wmb() in netdev_rss_key_fill(). */
+	smp_rmb();
+
 	for (int i = 0; i < NETDEV_RSS_KEY_LEN; i++) {
-		pos = hex_byte_pack(pos, netdev_rss_key[i]);
+		pos = hex_byte_pack(pos, initialized ? netdev_rss_key[i] : 0);
 		*pos++ = ':';
 	}
 	*(--pos) = 0;
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 4b0bc503f9307880e436d5ee30f75476264a0c6b..b2820f02ca2790a8a3c13395e20040ee6976006f 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -34,6 +34,7 @@
 #include <net/netdev_lock.h>
 #include <net/netdev_queues.h>
 
+#include "../core/dev.h"
 #include "common.h"
 
 /* State held across locks and calls for commands which have devlink fallback */
@@ -1302,11 +1303,17 @@ static int ethtool_copy_validate_indir(u32 *indir, void __user *useraddr,
 }
 
 u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
+bool netdev_rss_key_initialized __read_mostly;
 
 void netdev_rss_key_fill(void *buffer, size_t len)
 {
 	BUG_ON(len > sizeof(netdev_rss_key));
 	net_get_random_once(netdev_rss_key, sizeof(netdev_rss_key));
+	if (unlikely(!READ_ONCE(netdev_rss_key_initialized))) {
+		/* Pair with smp_rmb() in proc_do_rss_key(). */
+		smp_wmb();
+		WRITE_ONCE(netdev_rss_key_initialized, true);
+	}
 	memcpy(buffer, netdev_rss_key, len);
 }
 EXPORT_SYMBOL(netdev_rss_key_fill);
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH v2 net-next 2/4] net: ethtool: generate RSS keys that spread flows over all queues

[Eric Dumazet]

netdev_rss_key_fill() returns a key made of uniformly random bytes. That
is not enough, because the Toeplitz hash is linear over GF(2).

Walking the hash input MSB first, each set bit contributes a 32-bit
sliding window of the key, and hardware indexes the indirection table with
the low order bits of the result. Only the tail of each window therefore
reaches the queue index:

        v(i) = key bits [i + 32 - q .. i + 31]

with q = log2(number of RX queues). Consecutive input bits give windows
overlapping in q - 1 positions, so the q vectors belonging to the q lowest
bits of a header field form a Toeplitz matrix built from 2 * q - 1 key
bits, rather than q * q independent ones. Over GF(2) a random Toeplitz
matrix is singular with probability exactly 1/2, whatever its size.

When it is singular, flows differing only in the low order bits of that
field cannot reach all the queues. This is not theoretical: a burst of
connections draws ephemeral ports from a narrow range, and on one affected
host only 4 of the 16 RX queues received any traffic at all, until its key
was replaced.

Keep drawing the key at random, since it is a secret that stops a remote
attacker from steering flows onto a single queue, but force the handful of
bits that decide this. Writing d[t] for key bit (lsb + 31 - t), where lsb
is the position of the least significant bit of a field in the hash input,
the matrices of all the q values up to 8 are non singular if and only if

        d[2 * i] = 1 ^ d[i] ^ d[i + 1] ^ ... ^ d[2 * i - 1]

The odd positions stay free, so this is a one pass fixup rather than a
search. It is in fact a bijection from those free positions onto the set
of the values having the property, so the key stays uniformly distributed
over that set and the whole cost is 8 bits of entropy per position.
Searching for such a key by rejection would not have been an option: a
freshly drawn one has the property everywhere with probability 2^-1008.

Apply this at every 16-bit aligned position of the key, rather than at the
offsets of the 2-tuple and 4-tuple layouts only. The core does not get to
know what a given NIC hashes. Hardware may select the bytes it feeds to
Toeplitz out of a header window with a bitmap, and hash an encapsulated
header: for PSP over UDP over IPv6 it can pick the outer addresses and the
inner TCP ports, which sit 78 bytes into the frame and read key bits well
past the 40 bytes an IPv6 4-tuple needs. Hashed fields are 16 bits wide at
the smallest and are not expected to straddle that grid, so covering the
grid covers the layouts that were never written down, at no cost in code.

This spends 8 bits of entropy per position, 1008 bits out of the 2048 bits
of netdev_rss_key, leaving 1040 bits. 8 is also the largest usable bound,
as each q constrains 2 * q - 1 bits and anything larger would make the
ranges of two adjacent positions overlap.

What the fixup leaves behind is visible structure: 8 of every 16 bits are
derived from the 8 others, so a 16-bit aligned word of the key takes only
2^8 values and about 27 of the 128 words of netdev_rss_key duplicate
another one. That much is forced rather than an artefact of this
implementation, 1040 bits spread over 128 words being a little over 8 bits
each, but it has one consequence worth removing. Two 32-bit windows a
whole number of words apart now collide with probability 2^-16 instead of
2^-32, and two input bits reading the same window are indistinguishable to
the hash, since flipping both of them leaves it unchanged. Over 500 keys,
23% of them had such a pair, where a uniformly random key has one with
probability 2^-11.

So draw another key when that happens. Four out of five pass. Which
distances to look at follows from where the structure is: covering the
multiples of 16 leaves 1.2e-3 expected colliding pairs per key, still 2.6
times the 4.6e-4 of a plain random key, and almost all of that excess sits
at a distance of 8 modulo 16. Covering every multiple of 8 brings the total
down to 4.2e-4, below what a plain random key gives over all distances, and
within a few percent of the 4.0e-4 it gives over the distances that are
left.

Two windows a multiple of 8 bits apart are two windows at the same offset
modulo 8, so this is a handful of pairwise sweeps rather than one pass over
the key per distance. DO_ONCE() runs the generator under a spinlock with
hard IRQs disabled, so keep the windows of a class in an array rather than
recomputing both sides of every pair: 116 us instead of 254 us for the
worst case, a 2048-bit key with no collision anywhere, at a cost of 512
bytes of stack.

The shared key is fixed up once and every driver prefix inherits both
properties. Move netdev_rss_key and netdev_rss_key_fill() from
net/ethtool/ioctl.c to net/ethtool/common.c, and move the netdev_rss_key
declaration out of include/linux/netdevice.h into net/core/dev.h.

Checked against an independent Toeplitz implementation: for every 16-bit
aligned position and every q in 1..8, an aligned block of 2^q consecutive
values of a field ending there lands on the 2^q queues exactly once each.
Over 20 random keys that is 20160 checks, which 50.1% of plain random keys
fail and none of the generated keys do, for an average of 502 rewritten
bits out of 2048.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 Documentation/networking/scaling.rst |   9 ++
 include/linux/netdevice.h            |   1 -
 net/core/dev.h                       |   1 +
 net/ethtool/common.c                 | 174 +++++++++++++++++++++++++++
 net/ethtool/ioctl.c                  |  16 ---
 5 files changed, 184 insertions(+), 17 deletions(-)

diff --git a/Documentation/networking/scaling.rst b/Documentation/networking/scaling.rst
index 6c261eb48845a40516f201233df13694863ee8cd..6c9836000a8d15c209d52fe78e46d346156893e4 100644
--- a/Documentation/networking/scaling.rst
+++ b/Documentation/networking/scaling.rst
@@ -48,6 +48,15 @@ count is not a power of two. NICs should provide an indirection table
 at least 4 times larger than the queue count. 4x table results in ~16%
 imbalance between the queues, which is acceptable for most applications.
 
+The Toeplitz hash is linear over GF(2), so the quality of the hash key
+matters as much as its randomness. For a key made of uniformly random
+bytes, the q lowest order bits of a given header field fail to spread
+flows over all 2^q queues with probability 1/2, and a burst of connections
+picking nearly consecutive ephemeral ports then lands on a fraction of the
+queues while the others stay idle. The netdev_rss_key_fill() helper draws
+a random key that is free of this defect; drivers should use it rather
+than seeding a key of their own.
+
 Some NICs support symmetric RSS hashing where, if the IP (source address,
 destination address) and TCP/UDP (source port, destination port) tuples
 are swapped, the computed hash is the same. This is beneficial in some
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5d16737167eed1b96bd7e0f03bafb598160d94ba..d037faff7c44b61c80d8f572c23b49d2a8c1ab8f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5323,7 +5323,6 @@ void netdev_lower_state_changed(struct net_device *lower_dev,
 				void *lower_state_info);
 
 #define NETDEV_RSS_KEY_LEN 256
-extern u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
 void netdev_rss_key_fill(void *buffer, size_t len);
 
 int skb_checksum_help(struct sk_buff *skb);
diff --git a/net/core/dev.h b/net/core/dev.h
index 0127b4d03e5251e0ad695aa649d0c52c2248bc8c..a5e22b2eae5231b013fcf246922c8bd3ac787e69 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -95,6 +95,7 @@ extern int		netdev_unregister_timeout_secs;
 extern int		weight_p;
 extern int		dev_weight_rx_bias;
 extern int		dev_weight_tx_bias;
+extern u8		netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
 extern bool		netdev_rss_key_initialized;
 
 extern struct rw_semaphore dev_addr_sem;
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 23db40618fed147c4fa3de754b100cda1f98f5cd..b4e766e60d38ae22ea3cb52e74afae24a2b66946 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -2,6 +2,9 @@
 
 #include <linux/ethtool_netlink.h>
 #include <linux/net_tstamp.h>
+#include <linux/once.h>
+#include <linux/random.h>
+#include <linux/unaligned.h>
 #include <linux/phy.h>
 #include <linux/rtnetlink.h>
 #include <linux/ptp_clock_kernel.h>
@@ -1400,3 +1403,174 @@ enum ethtool_link_medium ethtool_str_to_medium(const char *str)
 	return ETHTOOL_LINK_MEDIUM_NONE;
 }
 EXPORT_SYMBOL_GPL(ethtool_str_to_medium);
+
+u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
+bool netdev_rss_key_initialized __read_mostly;
+
+/* Toeplitz is linear over GF(2): the hash is the XOR of the 32-bit key
+ * windows selected by the set bits of the input, and hardware indexes the
+ * indirection table with the low order bits of the hash. Only the tail of
+ * each window therefore matters for queue selection:
+ *
+ *	v(i) = key bits [i + 32 - q .. i + 31]
+ *
+ * for input bit @i, with q = log2(number of RX queues). Consecutive input
+ * bits give windows overlapping in q - 1 positions, so the matrix formed by
+ * the windows of the q lowest bits of a header field is a Toeplitz matrix
+ * built from 2 * q - 1 key bits, not from q * q independent ones. A random
+ * Toeplitz matrix over GF(2) is singular with probability 1/2, whatever its
+ * size, and when it is singular the flows of a burst differing only in the
+ * low order bits of that field (consecutive ephemeral ports, typically)
+ * cannot reach all the RX queues no matter how many of them are configured.
+ *
+ * Keep the key random, but constrain the few bits that decide this. The
+ * fixup is applied at every 16-bit aligned position of the key, rather than
+ * at the offsets of the one hash input layout the software happens to know
+ * about: hardware is free to hash whatever it wants, but the fields it picks
+ * are 16 bits wide at the smallest and are not expected to straddle that
+ * grid, so an encapsulated or offloaded layout is covered like the usual
+ * 2-tuple and 4-tuple ones. Each position constrains 2 * q - 1 bits, so
+ * NETDEV_RSS_KEY_QMAX is both enough for 256 queues and the largest value
+ * keeping the ranges of two adjacent positions disjoint.
+ */
+#define NETDEV_RSS_KEY_QMAX	8
+#define NETDEV_RSS_KEY_SPAN	(2 * NETDEV_RSS_KEY_QMAX - 1)
+
+static bool netdev_rss_key_bit(const u8 *key, unsigned int bit)
+{
+	return key[bit / BITS_PER_BYTE] & (0x80 >> (bit % BITS_PER_BYTE));
+}
+
+static void netdev_rss_key_assign_bit(u8 *key, unsigned int bit, bool value)
+{
+	u8 mask = 0x80 >> (bit % BITS_PER_BYTE);
+
+	if (value)
+		key[bit / BITS_PER_BYTE] |= mask;
+	else
+		key[bit / BITS_PER_BYTE] &= ~mask;
+}
+
+/* Writing d[t] for key bit (@lsb + 31 - t), the matrices of all the q values
+ * up to NETDEV_RSS_KEY_QMAX are non singular if and only if
+ *
+ *	d[2 * i] = 1 ^ d[i] ^ d[i + 1] ^ ... ^ d[2 * i - 1]
+ *
+ * The odd positions stay free, so this is a one pass fixup rather than a
+ * search. It is also a bijection onto the set of the values having the
+ * property, so the key stays uniformly distributed over that set. It costs
+ * NETDEV_RSS_KEY_QMAX bits of entropy per position.
+ */
+static void netdev_rss_key_fixup_field(u8 *key, unsigned int lsb)
+{
+	bool d[NETDEV_RSS_KEY_SPAN];
+	unsigned int i, t;
+
+	for (t = 0; t < NETDEV_RSS_KEY_SPAN; t++)
+		d[t] = netdev_rss_key_bit(key, lsb + 31 - t);
+
+	for (i = 0; 2 * i < NETDEV_RSS_KEY_SPAN; i++) {
+		bool value = true;
+
+		for (t = i; t < 2 * i; t++)
+			value ^= d[t];
+
+		d[2 * i] = value;
+	}
+
+	for (t = 0; t < NETDEV_RSS_KEY_SPAN; t++)
+		netdev_rss_key_assign_bit(key, lsb + 31 - t, d[t]);
+}
+
+/* The 32 key bits starting at @bit, which is what input bit @bit contributes
+ * to the hash. @bit + 32 must fit in the key.
+ */
+static u32 netdev_rss_key_window(const u8 *key, unsigned int bit)
+{
+	unsigned int byte = bit / BITS_PER_BYTE;
+	unsigned int shift = bit % BITS_PER_BYTE;
+	u32 window = get_unaligned_be32(key + byte);
+
+	if (shift)
+		window = (window << shift) |
+			 (key[byte + 4] >> (BITS_PER_BYTE - shift));
+
+	return window;
+}
+
+/* Two input bits contributing the same window are indistinguishable to the
+ * hash, since flipping both of them leaves it unchanged. For a uniformly
+ * random key that is a 2 ** -32 event per pair of positions, but the fixup
+ * makes it likelier: it derives 8 of every 16 bits from the 8 others, so a
+ * 16-bit aligned word only takes 2 ** 8 values and two windows a whole
+ * number of words apart collide with probability 2 ** -16 instead. Half a
+ * word apart is less affected but still well clear of the random odds, so
+ * cover every distance that is a multiple of 8. What is left after that is
+ * below what a plain random key gives.
+ *
+ * Two windows at a distance that is a multiple of 8 are two windows at the
+ * same offset modulo 8. Caching a whole class would put 253 u32 on the
+ * stack, so cache one class modulo 16 and stream the class 8 bits above it
+ * against it.
+ */
+static bool netdev_rss_key_aliases(const u8 *key, unsigned int bits)
+{
+	u32 windows[NETDEV_RSS_KEY_LEN * BITS_PER_BYTE / 16];
+	unsigned int i, j, n, r;
+
+	for (r = 0; r < 16; r++) {
+		n = 0;
+		for (i = r; i + 32 <= bits; i += 16)
+			windows[n++] = netdev_rss_key_window(key, i);
+
+		for (i = 0; i < n; i++)
+			for (j = i + 1; j < n; j++)
+				if (windows[i] == windows[j])
+					return true;
+
+		if (r >= 8)
+			continue;
+
+		for (i = r + 8; i + 32 <= bits; i += 16) {
+			u32 window = netdev_rss_key_window(key, i);
+
+			for (j = 0; j < n; j++)
+				if (windows[j] == window)
+					return true;
+		}
+	}
+
+	return false;
+}
+
+static void netdev_rss_key_init(u8 *key, size_t len)
+{
+	unsigned int lsb, bits = len * BITS_PER_BYTE;
+
+	/* Four keys out of five come out of the fixup free of aliases, so
+	 * drawing another one is both simpler and cheaper than repairing.
+	 */
+	do {
+		get_random_bytes(key, len);
+
+		/* A field ending at bit @lsb uses key bits [.. , @lsb + 31],
+		 * so stop as soon as a 32-bit window no longer fits in the
+		 * key.
+		 */
+		for (lsb = 15; lsb + 32 <= bits; lsb += 16)
+			netdev_rss_key_fixup_field(key, lsb);
+	} while (netdev_rss_key_aliases(key, bits));
+
+	/* Pair with smp_rmb() in proc_do_rss_key(). */
+	smp_wmb();
+	WRITE_ONCE(netdev_rss_key_initialized, true);
+}
+
+void netdev_rss_key_fill(void *buffer, size_t len)
+{
+	if (WARN_ON_ONCE(len > sizeof(netdev_rss_key)))
+		len = sizeof(netdev_rss_key);
+	DO_ONCE(netdev_rss_key_init, netdev_rss_key, sizeof(netdev_rss_key));
+	memcpy(buffer, netdev_rss_key, len);
+}
+EXPORT_SYMBOL(netdev_rss_key_fill);
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index b2820f02ca2790a8a3c13395e20040ee6976006f..2b449d08ddcf30e20caf86e9dd294f3e0819ac2d 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -34,7 +34,6 @@
 #include <net/netdev_lock.h>
 #include <net/netdev_queues.h>
 
-#include "../core/dev.h"
 #include "common.h"
 
 /* State held across locks and calls for commands which have devlink fallback */
@@ -1302,21 +1301,6 @@ static int ethtool_copy_validate_indir(u32 *indir, void __user *useraddr,
 	return 0;
 }
 
-u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
-bool netdev_rss_key_initialized __read_mostly;
-
-void netdev_rss_key_fill(void *buffer, size_t len)
-{
-	BUG_ON(len > sizeof(netdev_rss_key));
-	net_get_random_once(netdev_rss_key, sizeof(netdev_rss_key));
-	if (unlikely(!READ_ONCE(netdev_rss_key_initialized))) {
-		/* Pair with smp_rmb() in proc_do_rss_key(). */
-		smp_wmb();
-		WRITE_ONCE(netdev_rss_key_initialized, true);
-	}
-	memcpy(buffer, netdev_rss_key, len);
-}
-EXPORT_SYMBOL(netdev_rss_key_fill);
 
 static noinline_for_stack int ethtool_get_rxfh_indir(struct net_device *dev,
 						     void __user *useraddr)
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH v2 net-next 3/4] netdevsim: support reporting RSS key, indirection table, and flow hash fields

[Eric Dumazet]

Initialize a per-device RSS key in nsim_ethtool_init() using
netdev_rss_key_fill() and wire up get_rxfh_key_size, get_rxfh_indir_size,
get_rxfh, and get_rxfh_fields on nsim_ethtool_ops.

This allows RSS selftests under tools/testing/selftests/drivers/net/ to
exercise both /proc/sys/net/core/netdev_rss_key and ethtool netlink RSS
reporting against netdevsim without requiring physical hardware.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/netdevsim/ethtool.c   | 53 +++++++++++++++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h |  3 ++
 2 files changed, 56 insertions(+)

diff --git a/drivers/net/netdevsim/ethtool.c b/drivers/net/netdevsim/ethtool.c
index 025ea79879f3a93cb9a75c1a20464d81c4ca5cef..c3d2bc783d5571fd98547e8c5cb2fd9fb9b01351 100644
--- a/drivers/net/netdevsim/ethtool.c
+++ b/drivers/net/netdevsim/ethtool.c
@@ -205,7 +205,59 @@ static int nsim_get_ts_info(struct net_device *dev,
 	return 0;
 }
 
+static u32 nsim_get_rxfh_key_size(struct net_device *dev)
+{
+	return NETDEV_RSS_KEY_LEN;
+}
+
+static u32 nsim_get_rxfh_indir_size(struct net_device *dev)
+{
+	return NSIM_RSS_INDIR_SIZE;
+}
+
+static int nsim_get_rxfh(struct net_device *dev,
+			 struct ethtool_rxfh_param *rxfh)
+{
+	struct netdevsim *ns = netdev_priv(dev);
+	u32 i;
+
+	rxfh->hfunc = ETH_RSS_HASH_TOP;
+	if (rxfh->indir) {
+		for (i = 0; i < NSIM_RSS_INDIR_SIZE; i++)
+			rxfh->indir[i] = ethtool_rxfh_indir_default(i, ns->ethtool.channels);
+	}
+	if (rxfh->key)
+		memcpy(rxfh->key, ns->ethtool.rss_key,
+		       sizeof(ns->ethtool.rss_key));
+
+	return 0;
+}
+
+static int nsim_get_rxfh_fields(struct net_device *dev,
+				struct ethtool_rxfh_fields *info)
+{
+	switch (info->flow_type) {
+	case TCP_V4_FLOW:
+	case UDP_V4_FLOW:
+	case TCP_V6_FLOW:
+	case UDP_V6_FLOW:
+		info->data = RXH_IP_SRC | RXH_IP_DST |
+			     RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		return 0;
+	case IPV4_FLOW:
+	case IPV6_FLOW:
+		info->data = RXH_IP_SRC | RXH_IP_DST;
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static const struct ethtool_ops nsim_ethtool_ops = {
+	.get_rxfh_key_size		= nsim_get_rxfh_key_size,
+	.get_rxfh_indir_size		= nsim_get_rxfh_indir_size,
+	.get_rxfh			= nsim_get_rxfh,
+	.get_rxfh_fields		= nsim_get_rxfh_fields,
 	.supported_coalesce_params	= ETHTOOL_COALESCE_ALL_PARAMS,
 	.supported_ring_params		= ETHTOOL_RING_USE_TCP_DATA_SPLIT |
 					  ETHTOOL_RING_USE_HDS_THRS,
@@ -250,6 +302,7 @@ void nsim_ethtool_init(struct netdevsim *ns)
 	ns->ethtool.fec.active_fec = ETHTOOL_FEC_NONE;
 
 	ns->ethtool.channels = ns->nsim_bus_dev->num_queues;
+	netdev_rss_key_fill(ns->ethtool.rss_key, sizeof(ns->ethtool.rss_key));
 
 	ethtool = debugfs_create_dir("ethtool", ns->nsim_dev_port->ddir);
 	ns->ethtool_ddir = ethtool;
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index eb9d684e3bccf677f080786c201587d81e875aa2..fd5599cd3bb9f1638ce80e34247b5e34b997b9cb 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -87,10 +87,13 @@ struct nsim_ethtool_pauseparam {
 	bool report_stats_tx;
 };
 
+#define NSIM_RSS_INDIR_SIZE		128
+
 struct nsim_ethtool {
 	u32 get_err;
 	u32 set_err;
 	u32 channels;
+	u8 rss_key[NETDEV_RSS_KEY_LEN];
 	struct nsim_ethtool_pauseparam pauseparam;
 	struct ethtool_coalesce coalesce;
 	struct ethtool_ringparam ring;
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH v2 net-next 4/4] selftests: drivers: net: check the host and device RSS keys

[Eric Dumazet]

Add a selftest under tools/testing/selftests/drivers/net/rss_key.py
checking both the host RSS key (/proc/sys/net/core/netdev_rss_key) and
the RSS key, indirection table, and flow-hash layouts reported by a
device over ethtool netlink.

When invoked without NETIF, NetDrvEnv creates a 4-queue netdevsim device,
which calls netdev_rss_key_fill() and allows exercising both the host key
and the netlink RSS reporting path in a virtual machine without special
hardware. When invoked with NETIF, it checks the key and indirection
table of that interface.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 tools/testing/selftests/drivers/net/Makefile  |   1 +
 .../testing/selftests/drivers/net/rss_key.py  | 343 ++++++++++++++++++
 2 files changed, 344 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/rss_key.py

diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index b98c0e240b7d7d4e5800039930c1a99ebe6d2b22..79c6f998e7046fed855081ee6d2e4c4c12bae446 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -25,6 +25,7 @@ TEST_PROGS := \
 	psp.py \
 	queues.py \
 	ring_reconfig.py \
+	rss_key.py \
 	shaper.py \
 	so_txtime.py \
 	stats.py \
diff --git a/tools/testing/selftests/drivers/net/rss_key.py b/tools/testing/selftests/drivers/net/rss_key.py
new file mode 100755
index 0000000000000000000000000000000000000000..4f694a99e9a08ca6ad7570d43587dff2b4968350
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/rss_key.py
@@ -0,0 +1,343 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+"""
+Check the quality of the host RSS key (/proc/sys/net/core/netdev_rss_key)
+and that the RSS key a device actually uses spreads flows over all of the
+entries of its indirection table.
+
+The Toeplitz hash is linear over GF(2): the hash is the XOR of the 32 bit key
+windows selected by the set bits of the input, and hardware indexes the
+indirection table with the low order bits of the hash. The windows belonging
+to the q lowest bits of a header field therefore form a Toeplitz matrix, and
+when that matrix is singular the flows of a burst differing only in those
+bits, consecutive ephemeral ports typically, can not reach all of the 2 ** q
+entries of the table. A key drawn uniformly at random is singular for a given
+field and a given q with probability 1/2.
+
+netdev_rss_key_fill() generates keys that are non singular for every field of
+the hash input and every q up to RSS_KEY_QMAX.
+"""
+
+import errno
+import random
+
+from lib.py import ksft_run, ksft_exit, ksft_pr
+from lib.py import ksft_eq, ksft_ge
+from lib.py import KsftSkipEx
+from lib.py import NetDrvEnv, EthtoolFamily, NlError
+
+KEY_PATH = "/proc/sys/net/core/netdev_rss_key"
+
+# Shortest key able to hash an IPv6 4-tuple.
+MIN_KEY_LEN = 40
+
+# Matches NETDEV_RSS_KEY_QMAX, that is up to 256 entries of the table.
+RSS_KEY_QMAX = 8
+
+# "define" for the ID of the Toeplitz hash function
+ETH_RSS_HASH_TOP = 1
+
+FLOW_TYPES = ("tcp4", "udp4", "tcp6", "udp6")
+
+# Map ethtool netlink rxfh-fields flag names to rss_key_layout() codes.
+FIELD_NAMES = {
+    "ip-src": "s",
+    "ip-dst": "d",
+    "l3-proto": "t",
+    "l4-b-0-1": "f",
+    "l4-b-2-3": "n",
+    "ip6-fl": "l",
+}
+
+
+def rss_key_bit(buf, bit):
+    """Bit @bit of @buf, counting from the most significant bit of byte 0."""
+    return (buf[bit // 8] >> (7 - bit % 8)) & 1
+
+
+def rss_key_assign_bit(buf, bit, value):
+    mask = 0x80 >> (bit % 8)
+
+    if value:
+        buf[bit // 8] |= mask
+    else:
+        buf[bit // 8] &= ~mask
+
+
+def rss_key_window(key, bit):
+    """The 32 key bits starting at @bit, what input bit @bit contributes."""
+    value = 0
+
+    for i in range(32):
+        value = (value << 1) | rss_key_bit(key, bit + i)
+
+    return value
+
+
+def rss_key_toeplitz(key, inp, nbits):
+    """The Toeplitz hash of the @nbits long input @inp under @key."""
+    value = 0
+
+    for i in range(nbits):
+        if rss_key_bit(inp, i):
+            value ^= rss_key_window(key, i)
+
+    return value
+
+
+def rss_key_full_rank(key, lsb, q):
+    """Do the q low order bits of the field at @lsb reach all 2 ** q entries?
+
+    Gaussian elimination over GF(2) on the q windows involved, reduced to
+    their q low order bits, which are the ones indexing the table.
+    """
+    basis = {}
+
+    for j in range(q):
+        vector = rss_key_window(key, lsb - j) & ((1 << q) - 1)
+
+        while vector:
+            low = vector & -vector
+            if low not in basis:
+                basis[low] = vector
+                break
+            vector ^= basis[low]
+
+        if not vector:
+            return False
+
+    return True
+
+
+def rss_key_layout(fields, ipv6):
+    """Describe the hash input built from @fields.
+
+    @fields is the flow hash configuration, "sdfn" for a 4-tuple or "sd" for
+    a 2-tuple. Returns the list of (name, position of the least significant
+    bit) and the length of the input in bits, or None if the layout involves
+    something this does not know how to place.
+    """
+    addr_bits = 128 if ipv6 else 32
+    known = (("s", "saddr", addr_bits),
+             ("d", "daddr", addr_bits),
+             ("f", "sport", 16),
+             ("n", "dport", 16))
+
+    if set(fields) - {flag for flag, _, _ in known}:
+        return None, 0
+
+    layout = []
+    nbits = 0
+
+    for flag, name, width in known:
+        if flag not in fields:
+            continue
+        nbits += width
+        layout.append((name, nbits - 1))
+
+    return layout, nbits
+
+
+def _read_host_key():
+    """Return the host RSS key, skipping if it has not been generated."""
+    try:
+        with open(KEY_PATH, "r", encoding="ascii") as fp:
+            text = fp.read().strip()
+    except FileNotFoundError as exc:
+        raise KsftSkipEx(f"{KEY_PATH} is not available") from exc
+
+    key = bytes(int(byte, 16) for byte in text.split(":")) if text else b""
+
+    if not any(key):
+        raise KsftSkipEx("the host RSS key has not been generated yet, "
+                         "no driver has called netdev_rss_key_fill()")
+
+    return key
+
+
+def _get_rss(cfg):
+    """The key, indirection table, and flow-hash config of @cfg's device."""
+    try:
+        rss = cfg.ethnl.rss_get({"header": {"dev-index": cfg.ifindex}})
+    except NlError as exc:
+        if exc.error == errno.EOPNOTSUPP:
+            raise KsftSkipEx(f"{cfg.ifname} does not support RSS") from exc
+        raise
+
+    hkey = rss.get("hkey")
+    if not hkey or not any(hkey):
+        raise KsftSkipEx(f"{cfg.ifname} does not report an RSS key")
+
+    if rss.get("hfunc") != ETH_RSS_HASH_TOP:
+        raise KsftSkipEx(f"{cfg.ifname} does not use the Toeplitz hash")
+
+    if rss.get("input-xfrm"):
+        raise KsftSkipEx(f"{cfg.ifname} transforms the hash input")
+
+    indir = rss.get("indir")
+    if not indir:
+        raise KsftSkipEx(f"{cfg.ifname} does not report an indirection table")
+
+    if len(indir) & (len(indir) - 1):
+        raise KsftSkipEx(f"{cfg.ifname} has {len(indir)} indirection table "
+                         "entries, which is not a power of two")
+
+    return bytes(hkey), indir, rss.get("flow-hash", {})
+
+
+def _get_layouts(flow_hash):
+    """The hash input layouts in use, mapped to the flow types sharing them."""
+    layouts = {}
+
+    for fl_type in FLOW_TYPES:
+        nl_fields = flow_hash.get(fl_type)
+        if not nl_fields:
+            continue
+
+        fields = "".join(FIELD_NAMES.get(name, "?") for name in nl_fields)
+        layout, nbits = rss_key_layout(fields, fl_type.endswith("6"))
+        if layout is None:
+            ksft_pr(f"{fl_type}: not checked, hashes fields we can not place "
+                    f"({nl_fields})")
+            continue
+
+        layouts.setdefault((tuple(layout), nbits), []).append(fl_type)
+
+    if not layouts:
+        raise KsftSkipEx("no flow type with a hash input we can describe")
+
+    return layouts
+
+
+def test_host_rss_key_length(cfg) -> None:
+    key = _read_host_key()
+
+    ksft_pr(f"host RSS key is {len(key)} bytes")
+    ksft_ge(len(key), MIN_KEY_LEN, "key too short to hash an IPv6 4-tuple")
+
+
+def test_host_rss_key_spread(cfg) -> None:
+    key = _read_host_key()
+    degenerate = []
+
+    for ipv6 in (False, True):
+        layout, _ = rss_key_layout("sdfn", ipv6)
+        family = "IPv6" if ipv6 else "IPv4"
+
+        for name, lsb in layout:
+            if lsb + 32 > len(key) * 8:
+                continue
+
+            for q in range(1, RSS_KEY_QMAX + 1):
+                if not rss_key_full_rank(key, lsb, q):
+                    degenerate.append(f"{family} {name} over {1 << q} queues")
+
+    for bad in degenerate:
+        ksft_pr(f"degenerate: {bad}")
+
+    ksft_eq(len(degenerate), 0,
+            "the host RSS key does not spread flows over all the queues")
+
+
+def test_host_rss_key_grid(cfg) -> None:
+    """Sweep the whole key, not only the fields of the usual layouts."""
+    key = _read_host_key()
+    bits = len(key) * 8
+    positions = 0
+    degenerate = []
+
+    for lsb in range(15, bits - 31, 16):
+        positions += 1
+
+        for q in range(1, RSS_KEY_QMAX + 1):
+            if not rss_key_full_rank(key, lsb, q):
+                degenerate.append(f"field ending at bit {lsb} "
+                                  f"over {1 << q} queues")
+
+    ksft_pr(f"checked {positions} positions of the {len(key)} byte key")
+
+    for bad in degenerate[:8]:
+        ksft_pr(f"degenerate: {bad}")
+
+    ksft_eq(len(degenerate), 0,
+            "the host RSS key does not spread flows over all the queues "
+            "at every 16-bit aligned position")
+
+
+def test_dev_rss_key_rank(cfg) -> None:
+    """The key has to be non singular for the size of the table."""
+    hkey, indir, flow_hash = _get_rss(cfg)
+    q = min((len(indir) - 1).bit_length(), RSS_KEY_QMAX)
+    degenerate = []
+
+    if not q:
+        raise KsftSkipEx("the indirection table has a single entry")
+
+    for (layout, _), fl_types in _get_layouts(flow_hash).items():
+        for name, lsb in layout:
+            if lsb + 32 > len(hkey) * 8:
+                ksft_pr(f"{name}: not checked, the key is {len(hkey)} bytes")
+                continue
+
+            if not rss_key_full_rank(hkey, lsb, q):
+                degenerate.append(f"{'/'.join(fl_types)} {name}")
+
+    for bad in degenerate:
+        ksft_pr(f"degenerate: {bad}")
+
+    ksft_eq(len(degenerate), 0,
+            f"the key of {cfg.ifname} does not spread flows over the "
+            f"{1 << q} entries of its indirection table")
+
+
+def test_dev_rss_key_spread(cfg) -> None:
+    """Hash bursts differing in one field only, and place them in the table."""
+    hkey, indir, flow_hash = _get_rss(cfg)
+    q = (len(indir) - 1).bit_length()
+    collisions = []
+
+    if q > RSS_KEY_QMAX:
+        raise KsftSkipEx(f"{len(indir)} indirection table entries is more "
+                         "than the kernel guarantees")
+    if not q:
+        raise KsftSkipEx("the indirection table has a single entry")
+
+    for (layout, nbits), fl_types in _get_layouts(flow_hash).items():
+        if nbits + 31 > len(hkey) * 8:
+            ksft_pr(f"{'/'.join(fl_types)}: not checked, the key is "
+                    f"{len(hkey)} bytes, input needs {(nbits + 31 + 7) // 8}")
+            continue
+
+        for name, lsb in layout:
+            inp = bytearray(random.randbytes(nbits // 8))
+            entries = set()
+            for value in range(1 << q):
+                for bit in range(q):
+                    rss_key_assign_bit(inp, lsb - bit, value & (1 << bit))
+                hash_ = rss_key_toeplitz(hkey, inp, nbits)
+                entries.add(hash_ & (len(indir) - 1))
+
+            if len(entries) != 1 << q:
+                collisions.append(f"{'/'.join(fl_types)} {name} reaches "
+                                  f"{len(entries)} of the {1 << q} entries")
+
+    for bad in collisions:
+        ksft_pr(bad)
+
+    ksft_eq(len(collisions), 0,
+            f"flows differing in one field only do not fill the "
+            f"indirection table of {cfg.ifname}")
+
+
+def main() -> None:
+    """ Ksft boiler plate main """
+
+    with NetDrvEnv(__file__, queue_count=4) as cfg:
+        cfg.ethnl = EthtoolFamily()
+        ksft_run(globs=globals(), case_pfx={"test_"}, args=(cfg, ))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH v2 net-next 1/4] net: synchronize proc_do_rss_key() with netdev_rss_key_fill()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new `extern bool netdev_rss_key_initialized;` is added to the
  extern block in net/core/dev.h that is introduced by the comment `/*…

--

net: synchronize proc_do_rss_key() with netdev_rss_key_fill()

This adds a netdev_rss_key_initialized boolean that netdev_rss_key_fill()
publishes with an smp_wmb() once netdev_rss_key[] has been populated, and
pairs it with an smp_rmb() in proc_do_rss_key(), which now prints zero
bytes until the flag is set.

> diff --git a/net/core/dev.h b/net/core/dev.h
> index b757faead4d1a..0127b4d03e525 100644
> --- a/net/core/dev.h
> +++ b/net/core/dev.h
> @@ -95,6 +95,7 @@ extern int		netdev_unregister_timeout_secs;
>  extern int		weight_p;
>  extern int		dev_weight_rx_bias;
>  extern int		dev_weight_tx_bias;
> +extern bool		netdev_rss_key_initialized;

[Severity: Low]
This isn't a bug, but is this the right block for the new extern?  The
block is introduced by this comment in net/core/dev.h:

	/* sysctls not referred to from outside net/core/ */
	extern int		netdev_unregister_timeout_secs;
	extern int		weight_p;
	extern int		dev_weight_rx_bias;
	extern int		dev_weight_tx_bias;
	extern bool		netdev_rss_key_initialized;

netdev_rss_key_initialized is not backed by a sysctl entry in
net/core/sysctl_net_core.c, and it is defined and written outside
net/core/, in netdev_rss_key_fill() in net/ethtool/ioctl.c, which is why
this patch also needs to add #include "../core/dev.h" there.

Also, netdev_rss_key[] and netdev_rss_key_fill() are declared in
include/linux/netdevice.h, so the flag that says whether the key is valid
is not visible at the declaration site of the key it describes.  Would
declaring it next to netdev_rss_key in include/linux/netdevice.h, or
adjusting the comment, be clearer?

[ ... ]

> diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
> index 4b0bc503f9307..b2820f02ca279 100644
> --- a/net/ethtool/ioctl.c
> +++ b/net/ethtool/ioctl.c
> @@ -34,6 +34,7 @@
>  #include <net/netdev_lock.h>
>  #include <net/netdev_queues.h>
>  
> +#include "../core/dev.h"
>  #include "common.h"
>  
>  /* State held across locks and calls for commands which have devlink fallback */
> @@ -1302,11 +1303,17 @@ static int ethtool_copy_validate_indir(u32 *indir, void __user *useraddr,
>  }
>  
>  u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
> +bool netdev_rss_key_initialized __read_mostly;
>  
>  void netdev_rss_key_fill(void *buffer, size_t len)
>  {
>  	BUG_ON(len > sizeof(netdev_rss_key));
>  	net_get_random_once(netdev_rss_key, sizeof(netdev_rss_key));
> +	if (unlikely(!READ_ONCE(netdev_rss_key_initialized))) {
> +		/* Pair with smp_rmb() in proc_do_rss_key(). */
> +		smp_wmb();
> +		WRITE_ONCE(netdev_rss_key_initialized, true);
> +	}
>  	memcpy(buffer, netdev_rss_key, len);
>  }
>  EXPORT_SYMBOL(netdev_rss_key_fill);

The same placement mismatch appears to persist later in the series: in
"net: ethtool: generate RSS keys that spread flows over all queues" both
netdev_rss_key and netdev_rss_key_initialized are declared in that same
net/core/dev.h block while being defined in net/ethtool/common.c.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922163458.3900996-1-edumazet%40google.com

Re: [PATCH v2 net-next 2/4] net: ethtool: generate RSS keys that spread flows over all queues

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] netdev_rss_key_init() (net/ethtool/common.c:1546) is invoked
  through DO_ONCE() from netdev_rss_key_fill()…
- [Medium] The fixup introduces key-independent collision families in the
  bit that selects the queue.
- [Low] Stale cross-file memory-ordering comment.
- [Low] net/core/dev.h:93 introduces the block with '/* sysctls not
  referred to from outside net/core/ */', and this patch adds 'extern u8…

--

net: ethtool: generate RSS keys that spread flows over all queues

This moves netdev_rss_key and netdev_rss_key_fill() from net/ethtool/ioctl.c
to net/ethtool/common.c and replaces the plain random draw with a generator
that forces some key bits so the Toeplitz matrices of the q lowest bits of a
hashed field are non singular, then rejects keys where two 32-bit key windows
alias.

> diff --git a/net/core/dev.h b/net/core/dev.h
> index 0127b4d03e525..a5e22b2eae523 100644
> --- a/net/core/dev.h
> +++ b/net/core/dev.h
> @@ -95,6 +95,7 @@ extern int		netdev_unregister_timeout_secs;
>  extern int		weight_p;
>  extern int		dev_weight_rx_bias;
>  extern int		dev_weight_tx_bias;
> +extern u8		netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
>  extern bool		netdev_rss_key_initialized;

[Severity: Low]
This isn't a bug, but the block these two lines join is introduced by:

net/core/dev.h:
	/* sysctls not referred to from outside net/core/ */
	extern int		netdev_unregister_timeout_secs;

netdev_rss_key is now defined in net/ethtool/common.c and read from
net/ethtool/common.c:netdev_rss_key_fill(), and netdev_rss_key_initialized
(added by the preceding patch of the series) is defined there too.

Should the grouping comment be adjusted, or should these two declarations
live somewhere that is not described as net/core/ private?

> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index 23db40618fed1..b4e766e60d38a 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c

[ ... ]

> @@ -1400,3 +1403,174 @@ enum ethtool_link_medium ethtool_str_to_medium(const char *str)
>  	return ETHTOOL_LINK_MEDIUM_NONE;
>  }
>  EXPORT_SYMBOL_GPL(ethtool_str_to_medium);
> +
> +u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
> +bool netdev_rss_key_initialized __read_mostly;

[ ... ]

> +static void netdev_rss_key_fixup_field(u8 *key, unsigned int lsb)
> +{
> +	bool d[NETDEV_RSS_KEY_SPAN];
> +	unsigned int i, t;
> +
> +	for (t = 0; t < NETDEV_RSS_KEY_SPAN; t++)
> +		d[t] = netdev_rss_key_bit(key, lsb + 31 - t);
> +
> +	for (i = 0; 2 * i < NETDEV_RSS_KEY_SPAN; i++) {
> +		bool value = true;
> +
> +		for (t = i; t < 2 * i; t++)
> +			value ^= d[t];
> +
> +		d[2 * i] = value;
> +	}

[Severity: Medium]
Does this fixup create collision families that hold for every generated key?

d[t] is the key bit that field bit t contributes to the lowest hash bit, and
the loop above enforces, for i = 0..7, that the XOR of d over the set
S_i = {i .. 2i} equals 1:

	d[0] = 1
	d[1] ^ d[2] = 1
	d[2] ^ d[3] ^ d[4] = 1
	...

XORing any two of those relations gives a set of field bit positions over
which the contributions to the lowest hash bit cancel for any key the
generator produces. On a 16-bit field, S_0 xor S_i for i = 1..7 gives seven
independent masks:

	0x0007, 0x001d, 0x0079, 0x01f1, 0x07e1, 0x1fc1, 0x7f81

So starting from source port 0x8000 and XORing arbitrary subsets of those
masks yields 128 distinct source ports, all of them above 0x8000 and so all
unprivileged, that always land on the same value of the lowest hash bit.

With ethtool_rxfh_indir_default(), the queue is index % nq, so for a
power-of-two queue count those 128 flows are confined to half the queues,
and to a single queue when nq is 2:

include/linux/ethtool.h:ethtool_rxfh_indir_default() {
	return index % n_rx_rings;
}

drivers/net/ethernet/google/gve/gve_main.c:gve_init_rss_config() combines
both, filling hash_lut with ethtool_rxfh_indir_default() and the key with
netdev_rss_key_fill().

netdev_rss_key_aliases() does not catch this since it compares whole 32-bit
windows, whereas the relations above are XOR relations among their lowest
bits only.

The changelog says the key "is a secret that stops a remote attacker from
steering flows onto a single queue". With the relations being forced by the
q = 1 and q = 2 guarantee (non-singularity alone gives d[0] = 1 and
d[2] = 1 ^ d[1]), is that still accurate, and could the commit message and
the new scaling.rst paragraph say what a chosen-port attacker can still do?

[ ... ]

> +static bool netdev_rss_key_aliases(const u8 *key, unsigned int bits)
> +{
> +	u32 windows[NETDEV_RSS_KEY_LEN * BITS_PER_BYTE / 16];
> +	unsigned int i, j, n, r;
> +
> +	for (r = 0; r < 16; r++) {
> +		n = 0;
> +		for (i = r; i + 32 <= bits; i += 16)
> +			windows[n++] = netdev_rss_key_window(key, i);
> +
> +		for (i = 0; i < n; i++)
> +			for (j = i + 1; j < n; j++)
> +				if (windows[i] == windows[j])
> +					return true;

[ ... ]

> +static void netdev_rss_key_init(u8 *key, size_t len)
> +{
> +	unsigned int lsb, bits = len * BITS_PER_BYTE;
> +
> +	/* Four keys out of five come out of the fixup free of aliases, so
> +	 * drawing another one is both simpler and cheaper than repairing.
> +	 */
> +	do {
> +		get_random_bytes(key, len);
> +
> +		/* A field ending at bit @lsb uses key bits [.. , @lsb + 31],
> +		 * so stop as soon as a 32-bit window no longer fits in the
> +		 * key.
> +		 */
> +		for (lsb = 15; lsb + 32 <= bits; lsb += 16)
> +			netdev_rss_key_fixup_field(key, lsb);
> +	} while (netdev_rss_key_aliases(key, bits));

[Severity: Medium]
How long can this loop keep hard interrupts disabled?

DO_ONCE() below runs the callback under the global once_lock with IRQs off:

lib/once.c:__do_once_start() {
	spin_lock_irqsave(&once_lock, *flags);

and the lock is only released in __do_once_done() after the callback
returns. Previously the callback was a single 256-byte get_random_bytes().
It now does get_random_bytes(256), 126 netdev_rss_key_fixup_field() calls
and netdev_rss_key_aliases(), which builds up to 127 windows for each of 16
residues and compares all pairs.

The changelog measures one pass:

	"DO_ONCE() runs the generator under a spinlock with hard IRQs
	disabled, so keep the windows of a class in an array rather than
	recomputing both sides of every pair: 116 us instead of 254 us for
	the worst case"

and the do/while has no cap, so with roughly one draw in five rejected the
duration is only bounded probabilistically. Other CPUs entering
netdev_rss_key_fill() spin on once_lock for that whole time.

Would it be reasonable to generate the key outside the once lock and only
publish it there? Note DO_ONCE_SLEEPABLE() does not look usable as a
replacement: tg3 reaches netdev_rss_key_fill() from tg3_reset_hw(), called
by tg3_init_hw() under tg3_full_lock(), i.e. with a spinlock held.

> +
> +	/* Pair with smp_rmb() in proc_do_rss_key(). */
> +	smp_wmb();
> +	WRITE_ONCE(netdev_rss_key_initialized, true);
> +}

[Severity: Low]
The barrier moved here from netdev_rss_key_fill(), but the reader side still
points at the old place:

net/core/sysctl_net_core.c:proc_do_rss_key() {
	initialized = READ_ONCE(netdev_rss_key_initialized);
	/* Pair with smp_wmb() in netdev_rss_key_fill(). */
	smp_rmb();

After this patch netdev_rss_key_fill() contains no barrier at all, so only
one half of the documented pairing names the other. That reader comment came
in with the preceding patch of the series ("net: synchronize
proc_do_rss_key() with netdev_rss_key_fill()") and no later patch in the
series touches net/core/sysctl_net_core.c. Should it be updated to name
netdev_rss_key_init()?

> +
> +void netdev_rss_key_fill(void *buffer, size_t len)
> +{
> +	if (WARN_ON_ONCE(len > sizeof(netdev_rss_key)))
> +		len = sizeof(netdev_rss_key);
> +	DO_ONCE(netdev_rss_key_init, netdev_rss_key, sizeof(netdev_rss_key));
> +	memcpy(buffer, netdev_rss_key, len);
> +}
> +EXPORT_SYMBOL(netdev_rss_key_fill);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922163458.3900996-1-edumazet%40google.com

Re: [PATCH v2 net-next 2/4] net: ethtool: generate RSS keys that spread flows over all queues

[Eric Dumazet]

On Thu, Sep 24, 2026 at 12:36 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 4 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 2 · Low: 2
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] netdev_rss_key_init() (net/ethtool/common.c:1546) is invoked
>   through DO_ONCE() from netdev_rss_key_fill()…
> - [Medium] The fixup introduces key-independent collision families in the
>   bit that selects the queue.
> - [Low] Stale cross-file memory-ordering comment.
> - [Low] net/core/dev.h:93 introduces the block with '/* sysctls not
>   referred to from outside net/core/ */', and this patch adds 'extern u8…
>
> --
>
> net: ethtool: generate RSS keys that spread flows over all queues
>
> This moves netdev_rss_key and netdev_rss_key_fill() from net/ethtool/ioctl.c
> to net/ethtool/common.c and replaces the plain random draw with a generator
> that forces some key bits so the Toeplitz matrices of the q lowest bits of a
> hashed field are non singular, then rejects keys where two 32-bit key windows
> alias.
>
> > diff --git a/net/core/dev.h b/net/core/dev.h
> > index 0127b4d03e525..a5e22b2eae523 100644
> > --- a/net/core/dev.h
> > +++ b/net/core/dev.h
> > @@ -95,6 +95,7 @@ extern int          netdev_unregister_timeout_secs;
> >  extern int           weight_p;
> >  extern int           dev_weight_rx_bias;
> >  extern int           dev_weight_tx_bias;
> > +extern u8            netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
> >  extern bool          netdev_rss_key_initialized;
>
> [Severity: Low]
> This isn't a bug, but the block these two lines join is introduced by:
>
> net/core/dev.h:
>         /* sysctls not referred to from outside net/core/ */
>         extern int              netdev_unregister_timeout_secs;
>
> netdev_rss_key is now defined in net/ethtool/common.c and read from
> net/ethtool/common.c:netdev_rss_key_fill(), and netdev_rss_key_initialized
> (added by the preceding patch of the series) is defined there too.
>
> Should the grouping comment be adjusted, or should these two declarations
> live somewhere that is not described as net/core/ private?
>
> > diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> > index 23db40618fed1..b4e766e60d38a 100644
> > --- a/net/ethtool/common.c
> > +++ b/net/ethtool/common.c
>
> [ ... ]
>
> > @@ -1400,3 +1403,174 @@ enum ethtool_link_medium ethtool_str_to_medium(const char *str)
> >       return ETHTOOL_LINK_MEDIUM_NONE;
> >  }
> >  EXPORT_SYMBOL_GPL(ethtool_str_to_medium);
> > +
> > +u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
> > +bool netdev_rss_key_initialized __read_mostly;
>
> [ ... ]
>
> > +static void netdev_rss_key_fixup_field(u8 *key, unsigned int lsb)
> > +{
> > +     bool d[NETDEV_RSS_KEY_SPAN];
> > +     unsigned int i, t;
> > +
> > +     for (t = 0; t < NETDEV_RSS_KEY_SPAN; t++)
> > +             d[t] = netdev_rss_key_bit(key, lsb + 31 - t);
> > +
> > +     for (i = 0; 2 * i < NETDEV_RSS_KEY_SPAN; i++) {
> > +             bool value = true;
> > +
> > +             for (t = i; t < 2 * i; t++)
> > +                     value ^= d[t];
> > +
> > +             d[2 * i] = value;
> > +     }
>
> [Severity: Medium]
> Does this fixup create collision families that hold for every generated key?
>
> d[t] is the key bit that field bit t contributes to the lowest hash bit, and
> the loop above enforces, for i = 0..7, that the XOR of d over the set
> S_i = {i .. 2i} equals 1:
>
>         d[0] = 1
>         d[1] ^ d[2] = 1
>         d[2] ^ d[3] ^ d[4] = 1
>         ...
>
> XORing any two of those relations gives a set of field bit positions over
> which the contributions to the lowest hash bit cancel for any key the
> generator produces. On a 16-bit field, S_0 xor S_i for i = 1..7 gives seven
> independent masks:
>
>         0x0007, 0x001d, 0x0079, 0x01f1, 0x07e1, 0x1fc1, 0x7f81
>
> So starting from source port 0x8000 and XORing arbitrary subsets of those
> masks yields 128 distinct source ports, all of them above 0x8000 and so all
> unprivileged, that always land on the same value of the lowest hash bit.
>

This is a mathematical consequence of Toeplitz linearity over GF(2), not an
artifact of the generator: the q x q matrix mapping the q lowest bits of a
field to the q lowest bits of the hash is the Hankel matrix M_q(r, c) =
d[r + c]. Over GF(2), det(M_1) = ... = det(M_8) = 1 holds if and only if
d[2 * i] = 1 ^ d[i] ^ ... ^ d[2 * i - 1] for i = 0..7. Every Toeplitz key
that is non-singular for all power-of-two queue counts up to 256 satisfies
these relations.

Note that these relations only constrain bit 0 of the hash (queue parity).
For 2^q queues (q >= 2), the remaining q - 1 bits of the queue index
(r = 1..q - 1) depend on d[t + r], which include the unconstrained odd bits
(d[1], d[3], ..., d[15], ...) that remain secret and uniformly random.

> With ethtool_rxfh_indir_default(), the queue is index % nq, so for a
> power-of-two queue count those 128 flows are confined to half the queues,
> and to a single queue when nq is 2:
>
> include/linux/ethtool.h:ethtool_rxfh_indir_default() {
>         return index % n_rx_rings;
> }
>
> drivers/net/ethernet/google/gve/gve_main.c:gve_init_rss_config() combines
> both, filling hash_lut with ethtool_rxfh_indir_default() and the key with
> netdev_rss_key_fill().
>
> netdev_rss_key_aliases() does not catch this since it compares whole 32-bit
> windows, whereas the relations above are XOR relations among their lowest
> bits only.
>
> The changelog says the key "is a secret that stops a remote attacker from
> steering flows onto a single queue". With the relations being forced by the
> q = 1 and q = 2 guarantee (non-singularity alone gives d[0] = 1 and
> d[2] = 1 ^ d[1]), is that still accurate, and could the commit message and
> the new scaling.rst paragraph say what a chosen-port attacker can still do?
>
> [ ... ]
>
> > +static bool netdev_rss_key_aliases(const u8 *key, unsigned int bits)
> > +{
> > +     u32 windows[NETDEV_RSS_KEY_LEN * BITS_PER_BYTE / 16];
> > +     unsigned int i, j, n, r;
> > +
> > +     for (r = 0; r < 16; r++) {
> > +             n = 0;
> > +             for (i = r; i + 32 <= bits; i += 16)
> > +                     windows[n++] = netdev_rss_key_window(key, i);
> > +
> > +             for (i = 0; i < n; i++)
> > +                     for (j = i + 1; j < n; j++)
> > +                             if (windows[i] == windows[j])
> > +                                     return true;
>
> [ ... ]
>
> > +static void netdev_rss_key_init(u8 *key, size_t len)
> > +{
> > +     unsigned int lsb, bits = len * BITS_PER_BYTE;
> > +
> > +     /* Four keys out of five come out of the fixup free of aliases, so
> > +      * drawing another one is both simpler and cheaper than repairing.
> > +      */
> > +     do {
> > +             get_random_bytes(key, len);
> > +
> > +             /* A field ending at bit @lsb uses key bits [.. , @lsb + 31],
> > +              * so stop as soon as a 32-bit window no longer fits in the
> > +              * key.
> > +              */
> > +             for (lsb = 15; lsb + 32 <= bits; lsb += 16)
> > +                     netdev_rss_key_fixup_field(key, lsb);
> > +     } while (netdev_rss_key_aliases(key, bits));
>
> [Severity: Medium]
> How long can this loop keep hard interrupts disabled?
>
> DO_ONCE() below runs the callback under the global once_lock with IRQs off:
>
> lib/once.c:__do_once_start() {
>         spin_lock_irqsave(&once_lock, *flags);
>
> and the lock is only released in __do_once_done() after the callback
> returns. Previously the callback was a single 256-byte get_random_bytes().
> It now does get_random_bytes(256), 126 netdev_rss_key_fixup_field() calls
> and netdev_rss_key_aliases(), which builds up to 127 windows for each of 16
> residues and compares all pairs.

This runs once per boot when the first NIC driver initializes RSS (typically
during driver probe), and as noted in the changelog we already cache each
residue class on the stack specifically to keep a pass down to ~116 us, with
~80% of keys passing on the first draw.

Re: [PATCH v2 net-next 0/4] net: ethtool: make netdev_rss_key_fill() spread flows over all queues

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 22 Sep 2026 16:34:54 +0000 you wrote:
> netdev_rss_key_fill() hands drivers a uniformly random key. Because the
> Toeplitz hash is linear over GF(2), a random key is singular for a given
> header field and queue count with probability 1/2: flows differing only
> in the low order bits of that field (such as a burst of consecutive
> ephemeral ports) then cannot reach all RX queues. On one affected 16-queue
> host, only 4 queues received traffic until the key was replaced.
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/4] net: synchronize proc_do_rss_key() with netdev_rss_key_fill()
    https://git.kernel.org/netdev/net-next/c/9396e37c4860
  - [v2,net-next,2/4] net: ethtool: generate RSS keys that spread flows over all queues
    https://git.kernel.org/netdev/net-next/c/898d431abd25
  - [v2,net-next,3/4] netdevsim: support reporting RSS key, indirection table, and flow hash fields
    https://git.kernel.org/netdev/net-next/c/5fd4208098be
  - [v2,net-next,4/4] selftests: drivers: net: check the host and device RSS keys
    https://git.kernel.org/netdev/net-next/c/c8103dd8b9b4

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