Re: [PATCH net-next v7 11/13] net: mana: keep a user-configured RSS table across a queue rebuild

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: mana: State desynchronization on silent RSS table reset leaves queues unutilized
--

commit f7f07913c3b72971a54af27655b2426b066f266e
Author: Long Li <longli@microsoft.com>

net: mana: keep a user-configured RSS table across a queue rebuild

Preserve a user RSS table whenever all entries fit the requested queue
count. Regenerate driver defaults. On growth, a retained user table does
not steer RSS traffic to the added queues until the user updates it.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 2aa18cb10d902..7cf7ce9bfa28c 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c

[ ... ]

> @@ -3936,7 +3958,9 @@ int mana_alloc_queues(struct net_device *ndev)
>  		goto destroy_rxq;
>  	}
>  
> -	mana_rss_table_init(apc);
> +	/* Loss notification needs a netdev instance lock we may lack. */
> +	if (!mana_rss_table_keep(apc, apc->num_queues, &indir_lost))
> +		mana_rss_table_init(apc);

[Severity: Medium]
Does this silent reset desynchronize the driver from the ethtool core?

By silently reinitializing the table in mana_alloc_queues() without calling
ethtool_rxfh_indir_lost(), it leaves the ethtool core's
dev->ethtool->rss_indir_user_size > 0. If a user subsequently increases the
queue count, the ethtool core will falsely validate the small table, and
the newly added Rx queues will receive no RSS traffic.

The comment mentions lacking a netdev instance lock. However, callers of
mana_alloc_queues() like mana_open() and mana_attach() hold the RTNL lock,
which satisfies the locking requirements of ethtool_rxfh_indir_lost():

net/ethtool/common.c:ethtool_rxfh_indir_lost() {
	WARN_ONCE(!rtnl_is_locked() &&
		  !lockdep_is_held_type(&dev->ethtool->rss_lock, -1),
		  "RSS context lock assertion failed\n");
    ...
}

Can we safely call ethtool_rxfh_indir_lost() here to keep the states
synchronized?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791710735.git.weh@linux.microsoft.com?part=11