Re: [dpdk-dev] [PATCH v4 20/29] net/ena: use eal I/O device memory read/write API
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
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
Thank you very much!
Re: [dpdk-dev] [PATCH v12 0/6] add Tx preparation
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
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
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
> > >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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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)
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)
> > > # > > +# 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 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)
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
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
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)
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
ping for comments/review Thanks, Jan P.S. Sorry for replaying with whole code in previous messages.