This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
Date: Mon Jul 6 19:36:01 2026 +0000
ibmveth: Implement incremental MQ RX queue resize
Add helpers to register/deregister individual RX queues and scale
queue count up or down while the netdev stays up. Surviving queues
keep PHYP handles, buffer pools, and IRQ state across resize.
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c
> b/drivers/net/ethernet/ibm/ibmveth.c
> index ecc472ee8f71f..ac4d89a66a8db 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
[ ... ]
> @@ -1080,6 +1132,138 @@ static void ibmveth_free_buffer_pools(struct
> ibmveth_adapter *adapter)
> +static int
> +ibmveth_alloc_single_rx_queue(struct ibmveth_adapter *adapter, int queue_idx,
> + int rxq_entries)
> +{
[ ... ]
> + for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) {
> + adapter->rx_buff_pool[queue_idx][i].size =
> + adapter->rx_buff_pool[0][i].size;
> + adapter->rx_buff_pool[queue_idx][i].buff_size =
> + adapter->rx_buff_pool[0][i].buff_size;
> + adapter->rx_buff_pool[queue_idx][i].threshold =
> + adapter->rx_buff_pool[0][i].threshold;
> + adapter->rx_buff_pool[queue_idx][i].active =
> + adapter->rx_buff_pool[0][i].active;
> + }
[Severity: Critical]
Does this loop miss initializing pool->index?
If pool->index remains 0 for all buffer pools on the new queue, when
replenishing, 0 is shifted into the correlator.
Then when receiving, ibmveth_remove_buffer_from_pool() will extract pool=0,
which fetches from the wrong pool and could corrupt the free map of pool 0,
leading to a use-after-free or leak.
[ ... ]
> @@ -1192,6 +1376,49 @@ static int ibmveth_rxq_harvest_buffer(struct
> ibmveth_adapter *adapter,
> +static int
> +ibmveth_drain_rx_queue(struct ibmveth_adapter *adapter, int queue_index)
> +{
[ ... ]
> + while (drained < limit &&
> + ibmveth_rxq_pending_buffer(adapter, queue_index)) {
> + rc = ibmveth_rxq_harvest_buffer(adapter, queue_index, true);
Simon says: I think we can consider the issue below in the context of
possible follow-up, because I believe the use of this pattern
predates this patch.
[Severity: High]
Is a read memory barrier needed between reading the toggle bit in
ibmveth_rxq_pending_buffer() and reading the correlator in
ibmveth_rxq_harvest_buffer()?
Since the hypervisor could be actively writing new descriptors while this is
called, could a weakly-ordered CPU reorder the correlator load before the
toggle bit load, causing us to recycle the wrong buffer?
[ ... ]
> @@ -1491,6 +1718,175 @@ ibmveth_deregister_single_rx_queue(struct
> ibmveth_adapter *adapter,
> +static int
> +ibmveth_resize_rx_queues_incremental(struct ibmveth_adapter *adapter,
> + int new_count, int rxq_entries)
> +{
[ ... ]
> + rc = ibmveth_enable_irq(adapter, i);
> + if (rc) {
> + netdev_err(netdev,
> + "Failed to enable IRQ for queue %d:
> %d\n",
> + i, rc);
> + ibmveth_cleanup_single_rx_interrupt(adapter, i);
> + ibmveth_deregister_single_rx_queue(adapter, i);
> + ibmveth_free_single_rx_queue(adapter, i);
> + goto cleanup_new_queues;
> + }
> +
> + napi_enable(&adapter->napi[i]);
> + }
> +
> + adapter->num_rx_queues = new_count;
[Severity: High]
What happens if an interrupt fires on the newly added queue before
adapter->num_rx_queues is updated?
If an interrupt fires between ibmveth_enable_irq() and the update of
adapter->num_rx_queues, ibmveth_interrupt() might evaluate:
WARN_ON(qindex < 0 || qindex >= adapter->num_rx_queues)
to true and return IRQ_NONE without masking the interrupt.
Could this lead to an unhandled interrupt storm?
[ ... ]
> + } else {
> + netdev_dbg(netdev, "Scale-down: removing queues %d-%d\n",
> + new_count, old_count - 1);
> +
> + for (i = new_count; i < old_count; i++)
> + napi_disable(&adapter->napi[i]);
> +
> + for (i = new_count; i < old_count; i++)
> + ibmveth_drain_rx_queue(adapter, i);
> +
> + synchronize_net();
[Severity: High]
Can disabling NAPI before disabling the hardware interrupts create an
interrupt storm window?
If the hypervisor asserts an interrupt during synchronize_net(), the hardware
IRQ handler runs. Since NAPI is disabled, it returns IRQ_HANDLED without
masking the interrupt source at the hypervisor, leading immediately to an
IRQ storm and CPU lockup.
> +
> + rc = netif_set_real_num_rx_queues(netdev, new_count);
> + if (rc) {
> + netdev_err(netdev, "Failed to set real RX queues to %d:
> %d\n",
> + new_count, rc);
> + for (i = new_count; i < old_count; i++)
> + napi_enable(&adapter->napi[i]);
> + return rc;
> + }
[Severity: High]
Is it possible for this error path to permanently stall the RX queues?
It calls napi_enable() but doesn't call ibmveth_replenish_task(). Since the
queues were completely drained of their buffers by ibmveth_drain_rx_queue(),
will the hypervisor have any buffers to place incoming packets?
Without buffers, no interrupts will fire for these queues, and NAPI will
never run again.
