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

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Hardware Resource Exhaustion (MSI-X vectors) / Functional Regression
- [High] Ad-hoc Synchronization with Flags

Pre-existing issues:
- [High] State Loss (RSS Indirection Table)
--

--- Patch [3]: [PATCH net-next v2 03/13] net: mana: swap queue sets in mana_set_ringparam ---

commit e63909b90b30b5041af0319613913c9654a4cc80
Author: Long Li [off-list ref]

net: mana: swap queue sets in mana_set_ringparam

Convert the ring size path to pre-allocate and swap, for the same reasons
as the channel count path. An allocation failure returns the error with
the running configuration untouched, and the vport is never released.

[ ... ]

[ ... ]

+	/* Block RDMA from acquiring the vport for the duration. The vport
+	 * itself is never released, so vport_use_count stays > 0.
+	 */
+	mutex_lock(&apc->vport_mutex);
+	if (apc->channel_changing) {
+		mutex_unlock(&apc->vport_mutex);
+		return -EBUSY;
 	}
+	apc->channel_changing = true;
+	mutex_unlock(&apc->vport_mutex);

[Severity: High]
Does this code use apc->channel_changing as an ad-hoc lock?

Since this flag is set on entry and cleared on exit to prevent concurrent
RDMA vport access, could we use a proper synchronization primitive here instead
of a boolean flag?

 
-	apc->tx_queue_size = new_tx;
-	apc->rx_queue_size = new_rx;
+	scratch = mana_qset_scratch_alloc(apc);
+	if (!scratch) {
+		err = -ENOMEM;
+		goto clear_flag;
+	}
 
-	err = mana_attach(ndev);
+	err = mana_alloc_qset(scratch, apc->num_queues, new_rx, new_tx,
+			      apc->priv_flags, &newq);

[Severity: High]
By pre-allocating a complete new queue set alongside the existing one, does
this require twice as many MSI-X vectors simultaneously?

If the system is already fully utilizing its vectors, could mana_create_eq()
fail to acquire vectors from the MSI-X pool, causing the ring parameter
changes to reliably fail with -ENOSPC?

[Severity: High]
This is a pre-existing issue, but does this code cause a loss of the custom RSS
indirection table?

During mana_alloc_qset(), mana_rss_table_init() is called, which unconditionally
overwrites the indirection table with default round-robin values. If a user
has configured a custom RSS indirection table, will those settings be
irrevocably destroyed here?

[ ... ]

+clear_flag:
+	mutex_lock(&apc->vport_mutex);
+	apc->channel_changing = false;
+	mutex_unlock(&apc->vport_mutex);
 	return err;
 }

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