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 for Android: free password hash cracker in your pocket

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

Message-ID: <IA3PR11MB89865D0189D37BB3393B57F5E585A@IA3PR11MB8986.namprd11.prod.outlook.com>
Date: Thu, 8 Jan 2026 12:28:07 +0000
From: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>
To: Kohei Enju <enjuk@...zon.com>
CC: "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"kuba@...nel.org" <kuba@...nel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "pabeni@...hat.com" <pabeni@...hat.com>, "Kitszel,
 Przemyslaw" <przemyslaw.kitszel@...el.com>, "takkozu@...zon.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



> -----Original Message-----
> From: Kohei Enju <enjuk@...zon.com>
> Sent: Thursday, January 8, 2026 1:04 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@...el.com>
> Cc: andrew+netdev@...n.ch; Nguyen, Anthony L
> <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; Kitszel, Przemyslaw <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 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@...el.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?