From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E8CD241DCC; Mon, 13 Mar 2023 13:59:29 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C2D4B406BC; Mon, 13 Mar 2023 13:59:29 +0100 (CET) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by mails.dpdk.org (Postfix) with ESMTP id B2CD340151; Mon, 13 Mar 2023 13:59:28 +0100 (CET) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 32DBAhYl015833; Mon, 13 Mar 2023 05:59:27 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : mime-version : content-transfer-encoding : content-type; s=pfpt0220; bh=Q+pKFe/3p9smPHlwCpQKDXXHoAztTFbmvAM87YfVvjo=; b=BFky3nEk8FxqZgj/q3QLj7atUEg+C9ux18dmGBLc4sN/IbbmeCmTVHIrVchtFSvEgkOT GQArgZLzf4Ps3oD0CoFVC8F7lLcICiC4AtW2UjoqIzh1He+VY5nKKLgE1slSJkQBHvND SzfzHeMZ3o03qPDDAn2cX2g/fZIx5R2QCye09Bg+KBRvZuFb9swacHEi00lW8NjGeQF+ PPDvisrmKUDDf3Xesbq2b3fDRmvZPK7TPg4PMTbxecFkRACcdYg3UkMG48djSMYGuPXS 6Bb8sX+IFOL9nR6NRknDLhcd4eoWQb3yLAABBnjm2OgOarMe9sbO8Ct1iP69dcb1drW9 qw== Received: from dc5-exch01.marvell.com ([199.233.59.181]) by mx0a-0016f401.pphosted.com (PPS) with ESMTPS id 3p9xxms7k0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Mon, 13 Mar 2023 05:59:27 -0700 Received: from DC5-EXCH01.marvell.com (10.69.176.38) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Mon, 13 Mar 2023 05:59:25 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server id 15.0.1497.42 via Frontend Transport; Mon, 13 Mar 2023 05:59:25 -0700 Received: from cavium-DT10.. (unknown [10.28.34.39]) by maili.marvell.com (Postfix) with ESMTP id 41F5C5B6921; Mon, 13 Mar 2023 05:59:22 -0700 (PDT) From: Volodymyr Fialko To: , Reshma Pattan , David Marchand , Andrew Rybchenko CC: , , Volodymyr Fialko , Subject: [PATCH v2] reorder: fix registration of dynamic field in mbuf Date: Mon, 13 Mar 2023 13:59:12 +0100 Message-ID: <20230313125912.2650078-1-vfialko@marvell.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Proofpoint-GUID: QQjUOPMRrQDRL3gHx6PHGUDam9VUPW-l X-Proofpoint-ORIG-GUID: QQjUOPMRrQDRL3gHx6PHGUDam9VUPW-l X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22 definitions=2023-03-13_05,2023-03-13_02,2023-02-09_01 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org It's possible to initialize reorder buffer with user allocated memory via rte_reorder_init() function. In such case rte_reorder_create() is not required and reorder dynamic field in rte_mbuf will not be registered. Both reorder lib and mbuf dynamic field are using `rte_mcfg_tailq` read/write lock for synchronization, to avoid deadlocking move reorder buffer initialization before queue insertion. Fixes: 01f3496695b5 ("reorder: switch sequence number to dynamic mbuf field") Cc: stable@dpdk.org Signed-off-by: Volodymyr Fialko --- lib/reorder/rte_reorder.c | 94 +++++++++++++++++++++++++-------------- lib/reorder/rte_reorder.h | 1 + 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/lib/reorder/rte_reorder.c b/lib/reorder/rte_reorder.c index 6e029c9e02..66d2cc07b7 100644 --- a/lib/reorder/rte_reorder.c +++ b/lib/reorder/rte_reorder.c @@ -60,6 +60,11 @@ rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize, { const unsigned int min_bufsize = sizeof(*b) + (2 * size * sizeof(struct rte_mbuf *)); + static const struct rte_mbuf_dynfield reorder_seqn_dynfield_desc = { + .name = RTE_REORDER_SEQN_DYNFIELD_NAME, + .size = sizeof(rte_reorder_seqn_t), + .align = __alignof__(rte_reorder_seqn_t), + }; if (b == NULL) { RTE_LOG(ERR, REORDER, "Invalid reorder buffer parameter:" @@ -86,6 +91,14 @@ rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize, return NULL; } + rte_reorder_seqn_dynfield_offset = rte_mbuf_dynfield_register(&reorder_seqn_dynfield_desc); + if (rte_reorder_seqn_dynfield_offset < 0) { + RTE_LOG(ERR, REORDER, "Failed to register mbuf field for reorder sequence" + " number, rte_errno: %i\n", rte_errno); + rte_errno = ENOMEM; + return NULL; + } + memset(b, 0, bufsize); strlcpy(b->name, name, sizeof(b->name)); b->memsize = bufsize; @@ -98,21 +111,45 @@ rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize, return b; } +/* + * Insert new entry into global list. + * Returns pointer to already inserted entry if such exists, or to newly inserted one. + */ +static struct rte_tailq_entry* +rte_reorder_entry_insert(struct rte_tailq_entry *new_te) +{ + struct rte_reorder_list *reorder_list; + struct rte_reorder_buffer *b, *nb; + struct rte_tailq_entry *te; + + rte_mcfg_tailq_write_lock(); + + reorder_list = RTE_TAILQ_CAST(rte_reorder_tailq.head, rte_reorder_list); + /* guarantee there's no existing */ + TAILQ_FOREACH(te, reorder_list, next) { + b = (struct rte_reorder_buffer *) te->data; + nb = (struct rte_reorder_buffer *) new_te->data; + if (strncmp(nb->name, b->name, RTE_REORDER_NAMESIZE) == 0) + break; + } + + if (te == NULL) { + TAILQ_INSERT_TAIL(reorder_list, new_te, next); + te = new_te; + } + + rte_mcfg_tailq_write_unlock(); + + return te; +} + struct rte_reorder_buffer* rte_reorder_create(const char *name, unsigned socket_id, unsigned int size) { struct rte_reorder_buffer *b = NULL; - struct rte_tailq_entry *te; - struct rte_reorder_list *reorder_list; + struct rte_tailq_entry *te, *te_inserted; const unsigned int bufsize = sizeof(struct rte_reorder_buffer) + (2 * size * sizeof(struct rte_mbuf *)); - static const struct rte_mbuf_dynfield reorder_seqn_dynfield_desc = { - .name = RTE_REORDER_SEQN_DYNFIELD_NAME, - .size = sizeof(rte_reorder_seqn_t), - .align = __alignof__(rte_reorder_seqn_t), - }; - - reorder_list = RTE_TAILQ_CAST(rte_reorder_tailq.head, rte_reorder_list); /* Check user arguments. */ if (!rte_is_power_of_2(size)) { @@ -128,32 +165,12 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size) return NULL; } - rte_reorder_seqn_dynfield_offset = - rte_mbuf_dynfield_register(&reorder_seqn_dynfield_desc); - if (rte_reorder_seqn_dynfield_offset < 0) { - RTE_LOG(ERR, REORDER, "Failed to register mbuf field for reorder sequence number\n"); - rte_errno = ENOMEM; - return NULL; - } - - rte_mcfg_tailq_write_lock(); - - /* guarantee there's no existing */ - TAILQ_FOREACH(te, reorder_list, next) { - b = (struct rte_reorder_buffer *) te->data; - if (strncmp(name, b->name, RTE_REORDER_NAMESIZE) == 0) - break; - } - if (te != NULL) - goto exit; - /* allocate tailq entry */ te = rte_zmalloc("REORDER_TAILQ_ENTRY", sizeof(*te), 0); if (te == NULL) { RTE_LOG(ERR, REORDER, "Failed to allocate tailq entry\n"); rte_errno = ENOMEM; - b = NULL; - goto exit; + return NULL; } /* Allocate memory to store the reorder buffer structure. */ @@ -162,14 +179,23 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size) RTE_LOG(ERR, REORDER, "Memzone allocation failed\n"); rte_errno = ENOMEM; rte_free(te); + return NULL; } else { - rte_reorder_init(b, bufsize, name, size); + if (rte_reorder_init(b, bufsize, name, size) == NULL) { + rte_free(b); + rte_free(te); + return NULL; + } te->data = (void *)b; - TAILQ_INSERT_TAIL(reorder_list, te, next); } -exit: - rte_mcfg_tailq_write_unlock(); + te_inserted = rte_reorder_entry_insert(te); + if (te_inserted != te) { + rte_free(b); + rte_free(te); + return te_inserted->data; + } + return b; } diff --git a/lib/reorder/rte_reorder.h b/lib/reorder/rte_reorder.h index cc95015fa6..9a5998f2f6 100644 --- a/lib/reorder/rte_reorder.h +++ b/lib/reorder/rte_reorder.h @@ -82,6 +82,7 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size); * The initialized reorder buffer instance, or NULL on error * On error case, rte_errno will be set appropriately: * - EINVAL - invalid parameters + * - ENOMEM - not enough memory to register dynamic field */ struct rte_reorder_buffer * rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize, -- 2.34.1