Re: [dpdk-dev] [PATCH v4 20/29] net/ena: use eal I/O device memory read/write API

2017-01-17 Thread Jan Mędala
Jerin, Santosh,

As you are introducing improved I/O access API, I would like to ask to
change ENA platform code to:

* #define ENA_REG_WRITE32(value, reg) rte_write32_relaxed((value), (reg))*
* #define ENA_REG_READ32(reg) rte_read32_relaxed((reg))*

There is no more need to have read/write functions within platform code (
*readl()*, *writel()*, *writel_relaxed()*), as we can directly map our API
macro to DPDK platform implementation.
Also I would prefer to keep API within *drivers/net/ena/base/ena_eth_com.h*
and map define ENA_REG_WRITE32 to relaxed version (when barrier is needed
we call it explicitly in driver code, there is no need to have
ENA_REG_WRITE32_RELAXED). That will help us to manage code together between
the different platforms.

Should I do the change for ENA or do you want to keep the current flow and
take care of it?

​Best regards
Jan


Re: [dpdk-dev] [PATCH v4 20/29] net/ena: use eal I/O device memory read/write API

2017-01-17 Thread Jan Mędala
Jerin,

Thanks for the very quick replay.

Actually I would like to keep *ena_com* untouched, as this layer suppose to
be common between platforms. That's why it is better to leave definition in
*ena_plat_dpdk.h*. Here is the patch that I would like to propose:

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h
b/drivers/net/ena/base/ena_plat_dpdk.h
index 87c3bf13..7eaebf40 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -44,6 +44,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -224,18 +225,8 @@ typedef uint64_t dma_addr_t;
 #define ENA_MEM_ALLOC(dmadev, size) rte_zmalloc(NULL, size, 1)
 #define ENA_MEM_FREE(dmadev, ptr) ({ENA_TOUCH(dmadev); rte_free(ptr); })

-static inline void writel(u32 value, volatile void  *addr)
-{
-   *(volatile u32 *)addr = value;
-}
-
-static inline u32 readl(const volatile void *addr)
-{
-   return *(const volatile u32 *)addr;
-}
-
-#define ENA_REG_WRITE32(value, reg) writel((value), (reg))
-#define ENA_REG_READ32(reg) readl((reg))
+#define ENA_REG_WRITE32(value, reg) rte_write32_relaxed((value), (reg))
+#define ENA_REG_READ32(reg) rte_read32_relaxed((reg))

 #define ATOMIC32_INC(i32_ptr) rte_atomic32_inc(i32_ptr)
 #define ATOMIC32_DEC(i32_ptr) rte_atomic32_dec(i32_ptr)

Cheers,
Jan


Re: [dpdk-dev] [PATCH v4 20/29] net/ena: use eal I/O device memory read/write API

2017-01-17 Thread Jan Mędala
Thank you very much!


Re: [dpdk-dev] [PATCH v12 0/6] add Tx preparation

2016-12-09 Thread Jan Mędala
Hello,

Sorry for late response.

>From ENA perspective, we need to dig deeper about the requirements and use
cases, but I'm pretty confident (95%) that ena will need to implement
tx_prep() API. There is at least one scenario, when HW relay on partial
checksum.


Jan


Re: [dpdk-dev] [PATCH 19/28] net/ena: use eal I/O device memory read/write API

2016-12-14 Thread Jan Mędala
Despite the issue with naming convention (either it will be writel or
write32), I'm fine with this change and new API.

Acked-by: Jan Medala 

  Jan

2016-12-14 2:55 GMT+01:00 Jerin Jacob :

> From: Santosh Shukla 
>
> Replace the raw I/O device memory read/write access with eal
> abstraction for I/O device memory read/write access to fix
> portability issues across different architectures.
>
> Signed-off-by: Santosh Shukla 
> Signed-off-by: Jerin Jacob 
> CC: Jan Medala 
> CC: Jakub Palider 
> ---
>  drivers/net/ena/base/ena_plat_dpdk.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ena/base/ena_plat_dpdk.h
> b/drivers/net/ena/base/ena_plat_dpdk.h
> index 87c3bf1..4db07c7 100644
> --- a/drivers/net/ena/base/ena_plat_dpdk.h
> +++ b/drivers/net/ena/base/ena_plat_dpdk.h
> @@ -50,6 +50,7 @@
>  #include 
>
>  #include 
> +#include 
>
>  typedef uint64_t u64;
>  typedef uint32_t u32;
> @@ -226,12 +227,12 @@ typedef uint64_t dma_addr_t;
>
>  static inline void writel(u32 value, volatile void  *addr)
>  {
> -   *(volatile u32 *)addr = value;
> +   rte_writel(value, addr);
>  }
>
>  static inline u32 readl(const volatile void *addr)
>  {
> -   return *(const volatile u32 *)addr;
> +   return rte_readl(addr);
>  }
>
>  #define ENA_REG_WRITE32(value, reg) writel((value), (reg))
> --
> 2.5.5
>
>


Re: [dpdk-dev] [PATCH v12 0/6] add Tx preparation

2016-12-16 Thread Jan Mędala
​Hello,
​


> Is there any update on that subject?
>

​At this point we need to have only pseudo-header checksum for TSO. Maybe
there will be new requirements, but that's something I cannot predict at
this point.
​


> So it seems that standard pseudo-header checksum calculation should be
> enough.
> Is that correct?
>

​Yes, this will be enough at this point.
We also need to deliver a fix for issue we found during investigation of
tx_prep() API.

Thanks,
Jan​


Re: [dpdk-dev] [PATCH v12 0/6] add Tx preparation

2016-12-16 Thread Jan Mędala
>
> ​>At this point we need to have only pseudo-header checksum for TSO. Maybe
> there will be new requirements, but that's something I cannot predict at
> this point.
>
> Ok great, then we'll add a patch for ENA for v14, unless you guys would
> like to do it yourself.
> ​

​That'd be great!​

>We also need to deliver a fix for issue we found during investigation of
> tx_prep() API.
>
> Not sure what you are talking about...
> Is that some separate issue not related directly to tx_prepare()?
>

​Yes, this is something not related, but came up during investigation of
tx_prepare(), so this is an additional benefit of this topic :-)​

​Thanks
Jan


Re: [dpdk-dev] [PATCH 2/2] net/ena: fix return of hash control flushing

2017-03-14 Thread Jan Mędala
Acked-by: Jan Medala 

  Jan

2017-02-14 13:37 GMT+01:00 Yong Wang :

> In function ena_com_set_hash_ctrl(), the return value is assigned to
> "ret" variable, but it is not returned. Fix it by adding the return.
>
> Signed-off-by: Yong Wang 
> ---
>  drivers/net/ena/base/ena_com.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_
> com.c
> index 39356d2..38a0587 100644
> --- a/drivers/net/ena/base/ena_com.c
> +++ b/drivers/net/ena/base/ena_com.c
> @@ -2278,7 +2278,7 @@ int ena_com_set_hash_ctrl(struct ena_com_dev
> *ena_dev)
> sizeof(resp));
> if (unlikely(ret)) {
> ena_trc_err("Failed to set hash input. error: %d\n", ret);
> -   ret = ENA_COM_INVAL;
> +   return ENA_COM_INVAL;
> }
>
> return 0;
> --
> 1.8.3.1
>
>
>


Re: [dpdk-dev] [PATCH 1/2] net/ena: remove redundant variable

2017-03-14 Thread Jan Mędala
Acked-by: Jan Medala 

  Jan

2017-02-14 13:37 GMT+01:00 Yong Wang :

> Signed-off-by: Yong Wang 
> ---
>  drivers/net/ena/base/ena_com.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_
> com.c
> index bd6f3c6..39356d2 100644
> --- a/drivers/net/ena/base/ena_com.c
> +++ b/drivers/net/ena/base/ena_com.c
> @@ -2242,7 +2242,6 @@ int ena_com_set_hash_ctrl(struct ena_com_dev
> *ena_dev)
>  {
> struct ena_com_admin_queue *admin_queue = &ena_dev->admin_queue;
> struct ena_rss *rss = &ena_dev->rss;
> -   struct ena_admin_feature_rss_hash_control *hash_ctrl =
> rss->hash_ctrl;
> struct ena_admin_set_feat_cmd cmd;
> struct ena_admin_set_feat_resp resp;
> int ret;
> @@ -2269,7 +2268,8 @@ int ena_com_set_hash_ctrl(struct ena_com_dev
> *ena_dev)
> ena_trc_err("memory address set failed\n");
> return ret;
> }
> -   cmd.control_buffer.length = sizeof(*hash_ctrl);
> +   cmd.control_buffer.length =
> +   sizeof(struct ena_admin_feature_rss_hash_control);
>
> ret = ena_com_execute_admin_command(admin_queue,
> (struct ena_admin_aq_entry
> *)&cmd,
> --
> 1.8.3.1
>
>
>


Re: [dpdk-dev] [PATCH 4/4] net/ena: calculate partial checksum if DF bit is disabled

2017-04-12 Thread Jan Mędala
I would recommend to check if packet type is IPv4 before processing IPv4 header 
for DF flag.
This patch can break logic and go to unknown state when mbuf will contain IPv6 
packet. I believe that in case of IPv6 pkt the loop should be skipped to next 
mbuf, if exists.

Best regards,
Jan


[dpdk-dev] [PATCH v2 0/6] ena: update PMD to cooperate with latest ENA firmware

2016-06-29 Thread Jan Mędala
Bruce,

We've got a small minor fixes and we will create V3 version later this week
(targeting Friday).
What's final deadline for 16.07 integration?

Regards,

  Jan

2016-06-29 13:01 GMT+02:00 Bruce Richardson :

> On Fri, Jun 24, 2016 at 12:52:39PM +0100, Bruce Richardson wrote:
> > On Tue, Jun 21, 2016 at 02:05:57PM +0200, Jan Medala wrote:
> > > As requested, big patch splitted into logical pieces for easier review.
> > > Improved style and fixed icc compiler issues.
> > >
> >
> > Thanks for the patch split. However, many of the patches don't have a
> commit
> > message describing (at a high level), what the goal of that patch is and
> how
> > it goes about implementing the changes to achieve that goal. Can you add
> a
> > paragraph or two of commit message to each patch on the set to help
> anyone
> > looking at the code understand what is happening in each patch.
> >
> > Regards,
> > /Bruce
> >
>
> Any update on this set? Is a V3 coming for 16.07 timeframe?
>
> /Bruce
>
>


[dpdk-dev] [PATCH 0/4] DPDK polling-mode driver for Amazon Elastic Network Adapters (ENA)

2016-01-29 Thread Jan Mędala
Hello Thomas,

2016-01-28 16:40 GMT+01:00 Thomas Monjalon :

> >  lib/librte_eal/linuxapp/ena_uio/ena_uio_driver.c   |  276 +++
>
> Sorry the kernel module party is over.
> One day, igb_uio will be removed.
> I suggest to make a first version without interrupt support
> and work with Linux community to fix your issues.
>

Rationale to deliver UIO kernel module is based on lack of legacy INTx
interrupts in ENA device. It's operating only on MSI-X.
Currently we do not implement interrupt support (yet) in PMD, but hardware
was unable to operate under uio_generic_pci driver - probe is failing.
It might be possible to use igb_uio kmod with ENA and I will do necessary
investigation+experiments to prove that.

Could you elaborate on the topic of kernel module removal - how would it
look like, is there going to be generic PCI driver dedicated for MSI-X
devices?

Thanks,
Jan


[dpdk-dev] [PATCH v3 3/6] ena: disable readless communication regarding to HW revision

2016-07-05 Thread Jan Mędala
Bruce,

Here's explanation of readless communication (on behalf of Alex):

> "readless" refers to ability to read ENA registers without actually
> issuing read request from host (x86).
> Instead, host programs 2 registers on device that trigger DMA from device
> to host and report register value.
> However, this functionality is not going to be available in all type of
> devices. The decision if this mode is supported or not , is taken from
> revision_id in pci configuration space.


Regards

  Jan

2016-07-04 17:43 GMT+02:00 Bruce Richardson :

> On Thu, Jun 30, 2016 at 05:04:56PM +0200, Jan Medala wrote:
> > Depending on HW revision readless communcation between host and device
> > may be unavailable.
> > In that case prevent PMD of seting up readless communication mechanism.
> >
> The idea of readless communication is a strange one. Can you provide a bit
> more detail as to what "readless communication" refers to.
>
> /Bruce
>


[dpdk-dev] [PATCH v3 6/6] ena: fix for icc compiler

2016-07-05 Thread Jan Mędala
Bruce,


That'd be very kind of you if you can fix trailing zeros.


This patch

Fixes: b5b8cd9 ("ena: update of ENA communication layer")


Regards,

  Jan

2016-07-05 18:19 GMT+02:00 Bruce Richardson :

> On Tue, Jul 05, 2016 at 09:52:09AM +0100, Ferruh Yigit wrote:
> > On 6/30/2016 4:04 PM, Jan Medala wrote:
> > > Signed-off-by: Alexander Matushevsky 
> > > Signed-off-by: Jakub Palider 
> > > Signed-off-by: Jan Medala 
> >
> > The compilation error to fix is [1], it may be good to add what to fix
> > into commit log.
> >
> > [1]
> > == Build drivers/net/ena
> >   CC ena_ethdev.o
> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(943): error
> > #188: enumerated type mixed with another type
> > struct ena_com_create_io_ctx ctx = { 0 };
> >  ^
> >
> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(1036): error
> > #188: enumerated type mixed with another type
> > struct ena_com_create_io_ctx ctx = { 0 };
> >  ^
> > ...
> >
> > > --- a/drivers/net/ena/ena_ethdev.c
> > > +++ b/drivers/net/ena/ena_ethdev.c
> > > @@ -940,7 +940,10 @@ static int ena_tx_queue_setup(struct rte_eth_dev
> *dev,
> > >   __rte_unused unsigned int socket_id,
> > >   __rte_unused const struct rte_eth_txconf
> *tx_conf)
> > >  {
> > > -   struct ena_com_create_io_ctx ctx = { 0 };
> > > +   struct ena_com_create_io_ctx ctx =
> > > +   /* policy set to _HOST just to satisfy icc compiler */
> > > +   { ENA_ADMIN_PLACEMENT_POLICY_HOST,
> > > + ENA_COM_IO_QUEUE_DIRECTION_TX, 0, 0, 0, 0 };
> >
> > Trailing "0" are not required, compiler will take care of them.
> >
> Jan,
>
> any comment on this? If you want, I can remove the trailing zeros on apply
> rather than needing a V3. Is that ok?
>
> Again a fixes line is missing, can you supply one.
>
> /Bruce
>


[dpdk-dev] [PATCH v3 5/6] ena: fix memory management issue

2016-07-05 Thread Jan Mędala
Yes, this is correct.

  Jan

2016-07-05 18:13 GMT+02:00 Bruce Richardson :

> On Mon, Jul 04, 2016 at 05:27:50PM +0100, Bruce Richardson wrote:
> > On Thu, Jun 30, 2016 at 05:04:58PM +0200, Jan Medala wrote:
> > > After allocating memzone it's required to zeroize memory in it.
> > > Freeing memzone with function dedicated for memoryzones.
> > >
> >
> > Can you provide a fixes line for this patch?
> >
>
> From looking at git blame, it looks to be:
> Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")
>
> Please confirm that is ok.
>
> /Bruce
>
>


[dpdk-dev] [PATCH v3 5/6] ena: fix memory management issue

2016-07-05 Thread Jan Mędala
Yes, this is correct.

  Jan

2016-07-05 18:13 GMT+02:00 Bruce Richardson :

> On Mon, Jul 04, 2016 at 05:27:50PM +0100, Bruce Richardson wrote:
> > On Thu, Jun 30, 2016 at 05:04:58PM +0200, Jan Medala wrote:
> > > After allocating memzone it's required to zeroize memory in it.
> > > Freeing memzone with function dedicated for memoryzones.
> > >
> >
> > Can you provide a fixes line for this patch?
> >
>
> From looking at git blame, it looks to be:
> Fixes: 9ba7981ec992 ("ena: add communication layer for DPDK")
>
> Please confirm that is ok.
>
> /Bruce
>
>


[dpdk-dev] [PATCH v3 6/6] ena: fix for icc compiler

2016-07-05 Thread Jan Mędala
Uhm, wait, I think that sha is wrong, please let me check it double time.

  Jan

2016-07-05 19:03 GMT+02:00 Jan M?dala :

> Bruce,
>
>
> That'd be very kind of you if you can fix trailing zeros.
>
>
> This patch
>
> Fixes: b5b8cd9 ("ena: update of ENA communication layer")
>
>
> Regards,
>
>   Jan
>
> 2016-07-05 18:19 GMT+02:00 Bruce Richardson :
>
>> On Tue, Jul 05, 2016 at 09:52:09AM +0100, Ferruh Yigit wrote:
>> > On 6/30/2016 4:04 PM, Jan Medala wrote:
>> > > Signed-off-by: Alexander Matushevsky 
>> > > Signed-off-by: Jakub Palider 
>> > > Signed-off-by: Jan Medala 
>> >
>> > The compilation error to fix is [1], it may be good to add what to fix
>> > into commit log.
>> >
>> > [1]
>> > == Build drivers/net/ena
>> >   CC ena_ethdev.o
>> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(943): error
>> > #188: enumerated type mixed with another type
>> > struct ena_com_create_io_ctx ctx = { 0 };
>> >  ^
>> >
>> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(1036): error
>> > #188: enumerated type mixed with another type
>> > struct ena_com_create_io_ctx ctx = { 0 };
>> >  ^
>> > ...
>> >
>> > > --- a/drivers/net/ena/ena_ethdev.c
>> > > +++ b/drivers/net/ena/ena_ethdev.c
>> > > @@ -940,7 +940,10 @@ static int ena_tx_queue_setup(struct rte_eth_dev
>> *dev,
>> > >   __rte_unused unsigned int socket_id,
>> > >   __rte_unused const struct rte_eth_txconf
>> *tx_conf)
>> > >  {
>> > > -   struct ena_com_create_io_ctx ctx = { 0 };
>> > > +   struct ena_com_create_io_ctx ctx =
>> > > +   /* policy set to _HOST just to satisfy icc compiler */
>> > > +   { ENA_ADMIN_PLACEMENT_POLICY_HOST,
>> > > + ENA_COM_IO_QUEUE_DIRECTION_TX, 0, 0, 0, 0 };
>> >
>> > Trailing "0" are not required, compiler will take care of them.
>> >
>> Jan,
>>
>> any comment on this? If you want, I can remove the trailing zeros on apply
>> rather than needing a V3. Is that ok?
>>
>> Again a fixes line is missing, can you supply one.
>>
>> /Bruce
>>
>
>


[dpdk-dev] [PATCH v3 6/6] ena: fix for icc compiler

2016-07-05 Thread Jan Mędala
Let me point it this way:
??This patch fixes: [dpdk-dev,v3,1/6] ena: update of ENA communication layer

  Jan

2016-07-05 19:04 GMT+02:00 Jan M?dala :

> Uhm, wait, I think that sha is wrong, please let me check it double time.
>
>   Jan
>
> 2016-07-05 19:03 GMT+02:00 Jan M?dala :
>
>> Bruce,
>>
>>
>> That'd be very kind of you if you can fix trailing zeros.
>>
>>
>> This patch
>>
>> Fixes: b5b8cd9 ("ena: update of ENA communication layer")
>>
>>
>> Regards,
>>
>>   Jan
>>
>> 2016-07-05 18:19 GMT+02:00 Bruce Richardson :
>>
>>> On Tue, Jul 05, 2016 at 09:52:09AM +0100, Ferruh Yigit wrote:
>>> > On 6/30/2016 4:04 PM, Jan Medala wrote:
>>> > > Signed-off-by: Alexander Matushevsky 
>>> > > Signed-off-by: Jakub Palider 
>>> > > Signed-off-by: Jan Medala 
>>> >
>>> > The compilation error to fix is [1], it may be good to add what to fix
>>> > into commit log.
>>> >
>>> > [1]
>>> > == Build drivers/net/ena
>>> >   CC ena_ethdev.o
>>> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(943): error
>>> > #188: enumerated type mixed with another type
>>> > struct ena_com_create_io_ctx ctx = { 0 };
>>> >  ^
>>> >
>>> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(1036):
>>> error
>>> > #188: enumerated type mixed with another type
>>> > struct ena_com_create_io_ctx ctx = { 0 };
>>> >  ^
>>> > ...
>>> >
>>> > > --- a/drivers/net/ena/ena_ethdev.c
>>> > > +++ b/drivers/net/ena/ena_ethdev.c
>>> > > @@ -940,7 +940,10 @@ static int ena_tx_queue_setup(struct
>>> rte_eth_dev *dev,
>>> > >   __rte_unused unsigned int socket_id,
>>> > >   __rte_unused const struct rte_eth_txconf
>>> *tx_conf)
>>> > >  {
>>> > > -   struct ena_com_create_io_ctx ctx = { 0 };
>>> > > +   struct ena_com_create_io_ctx ctx =
>>> > > +   /* policy set to _HOST just to satisfy icc compiler */
>>> > > +   { ENA_ADMIN_PLACEMENT_POLICY_HOST,
>>> > > + ENA_COM_IO_QUEUE_DIRECTION_TX, 0, 0, 0, 0 };
>>> >
>>> > Trailing "0" are not required, compiler will take care of them.
>>> >
>>> Jan,
>>>
>>> any comment on this? If you want, I can remove the trailing zeros on
>>> apply
>>> rather than needing a V3. Is that ok?
>>>
>>> Again a fixes line is missing, can you supply one.
>>>
>>> /Bruce
>>>
>>
>>
>


[dpdk-dev] [PATCH v3 6/6] ena: fix for icc compiler

2016-07-06 Thread Jan Mędala
There are no obstacles.
We first produced patches 1-5, then tested on different compilers so patch
no. 6 was made.

Thank you Bruce for support!
I'm still learning DPDK's submission/upstream process so all comments and
gained knowledge will easier future patches.

  Jan

2016-07-06 10:14 GMT+02:00 Bruce Richardson :

> On Tue, Jul 05, 2016 at 07:10:54PM +0200, Jan M?dala wrote:
> > Let me point it this way:
> > ??This patch fixes: [dpdk-dev,v3,1/6] ena: update of ENA communication
> layer
> >
> >   Jan
>
> Since it fixes another patch in the same series, I think the two patches
> could
> actually be merged, rather than adding code with a known issue and later
> having
> a two-line(ish) patch to fix it.
> I will merge the two on apply, if there is no objection.
>
> /Bruce
>
> >
> > 2016-07-05 19:04 GMT+02:00 Jan M?dala :
> >
> > > Uhm, wait, I think that sha is wrong, please let me check it double
> time.
> > >
> > >   Jan
> > >
> > > 2016-07-05 19:03 GMT+02:00 Jan M?dala :
> > >
> > >> Bruce,
> > >>
> > >>
> > >> That'd be very kind of you if you can fix trailing zeros.
> > >>
> > >>
> > >> This patch
> > >>
> > >> Fixes: b5b8cd9 ("ena: update of ENA communication layer")
> > >>
> > >>
> > >> Regards,
> > >>
> > >>   Jan
> > >>
> > >> 2016-07-05 18:19 GMT+02:00 Bruce Richardson <
> bruce.richardson at intel.com>:
> > >>
> > >>> On Tue, Jul 05, 2016 at 09:52:09AM +0100, Ferruh Yigit wrote:
> > >>> > On 6/30/2016 4:04 PM, Jan Medala wrote:
> > >>> > > Signed-off-by: Alexander Matushevsky 
> > >>> > > Signed-off-by: Jakub Palider 
> > >>> > > Signed-off-by: Jan Medala 
> > >>> >
> > >>> > The compilation error to fix is [1], it may be good to add what to
> fix
> > >>> > into commit log.
> > >>> >
> > >>> > [1]
> > >>> > == Build drivers/net/ena
> > >>> >   CC ena_ethdev.o
> > >>> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(943):
> error
> > >>> > #188: enumerated type mixed with another type
> > >>> > struct ena_com_create_io_ctx ctx = { 0 };
> > >>> >  ^
> > >>> >
> > >>> > /tmp/dpdk_maintain/ena_v3/dpdk/drivers/net/ena/ena_ethdev.c(1036):
> > >>> error
> > >>> > #188: enumerated type mixed with another type
> > >>> > struct ena_com_create_io_ctx ctx = { 0 };
> > >>> >  ^
> > >>> > ...
> > >>> >
> > >>> > > --- a/drivers/net/ena/ena_ethdev.c
> > >>> > > +++ b/drivers/net/ena/ena_ethdev.c
> > >>> > > @@ -940,7 +940,10 @@ static int ena_tx_queue_setup(struct
> > >>> rte_eth_dev *dev,
> > >>> > >   __rte_unused unsigned int socket_id,
> > >>> > >   __rte_unused const struct
> rte_eth_txconf
> > >>> *tx_conf)
> > >>> > >  {
> > >>> > > -   struct ena_com_create_io_ctx ctx = { 0 };
> > >>> > > +   struct ena_com_create_io_ctx ctx =
> > >>> > > +   /* policy set to _HOST just to satisfy icc compiler
> */
> > >>> > > +   { ENA_ADMIN_PLACEMENT_POLICY_HOST,
> > >>> > > + ENA_COM_IO_QUEUE_DIRECTION_TX, 0, 0, 0, 0 };
> > >>> >
> > >>> > Trailing "0" are not required, compiler will take care of them.
> > >>> >
> > >>> Jan,
> > >>>
> > >>> any comment on this? If you want, I can remove the trailing zeros on
> > >>> apply
> > >>> rather than needing a V3. Is that ok?
> > >>>
> > >>> Again a fixes line is missing, can you supply one.
> > >>>
> > >>> /Bruce
> > >>>
> > >>
> > >>
> > >
>


[dpdk-dev] [PATCH] ena: fix doorbell submission when not needed

2016-07-07 Thread Jan Mędala
Avoid submitting doorbell when:
* no packets have been submitted to TX
* no free resources have been submitted while RX

Sending doorbell without actual work to be performed by device
violates ENA specification and can lead to unpredictable behavior.

Fixes: 1173fca25af9 ("ena: add polling-mode driver")

Signed-off-by: Alexander Matushevsky 
Signed-off-by: Jan Medala 
---
 drivers/net/ena/ena_ethdev.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 702289b..d68e7ec 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -920,10 +920,14 @@ static int ena_populate_rx_queue(struct ena_ring
*rxq, unsigned int count)
  next_to_use = ENA_RX_RING_IDX_NEXT(next_to_use, ring_size);
  }

- rte_wmb();
- rxq->next_to_use = next_to_use;
- /* let HW know that it can fill buffers with data */
- ena_com_write_sq_doorbell(rxq->ena_com_io_sq);
+ /* When we submitted free recources to device... */
+ if (i > 0) {
+ /* ...let HW know that it can fill buffers with data */
+ rte_wmb();
+ ena_com_write_sq_doorbell(rxq->ena_com_io_sq);
+
+ rxq->next_to_use = next_to_use;
+ }

  return i;
 }
@@ -1316,7 +1320,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue,
struct rte_mbuf **tx_pkts,
  struct ena_tx_buffer *tx_info;
  struct ena_com_buf *ebuf;
  uint16_t rc, req_id, total_tx_descs = 0;
- int sent_idx = 0;
+ uint16_t sent_idx = 0;
  int nb_hw_desc;

  /* Check adapter state */
@@ -1395,9 +1399,14 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue,
struct rte_mbuf **tx_pkts,
  next_to_use = ENA_TX_RING_IDX_NEXT(next_to_use, ring_size);
  }

- /* Let HW do it's best :-) */
- rte_wmb();
- ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
+ /* If there are ready packets to be xmitted... */
+ if (sent_idx > 0) {
+ /* ...let HW do its best :-) */
+ rte_wmb();
+ ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
+
+ tx_ring->next_to_use = next_to_use;
+ }

  /* Clear complete packets  */
  while (ena_com_tx_comp_req_id_get(tx_ring->ena_com_io_cq, &req_id) >= 0) {
@@ -1420,9 +1429,11 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue,
struct rte_mbuf **tx_pkts,
  break;
  }

- /* acknowledge completion of sent packets */
- ena_com_comp_ack(tx_ring->ena_com_io_sq, total_tx_descs);
- tx_ring->next_to_use = next_to_use;
+ if (total_tx_descs > 0) {
+ /* acknowledge completion of sent packets */
+ ena_com_comp_ack(tx_ring->ena_com_io_sq, total_tx_descs);
+ }
+
  return sent_idx;
 }

-- 
2.8.1


[dpdk-dev] dpdk daily build error on dpdk16.07-rc2

2016-07-15 Thread Jan Mędala
Hello Yu Liu,

I've sent second version of patch to test on icc16 compiler, Yongjie is
helping me to test issues on icc.

  Jan


[dpdk-dev] [PATCH] net/ena: fix icc compile error

2016-07-19 Thread Jan Mędala
Ferruh,

Actually we can stay with this patch. It's ok.

Acked-by: Jan Medala 

  Jan

2016-07-19 12:48 GMT+02:00 Ferruh Yigit :

> On 7/19/2016 10:33 AM, Ferruh Yigit wrote:
> > compile error:
> >   CC ena_com.o
> > .../drivers/net/ena/base/ena_com.c(346):
> > error #3656: variable "dev_node" may be used before its value is set
> > ENA_MEM_ALLOC_COHERENT_NODE(ena_dev->dmadev,
> > ^
> >
> > .../drivers/net/ena/base/ena_com.c(399):
> > error #3656: variable "prev_node" may be used before its value is set
> > ENA_MEM_ALLOC_COHERENT_NODE(ena_dev->dmadev,
> > ^
> >
> > Fixes: 3d3edc265fc8 ("net/ena: make coherent memory allocation
> NUMA-aware")
> >
> > Reported-by: Eoin Breen 
> > Signed-off-by: Ferruh Yigit 
>
> Self-Nack
> I wasn't aware Jan is already working on this and has a patch. We can
> drop this one.
>
> Thanks,
> ferruh
>
>


[dpdk-dev] [PATCH v5 0/4] DPDK polling-mode driver for Amazon Elastic Network Adapters (ENA)

2016-03-09 Thread Jan Mędala
I'd like to kindly request for review and comments.

 Jan


[dpdk-dev] [PATCH v7 4/4] ena: DPDK polling-mode driver for Amazon Elastic Network Adapters (ENA)

2016-03-17 Thread Jan Mędala
>
> >  #
> > +# Compile burst-oriented Amazon ENA PMD driver
> > +#
> > +CONFIG_RTE_LIBRTE_ENA_PMD=y
> > +CONFIG_RTE_LIBRTE_ENA_DEBUG_INIT=y
>
> Do you really want initialization debuggin to be on by default? Normally,
> we
> keep all debug options disabled.

This is actually error logging, so it's silent for user until there is
something wrong with initialization.
Do you want me to rename it to point more accurately it's role?

  Jan


[dpdk-dev] [PATCH v7 4/4] ena: DPDK polling-mode driver for Amazon Elastic Network Adapters (ENA)

2016-03-17 Thread Jan Mędala
2016-03-17 14:57 GMT+01:00 Thomas Monjalon :

> 2016-03-17 14:48, Jan M?dala:
> > >
> > > >  #
> > > > +# Compile burst-oriented Amazon ENA PMD driver
> > > > +#
> > > > +CONFIG_RTE_LIBRTE_ENA_PMD=y
> > > > +CONFIG_RTE_LIBRTE_ENA_DEBUG_INIT=y
> > >
> > > Do you really want initialization debuggin to be on by default?
> Normally,
> > > we
> > > keep all debug options disabled.
> >
> > This is actually error logging, so it's silent for user until there is
> > something wrong with initialization.
> > Do you want me to rename it to point more accurately it's role?
>
> There should not be any option at all to disable error logging.
>
OK, I'm going to fix that.


[dpdk-dev] [PATCH v8 0/4] DPDK polling-mode driver for Amazon Elastic Network Adapters (ENA)

2016-03-18 Thread Jan Mędala
Great news!
We are going to extend functionality of the ENA PMD, so from now I will
send patches based on v8 version.

Thanks,

  Jan

2016-03-18 18:04 GMT+01:00 Bruce Richardson :

> On Thu, Mar 17, 2016 at 03:31:14PM +0100, Jan Medala wrote:
> > v3:
> > Additional features for Amazon ENA:
> > * Low Latenycy Queue (LLQ) for Tx
> > * RSS
> > v4:
> > * Improved doc
> > * Improved style according to checkpatch script
> > * Fixed build problems on: i686, clang, +shared, +debug
> > v5:
> > * Removed 'cvos' environment code from ena Makefile
> > * Driver symbol version fixed to DPDK_16.04
> > * Max MTU is read from device attributes
> > v6:
> > * Updated ENA communication layer
> > * Added check if DPDK queue size is supported by device
> > * Checkpatch results: 6 warns >80, 0 warns >90, no whitespace issues
> > * defined likely/unlikely (can compile with ARM toolchain)
> > * Updated doc/guides/nics/overview.rst w/ ENA
> > * Removed metioned #pragma for "-Wcast-qual"
> > v7:
> > * Resolved Thomas's comments:
> >   - included  instead of own definition of
> > likely/unlikely
> >   - used RTE_MIN/RTE_MAX macros
> > v8:
> > * Fixed init (error) logging to be always available
> >
> > Jan Medala (4):
> >   ena: Amazon ENA documentation
> >   ena: Amazon ENA communication layer
> >   ena: Amazon ENA communication layer for DPDK platform
> >   ena: DPDK polling-mode driver for Amazon Elastic Network Adapters
> > (ENA)
> >
> Applied to dpdk-next-net/rel_16_04.
>
> Thanks for the contribution of a new PMD.
>
> /Bruce
>


[dpdk-dev] [PATCH v2 3/4] Amazon ENA communication layer for DPDK platform

2016-02-15 Thread Jan Mędala
Stephen,
This will be fixed

dpdk-dev,
ping for more comments/review

Thanks,
Jan

  Jan

2016-02-08 22:30 GMT+01:00 Stephen Hemminger :

> On Fri,  5 Feb 2016 19:20:28 +0100
> Jan Medala  wrote:
>
> > +
> > +typedef _Bool bool;
>
>
> > +#define true ((bool)1)
> > +#define false((bool)0)
>
> Why not just use  ??
>


[dpdk-dev] [PATCH v2 1/4] Amazon ENA PCI defines and documentation

2016-02-15 Thread Jan Mędala
Stephen,
does it mean we should insert those defines to PMD driver files?

dpdk-dev,
ping for more comments/review

Thanks,
Jan

  Jan

2016-02-08 22:27 GMT+01:00 Stephen Hemminger :

> On Fri,  5 Feb 2016 19:20:26 +0100
> Jan Medala  wrote:
>
> > +
> > +#ifndef PCI_VENDOR_ID_AMAZON
> > +/** Vendor ID used by Amazon devices */
> > +#define PCI_VENDOR_ID_AMAZON 0x1D0F
> > +#endif
> > +
>
> The goal is to eliminate the clobal rte_pci_dev_ids.h file, it doesn't
> scale.
>


[dpdk-dev] [PATCH v2 4/4] DPDK polling-mode driver for Amazon Elastic Network Adapters (ENA)

2016-02-15 Thread Jan Mędala
ping for comments/review

Thanks,

  Jan

2016-02-05 19:20 GMT+01:00 Jan Medala :

> This is a PMD for the Amazon ethernet ENA family.
> The driver operates variety of ENA adapters through feature negotiation
> with the adapter and upgradable commands set.
> ENA driver handles PCI Physical and Virtual ENA functions.
>
> Signed-off-by: Evgeny Schemeilin 
> Signed-off-by: Jan Medala 
> Signed-off-by: Jakub Palider 
> ---
>  config/common_linuxapp |   12 +
>  drivers/net/Makefile   |1 +
>  drivers/net/ena/Makefile   |   62 +++
>  drivers/net/ena/ena_ethdev.c   | 1129
> 
>  drivers/net/ena/ena_ethdev.h   |  151 ++
>  drivers/net/ena/ena_logs.h |   76 +++
>  drivers/net/ena/ena_platform.h |   58 +++
>  mk/rte.app.mk  |1 +
>  8 files changed, 1490 insertions(+)
>  create mode 100755 drivers/net/ena/Makefile
>  create mode 100644 drivers/net/ena/ena_ethdev.c
>  create mode 100755 drivers/net/ena/ena_ethdev.h
>  create mode 100644 drivers/net/ena/ena_logs.h
>  create mode 100644 drivers/net/ena/ena_platform.h
>
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index 74bc515..261a54f 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -249,6 +249,18 @@ CONFIG_RTE_LIBRTE_CXGBE_DEBUG_TX=n
>  CONFIG_RTE_LIBRTE_CXGBE_DEBUG_RX=n
>
>  #
> +# Compile burst-oriented Amazon ENA PMD driver
> +#
> +CONFIG_RTE_LIBRTE_ENA_PMD=y
> +CONFIG_RTE_LIBRTE_ENA_DEBUG_INIT=y
> +CONFIG_RTE_LIBRTE_ENA_DEBUG_RX=n
> +CONFIG_RTE_LIBRTE_ENA_DEBUG_TX=n
> +CONFIG_RTE_LIBRTE_ENA_DEBUG_TX_FREE=n
> +CONFIG_RTE_LIBRTE_ENA_DEBUG_DRIVER=n
> +CONFIG_RTE_LIBRTE_ENA_COM_DEBUG=n
> +CONFIG_RTE_EAL_ENA_UIO=y
> +
> +#
>  # Compile burst-oriented Cisco ENIC PMD driver
>  #
>  CONFIG_RTE_LIBRTE_ENIC_PMD=y
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 6e4497e..8f2649f 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -36,6 +36,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD) += bnx2x
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += bonding
>  DIRS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += cxgbe
>  DIRS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000
> +DIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += ena
>  DIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic
>  DIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += fm10k
>  DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e
> diff --git a/drivers/net/ena/Makefile b/drivers/net/ena/Makefile
> new file mode 100755
> index 000..960e4cd
> --- /dev/null
> +++ b/drivers/net/ena/Makefile
> @@ -0,0 +1,62 @@
> +#
> +# BSD LICENSE
> +#
> +# Copyright (c) 2015-2016 Amazon.com, Inc. or its affiliates.
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#
> +# * Redistributions of source code must retain the above copyright
> +# notice, this list of conditions and the following disclaimer.
> +# * Redistributions in binary form must reproduce the above copyright
> +# notice, this list of conditions and the following disclaimer in
> +# the documentation and/or other materials provided with the
> +# distribution.
> +# * Neither the name of copyright holder nor the names of its
> +# contributors may be used to endorse or promote products derived
> +# from this software without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +#
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_ena.a
> +CFLAGS += $(WERROR_FLAGS) -O2
> +INCLUDES :=-I$(SRCDIR) -I$(SRCDIR)/base/ena_defs -I$(SRCDIR)/base
> +
> +VPATH += $(SRCDIR)/base
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += ena_ethdev.c
> +SRCS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += ena_com.c
> +SRCS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += ena_eth_com.c
> +
> +# this lib depends upon:
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += lib/librte_eal lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += lib/librte_mempool lib/librte_mbuf
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += lib/librte_net lib/librte_malloc
> +
> +ifeq ($(CONFIG_RTE_EXEC_ENV),"cvos")
> +CFLAGS += -Wno-old-style-definition
> +endif
> +
> 

[dpdk-dev] [PATCH v2 2/4] Amazon ENA communication layer

2016-02-15 Thread Jan Mędala
ping for comments/review

Thanks,
  Jan

P.S. Sorry for replaying with whole code in previous messages.