[dpdk-dev] [PATCH 0/4] net/ena: minor bug fixes and improvements

[dpdk-dev] [PATCH 0/4] net/ena: minor bug fixes and improvements

[Michal Krawczyk]

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

Amit Bernstein (1):
  net/ena: Tx doorbell statistics fix

Ido Segev (1):
  net/ena: flush Rx buffers memory pool cache

Igor Chauskin (1):
  net/ena: fix Tx sq free space assessment

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         | 78 +++++++++++++++-------------
 3 files changed, 45 insertions(+), 37 deletions(-)

-- 
2.25.1

[dpdk-dev] [PATCH 1/4] net/ena: flush Rx buffers memory pool cache

[Michal Krawczyk]

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

[dpdk-dev] [PATCH 2/4] net/ena: Tx doorbell statistics fix

[Michal Krawczyk]

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

[dpdk-dev] [PATCH 3/4] net/ena: validate Rx req id upon acquiring the desc

[Michal Krawczyk]

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

[dpdk-dev] [PATCH 4/4] net/ena: fix Tx sq free space assessment

[Michal Krawczyk]

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