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? > + 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/[email protected]?part=2
