diff mbox

[dpdk-dev] enic: corrected the usage of VFIO_PRESENT

Message ID 1418372303-31565-1-git-send-email-ssujith@cisco.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Sujith Sankar Dec. 12, 2014, 8:18 a.m. UTC
This patch corrects the usage of the flag VFIO_PRESENT in enic driver.  
This has uncovered a few warnings, and this patch corrects those too.

Signed-off-by: Sujith Sankar <ssujith@cisco.com>
---
 lib/librte_pmd_enic/Makefile    |  1 +
 lib/librte_pmd_enic/enic.h      |  1 +
 lib/librte_pmd_enic/enic_main.c | 12 ++++++++----
 3 files changed, 10 insertions(+), 4 deletions(-)

Comments

Thomas Monjalon Dec. 15, 2014, 11:24 p.m. UTC | #1
2014-12-12 13:48, Sujith Sankar:
> This patch corrects the usage of the flag VFIO_PRESENT in enic driver.

Please, could you explain why the flag VFIO_PRESENT was not well used?

> This has uncovered a few warnings, and this patch corrects those too.
[...]
> --- a/lib/librte_pmd_enic/enic_main.c
> +++ b/lib/librte_pmd_enic/enic_main.c
> @@ -39,6 +39,7 @@
>  #include <sys/mman.h>
>  #include <fcntl.h>
>  #include <libgen.h>
> +#include <sys/ioctl.h>
>  
>  #include <rte_pci.h>
>  #include <rte_memzone.h>
> @@ -46,6 +47,7 @@
>  #include <rte_mbuf.h>
>  #include <rte_string_fns.h>
>  #include <rte_ethdev.h>
> +#include <eal_vfio.h>

This header was not designed to be included by PMDs.
It will break compilation on BSD.

>  #include "enic_compat.h"
>  #include "enic.h"
> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct rte_pci_device *hwdev,
>  	/* Nothing to be done */
>  }
>  
> +#ifndef VFIO_PRESENT
>  static void
>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
>  	void *arg)
> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
>  
>  	enic_log_q_error(enic);
>  }
> +#endif
Sujith Sankar Dec. 16, 2014, 4:12 a.m. UTC | #2
On 16/12/14 4:54 am, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:

>2014-12-12 13:48, Sujith Sankar:
>> This patch corrects the usage of the flag VFIO_PRESENT in enic driver.
>
>Please, could you explain why the flag VFIO_PRESENT was not well used?

Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
Hence VFIO specific code in enic was not getting compiled and some errors
were generated during run-time.

>
>> This has uncovered a few warnings, and this patch corrects those too.
>[...]
>> --- a/lib/librte_pmd_enic/enic_main.c
>> +++ b/lib/librte_pmd_enic/enic_main.c
>> @@ -39,6 +39,7 @@
>>  #include <sys/mman.h>
>>  #include <fcntl.h>
>>  #include <libgen.h>
>> +#include <sys/ioctl.h>
>>  
>>  #include <rte_pci.h>
>>  #include <rte_memzone.h>
>> @@ -46,6 +47,7 @@
>>  #include <rte_mbuf.h>
>>  #include <rte_string_fns.h>
>>  #include <rte_ethdev.h>
>> +#include <eal_vfio.h>
>
>This header was not designed to be included by PMDs.
>It will break compilation on BSD.

Is there an alternative to make VFIO_PRESENT available in enic?  Please
advise.

Thanks,
-Sujith

>
>>  #include "enic_compat.h"
>>  #include "enic.h"
>> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct
>>rte_pci_device *hwdev,
>>  	/* Nothing to be done */
>>  }
>>  
>> +#ifndef VFIO_PRESENT
>>  static void
>>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
>>  	void *arg)
>> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct
>>rte_intr_handle *handle,
>>  
>>  	enic_log_q_error(enic);
>>  }
>> +#endif
>
>-- 
>Thomas
Michael Qiu Dec. 16, 2014, 7:51 a.m. UTC | #3
On 12/16/2014 12:13 PM, Sujith Sankar (ssujith) wrote:
> On 16/12/14 4:54 am, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:
>
>> 2014-12-12 13:48, Sujith Sankar:
>>> This patch corrects the usage of the flag VFIO_PRESENT in enic driver.
>> Please, could you explain why the flag VFIO_PRESENT was not well used?
> Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
> Hence VFIO specific code in enic was not getting compiled and some errors
> were generated during run-time.
>
>>> This has uncovered a few warnings, and this patch corrects those too.
>> [...]
>>> --- a/lib/librte_pmd_enic/enic_main.c
>>> +++ b/lib/librte_pmd_enic/enic_main.c
>>> @@ -39,6 +39,7 @@
>>>  #include <sys/mman.h>
>>>  #include <fcntl.h>
>>>  #include <libgen.h>
>>> +#include <sys/ioctl.h>
>>>  
>>>  #include <rte_pci.h>
>>>  #include <rte_memzone.h>
>>> @@ -46,6 +47,7 @@
>>>  #include <rte_mbuf.h>
>>>  #include <rte_string_fns.h>
>>>  #include <rte_ethdev.h>
>>> +#include <eal_vfio.h>
>> This header was not designed to be included by PMDs.
>> It will break compilation on BSD.
> Is there an alternative to make VFIO_PRESENT available in enic?  Please
> advise.

You can remove  VFIO_PRESENT check, it all been done in eal, you can
check other nic pmds for reference.
And seems you done the interrupt logic all by your self?

Thanks,
Michael
>  
> Thanks,
> -Sujith
>
>>>  #include "enic_compat.h"
>>>  #include "enic.h"
>>> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct
>>> rte_pci_device *hwdev,
>>>  	/* Nothing to be done */
>>>  }
>>>  
>>> +#ifndef VFIO_PRESENT
>>>  static void
>>>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
>>>  	void *arg)
>>> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct
>>> rte_intr_handle *handle,
>>>  
>>>  	enic_log_q_error(enic);
>>>  }
>>> +#endif
>> -- 
>> Thomas
>
Sujith Sankar Dec. 16, 2014, 10 a.m. UTC | #4
On 16/12/14 1:21 pm, "Qiu, Michael" <michael.qiu@intel.com> wrote:

>On 12/16/2014 12:13 PM, Sujith Sankar (ssujith) wrote:
>> On 16/12/14 4:54 am, "Thomas Monjalon" <thomas.monjalon@6wind.com>
>>wrote:
>>
>>> 2014-12-12 13:48, Sujith Sankar:
>>>> This patch corrects the usage of the flag VFIO_PRESENT in enic driver.
>>> Please, could you explain why the flag VFIO_PRESENT was not well used?
>> Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
>> Hence VFIO specific code in enic was not getting compiled and some
>>errors
>> were generated during run-time.
>>
>>>> This has uncovered a few warnings, and this patch corrects those too.
>>> [...]
>>>> --- a/lib/librte_pmd_enic/enic_main.c
>>>> +++ b/lib/librte_pmd_enic/enic_main.c
>>>> @@ -39,6 +39,7 @@
>>>>  #include <sys/mman.h>
>>>>  #include <fcntl.h>
>>>>  #include <libgen.h>
>>>> +#include <sys/ioctl.h>
>>>>  
>>>>  #include <rte_pci.h>
>>>>  #include <rte_memzone.h>
>>>> @@ -46,6 +47,7 @@
>>>>  #include <rte_mbuf.h>
>>>>  #include <rte_string_fns.h>
>>>>  #include <rte_ethdev.h>
>>>> +#include <eal_vfio.h>
>>> This header was not designed to be included by PMDs.
>>> It will break compilation on BSD.
>> Is there an alternative to make VFIO_PRESENT available in enic?  Please
>> advise.
>
>You can remove  VFIO_PRESENT check, it all been done in eal, you can
>check other nic pmds for reference.
>And seems you done the interrupt logic all by your self?
>
>Thanks,
>Michael

Thanks for the comment, Michael.

Without the code under VFIO_PRESENT flag, I was getting false notification
of interrupt at the beginning (cat /proc/interrupts showed all 0s).
Let me try to root cause it.  I shall get back after some debugging and
testing.

There was one more reason behind doing interrupt logic in enic.  No matter
how many interrupts the user configures, enic pmd needs only one.
There is no way to communicate that to the EAL.  I thought doing interrupt
login in enic could avoid registering that many interrupts.

Thanks,
-Sujith

>>  
>> Thanks,
>> -Sujith
>>
>>>>  #include "enic_compat.h"
>>>>  #include "enic.h"
>>>> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct
>>>> rte_pci_device *hwdev,
>>>>  	/* Nothing to be done */
>>>>  }
>>>>  
>>>> +#ifndef VFIO_PRESENT
>>>>  static void
>>>>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
>>>>  	void *arg)
>>>> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct
>>>> rte_intr_handle *handle,
>>>>  
>>>>  	enic_log_q_error(enic);
>>>>  }
>>>> +#endif
>>> -- 
>>> Thomas
>>
>
Thomas Monjalon Dec. 16, 2014, 10:16 a.m. UTC | #5
2014-12-16 10:00, Sujith Sankar:
> On 16/12/14 1:21 pm, "Qiu, Michael" <michael.qiu@intel.com> wrote:
> >On 12/16/2014 12:13 PM, Sujith Sankar (ssujith) wrote:
> >> On 16/12/14 4:54 am, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:
> >>> 2014-12-12 13:48, Sujith Sankar:
> >>>> This patch corrects the usage of the flag VFIO_PRESENT in enic driver.
> >>> 
> >>> Please, could you explain why the flag VFIO_PRESENT was not well used?
> >> 
> >> Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
> >> Hence VFIO specific code in enic was not getting compiled and some errors
> >> were generated during run-time.
> >>
> >>>> This has uncovered a few warnings, and this patch corrects those too.
> >>> [...]
> >>>> --- a/lib/librte_pmd_enic/enic_main.c
> >>>> +++ b/lib/librte_pmd_enic/enic_main.c
> >>>> @@ -39,6 +39,7 @@
> >>>>  #include <sys/mman.h>
> >>>>  #include <fcntl.h>
> >>>>  #include <libgen.h>
> >>>> +#include <sys/ioctl.h>
> >>>>  
> >>>>  #include <rte_pci.h>
> >>>>  #include <rte_memzone.h>
> >>>> @@ -46,6 +47,7 @@
> >>>>  #include <rte_mbuf.h>
> >>>>  #include <rte_string_fns.h>
> >>>>  #include <rte_ethdev.h>
> >>>> +#include <eal_vfio.h>
> >>> 
> >>> This header was not designed to be included by PMDs.
> >>> It will break compilation on BSD.
> >> 
> >> Is there an alternative to make VFIO_PRESENT available in enic?  Please
> >> advise.
> >
> >You can remove  VFIO_PRESENT check, it all been done in eal, you can
> >check other nic pmds for reference.
> >And seems you done the interrupt logic all by your self?
> >
> >Thanks,
> >Michael
> 
> Thanks for the comment, Michael.
> 
> Without the code under VFIO_PRESENT flag, I was getting false notification
> of interrupt at the beginning (cat /proc/interrupts showed all 0s).
> Let me try to root cause it.  I shall get back after some debugging and
> testing.
> 
> There was one more reason behind doing interrupt logic in enic.  No matter
> how many interrupts the user configures, enic pmd needs only one.
> There is no way to communicate that to the EAL.  I thought doing interrupt
> login in enic could avoid registering that many interrupts.

If you think something is wrong or could be improved in EAL,
it's really better to patch it instead of workarounding in the PMD.

Thanks
Burakov, Anatoly Dec. 16, 2014, 10:22 a.m. UTC | #6
> On 16/12/14 4:54 am, "Thomas Monjalon" <thomas.monjalon@6wind.com>
> wrote:
> 
> >2014-12-12 13:48, Sujith Sankar:
> >> This patch corrects the usage of the flag VFIO_PRESENT in enic driver.
> >
> >Please, could you explain why the flag VFIO_PRESENT was not well used?
> 
> Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
> Hence VFIO specific code in enic was not getting compiled and some errors
> were generated during run-time.
> 
> >
> >> This has uncovered a few warnings, and this patch corrects those too.
> >[...]
> >> --- a/lib/librte_pmd_enic/enic_main.c
> >> +++ b/lib/librte_pmd_enic/enic_main.c
> >> @@ -39,6 +39,7 @@
> >>  #include <sys/mman.h>
> >>  #include <fcntl.h>
> >>  #include <libgen.h>
> >> +#include <sys/ioctl.h>
> >>
> >>  #include <rte_pci.h>
> >>  #include <rte_memzone.h>
> >> @@ -46,6 +47,7 @@
> >>  #include <rte_mbuf.h>
> >>  #include <rte_string_fns.h>
> >>  #include <rte_ethdev.h>
> >> +#include <eal_vfio.h>
> >
> >This header was not designed to be included by PMDs.
> >It will break compilation on BSD.
> 
> Is there an alternative to make VFIO_PRESENT available in enic?  Please
> advise.
> 
> Thanks,
> -Sujith
> 
> >
> >>  #include "enic_compat.h"
> >>  #include "enic.h"
> >> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct
> >>rte_pci_device *hwdev,
> >>  	/* Nothing to be done */
> >>  }
> >>
> >> +#ifndef VFIO_PRESENT
> >>  static void
> >>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
> >>  	void *arg)
> >> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct
> >>rte_intr_handle *handle,
> >>
> >>  	enic_log_q_error(enic);
> >>  }
> >> +#endif
> >
> >--
> >Thomas

Hi Sujith

Thomas is correct, VFIO code is designed to be EAL-only (mainly because it's Linuxapp-specific, and PMD's are intended to be cross-platform at least when it comes to compilation). Whatever it is that you're working around is better fixed in the EAL itself rather than in the PMD.

Thanks,
Anatoly
Sujith Sankar Dec. 16, 2014, 10:34 a.m. UTC | #7
On 16/12/14 3:52 pm, "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

>> On 16/12/14 4:54 am, "Thomas Monjalon" <thomas.monjalon@6wind.com>
>> wrote:
>> 
>> >2014-12-12 13:48, Sujith Sankar:
>> >> This patch corrects the usage of the flag VFIO_PRESENT in enic
>>driver.
>> >
>> >Please, could you explain why the flag VFIO_PRESENT was not well used?
>> 
>> Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
>> Hence VFIO specific code in enic was not getting compiled and some
>>errors
>> were generated during run-time.
>> 
>> >
>> >> This has uncovered a few warnings, and this patch corrects those too.
>> >[...]
>> >> --- a/lib/librte_pmd_enic/enic_main.c
>> >> +++ b/lib/librte_pmd_enic/enic_main.c
>> >> @@ -39,6 +39,7 @@
>> >>  #include <sys/mman.h>
>> >>  #include <fcntl.h>
>> >>  #include <libgen.h>
>> >> +#include <sys/ioctl.h>
>> >>
>> >>  #include <rte_pci.h>
>> >>  #include <rte_memzone.h>
>> >> @@ -46,6 +47,7 @@
>> >>  #include <rte_mbuf.h>
>> >>  #include <rte_string_fns.h>
>> >>  #include <rte_ethdev.h>
>> >> +#include <eal_vfio.h>
>> >
>> >This header was not designed to be included by PMDs.
>> >It will break compilation on BSD.
>> 
>> Is there an alternative to make VFIO_PRESENT available in enic?  Please
>> advise.
>> 
>> Thanks,
>> -Sujith
>> 
>> >
>> >>  #include "enic_compat.h"
>> >>  #include "enic.h"
>> >> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct
>> >>rte_pci_device *hwdev,
>> >>  	/* Nothing to be done */
>> >>  }
>> >>
>> >> +#ifndef VFIO_PRESENT
>> >>  static void
>> >>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
>> >>  	void *arg)
>> >> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct
>> >>rte_intr_handle *handle,
>> >>
>> >>  	enic_log_q_error(enic);
>> >>  }
>> >> +#endif
>> >
>> >--
>> >Thomas
>
>Hi Sujith
>
>Thomas is correct, VFIO code is designed to be EAL-only (mainly because
>it's Linuxapp-specific, and PMD's are intended to be cross-platform at
>least when it comes to compilation). Whatever it is that you're working
>around is better fixed in the EAL itself rather than in the PMD.

I agree with you and Thomas.  Let me find the root cause for the false
trigger. 

Thanks,
-Sujith

>
>Thanks,
>Anatoly
Burakov, Anatoly Dec. 16, 2014, 10:36 a.m. UTC | #8
> -----Original Message-----
> From: Sujith Sankar (ssujith) [mailto:ssujith@cisco.com]
> Sent: Tuesday, December 16, 2014 10:34 AM
> To: Burakov, Anatoly; Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
> 
> 
> 
> On 16/12/14 3:52 pm, "Burakov, Anatoly" <anatoly.burakov@intel.com>
> wrote:
> 
> >> On 16/12/14 4:54 am, "Thomas Monjalon"
> <thomas.monjalon@6wind.com>
> >> wrote:
> >>
> >> >2014-12-12 13:48, Sujith Sankar:
> >> >> This patch corrects the usage of the flag VFIO_PRESENT in enic
> >>driver.
> >> >
> >> >Please, could you explain why the flag VFIO_PRESENT was not well
> used?
> >>
> >> Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
> >> Hence VFIO specific code in enic was not getting compiled and some
> >>errors  were generated during run-time.
> >>
> >> >
> >> >> This has uncovered a few warnings, and this patch corrects those too.
> >> >[...]
> >> >> --- a/lib/librte_pmd_enic/enic_main.c
> >> >> +++ b/lib/librte_pmd_enic/enic_main.c
> >> >> @@ -39,6 +39,7 @@
> >> >>  #include <sys/mman.h>
> >> >>  #include <fcntl.h>
> >> >>  #include <libgen.h>
> >> >> +#include <sys/ioctl.h>
> >> >>
> >> >>  #include <rte_pci.h>
> >> >>  #include <rte_memzone.h>
> >> >> @@ -46,6 +47,7 @@
> >> >>  #include <rte_mbuf.h>
> >> >>  #include <rte_string_fns.h>
> >> >>  #include <rte_ethdev.h>
> >> >> +#include <eal_vfio.h>
> >> >
> >> >This header was not designed to be included by PMDs.
> >> >It will break compilation on BSD.
> >>
> >> Is there an alternative to make VFIO_PRESENT available in enic?
> >> Please advise.
> >>
> >> Thanks,
> >> -Sujith
> >>
> >> >
> >> >>  #include "enic_compat.h"
> >> >>  #include "enic.h"
> >> >> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct
> >> >>rte_pci_device *hwdev,
> >> >>  	/* Nothing to be done */
> >> >>  }
> >> >>
> >> >> +#ifndef VFIO_PRESENT
> >> >>  static void
> >> >>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
> >> >>  	void *arg)
> >> >> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct
> >> >>rte_intr_handle *handle,
> >> >>
> >> >>  	enic_log_q_error(enic);
> >> >>  }
> >> >> +#endif
> >> >
> >> >--
> >> >Thomas
> >
> >Hi Sujith
> >
> >Thomas is correct, VFIO code is designed to be EAL-only (mainly because
> >it's Linuxapp-specific, and PMD's are intended to be cross-platform at
> >least when it comes to compilation). Whatever it is that you're working
> >around is better fixed in the EAL itself rather than in the PMD.
> 
> I agree with you and Thomas.  Let me find the root cause for the false trigger.
> 
> Thanks,
> -Sujith
> 

You may find it in EAL VFIO interrupts code. When VFIO enables some interrupt types, it manually sends a trigger. Normally, this "trigger" just enables interrupts, but maybe for ENIC it's different. I therefore suggest you to look there first.

Thanks,
Anatoly
Sujith Sankar Dec. 16, 2014, 10:40 a.m. UTC | #9
On 16/12/14 4:06 pm, "Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

>> -----Original Message-----
>> From: Sujith Sankar (ssujith) [mailto:ssujith@cisco.com]
>> Sent: Tuesday, December 16, 2014 10:34 AM
>> To: Burakov, Anatoly; Thomas Monjalon
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] enic: corrected the usage of
>>VFIO_PRESENT
>> 
>> 
>> 
>> On 16/12/14 3:52 pm, "Burakov, Anatoly" <anatoly.burakov@intel.com>
>> wrote:
>> 
>> >> On 16/12/14 4:54 am, "Thomas Monjalon"
>> <thomas.monjalon@6wind.com>
>> >> wrote:
>> >>
>> >> >2014-12-12 13:48, Sujith Sankar:
>> >> >> This patch corrects the usage of the flag VFIO_PRESENT in enic
>> >>driver.
>> >> >
>> >> >Please, could you explain why the flag VFIO_PRESENT was not well
>> used?
>> >>
>> >> Without including eal_vfio.h, VFIO_PRESENT is not available in enic.
>> >> Hence VFIO specific code in enic was not getting compiled and some
>> >>errors  were generated during run-time.
>> >>
>> >> >
>> >> >> This has uncovered a few warnings, and this patch corrects those
>>too.
>> >> >[...]
>> >> >> --- a/lib/librte_pmd_enic/enic_main.c
>> >> >> +++ b/lib/librte_pmd_enic/enic_main.c
>> >> >> @@ -39,6 +39,7 @@
>> >> >>  #include <sys/mman.h>
>> >> >>  #include <fcntl.h>
>> >> >>  #include <libgen.h>
>> >> >> +#include <sys/ioctl.h>
>> >> >>
>> >> >>  #include <rte_pci.h>
>> >> >>  #include <rte_memzone.h>
>> >> >> @@ -46,6 +47,7 @@
>> >> >>  #include <rte_mbuf.h>
>> >> >>  #include <rte_string_fns.h>
>> >> >>  #include <rte_ethdev.h>
>> >> >> +#include <eal_vfio.h>
>> >> >
>> >> >This header was not designed to be included by PMDs.
>> >> >It will break compilation on BSD.
>> >>
>> >> Is there an alternative to make VFIO_PRESENT available in enic?
>> >> Please advise.
>> >>
>> >> Thanks,
>> >> -Sujith
>> >>
>> >> >
>> >> >>  #include "enic_compat.h"
>> >> >>  #include "enic.h"
>> >> >> @@ -561,6 +563,7 @@ enic_free_consistent(__rte_unused struct
>> >> >>rte_pci_device *hwdev,
>> >> >>  	/* Nothing to be done */
>> >> >>  }
>> >> >>
>> >> >> +#ifndef VFIO_PRESENT
>> >> >>  static void
>> >> >>  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
>> >> >>  	void *arg)
>> >> >> @@ -572,6 +575,7 @@ enic_intr_handler(__rte_unused struct
>> >> >>rte_intr_handle *handle,
>> >> >>
>> >> >>  	enic_log_q_error(enic);
>> >> >>  }
>> >> >> +#endif
>> >> >
>> >> >--
>> >> >Thomas
>> >
>> >Hi Sujith
>> >
>> >Thomas is correct, VFIO code is designed to be EAL-only (mainly because
>> >it's Linuxapp-specific, and PMD's are intended to be cross-platform at
>> >least when it comes to compilation). Whatever it is that you're working
>> >around is better fixed in the EAL itself rather than in the PMD.
>> 
>> I agree with you and Thomas.  Let me find the root cause for the false
>>trigger.
>> 
>> Thanks,
>> -Sujith
>> 
>
>You may find it in EAL VFIO interrupts code. When VFIO enables some
>interrupt types, it manually sends a trigger. Normally, this "trigger"
>just enables interrupts, but maybe for ENIC it's different. I therefore
>suggest you to look there first.

Ok.  Thanks for the info, Anatoly.
ENIC does not need that trigger.  Let me take a look at that first.

Thanks,
-Sujith

>
>Thanks,
>Anatoly
diff mbox

Patch

diff --git a/lib/librte_pmd_enic/Makefile b/lib/librte_pmd_enic/Makefile
index a2a623f..3271960 100644
--- a/lib/librte_pmd_enic/Makefile
+++ b/lib/librte_pmd_enic/Makefile
@@ -39,6 +39,7 @@  LIB = librte_pmd_enic.a
 
 CFLAGS += -I$(RTE_SDK)/lib/librte_pmd_enic/vnic/
 CFLAGS += -I$(RTE_SDK)/lib/librte_pmd_enic/
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal/
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS) -Wno-strict-aliasing
 
diff --git a/lib/librte_pmd_enic/enic.h b/lib/librte_pmd_enic/enic.h
index c43417c..c692bab 100644
--- a/lib/librte_pmd_enic/enic.h
+++ b/lib/librte_pmd_enic/enic.h
@@ -182,6 +182,7 @@  extern void enic_dev_stats_get(struct enic *enic,
 	struct rte_eth_stats *r_stats);
 extern void enic_dev_stats_clear(struct enic *enic);
 extern void enic_add_packet_filter(struct enic *enic);
+extern void *enic_err_intr_handler(void *arg);
 extern void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
 extern void enic_del_mac_address(struct enic *enic);
 extern unsigned int enic_cleanup_wq(struct enic *enic, struct vnic_wq *wq);
diff --git a/lib/librte_pmd_enic/enic_main.c b/lib/librte_pmd_enic/enic_main.c
index e4f43c5..469cb6c 100644
--- a/lib/librte_pmd_enic/enic_main.c
+++ b/lib/librte_pmd_enic/enic_main.c
@@ -39,6 +39,7 @@ 
 #include <sys/mman.h>
 #include <fcntl.h>
 #include <libgen.h>
+#include <sys/ioctl.h>
 
 #include <rte_pci.h>
 #include <rte_memzone.h>
@@ -46,6 +47,7 @@ 
 #include <rte_mbuf.h>
 #include <rte_string_fns.h>
 #include <rte_ethdev.h>
+#include <eal_vfio.h>
 
 #include "enic_compat.h"
 #include "enic.h"
@@ -561,6 +563,7 @@  enic_free_consistent(__rte_unused struct rte_pci_device *hwdev,
 	/* Nothing to be done */
 }
 
+#ifndef VFIO_PRESENT
 static void
 enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
 	void *arg)
@@ -572,6 +575,7 @@  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
 
 	enic_log_q_error(enic);
 }
+#endif
 
 int enic_enable(struct enic *enic)
 {
@@ -978,12 +982,13 @@  static void enic_eventfd_init(struct enic *enic)
 void *enic_err_intr_handler(void *arg)
 {
 	struct enic *enic = (struct enic *)arg;
-	unsigned int intr = enic_msix_err_intr(enic);
-	ssize_t size;
 	uint64_t data;
 
 	while (1) {
-		size = read(enic->eventfd, &data, sizeof(data));
+		if (-1 == read(enic->eventfd, &data, sizeof(data))) {
+			dev_err(enic, "eventfd read failed with error %d\n", errno);
+			continue;
+		}
 		dev_err(enic, "Err intr.\n");
 		vnic_intr_return_all_credits(&enic->intr);
 
@@ -1035,7 +1040,6 @@  static int enic_set_intr_mode(struct enic *enic)
 	int *fds;
 	int size;
 	int ret = -1;
-	int index;
 
 	if (enic->intr_count < 1) {
 		dev_err(enic, "Unsupported resource conf.\n");