Hi, this patchset contains few bug fixes for the ENA PMD and the version upgrade to v2.2.1. Besides that, the validation of the Rx req ID was optimized. Best regards, Michal --- v2: * Add patch preventing double doorbell on Tx. Amit Bernstein (1): net/ena: Tx doorbell statistics fix Ido Segev (1): net/ena: flush Rx buffers memory pool cache Igor Chauskin (2): net/ena: fix Tx sq free space assessment net/ena: prevent double doorbell Michal Krawczyk (1): net/ena: validate Rx req id upon acquiring the desc drivers/net/ena/base/ena_eth_com.c | 3 + drivers/net/ena/base/ena_plat_dpdk.h | 1 + drivers/net/ena/ena_ethdev.c | 85 +++++++++++++++------------- drivers/net/ena/ena_ethdev.h | 4 ++ 4 files changed, 54 insertions(+), 39 deletions(-) -- 2.25.1
From: Ido Segev <idose@amazon.com> As the refill called as part of ena_start(), we end up the refill progress with stuck buffers at the caller core cache. Calling to flush the cache results with invalidate this cache and free those stuck buffers. Fixes: 1173fca25af9 ("ena: add polling-mode driver") Cc: stable@dpdk.org Signed-off-by: Ido Segev <idose@amazon.com> Reviewed-by: Michal Krawczyk <mk@semihalf.com> Reviewed-by: Igor Chauskin <igorch@amazon.com> --- drivers/net/ena/ena_ethdev.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index 20ff3653c6..516e244295 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -1246,6 +1246,10 @@ static int ena_queue_start(struct ena_ring *ring) PMD_INIT_LOG(ERR, "Failed to populate rx ring !"); return ENA_COM_FAULT; } + /* Flush per-core RX buffers pools cache as they can be used on other + * cores as well. + */ + rte_mempool_cache_flush(NULL, ring->mb_pool); return 0; } -- 2.25.1
From: Amit Bernstein <amitbern@amazon.com> Increment Tx doorbell statistics on tx_pkt_burst after writing to doorbell and in case max burst size achieved Fixes: c7519ea5eb8d ("net/ena: call additional doorbells if needed") Cc: stable@dpdk.org Signed-off-by: Amit Bernstein <amitbern@amazon.com> Reviewed-by: Michal Krawczyk <mk@semihalf.com> Reviewed-by: Igor Chauskin <igorch@amazon.com> --- drivers/net/ena/ena_ethdev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index 516e244295..111e830bfa 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -2519,6 +2519,7 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf) "llq tx max burst size of queue %d achieved, writing doorbell to send burst\n", tx_ring->id); ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq); + tx_ring->tx_stats.doorbells++; } /* prepare the packet's descriptors to dma engine */ -- 2.25.1
Instead of veryfing the Rx descriptor each time it's being used in the driver code, now the verification happens on the HAL side. This simplifies code a lot as instead of doing 2 validations, only single one is needed. The driver have to check the rc value returned by the ena_com upon reading the Rx descriptor and trigger the reset if needed. It was previously the responsibility of the validate_rx_req_id() function. As part of the change, the version of the driver was bumped to v2.2.1. Fixes: 2061fe41f212 ("net/ena: linearize Tx mbuf") Signed-off-by: Ido Segev <idose@amazon.com> Signed-off-by: Michal Krawczyk <mk@semihalf.com> Reviewed-by: Igor Chauskin <igorch@amazon.com> --- drivers/net/ena/base/ena_eth_com.c | 3 +++ drivers/net/ena/base/ena_plat_dpdk.h | 1 + drivers/net/ena/ena_ethdev.c | 38 ++++++++-------------------- 3 files changed, 14 insertions(+), 28 deletions(-) diff --git a/drivers/net/ena/base/ena_eth_com.c b/drivers/net/ena/base/ena_eth_com.c index a35d92fbd3..5583a310a1 100644 --- a/drivers/net/ena/base/ena_eth_com.c +++ b/drivers/net/ena/base/ena_eth_com.c @@ -531,6 +531,7 @@ int ena_com_rx_pkt(struct ena_com_io_cq *io_cq, { struct ena_com_rx_buf_info *ena_buf = &ena_rx_ctx->ena_bufs[0]; struct ena_eth_io_rx_cdesc_base *cdesc = NULL; + u16 q_depth = io_cq->q_depth; u16 cdesc_idx = 0; u16 nb_hw_desc; u16 i = 0; @@ -559,6 +560,8 @@ int ena_com_rx_pkt(struct ena_com_io_cq *io_cq, do { ena_buf[i].len = cdesc->length; ena_buf[i].req_id = cdesc->req_id; + if (unlikely(ena_buf[i].req_id >= q_depth)) + return ENA_COM_EIO; if (++i >= nb_hw_desc) break; diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h index 48c77f0c19..a1d749f83f 100644 --- a/drivers/net/ena/base/ena_plat_dpdk.h +++ b/drivers/net/ena/base/ena_plat_dpdk.h @@ -51,6 +51,7 @@ typedef uint64_t dma_addr_t; #define ENA_COM_FAULT -EFAULT #define ENA_COM_TRY_AGAIN -EAGAIN #define ENA_COM_UNSUPPORTED -EOPNOTSUPP +#define ENA_COM_EIO -EIO #define ____cacheline_aligned __rte_cache_aligned diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index 111e830bfa..9ee9de6eb9 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -28,7 +28,7 @@ #define DRV_MODULE_VER_MAJOR 2 #define DRV_MODULE_VER_MINOR 2 -#define DRV_MODULE_VER_SUBMINOR 0 +#define DRV_MODULE_VER_SUBMINOR 1 #define ENA_IO_TXQ_IDX(q) (2 * (q)) #define ENA_IO_RXQ_IDX(q) (2 * (q) + 1) @@ -380,20 +380,6 @@ static inline void ena_tx_mbuf_prepare(struct rte_mbuf *mbuf, } } -static inline int validate_rx_req_id(struct ena_ring *rx_ring, uint16_t req_id) -{ - if (likely(req_id < rx_ring->ring_size)) - return 0; - - PMD_DRV_LOG(ERR, "Invalid rx req_id: %hu\n", req_id); - - rx_ring->adapter->reset_reason = ENA_REGS_RESET_INV_RX_REQ_ID; - rx_ring->adapter->trigger_reset = true; - ++rx_ring->rx_stats.bad_req_id; - - return -EFAULT; -} - static int validate_tx_req_id(struct ena_ring *tx_ring, u16 req_id) { struct ena_tx_buffer *tx_info = NULL; @@ -1486,10 +1472,6 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count) rte_prefetch0(mbufs[i + 4]); req_id = rxq->empty_rx_reqs[next_to_use]; - rc = validate_rx_req_id(rxq, req_id); - if (unlikely(rc)) - break; - rx_info = &rxq->rx_buffer_info[req_id]; rc = ena_add_single_rx_desc(rxq->ena_com_io_sq, mbuf, req_id); @@ -2114,8 +2096,6 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring, len = ena_bufs[buf].len; req_id = ena_bufs[buf].req_id; - if (unlikely(validate_rx_req_id(rx_ring, req_id))) - return NULL; rx_info = &rx_ring->rx_buffer_info[req_id]; @@ -2139,10 +2119,6 @@ static struct rte_mbuf *ena_rx_mbuf(struct ena_ring *rx_ring, ++buf; len = ena_bufs[buf].len; req_id = ena_bufs[buf].req_id; - if (unlikely(validate_rx_req_id(rx_ring, req_id))) { - rte_mbuf_raw_free(mbuf_head); - return NULL; - } rx_info = &rx_ring->rx_buffer_info[req_id]; RTE_ASSERT(rx_info->mbuf != NULL); @@ -2230,10 +2206,16 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, &ena_rx_ctx); if (unlikely(rc)) { PMD_DRV_LOG(ERR, "ena_com_rx_pkt error %d\n", rc); - rx_ring->adapter->reset_reason = - ENA_REGS_RESET_TOO_MANY_RX_DESCS; + if (rc == ENA_COM_NO_SPACE) { + ++rx_ring->rx_stats.bad_desc_num; + rx_ring->adapter->reset_reason = + ENA_REGS_RESET_TOO_MANY_RX_DESCS; + } else { + ++rx_ring->rx_stats.bad_req_id; + rx_ring->adapter->reset_reason = + ENA_REGS_RESET_INV_RX_REQ_ID; + } rx_ring->adapter->trigger_reset = true; - ++rx_ring->rx_stats.bad_desc_num; return 0; } -- 2.25.1
From: Igor Chauskin <igorch@amazon.com> Before starting transmission of Tx burst, the driver checked the available space in the sq and limited the number of packets for transmission accordingly. The calculation was incorrect for fragmented packets and potentially had significantly limited the length of Tx bursts. This patch removes the assessment and pushes packets to the sq as long as the burst is not exhausted and space is available in the sq. Correct evaluation of the required space isn't possible before the burst because it depends on the number of segments of each packet. This patch adds per-packet space evaluation for each packet before attempting to process it. In case there is not enough queue space, the burst will just stop without error. Signed-off-by: Igor Chauskin <igorch@amazon.com> Reviewed-by: Michal Krawczyk <mk@semihalf.com> --- drivers/net/ena/ena_ethdev.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index 9ee9de6eb9..4083568d5d 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -2359,8 +2359,8 @@ static void ena_update_hints(struct ena_adapter *adapter, } } -static int ena_check_and_linearize_mbuf(struct ena_ring *tx_ring, - struct rte_mbuf *mbuf) +static int ena_check_space_and_linearize_mbuf(struct ena_ring *tx_ring, + struct rte_mbuf *mbuf) { struct ena_com_dev *ena_dev; int num_segments, header_len, rc; @@ -2370,13 +2370,21 @@ static int ena_check_and_linearize_mbuf(struct ena_ring *tx_ring, header_len = mbuf->data_len; if (likely(num_segments < tx_ring->sgl_size)) - return 0; + goto checkspace; if (ena_dev->tx_mem_queue_type == ENA_ADMIN_PLACEMENT_POLICY_DEV && (num_segments == tx_ring->sgl_size) && (header_len < tx_ring->tx_max_header_size)) - return 0; + goto checkspace; + /* Checking for space for 2 additional metadata descriptors due to + * possible header split and metadata descriptor. Linearization will + * be needed so we reduce the segments number from num_segments to 1 + */ + if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq, 3)) { + PMD_DRV_LOG(DEBUG, "Not enough space in the tx queue\n"); + return ENA_COM_NO_MEM; + } ++tx_ring->tx_stats.linearize; rc = rte_pktmbuf_linearize(mbuf); if (unlikely(rc)) { @@ -2386,7 +2394,19 @@ static int ena_check_and_linearize_mbuf(struct ena_ring *tx_ring, return rc; } - return rc; + return 0; + +checkspace: + /* Checking for space for 2 additional metadata descriptors due to + * possible header split and metadata descriptor + */ + if (!ena_com_sq_have_enough_space(tx_ring->ena_com_io_sq, + num_segments + 2)) { + PMD_DRV_LOG(DEBUG, "Not enough space in the tx queue\n"); + return ENA_COM_NO_MEM; + } + + return 0; } static void ena_tx_map_mbuf(struct ena_ring *tx_ring, @@ -2473,7 +2493,7 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf) int nb_hw_desc; int rc; - rc = ena_check_and_linearize_mbuf(tx_ring, mbuf); + rc = ena_check_space_and_linearize_mbuf(tx_ring, mbuf); if (unlikely(rc)) return rc; @@ -2580,9 +2600,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, return 0; } - nb_pkts = RTE_MIN(ena_com_free_q_entries(tx_ring->ena_com_io_sq), - nb_pkts); - for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) { if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx])) break; -- 2.25.1
From: Igor Chauskin <igorch@amazon.com> Add per-tx-ring flag for packets that were pushed to HW but await doorbell. That is to prevent a situation when a doorbell is sent due to reaching Tx burst threshold and next send fails (e.g., due to queue full). In such case we shouldn't send another doorbell because there are no actual packets waiting for transmission. Signed-off-by: Igor Chauskin <igorch@amazon.com> Reviewed-by: Michal Krawczyk <mk@semihalf.com> --- drivers/net/ena/ena_ethdev.c | 7 +++++-- drivers/net/ena/ena_ethdev.h | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c index 4083568d5d..8baec80040 100644 --- a/drivers/net/ena/ena_ethdev.c +++ b/drivers/net/ena/ena_ethdev.c @@ -1282,6 +1282,7 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev, txq->ring_size = nb_desc; txq->size_mask = nb_desc - 1; txq->numa_socket_id = socket_id; + txq->pkts_without_db = false; txq->tx_buffer_info = rte_zmalloc("txq->tx_buffer_info", sizeof(struct ena_tx_buffer) * @@ -2522,6 +2523,7 @@ static int ena_xmit_mbuf(struct ena_ring *tx_ring, struct rte_mbuf *mbuf) tx_ring->id); ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq); tx_ring->tx_stats.doorbells++; + tx_ring->pkts_without_db = false; } /* prepare the packet's descriptors to dma engine */ @@ -2603,7 +2605,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) { if (ena_xmit_mbuf(tx_ring, tx_pkts[sent_idx])) break; - + tx_ring->pkts_without_db = true; rte_prefetch0(tx_pkts[ENA_IDX_ADD_MASKED(sent_idx, 4, tx_ring->size_mask)]); } @@ -2612,10 +2614,11 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, ena_com_free_q_entries(tx_ring->ena_com_io_sq); /* If there are ready packets to be xmitted... */ - if (sent_idx > 0) { + if (likely(tx_ring->pkts_without_db)) { /* ...let HW do its best :-) */ ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq); tx_ring->tx_stats.doorbells++; + tx_ring->pkts_without_db = false; } ena_tx_cleanup(tx_ring); diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h index 7bb74a1d06..ae235897ee 100644 --- a/drivers/net/ena/ena_ethdev.h +++ b/drivers/net/ena/ena_ethdev.h @@ -100,6 +100,10 @@ struct ena_ring { enum ena_ring_type type; enum ena_admin_placement_policy_type tx_mem_queue_type; + + /* Indicate there are Tx packets pushed to the device and wait for db */ + bool pkts_without_db; + /* Holds the empty requests for TX/RX OOO completions */ union { uint16_t *empty_tx_reqs; -- 2.25.1
On 1/26/2021 6:32 PM, Michal Krawczyk wrote:
> From: Igor Chauskin <igorch@amazon.com>
>
> Before starting transmission of Tx burst, the driver checked the
> available space in the sq and limited the number of packets for
> transmission accordingly.
> The calculation was incorrect for fragmented packets and potentially had
> significantly limited the length of Tx bursts.
>
> This patch removes the assessment and pushes packets to the sq as long
> as the burst is not exhausted and space is available in the sq.
>
> Correct evaluation of the required space isn't possible before the burst
> because it depends on the number of segments of each packet.
> This patch adds per-packet space evaluation for each packet before
> attempting to process it. In case there is not enough queue space, the
> burst will just stop without error.
>
> Signed-off-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Michal Krawczyk <mk@semihalf.com>
Hi Michal, Igor,
Can you please provide fixes line for the patch, and I assume you want it to be
backported?
On 1/26/2021 6:32 PM, Michal Krawczyk wrote:
> From: Igor Chauskin <igorch@amazon.com>
>
> Add per-tx-ring flag for packets that were pushed to HW but await
> doorbell. That is to prevent a situation when a doorbell is sent due to
> reaching Tx burst threshold and next send fails (e.g., due to queue
> full). In such case we shouldn't send another doorbell because there are
> no actual packets waiting for transmission.
>
> Signed-off-by: Igor Chauskin <igorch@amazon.com>
> Reviewed-by: Michal Krawczyk <mk@semihalf.com>
Same here, can you please provide fixes line for the patch?
pt., 29 sty 2021 o 13:07 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 1/26/2021 6:32 PM, Michal Krawczyk wrote:
> > From: Igor Chauskin <igorch@amazon.com>
> >
> > Before starting transmission of Tx burst, the driver checked the
> > available space in the sq and limited the number of packets for
> > transmission accordingly.
> > The calculation was incorrect for fragmented packets and potentially had
> > significantly limited the length of Tx bursts.
> >
> > This patch removes the assessment and pushes packets to the sq as long
> > as the burst is not exhausted and space is available in the sq.
> >
> > Correct evaluation of the required space isn't possible before the burst
> > because it depends on the number of segments of each packet.
> > This patch adds per-packet space evaluation for each packet before
> > attempting to process it. In case there is not enough queue space, the
> > burst will just stop without error.
> >
> > Signed-off-by: Igor Chauskin <igorch@amazon.com>
> > Reviewed-by: Michal Krawczyk <mk@semihalf.com>
>
> Hi Michal, Igor,
>
> Can you please provide fixes line for the patch, and I assume you want it to be
> backported?
>
Hi Ferruh,
thanks for the note! I'll update missing fixlines in v3 for both patch 4 and 5.
Thanks,
Michal
On 1/26/2021 6:32 PM, Michal Krawczyk wrote:
> Hi,
>
> this patchset contains few bug fixes for the ENA PMD and the version
> upgrade to v2.2.1. Besides that, the validation of the Rx req ID was
> optimized.
>
> Best regards,
> Michal
>
> ---
> v2:
> * Add patch preventing double doorbell on Tx.
>
> Amit Bernstein (1):
> net/ena: Tx doorbell statistics fix
>
> Ido Segev (1):
> net/ena: flush Rx buffers memory pool cache
>
> Igor Chauskin (2):
> net/ena: fix Tx sq free space assessment
> net/ena: prevent double doorbell
>
> Michal Krawczyk (1):
> net/ena: validate Rx req id upon acquiring the desc
>
For first three patches,
Applied to dpdk-next-net/main, thanks.
For 4/5 & 5/5 waiting for fixes line, no need to send new version for them, if
you can provide them, I can add in next-net.
pt., 29 sty 2021 o 13:17 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>
> On 1/26/2021 6:32 PM, Michal Krawczyk wrote:
> > Hi,
> >
> > this patchset contains few bug fixes for the ENA PMD and the version
> > upgrade to v2.2.1. Besides that, the validation of the Rx req ID was
> > optimized.
> >
> > Best regards,
> > Michal
> >
> > ---
> > v2:
> > * Add patch preventing double doorbell on Tx.
> >
> > Amit Bernstein (1):
> > net/ena: Tx doorbell statistics fix
> >
> > Ido Segev (1):
> > net/ena: flush Rx buffers memory pool cache
> >
> > Igor Chauskin (2):
> > net/ena: fix Tx sq free space assessment
> > net/ena: prevent double doorbell
> >
> > Michal Krawczyk (1):
> > net/ena: validate Rx req id upon acquiring the desc
> >
>
> For first three patches,
> Applied to dpdk-next-net/main, thanks.
>
>
> For 4/5 & 5/5 waiting for fixes line, no need to send new version for them, if
> you can provide them, I can add in next-net.
Patch 4/5 fixline:
Fixes: 2061fe41f212 ("net/ena: linearize Tx mbuf")
Cc: stable@dpdk.org
Patch 5/5 fixline:
Fixes: c7519ea5eb8d ("net/ena: call additional doorbells if needed")
Cc: stable@dpdk.org
Thanks!
Michal
On 1/29/2021 12:33 PM, Michał Krawczyk wrote:
> pt., 29 sty 2021 o 13:17 Ferruh Yigit <ferruh.yigit@intel.com> napisał(a):
>>
>> On 1/26/2021 6:32 PM, Michal Krawczyk wrote:
>>> Hi,
>>>
>>> this patchset contains few bug fixes for the ENA PMD and the version
>>> upgrade to v2.2.1. Besides that, the validation of the Rx req ID was
>>> optimized.
>>>
>>> Best regards,
>>> Michal
>>>
>>> ---
>>> v2:
>>> * Add patch preventing double doorbell on Tx.
>>>
>>> Amit Bernstein (1):
>>> net/ena: Tx doorbell statistics fix
>>>
>>> Ido Segev (1):
>>> net/ena: flush Rx buffers memory pool cache
>>>
>>> Igor Chauskin (2):
>>> net/ena: fix Tx sq free space assessment
>>> net/ena: prevent double doorbell
>>>
>>> Michal Krawczyk (1):
>>> net/ena: validate Rx req id upon acquiring the desc
>>>
>>
>> For first three patches,
>> Applied to dpdk-next-net/main, thanks.
>>
>>
>> For 4/5 & 5/5 waiting for fixes line, no need to send new version for them, if
>> you can provide them, I can add in next-net.
>
> Patch 4/5 fixline:
>
> Fixes: 2061fe41f212 ("net/ena: linearize Tx mbuf")
> Cc: stable@dpdk.org
>
> Patch 5/5 fixline:
>
> Fixes: c7519ea5eb8d ("net/ena: call additional doorbells if needed")
> Cc: stable@dpdk.org
>
For both,
Applied to dpdk-next-net/main, thanks.