> 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 <[email protected]> > 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?
This is a pre-existing upstream behavior. Patch 11/13 adds mana_rss_table_keep() and fixes it. Long
