DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
@ 2014-12-12  8:18 Sujith Sankar
  2014-12-15 23:24 ` Thomas Monjalon
  0 siblings, 1 reply; 10+ messages in thread
From: Sujith Sankar @ 2014-12-12  8:18 UTC (permalink / raw)
  To: dev

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(-)

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");
-- 
1.9.1

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-12  8:18 [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT Sujith Sankar
@ 2014-12-15 23:24 ` Thomas Monjalon
  2014-12-16  4:12   ` Sujith Sankar (ssujith)
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2014-12-15 23:24 UTC (permalink / raw)
  To: Sujith Sankar; +Cc: dev

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

-- 
Thomas

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-15 23:24 ` Thomas Monjalon
@ 2014-12-16  4:12   ` Sujith Sankar (ssujith)
  2014-12-16  7:51     ` Qiu, Michael
  2014-12-16 10:22     ` Burakov, Anatoly
  0 siblings, 2 replies; 10+ messages in thread
From: Sujith Sankar (ssujith) @ 2014-12-16  4:12 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev


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

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-16  4:12   ` Sujith Sankar (ssujith)
@ 2014-12-16  7:51     ` Qiu, Michael
  2014-12-16 10:00       ` Sujith Sankar (ssujith)
  2014-12-16 10:22     ` Burakov, Anatoly
  1 sibling, 1 reply; 10+ messages in thread
From: Qiu, Michael @ 2014-12-16  7:51 UTC (permalink / raw)
  To: Sujith Sankar (ssujith), Thomas Monjalon; +Cc: dev

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
>


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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-16  7:51     ` Qiu, Michael
@ 2014-12-16 10:00       ` Sujith Sankar (ssujith)
  2014-12-16 10:16         ` Thomas Monjalon
  0 siblings, 1 reply; 10+ messages in thread
From: Sujith Sankar (ssujith) @ 2014-12-16 10:00 UTC (permalink / raw)
  To: Qiu, Michael, Thomas Monjalon; +Cc: dev


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

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-16 10:00       ` Sujith Sankar (ssujith)
@ 2014-12-16 10:16         ` Thomas Monjalon
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2014-12-16 10:16 UTC (permalink / raw)
  To: Sujith Sankar (ssujith); +Cc: dev

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

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-16  4:12   ` Sujith Sankar (ssujith)
  2014-12-16  7:51     ` Qiu, Michael
@ 2014-12-16 10:22     ` Burakov, Anatoly
  2014-12-16 10:34       ` Sujith Sankar (ssujith)
  1 sibling, 1 reply; 10+ messages in thread
From: Burakov, Anatoly @ 2014-12-16 10:22 UTC (permalink / raw)
  To: Sujith Sankar (ssujith), Thomas Monjalon; +Cc: dev

> 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

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-16 10:22     ` Burakov, Anatoly
@ 2014-12-16 10:34       ` Sujith Sankar (ssujith)
  2014-12-16 10:36         ` Burakov, Anatoly
  0 siblings, 1 reply; 10+ messages in thread
From: Sujith Sankar (ssujith) @ 2014-12-16 10:34 UTC (permalink / raw)
  To: Burakov, Anatoly, Thomas Monjalon; +Cc: dev



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

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-16 10:34       ` Sujith Sankar (ssujith)
@ 2014-12-16 10:36         ` Burakov, Anatoly
  2014-12-16 10:40           ` Sujith Sankar (ssujith)
  0 siblings, 1 reply; 10+ messages in thread
From: Burakov, Anatoly @ 2014-12-16 10:36 UTC (permalink / raw)
  To: Sujith Sankar (ssujith), Thomas Monjalon; +Cc: dev

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

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

* Re: [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT
  2014-12-16 10:36         ` Burakov, Anatoly
@ 2014-12-16 10:40           ` Sujith Sankar (ssujith)
  0 siblings, 0 replies; 10+ messages in thread
From: Sujith Sankar (ssujith) @ 2014-12-16 10:40 UTC (permalink / raw)
  To: Burakov, Anatoly, Thomas Monjalon; +Cc: dev



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

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

end of thread, other threads:[~2014-12-16 10:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-12  8:18 [dpdk-dev] [PATCH] enic: corrected the usage of VFIO_PRESENT Sujith Sankar
2014-12-15 23:24 ` Thomas Monjalon
2014-12-16  4:12   ` Sujith Sankar (ssujith)
2014-12-16  7:51     ` Qiu, Michael
2014-12-16 10:00       ` Sujith Sankar (ssujith)
2014-12-16 10:16         ` Thomas Monjalon
2014-12-16 10:22     ` Burakov, Anatoly
2014-12-16 10:34       ` Sujith Sankar (ssujith)
2014-12-16 10:36         ` Burakov, Anatoly
2014-12-16 10:40           ` Sujith Sankar (ssujith)

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).