Re: [PATCH v4 04/10] net/iavf: fix memory leaks around IRQ resources

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Oct 21, 2026 at 03:25:44PM +0100, Anatoly Burakov wrote:
> `iavf_config_rx_queues_irqs()` allocates a queue-vector map and may
> enable eventfds and allocate an interrupt vector list. If queue startup
> then fails, those resources remain allocated. The queue-vector map is
> also retained after a normal stop, leaking it across start/stop cycles.
> 
> Release the resources on failed startup and free the queue-vector map
> on stop. Clean up eventfds when interrupt setup fails as well.
> 
> Fixes: f593944fc988 ("net/iavf: enable IRQ mapping configuration for large VF")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>  drivers/net/intel/iavf/iavf_ethdev.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
> index b5beaee8604..891880cf6e6 100644
> --- a/drivers/net/intel/iavf/iavf_ethdev.c
> +++ b/drivers/net/intel/iavf/iavf_ethdev.c
> @@ -863,7 +863,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
>  	if (rte_intr_cap_multiple(intr_handle) &&
>  	    dev->data->dev_conf.intr_conf.rxq) {
>  		if (rte_intr_efd_enable(intr_handle, dev->data->nb_rx_queues))
> -			return -1;
> +			goto efd_alloc_err;
>  	}

This change doesn't make sense to me. If the enable fails, we should not
need to call disable when handling that error.

Independently AI review flags this that you might want to consider:

Info
Pre-existing and not introduced by this commit: the new failure branch at iavf_ethdev.c:866 calls cleanup after rte_intr_efd_enable() fails, but that helper can create eventfds before returning an error and before recording the completed count at eal_interrupts.c:1580. The cleanup closes only the recorded eventfds at eal_interrupts.c:1623, so a partial allocation failure can still leak descriptors. Track and release partially created eventfds in the helper or cleanup path. Confidence: 9/10.

>  
>  	if (rte_intr_dp_is_en(intr_handle)) {
> @@ -871,7 +871,7 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
>  						    dev->data->nb_rx_queues)) {
>  			PMD_DRV_LOG(ERR, "Failed to allocate %d rx intr_vec",
>  				    dev->data->nb_rx_queues);
> -			return -1;
> +			goto qv_map_alloc_err;
>  		}
>  	}
>  
> @@ -981,6 +981,8 @@ static int iavf_config_rx_queues_irqs(struct rte_eth_dev *dev,
>  
>  qv_map_alloc_err:
>  	rte_intr_vec_list_free(intr_handle);
> +efd_alloc_err:
> +	rte_intr_efd_disable(intr_handle);
>  
>  	return -1;
>  }
> @@ -1034,12 +1036,15 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  	struct rte_intr_handle *intr_handle = dev->intr_handle;
>  	uint16_t num_queue_pairs;
>  	uint16_t index = 0;
> +	bool wb_on_itr;
>  
>  	PMD_INIT_FUNC_TRACE();
>  
>  	if (adapter->closed)
>  		return -1;
>  
> +	wb_on_itr = (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) != 0;
> +
>  	vf->max_pkt_len = dev->data->mtu + IAVF_ETH_OVERHEAD;
>  	vf->num_queue_pairs = RTE_MAX(dev->data->nb_rx_queues,
>  				      dev->data->nb_tx_queues);
> @@ -1089,7 +1094,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  	}
>  	/* re-enable intr again, because efd assign may change */
>  	if (dev->data->dev_conf.intr_conf.rxq != 0) {
> -		if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR)
> +		if (wb_on_itr)
>  			rte_intr_disable(intr_handle);
>  		rte_intr_enable(intr_handle);
>  	}
> @@ -1107,7 +1112,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  
>  	if (iavf_start_queues(dev) != 0) {
>  		PMD_DRV_LOG(ERR, "enable queues failed");
> -		goto error;
> +		goto irq_error;
>  	}
>  
>  	iavf_phc_sync_alarm_start(dev);
> @@ -1118,6 +1123,13 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  
>  	return 0;
>  
> +irq_error:
> +	if (!wb_on_itr && dev->data->dev_conf.intr_conf.rxq != 0)
> +		rte_intr_disable(intr_handle);
> +	rte_intr_efd_disable(intr_handle);
> +	rte_intr_vec_list_free(intr_handle);
> +	rte_free(vf->qv_map);
> +	vf->qv_map = NULL;
>  error:
>  	return -1;
>  }
> @@ -1150,6 +1162,8 @@ iavf_dev_stop(struct rte_eth_dev *dev)
>  	rte_intr_vec_list_free(intr_handle);
>  
>  	iavf_stop_queues(dev);
> +	rte_free(vf->qv_map);
> +	vf->qv_map = NULL;
>  
>  	adapter->stopped = 1;
>  	dev->data->dev_started = 0;
> -- 
> 2.52.0
>