Re: [PATCH v2 net-next 1/4] net: synchronize proc_do_rss_key() with netdev_rss_key_fill()
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-23 22:36:42
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.
quoted hunk ↗ jump to hunk
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? [ ... ]
quoted hunk ↗ jump to hunk
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