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 1C767A00BE; Wed, 20 Apr 2022 11:59:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EE1FB4068E; Wed, 20 Apr 2022 11:59:25 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id D639940687 for ; Wed, 20 Apr 2022 11:59:24 +0200 (CEST) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v1 3/5] ethdev: add API for direct rearm mode Date: Wed, 20 Apr 2022 11:59:23 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D86FE3@smartserver.smartshare.dk> In-Reply-To: <20220420081650.2043183-4-feifei.wang2@arm.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v1 3/5] ethdev: add API for direct rearm mode Thread-Index: AdhUjxD2rCt08UyPT7i+63A+rOY+MQABsfeg References: <20220420081650.2043183-1-feifei.wang2@arm.com> <20220420081650.2043183-4-feifei.wang2@arm.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Feifei Wang" , "Thomas Monjalon" , "Ferruh Yigit" , "Andrew Rybchenko" , "Ray Kinsella" 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, 20 April 2022 10.17 >=20 > Add API for enabling direct rearm mode and for mapping RX and TX > queues. Currently, the API supports 1:1(txq : rxq) mapping. >=20 > Suggested-by: Honnappa Nagarahalli > Signed-off-by: Feifei Wang > Reviewed-by: Ruifeng Wang > Reviewed-by: Honnappa Nagarahalli > --- > lib/ethdev/ethdev_driver.h | 15 +++++++++++++++ > lib/ethdev/rte_ethdev.c | 14 ++++++++++++++ > lib/ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++ > lib/ethdev/version.map | 1 + > 4 files changed, 61 insertions(+) >=20 > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > index 69d9dc21d8..22022f6da9 100644 > --- a/lib/ethdev/ethdev_driver.h > +++ b/lib/ethdev/ethdev_driver.h > @@ -485,6 +485,16 @@ typedef int (*eth_rx_enable_intr_t)(struct > rte_eth_dev *dev, > typedef int (*eth_rx_disable_intr_t)(struct rte_eth_dev *dev, > uint16_t rx_queue_id); >=20 > +/** @internal Enable direct rearm of a receive queue of an Ethernet > device. */ > +typedef int (*eth_rx_direct_rearm_enable_t)(struct rte_eth_dev *dev, > + uint16_t queue_id); > + > +/**< @internal map Rx/Tx queue of direct rearm mode */ > +typedef int (*eth_rx_direct_rearm_map_t)(struct rte_eth_dev *dev, > + uint16_t rx_queue_id, > + uint16_t tx_port_id, > + uint16_t tx_queue_id); > + > /** @internal Release memory resources allocated by given Rx/Tx = queue. > */ > typedef void (*eth_queue_release_t)(struct rte_eth_dev *dev, > uint16_t queue_id); > @@ -1152,6 +1162,11 @@ struct eth_dev_ops { > /** Disable Rx queue interrupt */ > eth_rx_disable_intr_t rx_queue_intr_disable; >=20 > + /** Enable Rx queue direct rearm mode */ > + eth_rx_direct_rearm_enable_t rx_queue_direct_rearm_enable; A disable function seems to be missing. > + /** Map Rx/Tx queue for direct rearm mode */ > + eth_rx_direct_rearm_map_t rx_queue_direct_rearm_map; > + > eth_tx_queue_setup_t tx_queue_setup;/**< Set up device Tx > queue */ > eth_queue_release_t tx_queue_release; /**< Release Tx > queue */ > eth_tx_done_cleanup_t tx_done_cleanup;/**< Free Tx ring > mbufs */ > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > index 29a3d80466..8e6f0284f4 100644 > --- a/lib/ethdev/rte_ethdev.c > +++ b/lib/ethdev/rte_ethdev.c > @@ -2139,6 +2139,20 @@ rte_eth_tx_hairpin_queue_setup(uint16_t = port_id, > uint16_t tx_queue_id, > return eth_err(port_id, ret); > } >=20 > +int > +rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t rx_queue_id, > + uint16_t tx_port_id, uint16_t tx_queue_id) > +{ > + struct rte_eth_dev *dev; > + > + dev =3D &rte_eth_devices[rx_port_id]; > + (*dev->dev_ops->rx_queue_direct_rearm_enable)(dev, rx_queue_id); > + (*dev->dev_ops->rx_queue_direct_rearm_map)(dev, rx_queue_id, > + tx_port_id, tx_queue_id); Here you enable the mapping before you configure it. It could cause the = driver to use an uninitialized map, if it processes packets between = these two function calls. Error handling is missing. Not all drivers support this feature, and the = parameters should be validated. Regarding driver support, the driver should also expose a capability = flag to the application, similar to the RTE_ETH_DEV_CAPA_RXQ_SHARE or = RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE flags. The documentation for this flag = could include the description of all the restrictions to using it. > + > + return 0; > +} > + > int > rte_eth_hairpin_bind(uint16_t tx_port, uint16_t rx_port) > { > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h > index 04cff8ee10..4a431fcbed 100644 > --- a/lib/ethdev/rte_ethdev.h > +++ b/lib/ethdev/rte_ethdev.h > @@ -5190,6 +5190,37 @@ __rte_experimental > int rte_eth_dev_hairpin_capability_get(uint16_t port_id, > struct rte_eth_hairpin_cap *cap); >=20 > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change, or be removed, without prior > notice > + * > + * Enable direct re-arm mode. In this mode the RX queue will be re- > armed using > + * buffers that have completed transmission on the transmit side. > + * > + * @note > + * It is assumed that the buffers have completed transmission = belong > to the > + * mempool used at the receive side, and have refcnt =3D 1. > + * > + * @param rx_port_id > + * Port identifying the receive side. > + * @param rx_queue_id > + * The index of the receive queue identifying the receive side. > + * The value must be in the range [0, nb_rx_queue - 1] previously > supplied > + * to rte_eth_dev_configure(). > + * @param tx_port_id > + * Port identifying the transmit side. > + * @param tx_queue_id > + * The index of the transmit queue identifying the transmit side. > + * The value must be in the range [0, nb_tx_queue - 1] previously > supplied > + * to rte_eth_dev_configure(). > + * > + * @return > + * - (0) if successful. > + */ > +__rte_experimental > +int rte_eth_direct_rxrearm_map(uint16_t rx_port_id, uint16_t > rx_queue_id, > + uint16_t tx_port_id, uint16_t tx_queue_id); > + I agree with the parameters to your proposed API here. Since the = relevant use case only needs 1:1 mapping, exposing an API function to = take some sort of array with N:M mappings would be premature, and = probably not ever come into play anyway. How do you remove, disable and/or change a mapping? > /** > * @warning > * @b EXPERIMENTAL: this structure may change without prior notice. > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map > index 20391ab29e..68d664498c 100644 > --- a/lib/ethdev/version.map > +++ b/lib/ethdev/version.map > @@ -279,6 +279,7 @@ EXPERIMENTAL { > rte_flow_async_action_handle_create; > rte_flow_async_action_handle_destroy; > rte_flow_async_action_handle_update; > + rte_eth_direct_rxrearm_map; > }; >=20 > INTERNAL { > -- > 2.25.1 >=20