Thread (31 messages) 31 messages, 3 authors, 20d ago

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

From: Long Li <longli@microsoft.com>
Date: 2026-08-14 18:24:35
Also in: sashiko-reviews

quoted hunk ↗ jump to hunk
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 [off-list ref]
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.
quoted
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
[ ... ]
quoted
@@ -659,42 +659,88 @@ static int mana_set_channels(struct net_device
*ndev,
quoted
 	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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help