Re: [PATCH net-next v2 13/14] ixgbe: preserve RSS indirection table across admin down/up

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 22 Oct 2025 12:40:45 +0900 Kohei Enju wrote:
> On Tue, 21 Oct 2025 16:10:06 -0700, Jakub Kicinski wrote:
> >On Tue, 21 Oct 2025 12:59:34 +0900 Kohei Enju wrote:  
> >> For example, consider a scenario where the queue count is 8 with user
> >> configuration containing values from 0 to 7. When queue count changes
> >> from 8 to 4 and we skip the reinitialization in this scenario, entries
> >> pointing to queues 4-7 become invalid. The same issue applies when the
> >> RETA table size changes.  
> >
> >Core should reject this. See ethtool_check_max_channel()  
> 
> Indeed, you're right that the situation above will be rejected. I missed
> it.
> 
> BTW, I think reinitializing the RETA table when queue count changes or
> RETA table size changes is reasonable for predictability and safety.
> Does this approach make sense to you?

Yes, if !netif_is_rxfh_configured() re-initializing is expected.

> >> Furthermore, IIUC, adding netif_is_rxfh_configured() to the current
> >> condition wouldn't provide additional benefit. When parameters remain
> >> unchanged, regardless of netif_is_rxfh_configured(), we already preserve
> >> the RETA entries which might be user-configured or default values,   
> >
> >User may decide to "isolate" (take out of RSS) a lower queue,
> >to configure it for AF_XDP or other form of zero-copy. Install
> >explicit rules to direct traffic to that queue. If you reset
> >the RSS table random traffic will get stranded in the ZC queue
> >(== dropped).
> 
> You're correct about the ZC queue scenario. The original implementation
> (before this patch) would indeed cause this problem by unconditionally
> reinitializing.
> 
> I believe this patch addresses that issue - it preserves the user
> configuration since neither queue count nor RETA table size changes in
> that case. If I'm misunderstanding your scenario, please let me know.
> 
> I could update the logic to explicitly check netif_is_rxfh_configured()
> as in [1], though the actual behavior would be the same as [2] since
> the default RETA table is a deterministic function of (rss_indices,
> reta_entries):
> 
> [1] Check user configuration explicitly:
>     if (!netif_is_rxfh_configured(adapter->netdev) ||
>         adapter->last_rss_indices != rss_i ||
>         adapter->last_reta_entries != reta_entries) {
>         // reinitialize
>     }
> 
> [2] Current patch:
>     if (adapter->last_rss_indices != rss_i ||
>         adapter->last_reta_entries != reta_entries) {
>         // reinitialize
>     }
> 
> Do you have any preference between these approaches, or would you
> recommend a different solution?

I was expecting something like:

if (netif_is_rxfh_configured(adapter->netdev)) {
	if (!check_that_rss_is_okay()) {
		/* This should never happen, barring FW errors etc */
		warn("user configuration lost due to XYZ");
		reinit();
	}
} else if (...rss_ind != rss_id ||
           ...reta_entries != reta_entries) {
	reinit();
}