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 C7264A00C2; Wed, 4 Jan 2023 09:21:48 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B3DD540A82; Wed, 4 Jan 2023 09:21:48 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id F303240697 for ; Wed, 4 Jan 2023 09:21:46 +0100 (CET) Content-class: urn:content-classes:message Subject: RE: [PATCH v3 1/3] ethdev: enable direct rearm with separate API MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Date: Wed, 4 Jan 2023 09:21:43 +0100 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D8761D@smartserver.smartshare.dk> In-Reply-To: <20230104073043.1120168-2-feifei.wang2@arm.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v3 1/3] ethdev: enable direct rearm with separate API Thread-Index: AdkgDn6w2ZZVhCd1TniHhw9vq7HYegAAxeTg References: <20220420081650.2043183-1-feifei.wang2@arm.com> <20230104073043.1120168-1-feifei.wang2@arm.com> <20230104073043.1120168-2-feifei.wang2@arm.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Feifei Wang" , "Thomas Monjalon" , "Ferruh Yigit" , "Andrew Rybchenko" Cc: , , , "Honnappa Nagarahalli" , "Ruifeng Wang" 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 > From: Feifei Wang [mailto:feifei.wang2@arm.com] > Sent: Wednesday, 4 January 2023 08.31 >=20 > Add 'tx_fill_sw_ring' and 'rx_flush_descriptor' API into direct rearm > mode for separate Rx and Tx Operation. And this can support different > multiple sources in direct rearm mode. For examples, Rx driver is > ixgbe, > and Tx driver is i40e. >=20 > Suggested-by: Honnappa Nagarahalli > Suggested-by: Ruifeng Wang > Signed-off-by: Feifei Wang > Reviewed-by: Ruifeng Wang > Reviewed-by: Honnappa Nagarahalli > --- This feature looks very promising for performance. I am pleased to see = progress on it. Please confirm that the fast path functions are still thread safe, i.e. = one EAL thread may be calling rte_eth_rx_burst() while another EAL = thread is calling rte_eth_tx_burst(). A few more comments below. > lib/ethdev/ethdev_driver.h | 10 ++ > lib/ethdev/ethdev_private.c | 2 + > lib/ethdev/rte_ethdev.c | 52 +++++++++++ > lib/ethdev/rte_ethdev.h | 174 = +++++++++++++++++++++++++++++++++++ > lib/ethdev/rte_ethdev_core.h | 11 +++ > lib/ethdev/version.map | 6 ++ > 6 files changed, 255 insertions(+) >=20 > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > index 6a550cfc83..bc539ec862 100644 > --- a/lib/ethdev/ethdev_driver.h > +++ b/lib/ethdev/ethdev_driver.h > @@ -59,6 +59,10 @@ struct rte_eth_dev { > eth_rx_descriptor_status_t rx_descriptor_status; > /** Check the status of a Tx descriptor */ > eth_tx_descriptor_status_t tx_descriptor_status; > + /** Fill Rx sw-ring with Tx buffers in direct rearm mode */ > + eth_tx_fill_sw_ring_t tx_fill_sw_ring; What is "Rx sw-ring"? Please confirm that this is not an Intel PMD = specific term and/or implementation detail, e.g. by providing a = conceptual implementation for a non-Intel PMD, e.g. mlx5. Please note: I do not request the ability to rearm between drivers from = different vendors, I only request that the public ethdev API uses = generic terms and concepts, so any NIC vendor can implement the = direct-rearm functions in their PMDs. > + /** Flush Rx descriptor in direct rearm mode */ > + eth_rx_flush_descriptor_t rx_flush_descriptor; descriptor -> descriptors. There are more than one. Both in comment and = function name. [...] > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index c129ca1eaf..381c3d535f 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -1818,6 +1818,17 @@ struct rte_eth_txq_info { > uint8_t queue_state; /**< one of RTE_ETH_QUEUE_STATE_*. */ > } __rte_cache_min_aligned; >=20 > +/** > + * @internal > + * Structure used to hold pointers to internal ethdev Rx rearm data. > + * The main purpose is to load Rx queue rearm data in direct rearm > mode. > + */ > +struct rte_eth_rxq_rearm_data { > + void *rx_sw_ring; > + uint16_t *rearm_start; > + uint16_t *rearm_nb; > +} __rte_cache_min_aligned; Please add descriptions to the fields in this structure.