Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/135482/?format=api
{ "id": 135482, "url": "http://patches.dpdk.org/api/patches/135482/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/20231221180529.18687-8-ajit.khaparde@broadcom.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<20231221180529.18687-8-ajit.khaparde@broadcom.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20231221180529.18687-8-ajit.khaparde@broadcom.com", "date": "2023-12-21T18:05:18", "name": "[07/18] net/bnxt: reattempt mbuf allocation for Rx and AGG rings", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "ec454c9dad2c8a70642eab655ad7ad4abd6419b3", "submitter": { "id": 501, "url": "http://patches.dpdk.org/api/people/501/?format=api", "name": "Ajit Khaparde", "email": "ajit.khaparde@broadcom.com" }, "delegate": { "id": 1766, "url": "http://patches.dpdk.org/api/users/1766/?format=api", "username": "ajitkhaparde", "first_name": "Ajit", "last_name": "Khaparde", "email": "ajit.khaparde@broadcom.com" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/20231221180529.18687-8-ajit.khaparde@broadcom.com/mbox/", "series": [ { "id": 30650, "url": "http://patches.dpdk.org/api/series/30650/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=30650", "date": "2023-12-21T18:05:11", "name": "bnxt patchset", "version": 1, "mbox": "http://patches.dpdk.org/series/30650/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/135482/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/135482/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id D6A6143752;\n\tThu, 21 Dec 2023 19:06:37 +0100 (CET)", "from mails.dpdk.org (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id 4686B42EC4;\n\tThu, 21 Dec 2023 19:05:48 +0100 (CET)", "from mail-pl1-f179.google.com (mail-pl1-f179.google.com\n [209.85.214.179])\n by mails.dpdk.org (Postfix) with ESMTP id 7009742E9E\n for <dev@dpdk.org>; Thu, 21 Dec 2023 19:05:45 +0100 (CET)", "by mail-pl1-f179.google.com with SMTP id\n d9443c01a7336-1d04c097e34so8035405ad.0\n for <dev@dpdk.org>; Thu, 21 Dec 2023 10:05:45 -0800 (PST)", "from C02GC2QQMD6T.wifi.broadcom.net ([192.19.223.252])\n by smtp.gmail.com with ESMTPSA id\n j17-20020a170902da9100b001d3b3ac2d7bsm1916379plx.245.2023.12.21.10.05.42\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Thu, 21 Dec 2023 10:05:43 -0800 (PST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=broadcom.com; s=google; t=1703181944; x=1703786744; darn=dpdk.org;\n h=mime-version:references:in-reply-to:message-id:date:subject:cc:to\n :from:from:to:cc:subject:date:message-id:reply-to;\n bh=YddV6JzjyB2l1hKlMCgBNjI9dWf95KTA0f56wSF9fmY=;\n b=IMJbsHmsmGLc2SNy8p6UGtkgea5fMRuueYypauPvG3Z/Gg2JcXS0I0a9Y3QIOLZV5k\n jn2Nju1gCOCqOlY4Nlg/eO1PTOdGRB1EaYEK928Po7CAwdh+pac6J3NaqJSsPLrxztgR\n diQh901R+6nGayr5JpIavfVfDC0Gf5IJWv+PA=", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20230601; t=1703181944; x=1703786744;\n h=mime-version:references:in-reply-to:message-id:date:subject:cc:to\n :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n bh=YddV6JzjyB2l1hKlMCgBNjI9dWf95KTA0f56wSF9fmY=;\n b=olxQ23HTUWcI+sLTqc6lyCvkk2baJnmR/coXO0Xi1M+VQwF5u5e0SYx8cwhVeAelO7\n w9Ij5fLHdrurNTKAxVwvu3cAJfKvK2VOah7PpAIq+Um3PH+eRU0Mr0P92BLsjDSQ7PgW\n vbba1/yGtiNT4PhDfiA/zhQjwhrOrcKSZAWOZx2rceinykzuowckzJjWKVXDTbn2H6b7\n 1POQZELKesHBNOtU3roVhAXyZ+SBxHftt4RuJwWZiPDlHx2mCrs+i5aWlhZQ5jwHNeK3\n qLG9MXRISTuQIXIDQIvKH03I9buf9mu/va4sd+8yxtwopZmJEjITySBG2aO73pFhr2nF\n WyIw==", "X-Gm-Message-State": "AOJu0YwhKLUT+nKmvkp7itqJwwDdza6ss1MWR9OphwihSRWTMjd2ynAU\n 8TNYygbFRZWZAObZvLJ2oF55Z97ze7sWRcyfuTFQ8t0hlm5kBfDJbffPVFbLbdNnHxXZ8YuUern\n XWIM5mAZLwavRbvEmxESfJwsyrgipDH/xwxjVm6IbkTDmJ0NqLT9PnwY1uhzOtSbk3x7qSJc+vR\n 0=", "X-Google-Smtp-Source": "\n AGHT+IEVKHlOxXB9/cIhfsKBZn5yiHj0y5XQC4CRryyl56kTnJxCE8NBR3kNgZiASGhgLgiPvJivkg==", "X-Received": "by 2002:a17:902:7209:b0:1d3:f783:5133 with SMTP id\n ba9-20020a170902720900b001d3f7835133mr1143432plb.0.1703181943797;\n Thu, 21 Dec 2023 10:05:43 -0800 (PST)", "From": "Ajit Khaparde <ajit.khaparde@broadcom.com>", "To": "dev@dpdk.org", "Cc": "Somnath Kotur <somnath.kotur@broadcom.com>", "Subject": "[PATCH 07/18] net/bnxt: reattempt mbuf allocation for Rx and AGG\n rings", "Date": "Thu, 21 Dec 2023 10:05:18 -0800", "Message-Id": "<20231221180529.18687-8-ajit.khaparde@broadcom.com>", "X-Mailer": "git-send-email 2.39.2 (Apple Git-143)", "In-Reply-To": "<20231221180529.18687-1-ajit.khaparde@broadcom.com>", "References": "<20231221180529.18687-1-ajit.khaparde@broadcom.com>", "MIME-Version": "1.0", "Content-Type": "multipart/signed; protocol=\"application/pkcs7-signature\";\n micalg=sha-256; boundary=\"000000000000c9885b060d08f22e\"", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org" }, "content": "Normally the PMD allocates a new mbuf for every mbuf consumed.\nIn case of mbuf alloc failure, that slot in the Rx or AGG ring remains\nempty till a new mbuf is not allocated for that slot. If this happens\ntoo frequently the Rx ring or the aggregation ring could be completely\ndrained of mbufs and can cause unexpected behavior.\n\nTo prevent this, in case of an mbuf allocation failure, set a bit and\ntry to reattempt mbuf allocation to fill the empty slots. Since this\nshould not happen under normal circumstances, it should not impact\nregular Rx performance.\n\nThe need_realloc bit is set in the RxQ if mbuf allocation fails for\nRx ring or the AGG ring.\n\nAs long as the application calls the Rx burst function even in cases\nwhere the Rx rings became completely empty, the logic should be able to\nreattempt buffer allocation for the associated Rx and aggregation rings.\n\nSigned-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>\nReviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>\n---\n drivers/net/bnxt/bnxt_rxq.h | 1 +\n drivers/net/bnxt/bnxt_rxr.c | 101 ++++++++++++++++++++++--------------\n 2 files changed, 64 insertions(+), 38 deletions(-)", "diff": "diff --git a/drivers/net/bnxt/bnxt_rxq.h b/drivers/net/bnxt/bnxt_rxq.h\nindex b9908be5f4..77bc382a1d 100644\n--- a/drivers/net/bnxt/bnxt_rxq.h\n+++ b/drivers/net/bnxt/bnxt_rxq.h\n@@ -41,6 +41,7 @@ struct bnxt_rx_queue {\n \tstruct bnxt_cp_ring_info\t*cp_ring;\n \tstruct rte_mbuf\t\t\tfake_mbuf;\n \tuint64_t\t\t\trx_mbuf_alloc_fail;\n+\tuint8_t\t\t\t\tneed_realloc;\n \tconst struct rte_memzone *mz;\n };\n \ndiff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c\nindex b919922a64..c5c9f9e6e6 100644\n--- a/drivers/net/bnxt/bnxt_rxr.c\n+++ b/drivers/net/bnxt/bnxt_rxr.c\n@@ -50,6 +50,8 @@ static inline int bnxt_alloc_rx_data(struct bnxt_rx_queue *rxq,\n \tmbuf = __bnxt_alloc_rx_data(rxq->mb_pool);\n \tif (!mbuf) {\n \t\t__atomic_fetch_add(&rxq->rx_mbuf_alloc_fail, 1, __ATOMIC_RELAXED);\n+\t\t/* If buff has failed already, setting this again won't hurt */\n+\t\trxq->need_realloc = 1;\n \t\treturn -ENOMEM;\n \t}\n \n@@ -85,6 +87,8 @@ static inline int bnxt_alloc_ag_data(struct bnxt_rx_queue *rxq,\n \tmbuf = __bnxt_alloc_rx_data(rxq->mb_pool);\n \tif (!mbuf) {\n \t\t__atomic_fetch_add(&rxq->rx_mbuf_alloc_fail, 1, __ATOMIC_RELAXED);\n+\t\t/* If buff has failed already, setting this again won't hurt */\n+\t\trxq->need_realloc = 1;\n \t\treturn -ENOMEM;\n \t}\n \n@@ -139,7 +143,6 @@ static void bnxt_rx_ring_reset(void *arg)\n \tint i, rc = 0;\n \tstruct bnxt_rx_queue *rxq;\n \n-\n \tfor (i = 0; i < (int)bp->rx_nr_rings; i++) {\n \t\tstruct bnxt_rx_ring_info *rxr;\n \n@@ -357,7 +360,8 @@ static int bnxt_rx_pages(struct bnxt_rx_queue *rxq,\n \t\tRTE_ASSERT(ag_cons <= rxr->ag_ring_struct->ring_mask);\n \t\tag_buf = &rxr->ag_buf_ring[ag_cons];\n \t\tag_mbuf = *ag_buf;\n-\t\tRTE_ASSERT(ag_mbuf != NULL);\n+\t\tif (ag_mbuf == NULL)\n+\t\t\treturn -EBUSY;\n \n \t\tag_mbuf->data_len = rte_le_to_cpu_16(rxcmp->len);\n \n@@ -452,7 +456,7 @@ static inline struct rte_mbuf *bnxt_tpa_end(\n \tRTE_ASSERT(mbuf != NULL);\n \n \tif (agg_bufs) {\n-\t\tbnxt_rx_pages(rxq, mbuf, raw_cp_cons, agg_bufs, tpa_info);\n+\t\t(void)bnxt_rx_pages(rxq, mbuf, raw_cp_cons, agg_bufs, tpa_info);\n \t}\n \tmbuf->l4_len = payload_offset;\n \n@@ -1230,8 +1234,11 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,\n \t\tbnxt_set_mark_in_mbuf(rxq->bp, rxcmp1, mbuf);\n \n reuse_rx_mbuf:\n-\tif (agg_buf)\n-\t\tbnxt_rx_pages(rxq, mbuf, &tmp_raw_cons, agg_buf, NULL);\n+\tif (agg_buf) {\n+\t\trc = bnxt_rx_pages(rxq, mbuf, &tmp_raw_cons, agg_buf, NULL);\n+\t\tif (rc != 0)\n+\t\t\treturn -EBUSY;\n+\t}\n \n #ifdef BNXT_DEBUG\n \tif (rxcmp1->errors_v2 & RX_CMP_L2_ERRORS) {\n@@ -1293,6 +1300,48 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,\n \treturn rc;\n }\n \n+static void bnxt_reattempt_buffer_alloc(struct bnxt_rx_queue *rxq)\n+{\n+\tstruct bnxt_rx_ring_info *rxr = rxq->rx_ring;\n+\tstruct bnxt_ring *ring;\n+\tuint16_t raw_prod;\n+\tuint32_t cnt;\n+\n+\t/* Assume alloc passes. On failure,\n+\t * need_realloc will be set inside bnxt_alloc_XY_data.\n+\t */\n+\trxq->need_realloc = 0;\n+\tif (!bnxt_need_agg_ring(rxq->bp->eth_dev))\n+\t\tgoto alloc_rx;\n+\n+\traw_prod = rxr->ag_raw_prod;\n+\tbnxt_prod_ag_mbuf(rxq);\n+\tif (raw_prod != rxr->ag_raw_prod)\n+\t\tbnxt_db_write(&rxr->ag_db, rxr->ag_raw_prod);\n+\n+alloc_rx:\n+\traw_prod = rxr->rx_raw_prod;\n+\tring = rxr->rx_ring_struct;\n+\tfor (cnt = 0; cnt < ring->ring_size; cnt++) {\n+\t\tstruct rte_mbuf **rx_buf;\n+\t\tuint16_t ndx;\n+\n+\t\tndx = RING_IDX(ring, raw_prod + cnt);\n+\t\trx_buf = &rxr->rx_buf_ring[ndx];\n+\n+\t\t/* Buffer already allocated for this index. */\n+\t\tif (*rx_buf != NULL && *rx_buf != &rxq->fake_mbuf)\n+\t\t\tcontinue;\n+\n+\t\t/* This slot is empty. Alloc buffer for Rx */\n+\t\tif (bnxt_alloc_rx_data(rxq, rxr, raw_prod + cnt))\n+\t\t\tbreak;\n+\n+\t\trxr->rx_raw_prod = raw_prod + cnt;\n+\t\tbnxt_db_write(&rxr->rx_db, rxr->rx_raw_prod);\n+\t}\n+}\n+\n uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,\n \t\t\t uint16_t nb_pkts)\n {\n@@ -1302,7 +1351,6 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,\n \tuint16_t rx_raw_prod = rxr->rx_raw_prod;\n \tuint16_t ag_raw_prod = rxr->ag_raw_prod;\n \tuint32_t raw_cons = cpr->cp_raw_cons;\n-\tbool alloc_failed = false;\n \tuint32_t cons;\n \tint nb_rx_pkts = 0;\n \tint nb_rep_rx_pkts = 0;\n@@ -1358,10 +1406,8 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,\n \t\t\t\tbreak;\n \t\t\telse if (rc == -ENODEV)\t/* completion for representor */\n \t\t\t\tnb_rep_rx_pkts++;\n-\t\t\telse if (rc == -ENOMEM) {\n+\t\t\telse if (rc == -ENOMEM)\n \t\t\t\tnb_rx_pkts++;\n-\t\t\t\talloc_failed = true;\n-\t\t\t}\n \t\t} else if (!BNXT_NUM_ASYNC_CPR(rxq->bp)) {\n \t\t\tevt =\n \t\t\tbnxt_event_hwrm_resp_handler(rxq->bp,\n@@ -1372,7 +1418,12 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,\n \t\t}\n \n \t\traw_cons = NEXT_RAW_CMP(raw_cons);\n-\t\tif (nb_rx_pkts == nb_pkts || nb_rep_rx_pkts == nb_pkts || evt)\n+\t\t/*\n+\t\t * The HW reposting may fall behind if mbuf allocation has\n+\t\t * failed. Break and reattempt allocation to prevent that.\n+\t\t */\n+\t\tif (nb_rx_pkts == nb_pkts || nb_rep_rx_pkts == nb_pkts || evt ||\n+\t\t rxq->need_realloc != 0)\n \t\t\tbreak;\n \t}\n \n@@ -1395,35 +1446,9 @@ uint16_t bnxt_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,\n \t/* Ring the AGG ring DB */\n \tif (ag_raw_prod != rxr->ag_raw_prod)\n \t\tbnxt_db_write(&rxr->ag_db, rxr->ag_raw_prod);\n-\n-\t/* Attempt to alloc Rx buf in case of a previous allocation failure. */\n-\tif (alloc_failed) {\n-\t\tint cnt;\n-\n-\t\trx_raw_prod = RING_NEXT(rx_raw_prod);\n-\t\tfor (cnt = 0; cnt < nb_rx_pkts + nb_rep_rx_pkts; cnt++) {\n-\t\t\tstruct rte_mbuf **rx_buf;\n-\t\t\tuint16_t ndx;\n-\n-\t\t\tndx = RING_IDX(rxr->rx_ring_struct, rx_raw_prod + cnt);\n-\t\t\trx_buf = &rxr->rx_buf_ring[ndx];\n-\n-\t\t\t/* Buffer already allocated for this index. */\n-\t\t\tif (*rx_buf != NULL && *rx_buf != &rxq->fake_mbuf)\n-\t\t\t\tcontinue;\n-\n-\t\t\t/* This slot is empty. Alloc buffer for Rx */\n-\t\t\tif (!bnxt_alloc_rx_data(rxq, rxr, rx_raw_prod + cnt)) {\n-\t\t\t\trxr->rx_raw_prod = rx_raw_prod + cnt;\n-\t\t\t\tbnxt_db_write(&rxr->rx_db, rxr->rx_raw_prod);\n-\t\t\t} else {\n-\t\t\t\tPMD_DRV_LOG(ERR, \"Alloc mbuf failed\\n\");\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n done:\n+\tif (unlikely(rxq->need_realloc))\n+\t\tbnxt_reattempt_buffer_alloc(rxq);\n \treturn nb_rx_pkts;\n }\n \n", "prefixes": [ "07/18" ] }