[dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[Jeff Guo]

udev could not detect remove and add event of device when hotplug in
and out devices, that related with the fix about using pointer of
rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
that would result igb uio irq failure when kernel version after than 3.17.
so this patch correct it by use kernel version check before handle the
pci interrupt.

Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 9f0fb5e..14ce2ea 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -192,9 +192,14 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 static irqreturn_t
 igbuio_pci_irqhandler(int irq, void *dev_id)
 {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
 	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
 	struct uio_info *info = &udev->info;
-
+#else
+	struct uio_device *idev = (struct uio_device *)dev_id;
+	struct uio_info *info = idev->info;
+	struct rte_uio_pci_dev *udev = info->priv;
+#endif
 	/* Legacy mode need to mask in hardware */
 	if (udev->mode == RTE_INTR_MODE_LEGACY &&
 	    !pci_check_and_mask_intx(udev->pdev))
@@ -279,9 +284,15 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 	}
 
 	if (udev->info.irq != UIO_IRQ_NONE)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
 		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
 				  udev->info.irq_flags, udev->info.name,
 				  udev);
+#else
+		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
+				  udev->info.irq_flags, udev->info.name,
+				  udev->info.uio_dev);
+#endif
 	dev_info(&udev->pdev->dev, "uio device registered with irq %lx\n",
 		 udev->info.irq);
 
@@ -292,7 +303,11 @@ static void
 igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 {
 	if (udev->info.irq) {
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
 		free_irq(udev->info.irq, udev);
+#else
+		free_irq(udev->info.irq, udev->info.uio_dev);
+#endif
 		udev->info.irq = 0;
 	}
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[Wu, Jingjing]

> -----Original Message-----
> From: Guo, Jia
> Sent: Tuesday, January 30, 2018 6:01 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Tan,
> Jianfeng <jianfeng.tan@intel.com>
> Cc: thomas@monjalon.net; dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
> Subject: [PATCH] igb_uio: fix uevent montior issue
> 
> udev could not detect remove and add event of device when hotplug in
> and out devices, that related with the fix about using pointer of
> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
> that would result igb uio irq failure when kernel version after than 3.17.
> so this patch correct it by use kernel version check before handle the
> pci interrupt.
> 
> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Reviewed-by: Jingjing Wu <jingjing.wu@intel.com>

Re: [dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[Thomas Monjalon]

30/01/2018 11:01, Jeff Guo:
> udev could not detect remove and add event of device when hotplug in
> and out devices, that related with the fix about using pointer of
> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
> that would result igb uio irq failure when kernel version after than 3.17.
> so this patch correct it by use kernel version check before handle the
> pci interrupt.
> 
> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
>  	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
>  	struct uio_info *info = &udev->info;
> -
> +#else
> +	struct uio_device *idev = (struct uio_device *)dev_id;
> +	struct uio_info *info = idev->info;
> +	struct rte_uio_pci_dev *udev = info->priv;
> +#endif

Can we avoid checking Linux version number?
This method won't work with kernel backports.

Re: [dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[Guo, Jia]

On 2/9/2018 6:01 AM, Thomas Monjalon wrote:
> 30/01/2018 11:01, Jeff Guo:
>> udev could not detect remove and add event of device when hotplug in
>> and out devices, that related with the fix about using pointer of
>> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
>> that would result igb uio irq failure when kernel version after than 3.17.
>> so this patch correct it by use kernel version check before handle the
>> pci interrupt.
>>
>> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
>>   	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
>>   	struct uio_info *info = &udev->info;
>> -
>> +#else
>> +	struct uio_device *idev = (struct uio_device *)dev_id;
>> +	struct uio_info *info = idev->info;
>> +	struct rte_uio_pci_dev *udev = info->priv;
>> +#endif
> Can we avoid checking Linux version number?
> This method won't work with kernel backports.
i don't think that is a bug rather than a kernel feature parameter 
change , so i am not sure if it will definitely a chance exist for the 
kernel backport fix for that.

Re: [dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[Thomas Monjalon]

12/02/2018 09:28, Guo, Jia:
> 
> On 2/9/2018 6:01 AM, Thomas Monjalon wrote:
> > 30/01/2018 11:01, Jeff Guo:
> >> udev could not detect remove and add event of device when hotplug in
> >> and out devices, that related with the fix about using pointer of
> >> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
> >> that would result igb uio irq failure when kernel version after than 3.17.
> >> so this patch correct it by use kernel version check before handle the
> >> pci interrupt.
> >>
> >> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
> >> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> >> ---
> >> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
> >>   	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
> >>   	struct uio_info *info = &udev->info;
> >> -
> >> +#else
> >> +	struct uio_device *idev = (struct uio_device *)dev_id;
> >> +	struct uio_info *info = idev->info;
> >> +	struct rte_uio_pci_dev *udev = info->priv;
> >> +#endif
> > Can we avoid checking Linux version number?
> > This method won't work with kernel backports.
> i don't think that is a bug rather than a kernel feature parameter 
> change , so i am not sure if it will definitely a chance exist for the 
> kernel backport fix for that.

Features are also backported in some kernels.

Re: [dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[Ferruh Yigit]

On 2/12/2018 8:28 AM, Guo, Jia wrote:
> 
> 
> On 2/9/2018 6:01 AM, Thomas Monjalon wrote:
>> 30/01/2018 11:01, Jeff Guo:
>>> udev could not detect remove and add event of device when hotplug in
>>> and out devices, that related with the fix about using pointer of
>>> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
>>> that would result igb uio irq failure when kernel version after than 3.17.
>>> so this patch correct it by use kernel version check before handle the
>>> pci interrupt.
>>>
>>> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>> ---
>>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
>>>   	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
>>>   	struct uio_info *info = &udev->info;
>>> -
>>> +#else
>>> +	struct uio_device *idev = (struct uio_device *)dev_id;
>>> +	struct uio_info *info = idev->info;
>>> +	struct rte_uio_pci_dev *udev = info->priv;
>>> +#endif
>> Can we avoid checking Linux version number?
>> This method won't work with kernel backports.
> i don't think that is a bug rather than a kernel feature parameter 
> change , so i am not sure if it will definitely a chance exist for the 
> kernel backport fix for that.

Hi Jeff,

What we tend to do is add version check to compat.h and define a feature based
on that check and use feature define in .c . This makes .c file more readable
and if in the future we need to add more check because of backported kernels
(mostly we need to do) that checks all goes into compat.h and .c file remains clean.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] igb_uio: fix uevent montior issue

[Guo, Jia]

On 2/13/2018 5:32 PM, Ferruh Yigit wrote:
> On 2/12/2018 8:28 AM, Guo, Jia wrote:
>>
>> On 2/9/2018 6:01 AM, Thomas Monjalon wrote:
>>> 30/01/2018 11:01, Jeff Guo:
>>>> udev could not detect remove and add event of device when hotplug in
>>>> and out devices, that related with the fix about using pointer of
>>>> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
>>>> that would result igb uio irq failure when kernel version after than 3.17.
>>>> so this patch correct it by use kernel version check before handle the
>>>> pci interrupt.
>>>>
>>>> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
>>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>>> ---
>>>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
>>>>    	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
>>>>    	struct uio_info *info = &udev->info;
>>>> -
>>>> +#else
>>>> +	struct uio_device *idev = (struct uio_device *)dev_id;
>>>> +	struct uio_info *info = idev->info;
>>>> +	struct rte_uio_pci_dev *udev = info->priv;
>>>> +#endif
>>> Can we avoid checking Linux version number?
>>> This method won't work with kernel backports.
>> i don't think that is a bug rather than a kernel feature parameter
>> change , so i am not sure if it will definitely a chance exist for the
>> kernel backport fix for that.
> Hi Jeff,
>
> What we tend to do is add version check to compat.h and define a feature based
> on that check and use feature define in .c . This makes .c file more readable
> and if in the future we need to add more check because of backported kernels
> (mostly we need to do) that checks all goes into compat.h and .c file remains clean.
>
> Thanks,
> ferruh
make sense, let me check the compat.h file and use it in this case.

[dpdk-dev] [PATCH V2] igb_uio: fix uevent montior issue

[Jeff Guo]

udev could not detect remove and add event of device when hotplug in
and out devices, that related with the fix about using pointer of
rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
that would result igb uio irq failure when kernel version after than 3.17.

The root cause is that the older version of Linux kernel don't expose the
uio_device structure, only for the kernel version after than 3.17 use
uio_device. so this patch correct it by use a macro to check before handle
the pci interrupt.

Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v2->v1:
use macro in compat.h to replace of version check in .c file, benifit for
future backport and make more readable. 
---
 lib/librte_eal/linuxapp/igb_uio/compat.h  |  4 ++++
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
index ce456d4..2c61190 100644
--- a/lib/librte_eal/linuxapp/igb_uio/compat.h
+++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
@@ -132,3 +132,7 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 5, 0)
 #define HAVE_PCI_MSI_MASK_IRQ 1
 #endif
+
+#if LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)
+#define HAVE_UIO_DEVICE_STRUCTURE 1
+#endif
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 4cae4dd..99018f4 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -192,8 +192,14 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
 static irqreturn_t
 igbuio_pci_irqhandler(int irq, void *dev_id)
 {
+#ifndef HAVE_UIO_DEVICE_STRUCTURE
 	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
 	struct uio_info *info = &udev->info;
+#else
+	struct uio_device *idev = (struct uio_device *)dev_id;
+	struct uio_info *info = idev->info;
+	struct rte_uio_pci_dev *udev = info->priv;
+#endif
 
 	/* Legacy mode need to mask in hardware */
 	if (udev->mode == RTE_INTR_MODE_LEGACY &&
@@ -279,9 +285,15 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
 	}
 
 	if (udev->info.irq != UIO_IRQ_NONE)
+#ifndef HAVE_UIO_DEVICE_STRUCTURE
 		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
 				  udev->info.irq_flags, udev->info.name,
 				  udev);
+#else
+		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
+				  udev->info.irq_flags, udev->info.name,
+				  udev->info.uio_dev);
+#endif
 	dev_info(&udev->pdev->dev, "uio device registered with irq %ld\n",
 		 udev->info.irq);
 
@@ -292,7 +304,11 @@ static void
 igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 {
 	if (udev->info.irq) {
+#ifndef HAVE_UIO_DEVICE_STRUCTURE
 		free_irq(udev->info.irq, udev);
+#else
+		free_irq(udev->info.irq, udev->info.uio_dev);
+#endif
 		udev->info.irq = 0;
 	}
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH V2] igb_uio: fix uevent montior issue

[Ferruh Yigit]

On 2/27/2018 7:20 AM, Jeff Guo wrote:
> udev could not detect remove and add event of device when hotplug in
> and out devices, that related with the fix about using pointer of
> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
> that would result igb uio irq failure when kernel version after than 3.17.
> 
> The root cause is that the older version of Linux kernel don't expose the
> uio_device structure, only for the kernel version after than 3.17 use
> uio_device. so this patch correct it by use a macro to check before handle
> the pci interrupt.
> 
> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v2->v1:
> use macro in compat.h to replace of version check in .c file, benifit for
> future backport and make more readable. 
> ---
>  lib/librte_eal/linuxapp/igb_uio/compat.h  |  4 ++++
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
> index ce456d4..2c61190 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/compat.h
> +++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
> @@ -132,3 +132,7 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 5, 0)
>  #define HAVE_PCI_MSI_MASK_IRQ 1
>  #endif
> +
> +#if LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)
> +#define HAVE_UIO_DEVICE_STRUCTURE 1
> +#endif
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index 4cae4dd..99018f4 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -192,8 +192,14 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
>  static irqreturn_t
>  igbuio_pci_irqhandler(int irq, void *dev_id)
>  {
> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>  	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
>  	struct uio_info *info = &udev->info;
> +#else
> +	struct uio_device *idev = (struct uio_device *)dev_id;
> +	struct uio_info *info = idev->info;
> +	struct rte_uio_pci_dev *udev = info->priv;
> +#endif
>  
>  	/* Legacy mode need to mask in hardware */
>  	if (udev->mode == RTE_INTR_MODE_LEGACY &&
> @@ -279,9 +285,15 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>  	}
>  
>  	if (udev->info.irq != UIO_IRQ_NONE)
> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>  		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
>  				  udev->info.irq_flags, udev->info.name,
>  				  udev);
> +#else
> +		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
> +				  udev->info.irq_flags, udev->info.name,
> +				  udev->info.uio_dev);
> +#endif

Hi Jeff,

Can you please describe how this is solving the problem. Isn't only requirement
for dev_id to be unique? Why it differs to pass uio_dev instead of udev pointer?

>  	dev_info(&udev->pdev->dev, "uio device registered with irq %ld\n",
>  		 udev->info.irq);
>  
> @@ -292,7 +304,11 @@ static void
>  igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
>  {
>  	if (udev->info.irq) {
> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>  		free_irq(udev->info.irq, udev);
> +#else
> +		free_irq(udev->info.irq, udev->info.uio_dev);
> +#endif
>  		udev->info.irq = 0;
>  	}
>  
> 

Re: [dpdk-dev] [PATCH V2] igb_uio: fix uevent montior issue

[Guo, Jia]

On 3/27/2018 2:28 AM, Ferruh Yigit wrote:
> On 2/27/2018 7:20 AM, Jeff Guo wrote:
>> udev could not detect remove and add event of device when hotplug in
>> and out devices, that related with the fix about using pointer of
>> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
>> that would result igb uio irq failure when kernel version after than 3.17.
>>
>> The root cause is that the older version of Linux kernel don't expose the
>> uio_device structure, only for the kernel version after than 3.17 use
>> uio_device. so this patch correct it by use a macro to check before handle
>> the pci interrupt.
>>
>> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v2->v1:
>> use macro in compat.h to replace of version check in .c file, benifit for
>> future backport and make more readable.
>> ---
>>   lib/librte_eal/linuxapp/igb_uio/compat.h  |  4 ++++
>>   lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 16 ++++++++++++++++
>>   2 files changed, 20 insertions(+)
>>
>> diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
>> index ce456d4..2c61190 100644
>> --- a/lib/librte_eal/linuxapp/igb_uio/compat.h
>> +++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
>> @@ -132,3 +132,7 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
>>   #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 5, 0)
>>   #define HAVE_PCI_MSI_MASK_IRQ 1
>>   #endif
>> +
>> +#if LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)
>> +#define HAVE_UIO_DEVICE_STRUCTURE 1
>> +#endif
>> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> index 4cae4dd..99018f4 100644
>> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>> @@ -192,8 +192,14 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
>>   static irqreturn_t
>>   igbuio_pci_irqhandler(int irq, void *dev_id)
>>   {
>> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>>   	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
>>   	struct uio_info *info = &udev->info;
>> +#else
>> +	struct uio_device *idev = (struct uio_device *)dev_id;
>> +	struct uio_info *info = idev->info;
>> +	struct rte_uio_pci_dev *udev = info->priv;
>> +#endif
>>   
>>   	/* Legacy mode need to mask in hardware */
>>   	if (udev->mode == RTE_INTR_MODE_LEGACY &&
>> @@ -279,9 +285,15 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>>   	}
>>   
>>   	if (udev->info.irq != UIO_IRQ_NONE)
>> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>>   		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
>>   				  udev->info.irq_flags, udev->info.name,
>>   				  udev);
>> +#else
>> +		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
>> +				  udev->info.irq_flags, udev->info.name,
>> +				  udev->info.uio_dev);
>> +#endif
> Hi Jeff,
>
> Can you please describe how this is solving the problem. Isn't only requirement
> for dev_id to be unique? Why it differs to pass uio_dev instead of udev pointer?

Hi, ferruh


yes, this is because the uio_device definition is not exposed in kernel 
earlier than 3.17,  you could check the history of commit 
(6b9ed026a8704b9e5ee5da7997617ef7cc82e114), igb_uio: fix build with 
kernel <= 3.17, which fix it by

use using pointer of rte_uio_pci_dev instead.

>>   	dev_info(&udev->pdev->dev, "uio device registered with irq %ld\n",
>>   		 udev->info.irq);
>>   
>> @@ -292,7 +304,11 @@ static void
>>   igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
>>   {
>>   	if (udev->info.irq) {
>> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>>   		free_irq(udev->info.irq, udev);
>> +#else
>> +		free_irq(udev->info.irq, udev->info.uio_dev);
>> +#endif
>>   		udev->info.irq = 0;
>>   	}
>>   
>>

Re: [dpdk-dev] [PATCH V2] igb_uio: fix uevent montior issue

[Ferruh Yigit]

On 3/27/2018 8:46 AM, Guo, Jia wrote:
> 
> 
> On 3/27/2018 2:28 AM, Ferruh Yigit wrote:
>> On 2/27/2018 7:20 AM, Jeff Guo wrote:
>>> udev could not detect remove and add event of device when hotplug in
>>> and out devices, that related with the fix about using pointer of
>>> rte_uio_pci_dev as dev_id instead of uio_device for irq device handler,
>>> that would result igb uio irq failure when kernel version after than 3.17.
>>>
>>> The root cause is that the older version of Linux kernel don't expose the
>>> uio_device structure, only for the kernel version after than 3.17 use
>>> uio_device. so this patch correct it by use a macro to check before handle
>>> the pci interrupt.
>>>
>>> Fixes: 6b9ed026a870 ("igb_uio: fix build with kernel <= 3.17")
>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>> ---
>>> v2->v1:
>>> use macro in compat.h to replace of version check in .c file, benifit for
>>> future backport and make more readable. 
>>> ---
>>>  lib/librte_eal/linuxapp/igb_uio/compat.h  |  4 ++++
>>>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 16 ++++++++++++++++
>>>  2 files changed, 20 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/igb_uio/compat.h b/lib/librte_eal/linuxapp/igb_uio/compat.h
>>> index ce456d4..2c61190 100644
>>> --- a/lib/librte_eal/linuxapp/igb_uio/compat.h
>>> +++ b/lib/librte_eal/linuxapp/igb_uio/compat.h
>>> @@ -132,3 +132,7 @@ static bool pci_check_and_mask_intx(struct pci_dev *pdev)
>>>  #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 5, 0)
>>>  #define HAVE_PCI_MSI_MASK_IRQ 1
>>>  #endif
>>> +
>>> +#if LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)
>>> +#define HAVE_UIO_DEVICE_STRUCTURE 1
>>> +#endif
>>> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> index 4cae4dd..99018f4 100644
>>> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>>> @@ -192,8 +192,14 @@ igbuio_pci_irqcontrol(struct uio_info *info, s32 irq_state)
>>>  static irqreturn_t
>>>  igbuio_pci_irqhandler(int irq, void *dev_id)
>>>  {
>>> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>>>  	struct rte_uio_pci_dev *udev = (struct rte_uio_pci_dev *)dev_id;
>>>  	struct uio_info *info = &udev->info;
>>> +#else
>>> +	struct uio_device *idev = (struct uio_device *)dev_id;
>>> +	struct uio_info *info = idev->info;
>>> +	struct rte_uio_pci_dev *udev = info->priv;
>>> +#endif
>>>  
>>>  	/* Legacy mode need to mask in hardware */
>>>  	if (udev->mode == RTE_INTR_MODE_LEGACY &&
>>> @@ -279,9 +285,15 @@ igbuio_pci_enable_interrupts(struct rte_uio_pci_dev *udev)
>>>  	}
>>>  
>>>  	if (udev->info.irq != UIO_IRQ_NONE)
>>> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>>>  		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
>>>  				  udev->info.irq_flags, udev->info.name,
>>>  				  udev);
>>> +#else
>>> +		err = request_irq(udev->info.irq, igbuio_pci_irqhandler,
>>> +				  udev->info.irq_flags, udev->info.name,
>>> +				  udev->info.uio_dev);
>>> +#endif
>> Hi Jeff,
>>
>> Can you please describe how this is solving the problem. Isn't only requirement
>> for dev_id to be unique? Why it differs to pass uio_dev instead of udev pointer?
> 
> Hi, ferruh
> 
> 
> yes, this is because the uio_device definition is not exposed in kernel earlier
> than 3.17,  you could check the history of commit
> (6b9ed026a8704b9e5ee5da7997617ef7cc82e114), igb_uio: fix build with kernel <=
> 3.17, which fix it by

I get this part.

The question is how this is fixing uevent monitor issue.
Isn't the last parameter of the request_irq() is "void *" that has been passed
to handler.
Why and where it differs if that pointer is "struct uio_device" or not?

> 
> use using pointer of rte_uio_pci_dev instead.
> 
>>>  	dev_info(&udev->pdev->dev, "uio device registered with irq %ld\n",
>>>  		 udev->info.irq);
>>>  
>>> @@ -292,7 +304,11 @@ static void
>>>  igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
>>>  {
>>>  	if (udev->info.irq) {
>>> +#ifndef HAVE_UIO_DEVICE_STRUCTURE
>>>  		free_irq(udev->info.irq, udev);
>>> +#else
>>> +		free_irq(udev->info.irq, udev->info.uio_dev);
>>> +#endif
>>>  		udev->info.irq = 0;
>>>  	}
>>>  
>>>
>