[2/3] net/vhost: fix leak in interrupt handle setup

Message ID 20230309123752.2237828-3-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series net/vhost pmd fixes for Rx interrupts |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

David Marchand March 9, 2023, 12:37 p.m. UTC
  Do a systematic cleanup if any part of the interrupt handle setup fails.

Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 53 ++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 19 deletions(-)
  

Comments

Chenbo Xia March 10, 2023, 3:16 a.m. UTC | #1
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, March 9, 2023 8:38 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>; Xia,
> Chenbo <chenbo.xia@intel.com>; Hyong Youb Kim <hyonkim@cisco.com>; Harman
> Kalra <hkalra@marvell.com>
> Subject: [PATCH 2/3] net/vhost: fix leak in interrupt handle setup
> 
> Do a systematic cleanup if any part of the interrupt handle setup fails.
> 
> Fixes: d61138d4f0e2 ("drivers: remove direct access to interrupt handle")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 53 ++++++++++++++++++++-----------
>  1 file changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c
> b/drivers/net/vhost/rte_eth_vhost.c
> index 198bf4d1f4..96deb18d91 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -686,25 +686,32 @@ eth_vhost_install_intr(struct rte_eth_dev *dev)
>  	dev->intr_handle =
> rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
>  	if (dev->intr_handle == NULL) {
>  		VHOST_LOG(ERR, "Fail to allocate intr_handle\n");
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto error;
> +	}
> +	if (rte_intr_efd_counter_size_set(dev->intr_handle,
> sizeof(uint64_t))) {
> +		ret = -rte_errno;
> +		goto error;
>  	}
> -	if (rte_intr_efd_counter_size_set(dev->intr_handle,
> sizeof(uint64_t)))
> -		return -rte_errno;
> 
>  	if (rte_intr_vec_list_alloc(dev->intr_handle, NULL, nb_rxq)) {
> -		VHOST_LOG(ERR,
> -			"Failed to allocate memory for interrupt vector\n");
> -		rte_intr_instance_free(dev->intr_handle);
> -		return -ENOMEM;
> +		VHOST_LOG(ERR, "Failed to allocate memory for interrupt
> vector\n");
> +		ret = -ENOMEM;
> +		goto error;
>  	}
> 
> 
>  	VHOST_LOG(INFO, "Prepare intr vec\n");
>  	for (i = 0; i < nb_rxq; i++) {
> -		if (rte_intr_vec_list_index_set(dev->intr_handle, i,
> RTE_INTR_VEC_RXTX_OFFSET + i))
> -			return -rte_errno;
> -		if (rte_intr_efds_index_set(dev->intr_handle, i, -1))
> -			return -rte_errno;
> +		if (rte_intr_vec_list_index_set(dev->intr_handle, i,
> +				RTE_INTR_VEC_RXTX_OFFSET + i)) {
> +			ret = -rte_errno;
> +			goto error;
> +		}
> +		if (rte_intr_efds_index_set(dev->intr_handle, i, -1)) {
> +			ret = -rte_errno;
> +			goto error;
> +		}
>  		vq = dev->data->rx_queues[i];
>  		if (!vq) {
>  			VHOST_LOG(INFO, "rxq-%d not setup yet, skip!\n", i);
> @@ -729,16 +736,24 @@ eth_vhost_install_intr(struct rte_eth_dev *dev)
>  		VHOST_LOG(INFO, "Installed intr vec for rxq-%d\n", i);
>  	}
> 
> -	if (rte_intr_nb_efd_set(dev->intr_handle, nb_rxq))
> -		return -rte_errno;
> -
> -	if (rte_intr_max_intr_set(dev->intr_handle, nb_rxq + 1))
> -		return -rte_errno;
> -
> -	if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_VDEV))
> -		return -rte_errno;
> +	if (rte_intr_nb_efd_set(dev->intr_handle, nb_rxq)) {
> +		ret = -rte_errno;
> +		goto error;
> +	}
> +	if (rte_intr_max_intr_set(dev->intr_handle, nb_rxq + 1)) {
> +		ret = -rte_errno;
> +		goto error;
> +	}
> +	if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_VDEV)) {
> +		ret = -rte_errno;
> +		goto error;
> +	}
> 
>  	return 0;
> +
> +error:
> +	eth_vhost_uninstall_intr(dev);
> +	return ret;
>  }
> 
>  static void
> --
> 2.39.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  

Patch

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 198bf4d1f4..96deb18d91 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -686,25 +686,32 @@  eth_vhost_install_intr(struct rte_eth_dev *dev)
 	dev->intr_handle = rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
 	if (dev->intr_handle == NULL) {
 		VHOST_LOG(ERR, "Fail to allocate intr_handle\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto error;
+	}
+	if (rte_intr_efd_counter_size_set(dev->intr_handle, sizeof(uint64_t))) {
+		ret = -rte_errno;
+		goto error;
 	}
-	if (rte_intr_efd_counter_size_set(dev->intr_handle, sizeof(uint64_t)))
-		return -rte_errno;
 
 	if (rte_intr_vec_list_alloc(dev->intr_handle, NULL, nb_rxq)) {
-		VHOST_LOG(ERR,
-			"Failed to allocate memory for interrupt vector\n");
-		rte_intr_instance_free(dev->intr_handle);
-		return -ENOMEM;
+		VHOST_LOG(ERR, "Failed to allocate memory for interrupt vector\n");
+		ret = -ENOMEM;
+		goto error;
 	}
 
 
 	VHOST_LOG(INFO, "Prepare intr vec\n");
 	for (i = 0; i < nb_rxq; i++) {
-		if (rte_intr_vec_list_index_set(dev->intr_handle, i, RTE_INTR_VEC_RXTX_OFFSET + i))
-			return -rte_errno;
-		if (rte_intr_efds_index_set(dev->intr_handle, i, -1))
-			return -rte_errno;
+		if (rte_intr_vec_list_index_set(dev->intr_handle, i,
+				RTE_INTR_VEC_RXTX_OFFSET + i)) {
+			ret = -rte_errno;
+			goto error;
+		}
+		if (rte_intr_efds_index_set(dev->intr_handle, i, -1)) {
+			ret = -rte_errno;
+			goto error;
+		}
 		vq = dev->data->rx_queues[i];
 		if (!vq) {
 			VHOST_LOG(INFO, "rxq-%d not setup yet, skip!\n", i);
@@ -729,16 +736,24 @@  eth_vhost_install_intr(struct rte_eth_dev *dev)
 		VHOST_LOG(INFO, "Installed intr vec for rxq-%d\n", i);
 	}
 
-	if (rte_intr_nb_efd_set(dev->intr_handle, nb_rxq))
-		return -rte_errno;
-
-	if (rte_intr_max_intr_set(dev->intr_handle, nb_rxq + 1))
-		return -rte_errno;
-
-	if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_VDEV))
-		return -rte_errno;
+	if (rte_intr_nb_efd_set(dev->intr_handle, nb_rxq)) {
+		ret = -rte_errno;
+		goto error;
+	}
+	if (rte_intr_max_intr_set(dev->intr_handle, nb_rxq + 1)) {
+		ret = -rte_errno;
+		goto error;
+	}
+	if (rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_VDEV)) {
+		ret = -rte_errno;
+		goto error;
+	}
 
 	return 0;
+
+error:
+	eth_vhost_uninstall_intr(dev);
+	return ret;
 }
 
 static void