patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v4 1/4] raw/ifpga/base: fix bug in IRQ functions
       [not found] <1600846213-18093-1-git-send-email-tianfei.zhang@intel.com>
@ 2020-09-23  7:30 ` Tianfei zhang
       [not found] ` <1601257218-6606-1-git-send-email-tianfei.zhang@intel.com>
       [not found] ` <1603443599-7356-1-git-send-email-tianfei.zhang@intel.com>
  2 siblings, 0 replies; 10+ messages in thread
From: Tianfei zhang @ 2020-09-23  7:30 UTC (permalink / raw)
  To: dev, wei.huang; +Cc: rosen.xu, stable, Tianfei zhang

From: Wei Huang <wei.huang@intel.com>

Using a pointer instead of using a structure and point to
ifpga_irq_handle[] in register and unregister interrupt
functions.
Treat positive return value of ifpga_unregister_msix_irq()
as sucessful.

Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 41 ++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index a50173264..374a7ff1d 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1337,17 +1337,18 @@ int
 ifpga_unregister_msix_irq(enum ifpga_irq_type type,
 		int vec_start, rte_intr_callback_fn handler, void *arg)
 {
-	struct rte_intr_handle intr_handle;
+	struct rte_intr_handle *intr_handle;
 
 	if (type == IFPGA_FME_IRQ)
-		intr_handle = ifpga_irq_handle[0];
+		intr_handle = &ifpga_irq_handle[0];
 	else if (type == IFPGA_AFU_IRQ)
-		intr_handle = ifpga_irq_handle[vec_start + 1];
+		intr_handle = &ifpga_irq_handle[vec_start + 1];
+	else
+		return 0;
 
-	rte_intr_efd_disable(&intr_handle);
+	rte_intr_efd_disable(intr_handle);
 
-	return rte_intr_callback_unregister(&intr_handle,
-			handler, arg);
+	return rte_intr_callback_unregister(intr_handle, handler, arg);
 }
 
 int
@@ -1357,7 +1358,7 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		void *arg)
 {
 	int ret;
-	struct rte_intr_handle intr_handle;
+	struct rte_intr_handle *intr_handle;
 	struct opae_adapter *adapter;
 	struct opae_manager *mgr;
 	struct opae_accelerator *acc;
@@ -1371,26 +1372,29 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		return -ENODEV;
 
 	if (type == IFPGA_FME_IRQ) {
-		intr_handle = ifpga_irq_handle[0];
+		intr_handle = &ifpga_irq_handle[0];
 		count = 1;
-	} else if (type == IFPGA_AFU_IRQ)
-		intr_handle = ifpga_irq_handle[vec_start + 1];
+	} else if (type == IFPGA_AFU_IRQ) {
+		intr_handle = &ifpga_irq_handle[vec_start + 1];
+	} else {
+		return -EINVAL;
+	}
 
-	intr_handle.type = RTE_INTR_HANDLE_VFIO_MSIX;
+	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
 
-	ret = rte_intr_efd_enable(&intr_handle, count);
+	ret = rte_intr_efd_enable(intr_handle, count);
 	if (ret)
 		return -ENODEV;
 
-	intr_handle.fd = intr_handle.efds[0];
+	intr_handle->fd = intr_handle->efds[0];
 
 	IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d, fd=%d\n",
-			name, intr_handle.vfio_dev_fd,
-			intr_handle.fd);
+			name, intr_handle->vfio_dev_fd,
+			intr_handle->fd);
 
 	if (type == IFPGA_FME_IRQ) {
 		struct fpga_fme_err_irq_set err_irq_set;
-		err_irq_set.evtfd = intr_handle.efds[0];
+		err_irq_set.evtfd = intr_handle->efds[0];
 
 		ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set);
 		if (ret)
@@ -1400,13 +1404,14 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		if (!acc)
 			return -EINVAL;
 
-		ret = opae_acc_set_irq(acc, vec_start, count, intr_handle.efds);
+		ret = opae_acc_set_irq(acc, vec_start, count,
+				intr_handle->efds);
 		if (ret)
 			return -EINVAL;
 	}
 
 	/* register interrupt handler using DPDK API */
-	ret = rte_intr_callback_register(&intr_handle,
+	ret = rte_intr_callback_register(intr_handle,
 			handler, (void *)arg);
 	if (ret)
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-stable] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
       [not found] ` <1601257218-6606-1-git-send-email-tianfei.zhang@intel.com>
@ 2020-09-28  1:40   ` Tianfei zhang
  2020-09-29  1:42     ` Xu, Rosen
  2020-10-15 18:56     ` Ferruh Yigit
  0 siblings, 2 replies; 10+ messages in thread
From: Tianfei zhang @ 2020-09-28  1:40 UTC (permalink / raw)
  To: dev, rosen.xu, wei.huang; +Cc: stable, Tianfei zhang

From: Wei Huang <wei.huang@intel.com>

Using a pointer instead of using a structure and point to
ifpga_irq_handle[] in register and unregister interrupt
functions.
Treat positive return value of ifpga_unregister_msix_irq()
as successful.

Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
v2: fix typo in commit log
---
 drivers/raw/ifpga/ifpga_rawdev.c | 41 ++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index a50173264..374a7ff1d 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1337,17 +1337,18 @@ int
 ifpga_unregister_msix_irq(enum ifpga_irq_type type,
 		int vec_start, rte_intr_callback_fn handler, void *arg)
 {
-	struct rte_intr_handle intr_handle;
+	struct rte_intr_handle *intr_handle;
 
 	if (type == IFPGA_FME_IRQ)
-		intr_handle = ifpga_irq_handle[0];
+		intr_handle = &ifpga_irq_handle[0];
 	else if (type == IFPGA_AFU_IRQ)
-		intr_handle = ifpga_irq_handle[vec_start + 1];
+		intr_handle = &ifpga_irq_handle[vec_start + 1];
+	else
+		return 0;
 
-	rte_intr_efd_disable(&intr_handle);
+	rte_intr_efd_disable(intr_handle);
 
-	return rte_intr_callback_unregister(&intr_handle,
-			handler, arg);
+	return rte_intr_callback_unregister(intr_handle, handler, arg);
 }
 
 int
@@ -1357,7 +1358,7 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		void *arg)
 {
 	int ret;
-	struct rte_intr_handle intr_handle;
+	struct rte_intr_handle *intr_handle;
 	struct opae_adapter *adapter;
 	struct opae_manager *mgr;
 	struct opae_accelerator *acc;
@@ -1371,26 +1372,29 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		return -ENODEV;
 
 	if (type == IFPGA_FME_IRQ) {
-		intr_handle = ifpga_irq_handle[0];
+		intr_handle = &ifpga_irq_handle[0];
 		count = 1;
-	} else if (type == IFPGA_AFU_IRQ)
-		intr_handle = ifpga_irq_handle[vec_start + 1];
+	} else if (type == IFPGA_AFU_IRQ) {
+		intr_handle = &ifpga_irq_handle[vec_start + 1];
+	} else {
+		return -EINVAL;
+	}
 
-	intr_handle.type = RTE_INTR_HANDLE_VFIO_MSIX;
+	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
 
-	ret = rte_intr_efd_enable(&intr_handle, count);
+	ret = rte_intr_efd_enable(intr_handle, count);
 	if (ret)
 		return -ENODEV;
 
-	intr_handle.fd = intr_handle.efds[0];
+	intr_handle->fd = intr_handle->efds[0];
 
 	IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d, fd=%d\n",
-			name, intr_handle.vfio_dev_fd,
-			intr_handle.fd);
+			name, intr_handle->vfio_dev_fd,
+			intr_handle->fd);
 
 	if (type == IFPGA_FME_IRQ) {
 		struct fpga_fme_err_irq_set err_irq_set;
-		err_irq_set.evtfd = intr_handle.efds[0];
+		err_irq_set.evtfd = intr_handle->efds[0];
 
 		ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set);
 		if (ret)
@@ -1400,13 +1404,14 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		if (!acc)
 			return -EINVAL;
 
-		ret = opae_acc_set_irq(acc, vec_start, count, intr_handle.efds);
+		ret = opae_acc_set_irq(acc, vec_start, count,
+				intr_handle->efds);
 		if (ret)
 			return -EINVAL;
 	}
 
 	/* register interrupt handler using DPDK API */
-	ret = rte_intr_callback_register(&intr_handle,
+	ret = rte_intr_callback_register(intr_handle,
 			handler, (void *)arg);
 	if (ret)
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
  2020-09-28  1:40   ` [dpdk-stable] [PATCH v2 " Tianfei zhang
@ 2020-09-29  1:42     ` Xu, Rosen
  2020-10-14  9:59       ` Zhang, Tianfei
  2020-10-15 13:14       ` [dpdk-stable] [dpdk-dev] " Zhang, Qi Z
  2020-10-15 18:56     ` Ferruh Yigit
  1 sibling, 2 replies; 10+ messages in thread
From: Xu, Rosen @ 2020-09-29  1:42 UTC (permalink / raw)
  To: Zhang, Tianfei, dev, Huang, Wei; +Cc: stable

Hi,

> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Monday, September 28, 2020 9:40
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Huang, Wei
> <wei.huang@intel.com>
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>
> Subject: [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
> 
> From: Wei Huang <wei.huang@intel.com>
> 
> Using a pointer instead of using a structure and point to ifpga_irq_handle[] in
> register and unregister interrupt functions.
> Treat positive return value of ifpga_unregister_msix_irq() as successful.
> 
> Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
> v2: fix typo in commit log
> ---
>  drivers/raw/ifpga/ifpga_rawdev.c | 41 ++++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c
> b/drivers/raw/ifpga/ifpga_rawdev.c
> index a50173264..374a7ff1d 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -1337,17 +1337,18 @@ int
>  ifpga_unregister_msix_irq(enum ifpga_irq_type type,
>  		int vec_start, rte_intr_callback_fn handler, void *arg)  {
> -	struct rte_intr_handle intr_handle;
> +	struct rte_intr_handle *intr_handle;
> 
>  	if (type == IFPGA_FME_IRQ)
> -		intr_handle = ifpga_irq_handle[0];
> +		intr_handle = &ifpga_irq_handle[0];
>  	else if (type == IFPGA_AFU_IRQ)
> -		intr_handle = ifpga_irq_handle[vec_start + 1];
> +		intr_handle = &ifpga_irq_handle[vec_start + 1];
> +	else
> +		return 0;
> 
> -	rte_intr_efd_disable(&intr_handle);
> +	rte_intr_efd_disable(intr_handle);
> 
> -	return rte_intr_callback_unregister(&intr_handle,
> -			handler, arg);
> +	return rte_intr_callback_unregister(intr_handle, handler, arg);
>  }
> 
>  int
> @@ -1357,7 +1358,7 @@ ifpga_register_msix_irq(struct rte_rawdev *dev,
> int port_id,
>  		void *arg)
>  {
>  	int ret;
> -	struct rte_intr_handle intr_handle;
> +	struct rte_intr_handle *intr_handle;
>  	struct opae_adapter *adapter;
>  	struct opae_manager *mgr;
>  	struct opae_accelerator *acc;
> @@ -1371,26 +1372,29 @@ ifpga_register_msix_irq(struct rte_rawdev *dev,
> int port_id,
>  		return -ENODEV;
> 
>  	if (type == IFPGA_FME_IRQ) {
> -		intr_handle = ifpga_irq_handle[0];
> +		intr_handle = &ifpga_irq_handle[0];
>  		count = 1;
> -	} else if (type == IFPGA_AFU_IRQ)
> -		intr_handle = ifpga_irq_handle[vec_start + 1];
> +	} else if (type == IFPGA_AFU_IRQ) {
> +		intr_handle = &ifpga_irq_handle[vec_start + 1];
> +	} else {
> +		return -EINVAL;
> +	}
> 
> -	intr_handle.type = RTE_INTR_HANDLE_VFIO_MSIX;
> +	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
> 
> -	ret = rte_intr_efd_enable(&intr_handle, count);
> +	ret = rte_intr_efd_enable(intr_handle, count);
>  	if (ret)
>  		return -ENODEV;
> 
> -	intr_handle.fd = intr_handle.efds[0];
> +	intr_handle->fd = intr_handle->efds[0];
> 
>  	IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d,
> fd=%d\n",
> -			name, intr_handle.vfio_dev_fd,
> -			intr_handle.fd);
> +			name, intr_handle->vfio_dev_fd,
> +			intr_handle->fd);
> 
>  	if (type == IFPGA_FME_IRQ) {
>  		struct fpga_fme_err_irq_set err_irq_set;
> -		err_irq_set.evtfd = intr_handle.efds[0];
> +		err_irq_set.evtfd = intr_handle->efds[0];
> 
>  		ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set);
>  		if (ret)
> @@ -1400,13 +1404,14 @@ ifpga_register_msix_irq(struct rte_rawdev *dev,
> int port_id,
>  		if (!acc)
>  			return -EINVAL;
> 
> -		ret = opae_acc_set_irq(acc, vec_start, count,
> intr_handle.efds);
> +		ret = opae_acc_set_irq(acc, vec_start, count,
> +				intr_handle->efds);
>  		if (ret)
>  			return -EINVAL;
>  	}
> 
>  	/* register interrupt handler using DPDK API */
> -	ret = rte_intr_callback_register(&intr_handle,
> +	ret = rte_intr_callback_register(intr_handle,
>  			handler, (void *)arg);
>  	if (ret)
>  		return -EINVAL;
> --
> 2.17.1

Acked-by: Rosen Xu <rosen.xu@intel.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
  2020-09-29  1:42     ` Xu, Rosen
@ 2020-10-14  9:59       ` Zhang, Tianfei
  2020-10-15 13:14       ` [dpdk-stable] [dpdk-dev] " Zhang, Qi Z
  1 sibling, 0 replies; 10+ messages in thread
From: Zhang, Tianfei @ 2020-10-14  9:59 UTC (permalink / raw)
  To: Xu, Rosen, dev, Huang, Wei, Yigit, Ferruh; +Cc: stable


> -----Original Message-----
> From: Zhang, Tianfei <tianfei.zhang@intel.com>
> Sent: Monday, September 28, 2020 9:40
> To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Huang, Wei 
> <wei.huang@intel.com>
> Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>
> Subject: [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
> 
> From: Wei Huang <wei.huang@intel.com>
> 
> Using a pointer instead of using a structure and point to 
> ifpga_irq_handle[] in register and unregister interrupt functions.
> Treat positive return value of ifpga_unregister_msix_irq() as successful.
> 
> Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> ---
> v2: fix typo in commit log
> ---
>  drivers/raw/ifpga/ifpga_rawdev.c | 41 
> ++++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/raw/ifpga/ifpga_rawdev.c
> b/drivers/raw/ifpga/ifpga_rawdev.c
> index a50173264..374a7ff1d 100644
> --- a/drivers/raw/ifpga/ifpga_rawdev.c
> +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> @@ -1337,17 +1337,18 @@ int
>  ifpga_unregister_msix_irq(enum ifpga_irq_type type,
>  		int vec_start, rte_intr_callback_fn handler, void *arg)  {
> -	struct rte_intr_handle intr_handle;
> +	struct rte_intr_handle *intr_handle;
> 
>  	if (type == IFPGA_FME_IRQ)
> -		intr_handle = ifpga_irq_handle[0];
> +		intr_handle = &ifpga_irq_handle[0];
>  	else if (type == IFPGA_AFU_IRQ)
> -		intr_handle = ifpga_irq_handle[vec_start + 1];
> +		intr_handle = &ifpga_irq_handle[vec_start + 1];
> +	else
> +		return 0;
> 
> -	rte_intr_efd_disable(&intr_handle);
> +	rte_intr_efd_disable(intr_handle);
> 
> -	return rte_intr_callback_unregister(&intr_handle,
> -			handler, arg);
> +	return rte_intr_callback_unregister(intr_handle, handler, arg);
>  }
> 
>  int
> @@ -1357,7 +1358,7 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, 
> int port_id,
>  		void *arg)
>  {
>  	int ret;
> -	struct rte_intr_handle intr_handle;
> +	struct rte_intr_handle *intr_handle;
>  	struct opae_adapter *adapter;
>  	struct opae_manager *mgr;
>  	struct opae_accelerator *acc;
> @@ -1371,26 +1372,29 @@ ifpga_register_msix_irq(struct rte_rawdev 
> *dev, int port_id,
>  		return -ENODEV;
> 
>  	if (type == IFPGA_FME_IRQ) {
> -		intr_handle = ifpga_irq_handle[0];
> +		intr_handle = &ifpga_irq_handle[0];
>  		count = 1;
> -	} else if (type == IFPGA_AFU_IRQ)
> -		intr_handle = ifpga_irq_handle[vec_start + 1];
> +	} else if (type == IFPGA_AFU_IRQ) {
> +		intr_handle = &ifpga_irq_handle[vec_start + 1];
> +	} else {
> +		return -EINVAL;
> +	}
> 
> -	intr_handle.type = RTE_INTR_HANDLE_VFIO_MSIX;
> +	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
> 
> -	ret = rte_intr_efd_enable(&intr_handle, count);
> +	ret = rte_intr_efd_enable(intr_handle, count);
>  	if (ret)
>  		return -ENODEV;
> 
> -	intr_handle.fd = intr_handle.efds[0];
> +	intr_handle->fd = intr_handle->efds[0];
> 
>  	IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d, fd=%d\n",
> -			name, intr_handle.vfio_dev_fd,
> -			intr_handle.fd);
> +			name, intr_handle->vfio_dev_fd,
> +			intr_handle->fd);
> 
>  	if (type == IFPGA_FME_IRQ) {
>  		struct fpga_fme_err_irq_set err_irq_set;
> -		err_irq_set.evtfd = intr_handle.efds[0];
> +		err_irq_set.evtfd = intr_handle->efds[0];
> 
>  		ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set);
>  		if (ret)
> @@ -1400,13 +1404,14 @@ ifpga_register_msix_irq(struct rte_rawdev 
> *dev, int port_id,
>  		if (!acc)
>  			return -EINVAL;
> 
> -		ret = opae_acc_set_irq(acc, vec_start, count,
> intr_handle.efds);
> +		ret = opae_acc_set_irq(acc, vec_start, count,
> +				intr_handle->efds);
>  		if (ret)
>  			return -EINVAL;
>  	}
> 
>  	/* register interrupt handler using DPDK API */
> -	ret = rte_intr_callback_register(&intr_handle,
> +	ret = rte_intr_callback_register(intr_handle,
>  			handler, (void *)arg);
>  	if (ret)
>  		return -EINVAL;
> --
> 2.17.1

>  Acked-by: Rosen Xu <rosen.xu@intel.com>

Hi Ferruh,

Would you like help to review those 4 small patches? Any comments? 

Best
Tianfei

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
  2020-09-29  1:42     ` Xu, Rosen
  2020-10-14  9:59       ` Zhang, Tianfei
@ 2020-10-15 13:14       ` Zhang, Qi Z
  1 sibling, 0 replies; 10+ messages in thread
From: Zhang, Qi Z @ 2020-10-15 13:14 UTC (permalink / raw)
  To: Xu, Rosen, Zhang, Tianfei, dev, Huang, Wei; +Cc: stable



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Xu, Rosen
> Sent: Tuesday, September 29, 2020 9:42 AM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>; dev@dpdk.org; Huang, Wei
> <wei.huang@intel.com>
> Cc: stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
> 
> Hi,
> 
> > -----Original Message-----
> > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Sent: Monday, September 28, 2020 9:40
> > To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Huang, Wei
> > <wei.huang@intel.com>
> > Cc: stable@dpdk.org; Zhang, Tianfei <tianfei.zhang@intel.com>
> > Subject: [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
> >
> > From: Wei Huang <wei.huang@intel.com>
> >
> > Using a pointer instead of using a structure and point to
> > ifpga_irq_handle[] in register and unregister interrupt functions.
> > Treat positive return value of ifpga_unregister_msix_irq() as successful.
> >
> > Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Huang <wei.huang@intel.com>
> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> > ---
> > v2: fix typo in commit log
> > ---
> >  drivers/raw/ifpga/ifpga_rawdev.c | 41
> > ++++++++++++++++++--------------
> >  1 file changed, 23 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/raw/ifpga/ifpga_rawdev.c
> > b/drivers/raw/ifpga/ifpga_rawdev.c
> > index a50173264..374a7ff1d 100644
> > --- a/drivers/raw/ifpga/ifpga_rawdev.c
> > +++ b/drivers/raw/ifpga/ifpga_rawdev.c
> > @@ -1337,17 +1337,18 @@ int
> >  ifpga_unregister_msix_irq(enum ifpga_irq_type type,
> >  		int vec_start, rte_intr_callback_fn handler, void *arg)  {
> > -	struct rte_intr_handle intr_handle;
> > +	struct rte_intr_handle *intr_handle;
> >
> >  	if (type == IFPGA_FME_IRQ)
> > -		intr_handle = ifpga_irq_handle[0];
> > +		intr_handle = &ifpga_irq_handle[0];
> >  	else if (type == IFPGA_AFU_IRQ)
> > -		intr_handle = ifpga_irq_handle[vec_start + 1];
> > +		intr_handle = &ifpga_irq_handle[vec_start + 1];
> > +	else
> > +		return 0;
> >
> > -	rte_intr_efd_disable(&intr_handle);
> > +	rte_intr_efd_disable(intr_handle);
> >
> > -	return rte_intr_callback_unregister(&intr_handle,
> > -			handler, arg);
> > +	return rte_intr_callback_unregister(intr_handle, handler, arg);
> >  }
> >
> >  int
> > @@ -1357,7 +1358,7 @@ ifpga_register_msix_irq(struct rte_rawdev *dev,
> > int port_id,
> >  		void *arg)
> >  {
> >  	int ret;
> > -	struct rte_intr_handle intr_handle;
> > +	struct rte_intr_handle *intr_handle;
> >  	struct opae_adapter *adapter;
> >  	struct opae_manager *mgr;
> >  	struct opae_accelerator *acc;
> > @@ -1371,26 +1372,29 @@ ifpga_register_msix_irq(struct rte_rawdev
> > *dev, int port_id,
> >  		return -ENODEV;
> >
> >  	if (type == IFPGA_FME_IRQ) {
> > -		intr_handle = ifpga_irq_handle[0];
> > +		intr_handle = &ifpga_irq_handle[0];
> >  		count = 1;
> > -	} else if (type == IFPGA_AFU_IRQ)
> > -		intr_handle = ifpga_irq_handle[vec_start + 1];
> > +	} else if (type == IFPGA_AFU_IRQ) {
> > +		intr_handle = &ifpga_irq_handle[vec_start + 1];
> > +	} else {
> > +		return -EINVAL;
> > +	}
> >
> > -	intr_handle.type = RTE_INTR_HANDLE_VFIO_MSIX;
> > +	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
> >
> > -	ret = rte_intr_efd_enable(&intr_handle, count);
> > +	ret = rte_intr_efd_enable(intr_handle, count);
> >  	if (ret)
> >  		return -ENODEV;
> >
> > -	intr_handle.fd = intr_handle.efds[0];
> > +	intr_handle->fd = intr_handle->efds[0];
> >
> >  	IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d, fd=%d\n",
> > -			name, intr_handle.vfio_dev_fd,
> > -			intr_handle.fd);
> > +			name, intr_handle->vfio_dev_fd,
> > +			intr_handle->fd);
> >
> >  	if (type == IFPGA_FME_IRQ) {
> >  		struct fpga_fme_err_irq_set err_irq_set;
> > -		err_irq_set.evtfd = intr_handle.efds[0];
> > +		err_irq_set.evtfd = intr_handle->efds[0];
> >
> >  		ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set);
> >  		if (ret)
> > @@ -1400,13 +1404,14 @@ ifpga_register_msix_irq(struct rte_rawdev
> > *dev, int port_id,
> >  		if (!acc)
> >  			return -EINVAL;
> >
> > -		ret = opae_acc_set_irq(acc, vec_start, count,
> > intr_handle.efds);
> > +		ret = opae_acc_set_irq(acc, vec_start, count,
> > +				intr_handle->efds);
> >  		if (ret)
> >  			return -EINVAL;
> >  	}
> >
> >  	/* register interrupt handler using DPDK API */
> > -	ret = rte_intr_callback_register(&intr_handle,
> > +	ret = rte_intr_callback_register(intr_handle,
> >  			handler, (void *)arg);
> >  	if (ret)
> >  		return -EINVAL;
> > --
> > 2.17.1
> 
> Acked-by: Rosen Xu <rosen.xu@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
  2020-09-28  1:40   ` [dpdk-stable] [PATCH v2 " Tianfei zhang
  2020-09-29  1:42     ` Xu, Rosen
@ 2020-10-15 18:56     ` Ferruh Yigit
  2020-10-16  5:46       ` Zhang, Tianfei
  1 sibling, 1 reply; 10+ messages in thread
From: Ferruh Yigit @ 2020-10-15 18:56 UTC (permalink / raw)
  To: Tianfei zhang, dev, rosen.xu, wei.huang; +Cc: stable

On 9/28/2020 2:40 AM, Tianfei zhang wrote:
> From: Wei Huang <wei.huang@intel.com>
> 
> Using a pointer instead of using a structure and point to
> ifpga_irq_handle[] in register and unregister interrupt
> functions.
> Treat positive return value of ifpga_unregister_msix_irq()
> as successful.
> 
> Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Huang <wei.huang@intel.com>
> Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>

I suggest commit log as following:

     raw/ifpga/base: fix interrupt handler instance usage

     Interrupt handler copied to the local 'intr_handle' variable by value
     before passing it to IRQ functions.
     This leads IRQ functions update the local variable instead of
     'ifpga_irq_handle'.

     Instead, using 'intr_handle' local variable as pointer to
     'ifpga_irq_handle' as intended.

     Also handle unsupported interrupt type requests properly, on unsupported
     interrupt case:
     'ifpga_unregister_msix_irq()' returns success
     'ifpga_register_msix_irq()' return failure.

     Fixes: e0a1aafe2af9 ("raw/ifpga: introduce IRQ functions")
     Cc: stable@dpdk.org


The "Also" part highlights that patch addressed two different issues, for next 
time please split different fixes to the different patches.

Title "fix bug" doesn't give much information, better to give some context.


And for the following part in the original commit log:
"
Treat positive return value of ifpga_unregister_msix_irq()
as successful.
"
It is missing in the patch, I see that part is in the next patch :)

+1 to update, since 'rte_intr_callback_unregister()' can return positive, but 
perhaps better to move the change too into its own patch.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions
  2020-10-15 18:56     ` Ferruh Yigit
@ 2020-10-16  5:46       ` Zhang, Tianfei
  0 siblings, 0 replies; 10+ messages in thread
From: Zhang, Tianfei @ 2020-10-16  5:46 UTC (permalink / raw)
  To: Yigit, Ferruh, dev, Xu, Rosen, Huang, Wei; +Cc: stable



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: 2020年10月16日 2:57
> To: Zhang, Tianfei <tianfei.zhang@intel.com>; dev@dpdk.org; Xu, Rosen
> <rosen.xu@intel.com>; Huang, Wei <wei.huang@intel.com>
> Cc: stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ
> functions
> 
> On 9/28/2020 2:40 AM, Tianfei zhang wrote:
> > From: Wei Huang <wei.huang@intel.com>
> >
> > Using a pointer instead of using a structure and point to
> > ifpga_irq_handle[] in register and unregister interrupt functions.
> > Treat positive return value of ifpga_unregister_msix_irq() as
> > successful.
> >
> > Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Huang <wei.huang@intel.com>
> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> 
> I suggest commit log as following:
> 
>      raw/ifpga/base: fix interrupt handler instance usage
> 
>      Interrupt handler copied to the local 'intr_handle' variable by value
>      before passing it to IRQ functions.
>      This leads IRQ functions update the local variable instead of
>      'ifpga_irq_handle'.
> 
>      Instead, using 'intr_handle' local variable as pointer to
>      'ifpga_irq_handle' as intended.
> 
Thanks Ferruh, this commit sounds better. 

>      Also handle unsupported interrupt type requests properly, on
> unsupported
>      interrupt case:
>      'ifpga_unregister_msix_irq()' returns success
>      'ifpga_register_msix_irq()' return failure.
> 
>      Fixes: e0a1aafe2af9 ("raw/ifpga: introduce IRQ functions")
>      Cc: stable@dpdk.org
> 
> 
> The "Also" part highlights that patch addressed two different issues, for next
> time please split different fixes to the different patches.

I will split in two patches in V3.
> 
> Title "fix bug" doesn't give much information, better to give some context.
> 
> 
> And for the following part in the original commit log:
> "
> Treat positive return value of ifpga_unregister_msix_irq() as successful.
> "
> It is missing in the patch, I see that part is in the next patch :)
> 
> +1 to update, since 'rte_intr_callback_unregister()' can return
> +positive, but
> perhaps better to move the change too into its own patch.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-stable] [PATCH v3 1/5] raw/ifpga/base: fix interrupt handler instance usage
       [not found] ` <1603443599-7356-1-git-send-email-tianfei.zhang@intel.com>
@ 2020-10-23  8:59   ` Tianfei zhang
  2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 2/5] raw/ifpga/base: handle unsupported interrupt type Tianfei zhang
  2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 3/5] raw/ifpga/base: fix return of IRQ unregister properly Tianfei zhang
  2 siblings, 0 replies; 10+ messages in thread
From: Tianfei zhang @ 2020-10-23  8:59 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: Wei Huang, stable, Tianfei zhang

From: Wei Huang <wei.huang@intel.com>

Interrupt handler copied to the local 'intr_handle' variable by value
before passing it to IRQ functions.
This leads IRQ functions update the local variable instead of
'ifpga_irq_handle'.

Instead, using 'intr_handle' local variable as pointer to
'ifpga_irq_handle' as intended.

Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
v2: fix typo in commit log
v3: slit into 2 patches, one is fix the pointer variable, other is
fix the return value.
---
 drivers/raw/ifpga/ifpga_rawdev.c | 34 ++++++++++++++++----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index a50173264..76b0f8a5b 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1337,17 +1337,16 @@ int
 ifpga_unregister_msix_irq(enum ifpga_irq_type type,
 		int vec_start, rte_intr_callback_fn handler, void *arg)
 {
-	struct rte_intr_handle intr_handle;
+	struct rte_intr_handle *intr_handle;
 
 	if (type == IFPGA_FME_IRQ)
-		intr_handle = ifpga_irq_handle[0];
+		intr_handle = &ifpga_irq_handle[0];
 	else if (type == IFPGA_AFU_IRQ)
-		intr_handle = ifpga_irq_handle[vec_start + 1];
+		intr_handle = &ifpga_irq_handle[vec_start + 1];
 
-	rte_intr_efd_disable(&intr_handle);
+	rte_intr_efd_disable(intr_handle);
 
-	return rte_intr_callback_unregister(&intr_handle,
-			handler, arg);
+	return rte_intr_callback_unregister(intr_handle, handler, arg);
 }
 
 int
@@ -1357,7 +1356,7 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		void *arg)
 {
 	int ret;
-	struct rte_intr_handle intr_handle;
+	struct rte_intr_handle *intr_handle;
 	struct opae_adapter *adapter;
 	struct opae_manager *mgr;
 	struct opae_accelerator *acc;
@@ -1371,26 +1370,26 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		return -ENODEV;
 
 	if (type == IFPGA_FME_IRQ) {
-		intr_handle = ifpga_irq_handle[0];
+		intr_handle = &ifpga_irq_handle[0];
 		count = 1;
 	} else if (type == IFPGA_AFU_IRQ)
-		intr_handle = ifpga_irq_handle[vec_start + 1];
+		intr_handle = &ifpga_irq_handle[vec_start + 1];
 
-	intr_handle.type = RTE_INTR_HANDLE_VFIO_MSIX;
+	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
 
-	ret = rte_intr_efd_enable(&intr_handle, count);
+	ret = rte_intr_efd_enable(intr_handle, count);
 	if (ret)
 		return -ENODEV;
 
-	intr_handle.fd = intr_handle.efds[0];
+	intr_handle->fd = intr_handle->efds[0];
 
 	IFPGA_RAWDEV_PMD_DEBUG("register %s irq, vfio_fd=%d, fd=%d\n",
-			name, intr_handle.vfio_dev_fd,
-			intr_handle.fd);
+			name, intr_handle->vfio_dev_fd,
+			intr_handle->fd);
 
 	if (type == IFPGA_FME_IRQ) {
 		struct fpga_fme_err_irq_set err_irq_set;
-		err_irq_set.evtfd = intr_handle.efds[0];
+		err_irq_set.evtfd = intr_handle->efds[0];
 
 		ret = opae_manager_ifpga_set_err_irq(mgr, &err_irq_set);
 		if (ret)
@@ -1400,13 +1399,14 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 		if (!acc)
 			return -EINVAL;
 
-		ret = opae_acc_set_irq(acc, vec_start, count, intr_handle.efds);
+		ret = opae_acc_set_irq(acc, vec_start, count,
+				intr_handle->efds);
 		if (ret)
 			return -EINVAL;
 	}
 
 	/* register interrupt handler using DPDK API */
-	ret = rte_intr_callback_register(&intr_handle,
+	ret = rte_intr_callback_register(intr_handle,
 			handler, (void *)arg);
 	if (ret)
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-stable] [PATCH v3 2/5] raw/ifpga/base: handle unsupported interrupt type
       [not found] ` <1603443599-7356-1-git-send-email-tianfei.zhang@intel.com>
  2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 1/5] raw/ifpga/base: fix interrupt handler instance usage Tianfei zhang
@ 2020-10-23  8:59   ` Tianfei zhang
  2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 3/5] raw/ifpga/base: fix return of IRQ unregister properly Tianfei zhang
  2 siblings, 0 replies; 10+ messages in thread
From: Tianfei zhang @ 2020-10-23  8:59 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: Wei Huang, stable, Tianfei zhang

From: Wei Huang <wei.huang@intel.com>

Handle unsupported interrupt type requests properly,
on unsupported interrupt case:
'ifpga_unregister_msix_irq()' returns success,
'ifpga_register_msix_irq()' return failure.

Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 76b0f8a5b..374a7ff1d 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1343,6 +1343,8 @@ ifpga_unregister_msix_irq(enum ifpga_irq_type type,
 		intr_handle = &ifpga_irq_handle[0];
 	else if (type == IFPGA_AFU_IRQ)
 		intr_handle = &ifpga_irq_handle[vec_start + 1];
+	else
+		return 0;
 
 	rte_intr_efd_disable(intr_handle);
 
@@ -1372,8 +1374,11 @@ ifpga_register_msix_irq(struct rte_rawdev *dev, int port_id,
 	if (type == IFPGA_FME_IRQ) {
 		intr_handle = &ifpga_irq_handle[0];
 		count = 1;
-	} else if (type == IFPGA_AFU_IRQ)
+	} else if (type == IFPGA_AFU_IRQ) {
 		intr_handle = &ifpga_irq_handle[vec_start + 1];
+	} else {
+		return -EINVAL;
+	}
 
 	intr_handle->type = RTE_INTR_HANDLE_VFIO_MSIX;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-stable] [PATCH v3 3/5] raw/ifpga/base: fix return of IRQ unregister properly
       [not found] ` <1603443599-7356-1-git-send-email-tianfei.zhang@intel.com>
  2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 1/5] raw/ifpga/base: fix interrupt handler instance usage Tianfei zhang
  2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 2/5] raw/ifpga/base: handle unsupported interrupt type Tianfei zhang
@ 2020-10-23  8:59   ` Tianfei zhang
  2 siblings, 0 replies; 10+ messages in thread
From: Tianfei zhang @ 2020-10-23  8:59 UTC (permalink / raw)
  To: dev, rosen.xu, qi.z.zhang; +Cc: Wei Huang, stable, Tianfei zhang

From: Wei Huang <wei.huang@intel.com>

Since 'rte_intr_callback_unregister()' can return positive
value as success, but 'ifpga_rawdev_destroy()' handle it as
an error.

Instead, only negative return is treated as failure.

Fixes: e0a1aafe ("raw/ifpga: introduce IRQ functions")
Cc: stable@dpdk.org

Signed-off-by: Wei Huang <wei.huang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
---
 drivers/raw/ifpga/ifpga_rawdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index 374a7ff1d..04ca5032a 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -1564,7 +1564,7 @@ ifpga_rawdev_destroy(struct rte_pci_device *pci_dev)
 		return -ENODEV;
 
 	if (ifpga_unregister_msix_irq(IFPGA_FME_IRQ, 0,
-				fme_interrupt_handler, mgr))
+				fme_interrupt_handler, mgr) < 0)
 		return -EINVAL;
 
 	opae_adapter_data_free(adapter->data);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-23 10:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1600846213-18093-1-git-send-email-tianfei.zhang@intel.com>
2020-09-23  7:30 ` [dpdk-stable] [PATCH v4 1/4] raw/ifpga/base: fix bug in IRQ functions Tianfei zhang
     [not found] ` <1601257218-6606-1-git-send-email-tianfei.zhang@intel.com>
2020-09-28  1:40   ` [dpdk-stable] [PATCH v2 " Tianfei zhang
2020-09-29  1:42     ` Xu, Rosen
2020-10-14  9:59       ` Zhang, Tianfei
2020-10-15 13:14       ` [dpdk-stable] [dpdk-dev] " Zhang, Qi Z
2020-10-15 18:56     ` Ferruh Yigit
2020-10-16  5:46       ` Zhang, Tianfei
     [not found] ` <1603443599-7356-1-git-send-email-tianfei.zhang@intel.com>
2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 1/5] raw/ifpga/base: fix interrupt handler instance usage Tianfei zhang
2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 2/5] raw/ifpga/base: handle unsupported interrupt type Tianfei zhang
2020-10-23  8:59   ` [dpdk-stable] [PATCH v3 3/5] raw/ifpga/base: fix return of IRQ unregister properly Tianfei zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).