public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Tony Nguyen <anthony.l.nguyen@intel.com>
To: Kohei Enju <kohei@enjuk.jp>, <takkozu@amazon.com>
Cc: <aleksandr.loktionov@intel.com>, <andrew+netdev@lunn.ch>,
	<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>,
	<piotr.kwapulinski@intel.com>, <pmenzel@molgen.mpg.de>,
	<przemyslaw.kitszel@intel.com>
Subject: Re: [PATCH iwl-next v4 3/3] igb: allow configuring RSS key via ethtool set_rxfh
Date: Tue, 27 Jan 2026 14:33:24 -0800	[thread overview]
Message-ID: <f521fae5-54d7-4a8a-a190-80e29b6275d8@intel.com> (raw)
In-Reply-To: <20260125131327.4469-1-kohei@enjuk.jp>



On 1/25/2026 5:12 AM, Kohei Enju wrote:
> On Tue, 20 Jan 2026 18:34:40 +0900, Takashi Kozu wrote:
> 
>> Change igc_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>`.
>>
>> 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;
> 
> Hi Kozu-san.
> 
> While preparing for testing, I noticed that now 'has_user_rss_key' is
> not necessary.
> 
> Since netdev_rss_key_fill() is called in igb_sw_init() and igb_sw_init()
> is called only once for the adapter's lifetime, adapter->rss_key
> wouldn't be changed except for user-intended change.
> 
> I'd drop that flag and related code (see below)

Hi Kohei,

I believe this igb implementation was based on your igc implementation 
which also has the same. Would it be possible for you to update the igc 
to do this as well?

Thanks,
Tony

>>   
>>   	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 b387121f0ea7..1db9c2447c91 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
>> @@ -3357,35 +3357,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;
> 
> here
> 
>> +		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 c703011b19ec..8dab133296ca 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -4051,7 +4051,8 @@ static int igb_sw_init(struct igb_adapter *adapter)
>>   	pci_read_config_word(pdev, PCI_COMMAND, &hw->bus.pci_cmd_word);
>>   
>>   	/* init RSS key */
>> -	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));
> 
> and this diff.
> 
>>   
>>   	/* set default ring sizes */
>>   	adapter->tx_ring_count = IGB_DEFAULT_TXD;
>> -- 
>> 2.52.0
> 


  reply	other threads:[~2026-01-27 22:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-20  9:34 [PATCH iwl-next v4 0/3] igb: add RSS key get/set support Takashi Kozu
2026-01-20  9:34 ` [PATCH iwl-next v4 1/3] igb: prepare for " Takashi Kozu
2026-01-20  9:36   ` Loktionov, Aleksandr
2026-01-20 15:26   ` Kwapulinski, Piotr
2026-01-20  9:34 ` [PATCH iwl-next v4 2/3] igb: expose RSS key via ethtool get_rxfh Takashi Kozu
2026-01-20  9:34 ` [PATCH iwl-next v4 3/3] igb: allow configuring RSS key via ethtool set_rxfh Takashi Kozu
2026-01-25 13:12   ` Kohei Enju
2026-01-27 22:33     ` Tony Nguyen [this message]
2026-01-28  6:09       ` [PATCH iwl-next v4 3/3] igb: allow configuring RSS key via Kohei Enju
2026-01-28 17:33         ` Tony Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f521fae5-54d7-4a8a-a190-80e29b6275d8@intel.com \
    --to=anthony.l.nguyen@intel.com \
    --cc=aleksandr.loktionov@intel.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=enjuk@amazon.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kohei@enjuk.jp \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=piotr.kwapulinski@intel.com \
    --cc=pmenzel@molgen.mpg.de \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=takkozu@amazon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox