> -----Original Message-----
> From: Kohei Enju <[email protected]>
> Sent: Thursday, January 8, 2026 1:04 PM
> To: Loktionov, Aleksandr <[email protected]>
> Cc: [email protected]; Nguyen, Anthony L
> <[email protected]>; [email protected];
> [email protected]; [email protected]; intel-wired-
> [email protected]; [email protected]; [email protected];
> [email protected]; Kitszel, Przemyslaw <[email protected]>;
> [email protected]
> 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 <[email protected]>
>
> >
> >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?
