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

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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
-- 
Thomas