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 BD15F41BAE; Thu, 2 Feb 2023 15:33:28 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 53EDB42D20; Thu, 2 Feb 2023 15:33:28 +0100 (CET) Received: from forward500c.mail.yandex.net (forward500c.mail.yandex.net [178.154.239.208]) by mails.dpdk.org (Postfix) with ESMTP id 32DED40EDC for ; Thu, 2 Feb 2023 15:33:27 +0100 (CET) Received: from iva1-adaa4d2a0364.qloud-c.yandex.net (iva1-adaa4d2a0364.qloud-c.yandex.net [IPv6:2a02:6b8:c0c:a0e:0:640:adaa:4d2a]) by forward500c.mail.yandex.net (Yandex) with ESMTP id A6E235F33C; Thu, 2 Feb 2023 17:33:26 +0300 (MSK) Received: by iva1-adaa4d2a0364.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id NXYj9vaZ5eA1-VYBIMtCD; Thu, 02 Feb 2023 17:33:25 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1675348405; bh=qrEVsZBCnTAPai+ZnB/WXE2D/lPqWkvDFhYQXogFRMs=; h=From:In-Reply-To:Cc:Date:References:To:Subject:Message-ID; b=UqTgc+rjgV9k7qtBR051ZVN5L+D7kiZaAovsF2oUTozy9uCgafR4dGgFOpYc965A0 pU+C/9CAhqdL5V//+UpzWab8qDS5WaYEpZ973fU+F69mtY4FtUzDLLuvffZqUeGbVM aWnrqCyqDp/SFxH2FqoNPHt+9bOVRBy4h65vzD3Q= Authentication-Results: iva1-adaa4d2a0364.qloud-c.yandex.net; dkim=pass header.i=@yandex.ru Message-ID: Date: Thu, 2 Feb 2023 14:33:23 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH v3 1/3] ethdev: enable direct rearm with separate API Content-Language: en-US To: Feifei Wang , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko Cc: dev@dpdk.org, nd@arm.com, Honnappa Nagarahalli , Ruifeng Wang References: <20220420081650.2043183-1-feifei.wang2@arm.com> <20230104073043.1120168-1-feifei.wang2@arm.com> <20230104073043.1120168-2-feifei.wang2@arm.com> From: Konstantin Ananyev In-Reply-To: <20230104073043.1120168-2-feifei.wang2@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 Hi Feifei, > 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. Thanks for your effort and thanks for taking comments provided into consideration. That approach looks much better then previous ones. Few nits below. Konstantin > Suggested-by: Honnappa Nagarahalli > Suggested-by: Ruifeng Wang > Signed-off-by: Feifei Wang > Reviewed-by: Ruifeng Wang > Reviewed-by: Honnappa Nagarahalli > --- > 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(+) > > 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; > + /** Flush Rx descriptor in direct rearm mode */ > + eth_rx_flush_descriptor_t rx_flush_descriptor; > > /** > * Device data that is shared between primary and secondary processes > @@ -504,6 +508,10 @@ typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev, > typedef void (*eth_txq_info_get_t)(struct rte_eth_dev *dev, > uint16_t tx_queue_id, struct rte_eth_txq_info *qinfo); > > +/**< @internal Get rearm data for a receive queue of an Ethernet device. */ > +typedef void (*eth_rxq_rearm_data_get_t)(struct rte_eth_dev *dev, > + uint16_t tx_queue_id, struct rte_eth_rxq_rearm_data *rxq_rearm_data); > + > typedef int (*eth_burst_mode_get_t)(struct rte_eth_dev *dev, > uint16_t queue_id, struct rte_eth_burst_mode *mode); > > @@ -1215,6 +1223,8 @@ struct eth_dev_ops { > eth_rxq_info_get_t rxq_info_get; > /** Retrieve Tx queue information */ > eth_txq_info_get_t txq_info_get; > + /** Get Rx queue rearm data */ > + eth_rxq_rearm_data_get_t rxq_rearm_data_get; > eth_burst_mode_get_t rx_burst_mode_get; /**< Get Rx burst mode */ > eth_burst_mode_get_t tx_burst_mode_get; /**< Get Tx burst mode */ > eth_fw_version_get_t fw_version_get; /**< Get firmware version */ > diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c > index 48090c879a..c5dd5e30f6 100644 > --- a/lib/ethdev/ethdev_private.c > +++ b/lib/ethdev/ethdev_private.c > @@ -276,6 +276,8 @@ eth_dev_fp_ops_setup(struct rte_eth_fp_ops *fpo, > fpo->rx_queue_count = dev->rx_queue_count; > fpo->rx_descriptor_status = dev->rx_descriptor_status; > fpo->tx_descriptor_status = dev->tx_descriptor_status; > + fpo->tx_fill_sw_ring = dev->tx_fill_sw_ring; > + fpo->rx_flush_descriptor = dev->rx_flush_descriptor; > > fpo->rxq.data = dev->data->rx_queues; > fpo->rxq.clbk = (void **)(uintptr_t)dev->post_rx_burst_cbs; > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > index 5d5e18db1e..2af5cb42fe 100644 > --- a/lib/ethdev/rte_ethdev.c > +++ b/lib/ethdev/rte_ethdev.c > @@ -3282,6 +3282,21 @@ rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id, uint16_t rx_queue_id, > stat_idx, STAT_QMAP_RX)); > } > > +int > +rte_eth_dev_direct_rearm(uint16_t rx_port_id, uint16_t rx_queue_id, > + uint16_t tx_port_id, uint16_t tx_rx_queue_id, > + struct rte_eth_rxq_rearm_data *rxq_rearm_data) > +{ > + int nb_rearm = 0; > + > + nb_rearm = rte_eth_tx_fill_sw_ring(tx_port_id, tx_rx_queue_id, rxq_rearm_data); > + > + if (nb_rearm > 0) > + return rte_eth_rx_flush_descriptor(rx_port_id, rx_queue_id, nb_rearm); > + > + return 0; > +} > + > int > rte_eth_dev_fw_version_get(uint16_t port_id, char *fw_version, size_t fw_size) > { > @@ -5323,6 +5338,43 @@ rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id, > return 0; > } > > +int > +rte_eth_rx_queue_rearm_data_get(uint16_t port_id, uint16_t queue_id, > + struct rte_eth_rxq_rearm_data *rxq_rearm_data) > +{ > + struct rte_eth_dev *dev; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > + dev = &rte_eth_devices[port_id]; > + > + if (queue_id >= dev->data->nb_rx_queues) { > + RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id); > + return -EINVAL; > + } > + > + if (rxq_rearm_data == NULL) { > + RTE_ETHDEV_LOG(ERR, "Cannot get ethdev port %u Rx queue %u rearm data to NULL\n", > + port_id, queue_id); > + return -EINVAL; > + } > + > + if (dev->data->rx_queues == NULL || > + dev->data->rx_queues[queue_id] == NULL) { > + RTE_ETHDEV_LOG(ERR, > + "Rx queue %"PRIu16" of device with port_id=%" > + PRIu16" has not been setup\n", > + queue_id, port_id); > + return -EINVAL; > + } > + > + if (*dev->dev_ops->rxq_rearm_data_get == NULL) > + return -ENOTSUP; > + > + dev->dev_ops->rxq_rearm_data_get(dev, queue_id, rxq_rearm_data); > + > + return 0; > +} > + > int > rte_eth_rx_burst_mode_get(uint16_t port_id, uint16_t queue_id, > struct rte_eth_burst_mode *mode) > 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; > > +/** > + * @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. > + */ I think this structure owes a lot more expalantion. What each fields suppose to do and what are the constraints, etc. In general, more doc will be needed to explain that feature. > +struct rte_eth_rxq_rearm_data { > + void *rx_sw_ring; That's misleading, we always suppose to store mbufs ptrs here, so why not be direct: struct rte_mbuf **rx_sw_ring; > + uint16_t *rearm_start; > + uint16_t *rearm_nb; I know that for Intel NICs uint16_t is sufficient, wonder would it always be for other vendors? Another thing to consider the case when ring position wrapping? Again I know that it is not required for Intel NICs, but would it be sufficient for API that supposed to be general? > +} __rte_cache_min_aligned; > + > /* Generic Burst mode flag definition, values can be ORed. */ > > /** > @@ -3184,6 +3195,34 @@ int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id, > uint16_t rx_queue_id, > uint8_t stat_idx); > > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice > + * > + * Directly put Tx freed buffers into Rx sw-ring and flush desc. > + * > + * @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(). > + * @param rxq_rearm_data > + * A pointer to a structure of type *rte_eth_txq_rearm_data* to be filled. > + * @return > + * - 0: Success > + */ > +__rte_experimental > +int rte_eth_dev_direct_rearm(uint16_t rx_port_id, uint16_t rx_queue_id, > + uint16_t tx_port_id, uint16_t tx_queue_id, > + struct rte_eth_rxq_rearm_data *rxq_rearm_data); I think we need one more parameter for that function 'uint16_t offset' or so. So _rearm_ will start to populate rx_sw_ring from *rearm_start + offset position. That way we can support populating from different sources. Or should 'offset' be part of truct rte_eth_rxq_rearm_data? > + > /** > * Retrieve the Ethernet address of an Ethernet device. > * > @@ -4782,6 +4821,27 @@ int rte_eth_rx_queue_info_get(uint16_t port_id, uint16_t queue_id, > int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id, > struct rte_eth_txq_info *qinfo); > > +/** > + * Get rearm data about given ports's Rx queue. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The Rx queue on the Ethernet device for which rearm data > + * will be got. > + * @param rxq_rearm_data > + * A pointer to a structure of type *rte_eth_txq_rearm_data* to be filled. > + * > + * @return > + * - 0: Success > + * - -ENODEV: If *port_id* is invalid. > + * - -ENOTSUP: routine is not supported by the device PMD. > + * - -EINVAL: The queue_id is out of range. > + */ > +__rte_experimental > +int rte_eth_rx_queue_rearm_data_get(uint16_t port_id, uint16_t queue_id, > + struct rte_eth_rxq_rearm_data *rxq_rearm_data); > + > /** > * Retrieve information about the Rx packet burst mode. > * > @@ -6103,6 +6163,120 @@ static inline int rte_eth_tx_descriptor_status(uint16_t port_id, > return (*p->tx_descriptor_status)(qd, offset); > } > > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice > + * > + * Fill Rx sw-ring with Tx buffers in direct rearm mode. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The index of the transmit queue. > + * The value must be in the range [0, nb_tx_queue - 1] previously supplied > + * to rte_eth_dev_configure(). > + * @param rxq_rearm_data > + * A pointer to a structure of type *rte_eth_rxq_rearm_data* to be filled with > + * the rearm data of a receive queue. > + * @return > + * - The number buffers correct to be filled in the Rx sw-ring. > + * - (-EINVAL) bad port or queue. > + * - (-ENODEV) bad port. > + * - (-ENOTSUP) if the device does not support this function. > + * > + */ > +__rte_experimental > +static inline int rte_eth_tx_fill_sw_ring(uint16_t port_id, > + uint16_t queue_id, struct rte_eth_rxq_rearm_data *rxq_rearm_data) > +{ > + struct rte_eth_fp_ops *p; > + void *qd; > + > +#ifdef RTE_ETHDEV_DEBUG_TX > + if (port_id >= RTE_MAX_ETHPORTS || > + queue_id >= RTE_MAX_QUEUES_PER_PORT) { > + RTE_ETHDEV_LOG(ERR, > + "Invalid port_id=%u or queue_id=%u\n", > + port_id, queue_id); > + return -EINVAL; > + } > +#endif > + > + p = &rte_eth_fp_ops[port_id]; > + qd = p->txq.data[queue_id]; > + > +#ifdef RTE_ETHDEV_DEBUG_TX > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0); > + > + if (qd == NULL) { > + RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u for port_id=%u\n", > + queue_id, port_id); > + return -ENODEV; > + } > +#endif > + > + if (p->tx_fill_sw_ring == NULL) > + return -ENOTSUP; > + > + return p->tx_fill_sw_ring(qd, rxq_rearm_data); > +} > + > +/** > + * @warning > + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice > + * > + * Flush Rx descriptor in direct rearm mode. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param queue_id > + * The index of the receive queue. > + * The value must be in the range [0, nb_rx_queue - 1] previously supplied > + * to rte_eth_dev_configure(). > + *@param nb_rearm > + * The number of Rx sw-ring buffers need to be flushed. > + * @return > + * - (0) if successful. > + * - (-EINVAL) bad port or queue. > + * - (-ENODEV) bad port. > + * - (-ENOTSUP) if the device does not support this function. > + */ > +__rte_experimental > +static inline int rte_eth_rx_flush_descriptor(uint16_t port_id, > + uint16_t queue_id, uint16_t nb_rearm) > +{ > + struct rte_eth_fp_ops *p; > + void *qd; > + > +#ifdef RTE_ETHDEV_DEBUG_RX > + if (port_id >= RTE_MAX_ETHPORTS || > + queue_id >= RTE_MAX_QUEUES_PER_PORT) { > + RTE_ETHDEV_LOG(ERR, > + "Invalid port_id=%u or queue_id=%u\n", > + port_id, queue_id); > + return -EINVAL; > + } > +#endif > + > + p = &rte_eth_fp_ops[port_id]; > + qd = p->rxq.data[queue_id]; > + > +#ifdef RTE_ETHDEV_DEBUG_RX > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0); > + > + if (qd == NULL) { > + RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u for port_id=%u\n", > + queue_id, port_id); > + return -ENODEV; > + } > +#endif > + > + if (p->rx_flush_descriptor == NULL) > + return -ENOTSUP; > + > + return p->rx_flush_descriptor(qd, nb_rearm); > +} > + > /** > * @internal > * Helper routine for rte_eth_tx_burst(). > diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h > index dcf8adab92..5ecb57f6f0 100644 > --- a/lib/ethdev/rte_ethdev_core.h > +++ b/lib/ethdev/rte_ethdev_core.h > @@ -56,6 +56,13 @@ typedef int (*eth_rx_descriptor_status_t)(void *rxq, uint16_t offset); > /** @internal Check the status of a Tx descriptor */ > typedef int (*eth_tx_descriptor_status_t)(void *txq, uint16_t offset); > > +/** @internal Fill Rx sw-ring with Tx buffers in direct rearm mode */ > +typedef int (*eth_tx_fill_sw_ring_t)(void *txq, > + struct rte_eth_rxq_rearm_data *rxq_rearm_data); > + > +/** @internal Flush Rx descriptor in direct rearm mode */ > +typedef int (*eth_rx_flush_descriptor_t)(void *rxq, uint16_t nb_rearm); > + > /** > * @internal > * Structure used to hold opaque pointers to internal ethdev Rx/Tx > @@ -90,6 +97,8 @@ struct rte_eth_fp_ops { > eth_rx_queue_count_t rx_queue_count; > /** Check the status of a Rx descriptor. */ > eth_rx_descriptor_status_t rx_descriptor_status; > + /** Flush Rx descriptor in direct rearm mode */ > + eth_rx_flush_descriptor_t rx_flush_descriptor; > /** Rx queues data. */ > struct rte_ethdev_qdata rxq; > uintptr_t reserved1[3]; > @@ -106,6 +115,8 @@ struct rte_eth_fp_ops { > eth_tx_prep_t tx_pkt_prepare; > /** 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; > /** Tx queues data. */ > struct rte_ethdev_qdata txq; > uintptr_t reserved2[3]; > diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map > index 17201fbe0f..f39f02a69b 100644 > --- a/lib/ethdev/version.map > +++ b/lib/ethdev/version.map > @@ -298,6 +298,12 @@ EXPERIMENTAL { > rte_flow_get_q_aged_flows; > rte_mtr_meter_policy_get; > rte_mtr_meter_profile_get; > + > + # added in 23.03 > + rte_eth_dev_direct_rearm; > + rte_eth_rx_flush_descriptor; > + rte_eth_rx_queue_rearm_data_get; > + rte_eth_tx_fill_sw_ring; > }; > > INTERNAL {