[v2,1/4] raw/ifpga/base: fix bug in IRQ functions

Message ID 1601257218-6606-2-git-send-email-tianfei.zhang@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Qi Zhang
Headers
Series raw/ifpga/base: An improvement for multi-process |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Zhang, Tianfei Sept. 28, 2020, 1:40 a.m. UTC
  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(-)
  

Comments

Xu, Rosen Sept. 29, 2020, 1:42 a.m. UTC | #1
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>
  
Zhang, Tianfei Oct. 14, 2020, 9:59 a.m. UTC | #2
> -----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
  
Qi Zhang Oct. 15, 2020, 1:14 p.m. UTC | #3
> -----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
  
Ferruh Yigit Oct. 15, 2020, 6:56 p.m. UTC | #4
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.
  
Zhang, Tianfei Oct. 16, 2020, 5:46 a.m. UTC | #5
> -----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.
  

Patch

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;