DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ajit Khaparde <ajit.khaparde@broadcom.com>
To: dev@dpdk.org
Cc: Somnath Kotur <somnath.kotur@broadcom.com>
Subject: [PATCH v3 07/18] net/bnxt: reattempt mbuf allocation for Rx and AGG rings
Date: Tue, 26 Dec 2023 20:21:08 -0800	[thread overview]
Message-ID: <20231227042119.72469-8-ajit.khaparde@broadcom.com> (raw)
In-Reply-To: <20231227042119.72469-1-ajit.khaparde@broadcom.com>

[-- Attachment #1: Type: text/plain, Size: 7128 bytes --]

Normally the PMD allocates a new mbuf for every mbuf consumed.
In case of mbuf alloc failure, that slot in the Rx or AGG ring remains
empty till a new mbuf is not allocated for that slot. If this happens
too frequently the Rx ring or the aggregation ring could be completely
drained of mbufs and can cause unexpected behavior.

To prevent this, in case of an mbuf allocation failure, set a bit and
try to reattempt mbuf allocation to fill the empty slots. Since this
should not happen under normal circumstances, it should not impact
regular Rx performance.

The need_realloc bit is set in the RxQ if mbuf allocation fails for
Rx ring or the AGG ring.

As long as the application calls the Rx burst function even in cases
where the Rx rings became completely empty, the logic should be able to
reattempt buffer allocation for the associated Rx and aggregation rings.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
---
 drivers/net/bnxt/bnxt_rxq.h |   1 +
 drivers/net/bnxt/bnxt_rxr.c | 101 ++++++++++++++++++++++--------------
 2 files changed, 64 insertions(+), 38 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_rxq.h b/drivers/net/bnxt/bnxt_rxq.h
index b9908be5f4..77bc382a1d 100644
--- a/drivers/net/bnxt/bnxt_rxq.h
+++ b/drivers/net/bnxt/bnxt_rxq.h
@@ -41,6 +41,7 @@ struct bnxt_rx_queue {
 	struct bnxt_cp_ring_info	*cp_ring;
 	struct rte_mbuf			fake_mbuf;
 	uint64_t			rx_mbuf_alloc_fail;
+	uint8_t				need_realloc;
 	const struct rte_memzone *mz;
 };
 
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index b919922a64..c5c9f9e6e6 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -50,6 +50,8 @@ static inline int bnxt_alloc_rx_data(struct bnxt_rx_queue *rxq,
 	mbuf = __bnxt_alloc_rx_data(rxq->mb_pool);
 	if (!mbuf) {
 		__atomic_fetch_add(&rxq->rx_mbuf_alloc_fail, 1, __ATOMIC_RELAXED);
+		/* If buff has failed already, setting this again won't hurt */
+		rxq->need_realloc = 1;
 		return -ENOMEM;
 	}
 
@@ -85,6 +87,8 @@ static inline int bnxt_alloc_ag_data(struct bnxt_rx_queue *rxq,
 	mbuf = __bnxt_alloc_rx_data(rxq->mb_pool);
 	if (!mbuf) {
 		__atomic_fetch_add(&rxq->rx_mbuf_alloc_fail, 1, __ATOMIC_RELAXED);
+		/* If buff has failed already, setting this again won't hurt */
+		rxq->need_realloc = 1;
 		return -ENOMEM;
 	}
 
@@ -139,7 +143,6 @@ static void bnxt_rx_ring_reset(void *arg)
 	int i, rc = 0;
 	struct bnxt_rx_queue *rxq;
 
-
 	for (i = 0; i < (int)bp->rx_nr_rings; i++) {
 		struct bnxt_rx_ring_info *rxr;
 
@@ -357,7 +360,8 @@ static int bnxt_rx_pages(struct bnxt_rx_queue *rxq,
 		RTE_ASSERT(ag_cons <= rxr->ag_ring_struct->ring_mask);
 		ag_buf = &rxr->ag_buf_ring[ag_cons];
 		ag_mbuf = *ag_buf;
-		RTE_ASSERT(ag_mbuf != NULL);
+		if (ag_mbuf == NULL)
+			return -EBUSY;
 
 		ag_mbuf->data_len = rte_le_to_cpu_16(rxcmp->len);
 
@@ -452,7 +456,7 @@ static inline struct rte_mbuf *bnxt_tpa_end(
 	RTE_ASSERT(mbuf != NULL);
 
 	if (agg_bufs) {
-		bnxt_rx_pages(rxq, mbuf, raw_cp_cons, agg_bufs, tpa_info);
+		(void)bnxt_rx_pages(rxq, mbuf, raw_cp_cons, agg_bufs, tpa_info);
 	}
 	mbuf->l4_len = payload_offset;
 
@@ -1230,8 +1234,11 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 		bnxt_set_mark_in_mbuf(rxq->bp, rxcmp1, mbuf);
 
 reuse_rx_mbuf:
-	if (agg_buf)
-		bnxt_rx_pages(rxq, mbuf, &tmp_raw_cons, agg_buf, NULL);
+	if (agg_buf) {
+		rc = bnxt_rx_pages(rxq, mbuf, &tmp_raw_cons, agg_buf, NULL);
+		if (rc != 0)
+			return -EBUSY;
+	}
 
 #ifdef BNXT_DEBUG
 	if (rxcmp1->errors_v2 & RX_CMP_L2_ERRORS) {
@@ -1293,6 +1300,48 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 	return rc;
 }
 
+static void bnxt_reattempt_buffer_alloc(struct bnxt_rx_queue *rxq)
+{
+	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
+	struct bnxt_ring *ring;
+	uint16_t raw_prod;
+	uint32_t cnt;
+
+	/* Assume alloc passes. On failure,
+	 * need_realloc will be set inside bnxt_alloc_XY_data.
+	 */
+	rxq->need_realloc = 0;
+	if (!bnxt_need_agg_ring(rxq->bp->eth_dev))
+		goto alloc_rx;
+
+	raw_prod = rxr->ag_raw_prod;
+	bnxt_prod_ag_mbuf(rxq);
+	if (raw_prod != rxr->ag_raw_prod)
+		bnxt_db_write(&rxr->ag_db, rxr->ag_raw_prod);
+
+alloc_rx:
+	raw_prod = rxr->rx_raw_prod;
+	ring = rxr->rx_ring_struct;
+	for (cnt = 0; cnt < ring->ring_size; cnt++) {
+		struct rte_mbuf **rx_buf;
+		uint16_t ndx;
+
+		ndx = RING_IDX(ring, raw_prod + cnt);
+		rx_buf = &rxr->rx_buf_ring[ndx];
+
+		/* Buffer already allocated for this index. */
+		if (*rx_buf != NULL && *rx_buf != &rxq->fake_mbuf)
+			continue;
+
+		/* This slot is empty. Alloc buffer for Rx */
+		if (bnxt_alloc_rx_data(rxq, rxr, raw_prod + cnt))
+			break;
+
+		rxr->rx_raw_prod = raw_prod + cnt;
+		bnxt_db_write(&rxr->rx_db, rxr->rx_raw_prod);
+	}
+}
+
 uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 			       uint16_t nb_pkts)
 {
@@ -1302,7 +1351,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	uint16_t rx_raw_prod = rxr->rx_raw_prod;
 	uint16_t ag_raw_prod = rxr->ag_raw_prod;
 	uint32_t raw_cons = cpr->cp_raw_cons;
-	bool alloc_failed = false;
 	uint32_t cons;
 	int nb_rx_pkts = 0;
 	int nb_rep_rx_pkts = 0;
@@ -1358,10 +1406,8 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 				break;
 			else if (rc == -ENODEV)	/* completion for representor */
 				nb_rep_rx_pkts++;
-			else if (rc == -ENOMEM) {
+			else if (rc == -ENOMEM)
 				nb_rx_pkts++;
-				alloc_failed = true;
-			}
 		} else if (!BNXT_NUM_ASYNC_CPR(rxq->bp)) {
 			evt =
 			bnxt_event_hwrm_resp_handler(rxq->bp,
@@ -1372,7 +1418,12 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		}
 
 		raw_cons = NEXT_RAW_CMP(raw_cons);
-		if (nb_rx_pkts == nb_pkts || nb_rep_rx_pkts == nb_pkts || evt)
+		/*
+		 * The HW reposting may fall behind if mbuf allocation has
+		 * failed. Break and reattempt allocation to prevent that.
+		 */
+		if (nb_rx_pkts == nb_pkts || nb_rep_rx_pkts == nb_pkts || evt ||
+		    rxq->need_realloc != 0)
 			break;
 	}
 
@@ -1395,35 +1446,9 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 	/* Ring the AGG ring DB */
 	if (ag_raw_prod != rxr->ag_raw_prod)
 		bnxt_db_write(&rxr->ag_db, rxr->ag_raw_prod);
-
-	/* Attempt to alloc Rx buf in case of a previous allocation failure. */
-	if (alloc_failed) {
-		int cnt;
-
-		rx_raw_prod = RING_NEXT(rx_raw_prod);
-		for (cnt = 0; cnt < nb_rx_pkts + nb_rep_rx_pkts; cnt++) {
-			struct rte_mbuf **rx_buf;
-			uint16_t ndx;
-
-			ndx = RING_IDX(rxr->rx_ring_struct, rx_raw_prod + cnt);
-			rx_buf = &rxr->rx_buf_ring[ndx];
-
-			/* Buffer already allocated for this index. */
-			if (*rx_buf != NULL && *rx_buf != &rxq->fake_mbuf)
-				continue;
-
-			/* This slot is empty. Alloc buffer for Rx */
-			if (!bnxt_alloc_rx_data(rxq, rxr, rx_raw_prod + cnt)) {
-				rxr->rx_raw_prod = rx_raw_prod + cnt;
-				bnxt_db_write(&rxr->rx_db, rxr->rx_raw_prod);
-			} else {
-				PMD_DRV_LOG(ERR, "Alloc  mbuf failed\n");
-				break;
-			}
-		}
-	}
-
 done:
+	if (unlikely(rxq->need_realloc))
+		bnxt_reattempt_buffer_alloc(rxq);
 	return nb_rx_pkts;
 }
 
-- 
2.39.2 (Apple Git-143)


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

  parent reply	other threads:[~2023-12-27  4:22 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-27  4:21 [PATCH v3 00/18] bnxt patchset Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 01/18] net/bnxt: add support for UDP GSO Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 02/18] net/bnxt: add support for compressed Rx CQE Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 03/18] net/bnxt: fix a typo while parsing link speed Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 04/18] net/bnxt: fix setting 50G and 100G forced speed Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 05/18] net/bnxt: fix speed change from 200G to 25G on Thor Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 06/18] net/bnxt: support backward compatibility Ajit Khaparde
2023-12-27  4:21 ` Ajit Khaparde [this message]
2023-12-27  4:21 ` [PATCH v3 08/18] net/bnxt: refactor Rx doorbell during Rx flush Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 09/18] net/bnxt: extend RSS hash support for P7 devices Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 10/18] net/bnxt: add flow query callback Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 11/18] net/bnxt: add ESP and AH header based RSS support Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 12/18] net/bnxt: set allmulti mode if multicast filter fails Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 13/18] net/bnxt: add VF FLR async event handler Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 14/18] net/bnxt: add tunnel TPA support Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 15/18] net/bnxt: add 400G get support for P7 devices Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 16/18] net/bnxt: query extended stats from firmware Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 17/18] net/bnxt: add AVX2 support for compressed CQE Ajit Khaparde
2023-12-27  4:21 ` [PATCH v3 18/18] net/bnxt: enable SSE mode " Ajit Khaparde
2023-12-29 16:21 ` [PATCH v3 00/18] bnxt patchset Ajit Khaparde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231227042119.72469-8-ajit.khaparde@broadcom.com \
    --to=ajit.khaparde@broadcom.com \
    --cc=dev@dpdk.org \
    --cc=somnath.kotur@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).