Re: [PATCH net-next v2 02/13] net: mana: swap queue sets in mana_set_channels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mana_set_channels silently drops user-configured RSS indirection tables
--

--- Patch [2]: [PATCH net-next v2 02/13] net: mana: swap queue sets in mana_set_channels ---
commit 20d58255702f027247917ff013e18e0c18e9845c
Author: Long Li <longli@microsoft.com>
Subject: net: mana: swap queue sets in mana_set_channels

This converts the channel count path away from detach/attach. It builds
the new queue set while the current one keeps serving traffic, then swaps
it in and retires the old one.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> index 04b7a5c0fdabc..b12291555eaeb 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
[ ... ]
> @@ -659,42 +659,88 @@ static int mana_set_channels(struct net_device *ndev,
>  	apc->channel_changing = true;
>  	mutex_unlock(&apc->vport_mutex);
>  
> -	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, new_count);
> -	if (err) {
> -		netdev_err(ndev, "Insufficient memory for new allocations");
> +	scratch = mana_qset_scratch_alloc(apc);
> +	if (!scratch) {
> +		err = -ENOMEM;
>  		goto clear_flag;
>  	}
>  
> -	err = mana_detach(ndev, false);
> -	if (err) {
> -		netdev_err(ndev, "mana_detach failed: %d\n", err);
> -		goto out;
> -	}
> +	err = mana_alloc_qset(scratch, new_count, apc->rx_queue_size,
> +			      apc->tx_queue_size, apc->priv_flags, &newq);

[Severity: Medium]
Does this silently drop a user-configured RSS indirection table?

By using mana_alloc_qset() here in mana_set_channels(), a custom RSS
indirection table configured via ethtool will be lost. When mana_alloc_qset()
is called, it unconditionally initializes the table with default values in
drivers/net/ethernet/microsoft/mana/mana_en.c:

mana_alloc_qset() {
    ...
    mana_rss_table_init(scratch);
    ...
    mana_qset_snapshot(scratch, out);
}

Because out->rxfh_indir_lost is explicitly set to false, the old indirection
table is freed without preserving the entries, and no ethtool_rxfh_indir_lost()
notification is sent.

Will this cause the interface to silently revert to a default hashing
distribution after a channel count change instead of preserving it?

> +	if (err)
> +		goto free_scratch; /* current qset untouched, nothing to undo */
>  
> -	apc->num_queues = new_count;
> -	err = mana_attach(ndev);
> +	err = mana_publish_qset(apc, &newq, &oldq);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813050418.2906468-1-longli@microsoft.com?part=2