[PATCH iwl-next v2 0/3] igb: add RSS key get/set support

[PATCH iwl-next v2 0/3] igb: add RSS key get/set support

[Takashi Kozu]

This series adds ethtool get/set support for the RSS hash key in the igb
driver.
- `ethtool -x <dev>` to display the RSS key
- `ethtool -X <dev> hkey <key>` to configure the RSS key

Without patch:

# ethtool -x $DEV | grep key -A1
RSS hash key:
Operation not supported
# ethtool -X $DEV hkey 00:00:00:00:00:00:00:00:00:00:00:00:000
Cannot set RX flow hash configuration:
  Hash key setting not supported


With patch:

# ethtool -x $DEV | grep key -A1
RSS hash key:
86:5d:11:56:bd:6f:20:38:3b:f8:bb:df:00:3a:b0:24:95:9f:f9:f4:25:a3:01:3e:4a:15:d6:7c:4d:af:39:7e:4a:95:f2:fd:f6:b6:26:f7

# ethtool -X $DEV hkey 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
# ethtool -x $DEV | grep key -A1
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

<Changelog>

  v2: Fix typos (igc_* → igb_*) 

  v1: https://lore.kernel.org/all/20251205082106.4028-5-takkozu@amazon.com/

Takashi Kozu (3):
  Store the RSS key inside struct igb_adapter and introduce the
  igb: expose RSS key via ethtool get_rxfh
  igb: allow configuring RSS key via ethtool set_rxfh

 drivers/net/ethernet/intel/igb/igb.h         |  4 +
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 77 +++++++++++++-------
 drivers/net/ethernet/intel/igb/igb_main.c    |  7 +-
 3 files changed, 58 insertions(+), 30 deletions(-)

-- 
2.52.0

[PATCH iwl-next v2 1/3] igb: prepare for RSS key get/set support

[Takashi Kozu]

Store the RSS key inside struct igb_adapter and introduce the
igb_write_rss_key() helper function. This allows the driver to program
the E1000 registers using a persistent RSS key, instead of using a
stack-local buffer in igb_setup_mrqc().

Tested-by: Kohei Enju <enjuk@amazon.com>
Signed-off-by: Takashi Kozu <takkozu@amazon.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  3 +++
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 12 ++++++++++++
 drivers/net/ethernet/intel/igb/igb_main.c    |  6 ++----
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 0fff1df81b7b..8c9b02058cec 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -495,6 +495,7 @@ struct hwmon_buff {
 #define IGB_N_PEROUT	2
 #define IGB_N_SDP	4
 #define IGB_RETA_SIZE	128
+#define IGB_RSS_KEY_SIZE	40
 
 enum igb_filter_match_flags {
 	IGB_FILTER_FLAG_ETHER_TYPE = 0x1,
@@ -655,6 +656,7 @@ struct igb_adapter {
 	struct i2c_client *i2c_client;
 	u32 rss_indir_tbl_init;
 	u8 rss_indir_tbl[IGB_RETA_SIZE];
+	u8 rss_key[IGB_RSS_KEY_SIZE];
 
 	unsigned long link_check_timeout;
 	int copper_tries;
@@ -735,6 +737,7 @@ void igb_down(struct igb_adapter *);
 void igb_reinit_locked(struct igb_adapter *);
 void igb_reset(struct igb_adapter *);
 int igb_reinit_queues(struct igb_adapter *);
+void igb_write_rss_key(struct igb_adapter *adapter);
 void igb_write_rss_indir_tbl(struct igb_adapter *);
 int igb_set_spd_dplx(struct igb_adapter *, u32, u8);
 int igb_setup_tx_resources(struct igb_ring *);
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 10e2445e0ded..8695ff28a7b8 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -3016,6 +3016,18 @@ static int igb_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
 	return ret;
 }
 
+void igb_write_rss_key(struct igb_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	u32 val;
+	int i;
+
+	for (i = 0; i < IGB_RSS_KEY_SIZE / 4; i++) {
+		val = get_unaligned_le32(&adapter->rss_key[i * 4]);
+		wr32(E1000_RSSRK(i), val);
+	}
+}
+
 static int igb_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 85f9589cc568..da0f550de605 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4525,11 +4525,9 @@ static void igb_setup_mrqc(struct igb_adapter *adapter)
 	struct e1000_hw *hw = &adapter->hw;
 	u32 mrqc, rxcsum;
 	u32 j, num_rx_queues;
-	u32 rss_key[10];
 
-	netdev_rss_key_fill(rss_key, sizeof(rss_key));
-	for (j = 0; j < 10; j++)
-		wr32(E1000_RSSRK(j), rss_key[j]);
+	netdev_rss_key_fill(adapter->rss_key, sizeof(adapter->rss_key));
+	igb_write_rss_key(adapter);
 
 	num_rx_queues = adapter->rss_queues;
 
-- 
2.52.0

[PATCH iwl-next v2 2/3] igb: expose RSS key via ethtool get_rxfh

[Takashi Kozu]

Implement igb_get_rxfh_key_size() and extend
igb_get_rxfh() to return the RSS key to userspace.

This can be tested using `ethtool -x <dev>`.

Tested-by: Kohei Enju <enjuk@amazon.com>
Signed-off-by: Takashi Kozu <takkozu@amazon.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 8695ff28a7b8..2953d079ebae 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -3285,11 +3285,13 @@ static int igb_get_rxfh(struct net_device *netdev,
 	int i;
 
 	rxfh->hfunc = ETH_RSS_HASH_TOP;
-	if (!rxfh->indir)
-		return 0;
-	for (i = 0; i < IGB_RETA_SIZE; i++)
-		rxfh->indir[i] = adapter->rss_indir_tbl[i];
 
+	if (rxfh->indir)
+		for (i = 0; i < IGB_RETA_SIZE; i++)
+			rxfh->indir[i] = adapter->rss_indir_tbl[i];
+
+	if (rxfh->key)
+		memcpy(rxfh->key, adapter->rss_key, sizeof(adapter->rss_key));
 	return 0;
 }
 
@@ -3328,6 +3330,11 @@ void igb_write_rss_indir_tbl(struct igb_adapter *adapter)
 	}
 }
 
+static u32 igb_get_rxfh_key_size(struct net_device *netdev)
+{
+	return IGB_RSS_KEY_SIZE;
+}
+
 static int igb_set_rxfh(struct net_device *netdev,
 			struct ethtool_rxfh_param *rxfh,
 			struct netlink_ext_ack *extack)
@@ -3491,6 +3498,7 @@ static const struct ethtool_ops igb_ethtool_ops = {
 	.get_module_eeprom	= igb_get_module_eeprom,
 	.get_rxfh_indir_size	= igb_get_rxfh_indir_size,
 	.get_rxfh		= igb_get_rxfh,
+	.get_rxfh_key_size	= igb_get_rxfh_key_size,
 	.set_rxfh		= igb_set_rxfh,
 	.get_rxfh_fields	= igb_get_rxfh_fields,
 	.set_rxfh_fields	= igb_set_rxfh_fields,
-- 
2.52.0

[PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh

[Takashi Kozu]

Change igb_set_rxfh() to accept and save a userspace-provided
RSS key. When a key is provided, store it in the adapter and write the
E1000 registers accordingly.

This can be tested using `ethtool -X <dev> hkey <key>`.

Tested-by: Kohei Enju <enjuk@amazon.com>
Signed-off-by: Takashi Kozu <takkozu@amazon.com>
---
 drivers/net/ethernet/intel/igb/igb.h         |  1 +
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 49 +++++++++++---------
 drivers/net/ethernet/intel/igb/igb_main.c    |  3 +-
 3 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 8c9b02058cec..2509ec30acf3 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -657,6 +657,7 @@ struct igb_adapter {
 	u32 rss_indir_tbl_init;
 	u8 rss_indir_tbl[IGB_RETA_SIZE];
 	u8 rss_key[IGB_RSS_KEY_SIZE];
+	bool has_user_rss_key;
 
 	unsigned long link_check_timeout;
 	int copper_tries;
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 2953d079ebae..ac045fbebade 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -3345,35 +3345,40 @@ static int igb_set_rxfh(struct net_device *netdev,
 	u32 num_queues;
 
 	/* We do not allow change in unsupported parameters */
-	if (rxfh->key ||
-	    (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
-	     rxfh->hfunc != ETH_RSS_HASH_TOP))
+	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
+	    rxfh->hfunc != ETH_RSS_HASH_TOP)
 		return -EOPNOTSUPP;
-	if (!rxfh->indir)
-		return 0;
 
-	num_queues = adapter->rss_queues;
+	if (rxfh->indir) {
+		num_queues = adapter->rss_queues;
 
-	switch (hw->mac.type) {
-	case e1000_82576:
-		/* 82576 supports 2 RSS queues for SR-IOV */
-		if (adapter->vfs_allocated_count)
-			num_queues = 2;
-		break;
-	default:
-		break;
-	}
+		switch (hw->mac.type) {
+		case e1000_82576:
+			/* 82576 supports 2 RSS queues for SR-IOV */
+			if (adapter->vfs_allocated_count)
+				num_queues = 2;
+			break;
+		default:
+			break;
+		}
 
-	/* Verify user input. */
-	for (i = 0; i < IGB_RETA_SIZE; i++)
-		if (rxfh->indir[i] >= num_queues)
-			return -EINVAL;
+		/* Verify user input. */
+		for (i = 0; i < IGB_RETA_SIZE; i++)
+			if (rxfh->indir[i] >= num_queues)
+				return -EINVAL;
 
 
-	for (i = 0; i < IGB_RETA_SIZE; i++)
-		adapter->rss_indir_tbl[i] = rxfh->indir[i];
+		for (i = 0; i < IGB_RETA_SIZE; i++)
+			adapter->rss_indir_tbl[i] = rxfh->indir[i];
+
+		igb_write_rss_indir_tbl(adapter);
+	}
 
-	igb_write_rss_indir_tbl(adapter);
+	if (rxfh->key) {
+		adapter->has_user_rss_key = true;
+		memcpy(adapter->rss_key, rxfh->key, sizeof(adapter->rss_key));
+		igb_write_rss_key(adapter);
+	}
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index da0f550de605..d42b3750f0b1 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4526,7 +4526,8 @@ static void igb_setup_mrqc(struct igb_adapter *adapter)
 	u32 mrqc, rxcsum;
 	u32 j, num_rx_queues;
 
-	netdev_rss_key_fill(adapter->rss_key, sizeof(adapter->rss_key));
+	if (!adapter->has_user_rss_key)
+		netdev_rss_key_fill(adapter->rss_key, sizeof(adapter->rss_key));
 	igb_write_rss_key(adapter);
 
 	num_rx_queues = adapter->rss_queues;
-- 
2.52.0

RE: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Takashi Kozu
> Sent: Thursday, January 8, 2026 6:20 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; Takashi Kozu <takkozu@amazon.com>; Kohei Enju
> <enjuk@amazon.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow
> configuring RSS key via ethtool set_rxfh
> 
> Change igb_set_rxfh() to accept and save a userspace-provided RSS key.
> When a key is provided, store it in the adapter and write the
> E1000 registers accordingly.
> 
> This can be tested using `ethtool -X <dev> hkey <key>`.
> 
> Tested-by: Kohei Enju <enjuk@amazon.com>
> Signed-off-by: Takashi Kozu <takkozu@amazon.com>
> ---
>  drivers/net/ethernet/intel/igb/igb.h         |  1 +
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 49 +++++++++++--------
> -
>  drivers/net/ethernet/intel/igb/igb_main.c    |  3 +-
>  3 files changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb.h
> b/drivers/net/ethernet/intel/igb/igb.h
> index 8c9b02058cec..2509ec30acf3 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -657,6 +657,7 @@ struct igb_adapter {
>  	u32 rss_indir_tbl_init;
>  	u8 rss_indir_tbl[IGB_RETA_SIZE];
>  	u8 rss_key[IGB_RSS_KEY_SIZE];
> +	bool has_user_rss_key;
> 
>  	unsigned long link_check_timeout;
>  	int copper_tries;
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 2953d079ebae..ac045fbebade 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -3345,35 +3345,40 @@ static int igb_set_rxfh(struct net_device
> *netdev,
>  	u32 num_queues;
> 
>  	/* We do not allow change in unsupported parameters */
> -	if (rxfh->key ||
> -	    (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
> -	     rxfh->hfunc != ETH_RSS_HASH_TOP))
> +	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
> +	    rxfh->hfunc != ETH_RSS_HASH_TOP)
>  		return -EOPNOTSUPP;
> -	if (!rxfh->indir)
> -		return 0;
> 
> -	num_queues = adapter->rss_queues;
> +	if (rxfh->indir) {
> +		num_queues = adapter->rss_queues;
> 
> -	switch (hw->mac.type) {
> -	case e1000_82576:
> -		/* 82576 supports 2 RSS queues for SR-IOV */
> -		if (adapter->vfs_allocated_count)
> -			num_queues = 2;
> -		break;
> -	default:
> -		break;
> -	}
> +		switch (hw->mac.type) {
> +		case e1000_82576:
> +			/* 82576 supports 2 RSS queues for SR-IOV */
> +			if (adapter->vfs_allocated_count)
> +				num_queues = 2;
> +			break;
> +		default:
> +			break;
> +		}
> 
> -	/* Verify user input. */
> -	for (i = 0; i < IGB_RETA_SIZE; i++)
> -		if (rxfh->indir[i] >= num_queues)
> -			return -EINVAL;
> +		/* Verify user input. */
> +		for (i = 0; i < IGB_RETA_SIZE; i++)
> +			if (rxfh->indir[i] >= num_queues)
> +				return -EINVAL;
> 
> 
> -	for (i = 0; i < IGB_RETA_SIZE; i++)
> -		adapter->rss_indir_tbl[i] = rxfh->indir[i];
> +		for (i = 0; i < IGB_RETA_SIZE; i++)
> +			adapter->rss_indir_tbl[i] = rxfh->indir[i];
> +
> +		igb_write_rss_indir_tbl(adapter);
> +	}
> 
> -	igb_write_rss_indir_tbl(adapter);
> +	if (rxfh->key) {
> +		adapter->has_user_rss_key = true;
> +		memcpy(adapter->rss_key, rxfh->key, sizeof(adapter-
> >rss_key));
> +		igb_write_rss_key(adapter);
It leads to race between ethtool RSS update and concurrent resets.
Because igb_setup_mrqc() (called during resets) also calls igb_write_rss_key(adapter).
Non-fatal but breaks RSS configuration guarantees.

I think ethtool can/should wait of reset/watchdog task to finish. 
I'm against adding locks, and just my personal opinion, it's better to implement igb_rss_key_update_task() in addition to reset and watchdog tasks to be used both in reset and ethtool path.

What do you think?

> +	}
> 
>  	return 0;
>  }
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index da0f550de605..d42b3750f0b1 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4526,7 +4526,8 @@ static void igb_setup_mrqc(struct igb_adapter
> *adapter)
>  	u32 mrqc, rxcsum;
>  	u32 j, num_rx_queues;
> 
> -	netdev_rss_key_fill(adapter->rss_key, sizeof(adapter-
> >rss_key));
> +	if (!adapter->has_user_rss_key)
> +		netdev_rss_key_fill(adapter->rss_key, sizeof(adapter-
> >rss_key));
>  	igb_write_rss_key(adapter);
> 
>  	num_rx_queues = adapter->rss_queues;
> --
> 2.52.0

Re: RE: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh

[Kohei Enju]

On Thu, 8 Jan 2026 07:29:19 +0000, Loktionov, Aleksandr wrote:

>> 
>> -	igb_write_rss_indir_tbl(adapter);
>> +	if (rxfh->key) {
>> +		adapter->has_user_rss_key = true;
>> +		memcpy(adapter->rss_key, rxfh->key, sizeof(adapter-
>> >rss_key));
>> +		igb_write_rss_key(adapter);
>It leads to race between ethtool RSS update and concurrent resets.
>Because igb_setup_mrqc() (called during resets) also calls igb_write_rss_key(adapter).
>Non-fatal but breaks RSS configuration guarantees.

At my first glance, rtnl lock serializes those operation, so it
doesn't seem to be racy as long as they are under the rtnl lock.

As far as I skimmed the codes, functions such as igb_open()/
igb_up()/igb_reset_task(), which finally call igb_write_rss_key() are
serialized by rtnl lock or serializes igb_write_rss_key() call by
locking rtnl.

Please let me know if I'm missing something and it's truly racy.

>
>I think ethtool can/should wait of reset/watchdog task to finish. 
>I'm against adding locks, and just my personal opinion, it's better to implement igb_rss_key_update_task() in addition to reset and watchdog tasks to be used both in reset and ethtool path.
>
>What do you think?

Re: [Intel-wired-lan] [PATCH iwl-next v2 1/3] igb: prepare for RSS key get/set support

[Paul Menzel]

Dear Takashi,


Thank you for the patch.

Am 08.01.26 um 06:20 schrieb Takashi Kozu:
> Store the RSS key inside struct igb_adapter and introduce the
> igb_write_rss_key() helper function. This allows the driver to program
> the E1000 registers using a persistent RSS key, instead of using a
> stack-local buffer in igb_setup_mrqc().
> 
> Tested-by: Kohei Enju <enjuk@amazon.com>
> Signed-off-by: Takashi Kozu <takkozu@amazon.com>
> ---
>   drivers/net/ethernet/intel/igb/igb.h         |  3 +++
>   drivers/net/ethernet/intel/igb/igb_ethtool.c | 12 ++++++++++++
>   drivers/net/ethernet/intel/igb/igb_main.c    |  6 ++----
>   3 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
> index 0fff1df81b7b..8c9b02058cec 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -495,6 +495,7 @@ struct hwmon_buff {
>   #define IGB_N_PEROUT	2
>   #define IGB_N_SDP	4
>   #define IGB_RETA_SIZE	128
> +#define IGB_RSS_KEY_SIZE	40
>   
>   enum igb_filter_match_flags {
>   	IGB_FILTER_FLAG_ETHER_TYPE = 0x1,
> @@ -655,6 +656,7 @@ struct igb_adapter {
>   	struct i2c_client *i2c_client;
>   	u32 rss_indir_tbl_init;
>   	u8 rss_indir_tbl[IGB_RETA_SIZE];
> +	u8 rss_key[IGB_RSS_KEY_SIZE];
>   
>   	unsigned long link_check_timeout;
>   	int copper_tries;
> @@ -735,6 +737,7 @@ void igb_down(struct igb_adapter *);
>   void igb_reinit_locked(struct igb_adapter *);
>   void igb_reset(struct igb_adapter *);
>   int igb_reinit_queues(struct igb_adapter *);
> +void igb_write_rss_key(struct igb_adapter *adapter);
>   void igb_write_rss_indir_tbl(struct igb_adapter *);
>   int igb_set_spd_dplx(struct igb_adapter *, u32, u8);
>   int igb_setup_tx_resources(struct igb_ring *);
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 10e2445e0ded..8695ff28a7b8 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -3016,6 +3016,18 @@ static int igb_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
>   	return ret;
>   }
>   
> +void igb_write_rss_key(struct igb_adapter *adapter)
> +{
> +	struct e1000_hw *hw = &adapter->hw;
> +	u32 val;
> +	int i;
> +
> +	for (i = 0; i < IGB_RSS_KEY_SIZE / 4; i++) {
> +		val = get_unaligned_le32(&adapter->rss_key[i * 4]);

Why is `get_unaligned_le32()` needed?

> +		wr32(E1000_RSSRK(i), val);

I probably wouldn’t get rid of `val`.

> +	}
> +}
> +
>   static int igb_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
>   {
>   	struct igb_adapter *adapter = netdev_priv(netdev);
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 85f9589cc568..da0f550de605 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4525,11 +4525,9 @@ static void igb_setup_mrqc(struct igb_adapter *adapter)
>   	struct e1000_hw *hw = &adapter->hw;
>   	u32 mrqc, rxcsum;
>   	u32 j, num_rx_queues;
> -	u32 rss_key[10];
>   
> -	netdev_rss_key_fill(rss_key, sizeof(rss_key));
> -	for (j = 0; j < 10; j++)
> -		wr32(E1000_RSSRK(j), rss_key[j]);
> +	netdev_rss_key_fill(adapter->rss_key, sizeof(adapter->rss_key));
> +	igb_write_rss_key(adapter);
>   
>   	num_rx_queues = adapter->rss_queues;

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

RE: RE: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kohei Enju <enjuk@amazon.com>
> Sent: Thursday, January 8, 2026 1:04 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> edumazet@google.com; enjuk@amazon.com; intel-wired-
> lan@lists.osuosl.org; kuba@kernel.org; netdev@vger.kernel.org;
> pabeni@redhat.com; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>;
> takkozu@amazon.com
> Subject: Re: RE: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow
> configuring RSS key via ethtool set_rxfh
> 
> On Thu, 8 Jan 2026 07:29:19 +0000, Loktionov, Aleksandr wrote:
> 
> >>
> >> -	igb_write_rss_indir_tbl(adapter);
> >> +	if (rxfh->key) {
> >> +		adapter->has_user_rss_key = true;
> >> +		memcpy(adapter->rss_key, rxfh->key, sizeof(adapter-
> >> >rss_key));
> >> +		igb_write_rss_key(adapter);
> >It leads to race between ethtool RSS update and concurrent resets.
> >Because igb_setup_mrqc() (called during resets) also calls
> igb_write_rss_key(adapter).
> >Non-fatal but breaks RSS configuration guarantees.
> 
> At my first glance, rtnl lock serializes those operation, so it
> doesn't seem to be racy as long as they are under the rtnl lock.
> 
> As far as I skimmed the codes, functions such as igb_open()/
> igb_up()/igb_reset_task(), which finally call igb_write_rss_key() are
> serialized by rtnl lock or serializes igb_write_rss_key() call by
> locking rtnl.
> 
> Please let me know if I'm missing something and it's truly racy.
I think you're right, and I've missed that missing rtnl_lock was added in upstream.

Thank you for clarification
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> 
> >
> >I think ethtool can/should wait of reset/watchdog task to finish.
> >I'm against adding locks, and just my personal opinion, it's better
> to implement igb_rss_key_update_task() in addition to reset and
> watchdog tasks to be used both in reset and ethtool path.
> >
> >What do you think?

RE: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Loktionov, Aleksandr
> Sent: Thursday, January 8, 2026 1:28 PM
> To: Kohei Enju <enjuk@amazon.com>
> Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> edumazet@google.com; intel-wired-lan@lists.osuosl.org;
> kuba@kernel.org; netdev@vger.kernel.org; pabeni@redhat.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; takkozu@amazon.com
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow
> configuring RSS key via ethtool set_rxfh
> 
> 
> 
> > -----Original Message-----
> > From: Kohei Enju <enjuk@amazon.com>
> > Sent: Thursday, January 8, 2026 1:04 PM
> > To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> > Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L
> > <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> > edumazet@google.com; enjuk@amazon.com; intel-wired-
> > lan@lists.osuosl.org; kuba@kernel.org; netdev@vger.kernel.org;
> > pabeni@redhat.com; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>;
> > takkozu@amazon.com
> > Subject: Re: RE: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb:
> allow
> > configuring RSS key via ethtool set_rxfh
> >
> > On Thu, 8 Jan 2026 07:29:19 +0000, Loktionov, Aleksandr wrote:
> >
> > >>
> > >> -	igb_write_rss_indir_tbl(adapter);
> > >> +	if (rxfh->key) {
> > >> +		adapter->has_user_rss_key = true;
> > >> +		memcpy(adapter->rss_key, rxfh->key, sizeof(adapter-
> > >> >rss_key));
> > >> +		igb_write_rss_key(adapter);
> > >It leads to race between ethtool RSS update and concurrent resets.
> > >Because igb_setup_mrqc() (called during resets) also calls
> > igb_write_rss_key(adapter).
> > >Non-fatal but breaks RSS configuration guarantees.
> >
> > At my first glance, rtnl lock serializes those operation, so it
> > doesn't seem to be racy as long as they are under the rtnl lock.
> >
> > As far as I skimmed the codes, functions such as igb_open()/
> > igb_up()/igb_reset_task(), which finally call igb_write_rss_key()
> are
> > serialized by rtnl lock or serializes igb_write_rss_key() call by
> > locking rtnl.
> >
> > Please let me know if I'm missing something and it's truly racy.
> I think you're right, and I've missed that missing rtnl_lock was added
> in upstream.
> 
> Thank you for clarification
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> 

Afterthought, I think it could be nice to place ASSERT_RTNL() to show it explicitly.
What do you think about this idea?

Re: [PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh

[Jakub Kicinski]

On Thu, 8 Jan 2026 14:20:15 +0900 Takashi Kozu wrote:
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index da0f550de605..d42b3750f0b1 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4526,7 +4526,8 @@ static void igb_setup_mrqc(struct igb_adapter *adapter)
>  	u32 mrqc, rxcsum;
>  	u32 j, num_rx_queues;
>  
> -	netdev_rss_key_fill(adapter->rss_key, sizeof(adapter->rss_key));
> +	if (!adapter->has_user_rss_key)
> +		netdev_rss_key_fill(adapter->rss_key, sizeof(adapter->rss_key));
>  	igb_write_rss_key(adapter);

This is an unusual construct. adapter->rss_key is driver state, does
something wipe it? It's normal to have to write the key into the device
after reset but initializing the driver state is usually done at probe,
just once. Then you don't have to worry whether the state is coming from
random or user.

Note that netdev_rss_key_fill() initializes its state once per boot so
it will not change its 'return' value without reboot.

Last but not least - would you be able to run:

tools/testing/selftests/drivers/net/hw/toeplitz.py
tools/testing/selftests/drivers/net/hw/rss_api.py

against this device? Some more help:
https://github.com/linux-netdev/nipa/wiki/Running-driver-tests

RE: [Intel-wired-lan] [PATCH iwl-next v2 1/3] igb: prepare for RSS key get/set support

[Kwapulinski, Piotr]

>-----Original Message-----
>From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Takashi Kozu
>Sent: Thursday, January 8, 2026 6:20 AM
>To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
>Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Takashi Kozu <takkozu@amazon.com>; Kohei Enju <enjuk@amazon.com>
>Subject: [Intel-wired-lan] [PATCH iwl-next v2 1/3] igb: prepare for RSS key get/set support
>
>Store the RSS key inside struct igb_adapter and introduce the
>igb_write_rss_key() helper function. This allows the driver to program the E1000 registers using a persistent RSS key, instead of using a stack-local buffer in igb_setup_mrqc().
>
>Tested-by: Kohei Enju <enjuk@amazon.com>
>Signed-off-by: Takashi Kozu <takkozu@amazon.com>
>---
> drivers/net/ethernet/intel/igb/igb.h         |  3 +++
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 12 ++++++++++++
> drivers/net/ethernet/intel/igb/igb_main.c    |  6 ++----
> 3 files changed, 17 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
>index 0fff1df81b7b..8c9b02058cec 100644
>--- a/drivers/net/ethernet/intel/igb/igb.h
>+++ b/drivers/net/ethernet/intel/igb/igb.h
>@@ -495,6 +495,7 @@ struct hwmon_buff {
> #define IGB_N_PEROUT	2
> #define IGB_N_SDP	4
> #define IGB_RETA_SIZE	128
>+#define IGB_RSS_KEY_SIZE	40
> 
> enum igb_filter_match_flags {
> 	IGB_FILTER_FLAG_ETHER_TYPE = 0x1,
>@@ -655,6 +656,7 @@ struct igb_adapter {
> 	struct i2c_client *i2c_client;
> 	u32 rss_indir_tbl_init;
> 	u8 rss_indir_tbl[IGB_RETA_SIZE];
>+	u8 rss_key[IGB_RSS_KEY_SIZE];
> 
> 	unsigned long link_check_timeout;
> 	int copper_tries;
>@@ -735,6 +737,7 @@ void igb_down(struct igb_adapter *);  void igb_reinit_locked(struct igb_adapter *);  void igb_reset(struct igb_adapter *);  int igb_reinit_queues(struct igb_adapter *);
>+void igb_write_rss_key(struct igb_adapter *adapter);
> void igb_write_rss_indir_tbl(struct igb_adapter *);  int igb_set_spd_dplx(struct igb_adapter *, u32, u8);  int igb_setup_tx_resources(struct igb_ring *); diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
>index 10e2445e0ded..8695ff28a7b8 100644
>--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
>+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
>@@ -3016,6 +3016,18 @@ static int igb_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
> 	return ret;
> }
> 
>+void igb_write_rss_key(struct igb_adapter *adapter) {
Please add the kernel-doc header.

>+	struct e1000_hw *hw = &adapter->hw;
>+	u32 val;
>+	int i;
Please consider declaring the the 'val' and 'i' within the scope of the 'for' loop.
Piotr

>+
>+	for (i = 0; i < IGB_RSS_KEY_SIZE / 4; i++) {
>+		val = get_unaligned_le32(&adapter->rss_key[i * 4]);
>+		wr32(E1000_RSSRK(i), val);
>+	}
>+}

[...]

RE: [Intel-wired-lan] [PATCH iwl-next v2 1/3] igb: prepare for RSS key get/set support

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Takashi Kozu
> Sent: Thursday, January 8, 2026 6:20 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; Takashi Kozu <takkozu@amazon.com>; Kohei Enju
> <enjuk@amazon.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2 1/3] igb: prepare for
> RSS key get/set support
> 
> Store the RSS key inside struct igb_adapter and introduce the
> igb_write_rss_key() helper function. This allows the driver to program
> the E1000 registers using a persistent RSS key, instead of using a
> stack-local buffer in igb_setup_mrqc().
> 
> Tested-by: Kohei Enju <enjuk@amazon.com>
> Signed-off-by: Takashi Kozu <takkozu@amazon.com>
> ---
>  drivers/net/ethernet/intel/igb/igb.h         |  3 +++
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 12 ++++++++++++
>  drivers/net/ethernet/intel/igb/igb_main.c    |  6 ++----
>  3 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb.h
> b/drivers/net/ethernet/intel/igb/igb.h
> index 0fff1df81b7b..8c9b02058cec 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -495,6 +495,7 @@ struct hwmon_buff {
>  #define IGB_N_PEROUT	2
>  #define IGB_N_SDP	4
>  #define IGB_RETA_SIZE	128
> +#define IGB_RSS_KEY_SIZE	40
> 
>  enum igb_filter_match_flags {
>  	IGB_FILTER_FLAG_ETHER_TYPE = 0x1,
> @@ -655,6 +656,7 @@ struct igb_adapter {
>  	struct i2c_client *i2c_client;
>  	u32 rss_indir_tbl_init;
>  	u8 rss_indir_tbl[IGB_RETA_SIZE];
> +	u8 rss_key[IGB_RSS_KEY_SIZE];
> 
>  	unsigned long link_check_timeout;
>  	int copper_tries;
> @@ -735,6 +737,7 @@ void igb_down(struct igb_adapter *);  void
> igb_reinit_locked(struct igb_adapter *);  void igb_reset(struct
> igb_adapter *);  int igb_reinit_queues(struct igb_adapter *);
> +void igb_write_rss_key(struct igb_adapter *adapter);
>  void igb_write_rss_indir_tbl(struct igb_adapter *);  int
> igb_set_spd_dplx(struct igb_adapter *, u32, u8);  int
> igb_setup_tx_resources(struct igb_ring *); diff --git
> a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 10e2445e0ded..8695ff28a7b8 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -3016,6 +3016,18 @@ static int igb_set_rxnfc(struct net_device
> *dev, struct ethtool_rxnfc *cmd)
>  	return ret;
>  }
> 
> +void igb_write_rss_key(struct igb_adapter *adapter) {
> +	struct e1000_hw *hw = &adapter->hw;
> +	u32 val;
> +	int i;
> +
> +	for (i = 0; i < IGB_RSS_KEY_SIZE / 4; i++) {
> +		val = get_unaligned_le32(&adapter->rss_key[i * 4]);
> +		wr32(E1000_RSSRK(i), val);
> +	}
> +}
> +
>  static int igb_get_eee(struct net_device *netdev, struct ethtool_keee
> *edata)  {
>  	struct igb_adapter *adapter = netdev_priv(netdev); diff --git
> a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index 85f9589cc568..da0f550de605 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4525,11 +4525,9 @@ static void igb_setup_mrqc(struct igb_adapter
> *adapter)
>  	struct e1000_hw *hw = &adapter->hw;
>  	u32 mrqc, rxcsum;
>  	u32 j, num_rx_queues;
> -	u32 rss_key[10];
> 
> -	netdev_rss_key_fill(rss_key, sizeof(rss_key));
> -	for (j = 0; j < 10; j++)
> -		wr32(E1000_RSSRK(j), rss_key[j]);
> +	netdev_rss_key_fill(adapter->rss_key, sizeof(adapter-
> >rss_key));
> +	igb_write_rss_key(adapter);
> 
>  	num_rx_queues = adapter->rss_queues;
> 
> --
> 2.52.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [Intel-wired-lan] [PATCH iwl-next v2 2/3] igb: expose RSS key via ethtool get_rxfh

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Takashi Kozu
> Sent: Thursday, January 8, 2026 6:20 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>;
> andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; Takashi Kozu <takkozu@amazon.com>; Kohei Enju
> <enjuk@amazon.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next v2 2/3] igb: expose RSS key
> via ethtool get_rxfh
> 
> Implement igb_get_rxfh_key_size() and extend
> igb_get_rxfh() to return the RSS key to userspace.
> 
> This can be tested using `ethtool -x <dev>`.
> 
> Tested-by: Kohei Enju <enjuk@amazon.com>
> Signed-off-by: Takashi Kozu <takkozu@amazon.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index 8695ff28a7b8..2953d079ebae 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -3285,11 +3285,13 @@ static int igb_get_rxfh(struct net_device
> *netdev,
>  	int i;
> 
>  	rxfh->hfunc = ETH_RSS_HASH_TOP;
> -	if (!rxfh->indir)
> -		return 0;
> -	for (i = 0; i < IGB_RETA_SIZE; i++)
> -		rxfh->indir[i] = adapter->rss_indir_tbl[i];
> 
> +	if (rxfh->indir)
> +		for (i = 0; i < IGB_RETA_SIZE; i++)
> +			rxfh->indir[i] = adapter->rss_indir_tbl[i];
> +
> +	if (rxfh->key)
> +		memcpy(rxfh->key, adapter->rss_key, sizeof(adapter-
> >rss_key));
>  	return 0;
>  }
> 
> @@ -3328,6 +3330,11 @@ void igb_write_rss_indir_tbl(struct igb_adapter
> *adapter)
>  	}
>  }
> 
> +static u32 igb_get_rxfh_key_size(struct net_device *netdev) {
> +	return IGB_RSS_KEY_SIZE;
> +}
> +
>  static int igb_set_rxfh(struct net_device *netdev,
>  			struct ethtool_rxfh_param *rxfh,
>  			struct netlink_ext_ack *extack)
> @@ -3491,6 +3498,7 @@ static const struct ethtool_ops igb_ethtool_ops
> = {
>  	.get_module_eeprom	= igb_get_module_eeprom,
>  	.get_rxfh_indir_size	= igb_get_rxfh_indir_size,
>  	.get_rxfh		= igb_get_rxfh,
> +	.get_rxfh_key_size	= igb_get_rxfh_key_size,
>  	.set_rxfh		= igb_set_rxfh,
>  	.get_rxfh_fields	= igb_get_rxfh_fields,
>  	.set_rxfh_fields	= igb_set_rxfh_fields,
> --
> 2.52.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Re: [Intel-wired-lan] [PATCH iwl-next v2 1/3] igb: prepare for RSS key get/set support

[Takashi Kozu]

> From: Paul Menzel <pmenzel@molgen.mpg.de>
> To: Takashi Kozu <takkozu@amazon.com>
> Cc: anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
> andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
> kuba@kernel.org, pabeni@redhat.com,
> intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
> Kohei Enju <enjuk@amazon.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 1/3] igb: prepare for RSS key get/set support
> Date: Thu, 8 Jan 2026 13:27:57 +0100 [thread overview]
> Message-ID: <2ad8d26a-794c-498b-a09b-5791acb0a9d5@molgen.mpg.de> (raw)
> In-Reply-To: <20260108052020.84218-6-takkozu@amazon.com>
> 
> Dear Takashi,
> 
> 
> Thank you for the patch.
> 
> Am 08.01.26 um 06:20 schrieb Takashi Kozu:
> > Store the RSS key inside struct igb_adapter and introduce the
> > igb_write_rss_key() helper function. This allows the driver to program
> > the E1000 registers using a persistent RSS key, instead of using a
> > stack-local buffer in igb_setup_mrqc().
> >
> > Tested-by: Kohei Enju <enjuk@amazon.com>
> > Signed-off-by: Takashi Kozu <takkozu@amazon.com>
> > ---
> > drivers/net/ethernet/intel/igb/igb.h | 3 +++
> > drivers/net/ethernet/intel/igb/igb_ethtool.c | 12 ++++++++++++
> > drivers/net/ethernet/intel/igb/igb_main.c | 6 ++----
> > 3 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
> > index 0fff1df81b7b..8c9b02058cec 100644
> > --- a/drivers/net/ethernet/intel/igb/igb.h
> > +++ b/drivers/net/ethernet/intel/igb/igb.h
> > @@ -495,6 +495,7 @@ struct hwmon_buff {
> > #define IGB_N_PEROUT 2
> > #define IGB_N_SDP 4
> > #define IGB_RETA_SIZE 128
> > +#define IGB_RSS_KEY_SIZE 40
> >
> > enum igb_filter_match_flags {
> > IGB_FILTER_FLAG_ETHER_TYPE = 0x1,
> > @@ -655,6 +656,7 @@ struct igb_adapter {
> > struct i2c_client *i2c_client;
> > u32 rss_indir_tbl_init;
> > u8 rss_indir_tbl[IGB_RETA_SIZE];
> > + u8 rss_key[IGB_RSS_KEY_SIZE];
> >
> > unsigned long link_check_timeout;
> > int copper_tries;
> > @@ -735,6 +737,7 @@ void igb_down(struct igb_adapter *);
> > void igb_reinit_locked(struct igb_adapter *);
> > void igb_reset(struct igb_adapter *);
> > int igb_reinit_queues(struct igb_adapter *);
> > +void igb_write_rss_key(struct igb_adapter *adapter);
> > void igb_write_rss_indir_tbl(struct igb_adapter *);
> > int igb_set_spd_dplx(struct igb_adapter *, u32, u8);
> > int igb_setup_tx_resources(struct igb_ring *);
> > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > index 10e2445e0ded..8695ff28a7b8 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> > @@ -3016,6 +3016,18 @@ static int igb_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
> > return ret;
> > }
> >
> > +void igb_write_rss_key(struct igb_adapter *adapter)
> > +{
> > + struct e1000_hw *hw = &adapter->hw;
> > + u32 val;
> > + int i;
> > +
> > + for (i = 0; i < IGB_RSS_KEY_SIZE / 4; i++) {
> > + val = get_unaligned_le32(&adapter->rss_key[i info.plist 4]);
> 
> Why is `get_unaligned_le32()` needed?

I think it's necessary in order to align to 4 bytes.

Re: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh

[Takashi Kozu]

> -----Original Message-----
> From: "Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>
> To: "Loktionov, Aleksandr" <aleksandr.loktionov@intel.com>,
> Kohei Enju <enjuk@amazon.com>
> Cc: "andrew+netdev@lunn.ch" <andrew+netdev@lunn.ch>,
> "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
> "davem@davemloft.net" <davem@davemloft.net>,
> "edumazet@google.com" <edumazet@google.com>,
> "intel-wired-lan@lists.osuosl.org"
> <intel-wired-lan@lists.osuosl.org>,
> "kuba@kernel.org" <kuba@kernel.org>,
> "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
> "pabeni@redhat.com" <pabeni@redhat.com>,
> "Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
> "takkozu@amazon.com" <takkozu@amazon.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow configuring RSS key via ethtool set_rxfh
> Date: Thu, 8 Jan 2026 13:03:12 +0000 [thread overview]
> Message-ID: <IA3PR11MB898612B0CDA9C5A5448733EEE585A@IA3PR11MB8986.namprd11.prod.outlook.com> (raw)
> In-Reply-To: <IA3PR11MB89865D0189D37BB3393B57F5E585A@IA3PR11MB8986.namprd11.prod.outlook.com>
> 
> 
> 
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> > Of Loktionov, Aleksandr
> > Sent: Thursday, January 8, 2026 1:28 PM
> > To: Kohei Enju <enjuk@amazon.com>
> > Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L
> > <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> > edumazet@google.com; intel-wired-lan@lists.osuosl.org;
> > kuba@kernel.org; netdev@vger.kernel.org; pabeni@redhat.com; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@intel.com>; takkozu@amazon.com
> > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb: allow
> > configuring RSS key via ethtool set_rxfh
> >
> >
> >
> > > -----Original Message-----
> > > From: Kohei Enju <enjuk@amazon.com>
> > > Sent: Thursday, January 8, 2026 1:04 PM
> > > To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> > > Cc: andrew+netdev@lunn.ch; Nguyen, Anthony L
> > > <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> > > edumazet@google.com; enjuk@amazon.com; intel-wired-
> > > lan@lists.osuosl.org; kuba@kernel.org; netdev@vger.kernel.org;
> > > pabeni@redhat.com; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@intel.com>;
> > > takkozu@amazon.com
> > > Subject: Re: RE: [Intel-wired-lan] [PATCH iwl-next v2 3/3] igb:
> > allow
> > > configuring RSS key via ethtool set_rxfh
> > >
> > > On Thu, 8 Jan 2026 07:29:19 +0000, Loktionov, Aleksandr wrote:
> > >
> > > >>
> > > >> - igb_write_rss_indir_tbl(adapter);
> > > >> + if (rxfh->key) {
> > > >> + adapter->has_user_rss_key = true;
> > > >> + memcpy(adapter->rss_key, rxfh->key, sizeof(adapter-
> > > >> >rss_key));
> > > >> + igb_write_rss_key(adapter);
> > > >It leads to race between ethtool RSS update and concurrent resets.
> > > >Because igb_setup_mrqc() (called during resets) also calls
> > > igb_write_rss_key(adapter).
> > > >Non-fatal but breaks RSS configuration guarantees.
> > >
> > > At my first glance, rtnl lock serializes those operation, so it
> > > doesn't seem to be racy as long as they are under the rtnl lock.
> > >
> > > As far as I skimmed the codes, functions such as igb_open()/
> > > igb_up()/igb_reset_task(), which finally call igb_write_rss_key()
> > are
> > > serialized by rtnl lock or serializes igb_write_rss_key() call by
> > > locking rtnl.
> > >
> > > Please let me know if I'm missing something and it's truly racy.
> > I think you're right, and I've missed that missing rtnl_lock was added
> > in upstream.
> >
> > Thank you for clarification
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> >
> 
> Afterthought, I think it could be nice to place ASSERT_RTNL() to show it explicitly.
> What do you think about this idea?

Sorry for the late reply. 
I think it's a good idea. I will add ASSERT_RTNL().