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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260108120400.75859-1-enjuk@amazon.com>
Date: Thu, 8 Jan 2026 21:03:58 +0900
From: Kohei Enju <enjuk@...zon.com>
To: <aleksandr.loktionov@...el.com>
CC: <andrew+netdev@...n.ch>, <anthony.l.nguyen@...el.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <enjuk@...zon.com>,
	<intel-wired-lan@...ts.osuosl.org>, <kuba@...nel.org>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<przemyslaw.kitszel@...el.com>, <takkozu@...zon.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 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?