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: [email protected]
>
> Signed-off-by: Anatoly Burakov <[email protected]>
> ---
> 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
>