[v11,1/4] ethdev: add API for mbufs recycle mode

  Add 'rte_eth_recycle_rx_queue_info_get' and 'rte_eth_recycle_mbufs'
APIs to recycle used mbufs from a transmit queue of an Ethernet device,
and move these mbufs into a mbuf ring for a receive queue of an Ethernet
device. This can bypass mempool 'put/get' operations hence saving CPU
cycles.

For each recycling mbufs, the rte_eth_recycle_mbufs() function performs
the following operations:
- Copy used *rte_mbuf* buffer pointers from Tx mbuf ring into Rx mbuf
ring.
- Replenish the Rx descriptors with the recycling *rte_mbuf* mbufs freed
from the Tx mbuf ring.

Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Suggested-by: Ruifeng Wang <ruifeng.wang@arm.com>
Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 doc/guides/rel_notes/release_23_11.rst |  15 ++
 lib/ethdev/ethdev_driver.h             |  10 ++
 lib/ethdev/ethdev_private.c            |   2 +
 lib/ethdev/rte_ethdev.c                |  31 +++++
 lib/ethdev/rte_ethdev.h                | 181 +++++++++++++++++++++++++
 lib/ethdev/rte_ethdev_core.h           |  23 +++-
 lib/ethdev/version.map                 |   3 +
 7 files changed, 259 insertions(+), 6 deletions(-)
  

On Tue, 22 Aug 2023 15:27:07 +0800
Feifei Wang <feifei.wang2@arm.com> wrote:

> +	if (queue_id >= dev->data->nb_rx_queues) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", 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;
> +	}

This could be changed to something like:

	ret = eth_dev_validate_rx_queue(dev, queue_id);
	if (unlikely(ret != 0))
		return ret;
  

Hi Feifei,

> Add 'rte_eth_recycle_rx_queue_info_get' and 'rte_eth_recycle_mbufs'
> APIs to recycle used mbufs from a transmit queue of an Ethernet device,
> and move these mbufs into a mbuf ring for a receive queue of an Ethernet
> device. This can bypass mempool 'put/get' operations hence saving CPU
> cycles.
> 
> For each recycling mbufs, the rte_eth_recycle_mbufs() function performs
> the following operations:
> - Copy used *rte_mbuf* buffer pointers from Tx mbuf ring into Rx mbuf
> ring.
> - Replenish the Rx descriptors with the recycling *rte_mbuf* mbufs freed
> from the Tx mbuf ring.
> 
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Suggested-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>   doc/guides/rel_notes/release_23_11.rst |  15 ++
>   lib/ethdev/ethdev_driver.h             |  10 ++
>   lib/ethdev/ethdev_private.c            |   2 +
>   lib/ethdev/rte_ethdev.c                |  31 +++++
>   lib/ethdev/rte_ethdev.h                | 181 +++++++++++++++++++++++++
>   lib/ethdev/rte_ethdev_core.h           |  23 +++-
>   lib/ethdev/version.map                 |   3 +
>   7 files changed, 259 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst
> index 4411bb32c1..02ee3867a0 100644
> --- a/doc/guides/rel_notes/release_23_11.rst
> +++ b/doc/guides/rel_notes/release_23_11.rst
> @@ -72,6 +72,13 @@ New Features
>        Also, make sure to start the actual text at the margin.
>        =======================================================
>   
> +* **Add mbufs recycling support.**
> +
> +  Added ``rte_eth_recycle_rx_queue_info_get`` and ``rte_eth_recycle_mbufs``
> +  APIs which allow the user to copy used mbufs from the Tx mbuf ring
> +  into the Rx mbuf ring. This feature supports the case that the Rx Ethernet
> +  device is different from the Tx Ethernet device with respective driver
> +  callback functions in ``rte_eth_recycle_mbufs``.
>   
>   Removed Items
>   -------------
> @@ -123,6 +130,14 @@ ABI Changes
>      Also, make sure to start the actual text at the margin.
>      =======================================================
>   
> +* ethdev: Added ``recycle_tx_mbufs_reuse`` and ``recycle_rx_descriptors_refill``
> +  fields to ``rte_eth_dev`` structure.
> +
> +* ethdev: Structure ``rte_eth_fp_ops`` was affected to add
> +  ``recycle_tx_mbufs_reuse`` and ``recycle_rx_descriptors_refill``
> +  fields, to move ``rxq`` and ``txq`` fields, to change the size of
> +  ``reserved1`` and ``reserved2`` fields.
> +
>   
>   Known Issues
>   ------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 980f837ab6..b0c55a8523 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -58,6 +58,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;
> +	/** Pointer to PMD transmit mbufs reuse function */
> +	eth_recycle_tx_mbufs_reuse_t recycle_tx_mbufs_reuse;
> +	/** Pointer to PMD receive descriptors refill function */
> +	eth_recycle_rx_descriptors_refill_t recycle_rx_descriptors_refill;
>   
>   	/**
>   	 * Device data that is shared between primary and secondary processes
> @@ -507,6 +511,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);
>   
> +typedef void (*eth_recycle_rxq_info_get_t)(struct rte_eth_dev *dev,
> +	uint16_t rx_queue_id,
> +	struct rte_eth_recycle_rxq_info *recycle_rxq_info);
> +
>   typedef int (*eth_burst_mode_get_t)(struct rte_eth_dev *dev,
>   	uint16_t queue_id, struct rte_eth_burst_mode *mode);
>   
> @@ -1250,6 +1258,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;
> +	/** Retrieve mbufs recycle Rx queue information */
> +	eth_recycle_rxq_info_get_t recycle_rxq_info_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 14ec8c6ccf..f8ab64f195 100644
> --- a/lib/ethdev/ethdev_private.c
> +++ b/lib/ethdev/ethdev_private.c
> @@ -277,6 +277,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->recycle_tx_mbufs_reuse = dev->recycle_tx_mbufs_reuse;
> +	fpo->recycle_rx_descriptors_refill = dev->recycle_rx_descriptors_refill;
>   
>   	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 0840d2b594..ea89a101a1 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -5876,6 +5876,37 @@ rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
>   	return 0;
>   }
>   
> +int
> +rte_eth_recycle_rx_queue_info_get(uint16_t port_id, uint16_t queue_id,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info)
> +{
> +	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 (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->recycle_rxq_info_get == NULL)
> +		return -ENOTSUP;
> +
> +	dev->dev_ops->recycle_rxq_info_get(dev, queue_id, recycle_rxq_info);
> +
> +	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 04a2564f22..9dc5749d83 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1820,6 +1820,30 @@ struct rte_eth_txq_info {
>   	uint8_t queue_state;        /**< one of RTE_ETH_QUEUE_STATE_*. */
>   } __rte_cache_min_aligned;
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice.
> + *
> + * Ethernet device Rx queue information structure for recycling mbufs.
> + * Used to retrieve Rx queue information when Tx queue reusing mbufs and moving
> + * them into Rx mbuf ring.
> + */
> +struct rte_eth_recycle_rxq_info {
> +	struct rte_mbuf **mbuf_ring; /**< mbuf ring of Rx queue. */
> +	struct rte_mempool *mp;     /**< mempool of Rx queue. */
> +	uint16_t *refill_head;      /**< head of Rx queue refilling mbufs. */
> +	uint16_t *receive_tail;     /**< tail of Rx queue receiving pkts. */
> +	uint16_t mbuf_ring_size;     /**< configured number of mbuf ring size. */
> +	/**
> +	 * Requirement on mbuf refilling batch size of Rx mbuf ring.
> +	 * For some PMD drivers, the number of Rx mbuf ring refilling mbufs
> +	 * should be aligned with mbuf ring size, in order to simplify
> +	 * ring wrapping around.
> +	 * Value 0 means that PMD drivers have no requirement for this.
> +	 */
> +	uint16_t refill_requirement;
> +} __rte_cache_min_aligned;
> +
>   /* Generic Burst mode flag definition, values can be ORed. */
>   
>   /**
> @@ -4853,6 +4877,31 @@ 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);
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Retrieve information about given ports's Rx queue for recycling mbufs.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param queue_id
> + *   The Rx queue on the Ethernet devicefor which information
> + *   will be retrieved.
> + * @param recycle_rxq_info
> + *   A pointer to a structure of type *rte_eth_recycle_rxq_info* 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_recycle_rx_queue_info_get(uint16_t port_id,
> +		uint16_t queue_id,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info);
> +
>   /**
>    * Retrieve information about the Rx packet burst mode.
>    *
> @@ -6527,6 +6576,138 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
>   	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>   }
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Recycle used mbufs from a transmit queue of an Ethernet device, and move
> + * these mbufs into a mbuf ring for a receive queue of an Ethernet device.
> + * This can bypass mempool path to save CPU cycles.
> + *
> + * The rte_eth_recycle_mbufs() function loops, with rte_eth_rx_burst() and
> + * rte_eth_tx_burst() functions, freeing Tx used mbufs and replenishing Rx
> + * descriptors. The number of recycling mbufs depends on the request of Rx mbuf
> + * ring, with the constraint of enough used mbufs from Tx mbuf ring.
> + *
> + * For each recycling mbufs, the rte_eth_recycle_mbufs() function performs the
> + * following operations:
> + *
> + * - Copy used *rte_mbuf* buffer pointers from Tx mbuf ring into Rx mbuf ring.
> + *
> + * - Replenish the Rx descriptors with the recycling *rte_mbuf* mbufs freed
> + *   from the Tx mbuf ring.
> + *
> + * This function spilts Rx and Tx path with different callback functions. The
> + * callback function recycle_tx_mbufs_reuse is for Tx driver. The callback
> + * function recycle_rx_descriptors_refill is for Rx driver. rte_eth_recycle_mbufs()
> + * can support the case that Rx Ethernet device is different from Tx Ethernet device.
> + *
> + * It is the responsibility of users to select the Rx/Tx queue pair to recycle
> + * mbufs. Before call this function, users must call rte_eth_recycle_rxq_info_get
> + * function to retrieve selected Rx queue information.
> + * @see rte_eth_recycle_rxq_info_get, struct rte_eth_recycle_rxq_info
> + *
> + * Currently, the rte_eth_recycle_mbufs() function can support to feed 1 Rx queue from
> + * 2 Tx queues in the same thread. Do not pair the Rx queue and Tx queue in different
> + * threads, in order to avoid memory error rewriting.
> + *
> + * @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 recycle_rxq_info
> + *   A pointer to a structure of type *rte_eth_recycle_rxq_info* which contains
> + *   the information of the Rx queue mbuf ring.
> + * @return
> + *   The number of recycling mbufs.
> + */
> +__rte_experimental
> +static inline uint16_t
> +rte_eth_recycle_mbufs(uint16_t rx_port_id, uint16_t rx_queue_id,
> +		uint16_t tx_port_id, uint16_t tx_queue_id,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info)
> +{
> +	struct rte_eth_fp_ops *p;
> +	void *qd;
> +	uint16_t nb_mbufs;
> +
> +#ifdef RTE_ETHDEV_DEBUG_TX
> +	if (tx_port_id >= RTE_MAX_ETHPORTS ||
> +			tx_queue_id >= RTE_MAX_QUEUES_PER_PORT) {
> +		RTE_ETHDEV_LOG(ERR,
> +				"Invalid tx_port_id=%u or tx_queue_id=%u\n",
> +				tx_port_id, tx_queue_id);
> +		return 0;
> +	}
> +#endif
> +
> +	/* fetch pointer to queue data */
> +	p = &rte_eth_fp_ops[tx_port_id];
> +	qd = p->txq.data[tx_queue_id];
> +
> +#ifdef RTE_ETHDEV_DEBUG_TX
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(tx_port_id, 0);
> +
> +	if (qd == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u for port_id=%u\n",
> +				tx_queue_id, tx_port_id);
> +		return 0;
> +	}
> +#endif
> +	if (p->recycle_tx_mbufs_reuse == NULL)
> +		return 0;
> +
> +	/* Copy used *rte_mbuf* buffer pointers from Tx mbuf ring
> +	 * into Rx mbuf ring.
> +	 */
> +	nb_mbufs = p->recycle_tx_mbufs_reuse(qd, recycle_rxq_info);

It is probably better to do that call after rx_port_id, rx_queue_id, 
etc. checks.
Otherwise with some errorneous params we can get mbufs, from TXQ,
'rx_refill' would not happen and we will return zero.
With that in place:
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>

> +
> +	/* If no recycling mbufs, return 0. */
> +	if (nb_mbufs == 0)
> +		return 0;
> +
> +#ifdef RTE_ETHDEV_DEBUG_RX
> +	if (rx_port_id >= RTE_MAX_ETHPORTS ||
> +			rx_queue_id >= RTE_MAX_QUEUES_PER_PORT) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid rx_port_id=%u or rx_queue_id=%u\n",
> +				rx_port_id, rx_queue_id);
> +		return 0;
> +	}
> +#endif
> +
> +	/* fetch pointer to queue data */
> +	p = &rte_eth_fp_ops[rx_port_id];
> +	qd = p->rxq.data[rx_queue_id];
> +
> +#ifdef RTE_ETHDEV_DEBUG_RX
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(rx_port_id, 0);
> +
> +	if (qd == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u for port_id=%u\n",
> +				rx_queue_id, rx_port_id);
> +		return 0;
> +	}
> +#endif
> +
> +	if (p->recycle_rx_descriptors_refill == NULL)
> +		return 0;
> +
> +	/* Replenish the Rx descriptors with the recycling
> +	 * into Rx mbuf ring.
> +	 */
> +	p->recycle_rx_descriptors_refill(qd, nb_mbufs);
> +
> +	return nb_mbufs;
> +}
> +
>   /**
>    * @warning
>    * @b EXPERIMENTAL: this API may change without prior notice
> diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h
> index 46e9721e07..a24ad7a6b2 100644
> --- a/lib/ethdev/rte_ethdev_core.h
> +++ b/lib/ethdev/rte_ethdev_core.h
> @@ -55,6 +55,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 Copy used mbufs from Tx mbuf ring into Rx mbuf ring */
> +typedef uint16_t (*eth_recycle_tx_mbufs_reuse_t)(void *txq,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info);
> +
> +/** @internal Refill Rx descriptors with the recycling mbufs */
> +typedef void (*eth_recycle_rx_descriptors_refill_t)(void *rxq, uint16_t nb);
> +
>   /**
>    * @internal
>    * Structure used to hold opaque pointers to internal ethdev Rx/Tx
> @@ -83,15 +90,17 @@ struct rte_eth_fp_ops {
>   	 * Rx fast-path functions and related data.
>   	 * 64-bit systems: occupies first 64B line
>   	 */
> +	/** Rx queues data. */
> +	struct rte_ethdev_qdata rxq;
>   	/** PMD receive function. */
>   	eth_rx_burst_t rx_pkt_burst;
>   	/** Get the number of used Rx descriptors. */
>   	eth_rx_queue_count_t rx_queue_count;
>   	/** Check the status of a Rx descriptor. */
>   	eth_rx_descriptor_status_t rx_descriptor_status;
> -	/** Rx queues data. */
> -	struct rte_ethdev_qdata rxq;
> -	uintptr_t reserved1[3];
> +	/** Refill Rx descriptors with the recycling mbufs. */
> +	eth_recycle_rx_descriptors_refill_t recycle_rx_descriptors_refill;
> +	uintptr_t reserved1[2];
>   	/**@}*/
>   
>   	/**@{*/
> @@ -99,15 +108,17 @@ struct rte_eth_fp_ops {
>   	 * Tx fast-path functions and related data.
>   	 * 64-bit systems: occupies second 64B line
>   	 */
> +	/** Tx queues data. */
> +	struct rte_ethdev_qdata txq;
>   	/** PMD transmit function. */
>   	eth_tx_burst_t tx_pkt_burst;
>   	/** PMD transmit prepare function. */
>   	eth_tx_prep_t tx_pkt_prepare;
>   	/** Check the status of a Tx descriptor. */
>   	eth_tx_descriptor_status_t tx_descriptor_status;
> -	/** Tx queues data. */
> -	struct rte_ethdev_qdata txq;
> -	uintptr_t reserved2[3];
> +	/** Copy used mbufs from Tx mbuf ring into Rx. */
> +	eth_recycle_tx_mbufs_reuse_t recycle_tx_mbufs_reuse;
> +	uintptr_t reserved2[2];
>   	/**@}*/
>   
>   } __rte_cache_aligned;
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index b965d6aa52..eec159dfdd 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -312,6 +312,9 @@ EXPERIMENTAL {
>   	rte_flow_async_action_list_handle_query_update;
>   	rte_flow_async_actions_update;
>   	rte_flow_restore_info_dynflag;
> +
> +	# added in 23.11
> +	rte_eth_recycle_rx_queue_info_get;
>   };
>   
>   INTERNAL {
  

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, August 22, 2023 10:03 PM
> To: Feifei Wang <Feifei.Wang2@arm.com>
> Cc: thomas@monjalon.net; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; nd
> <nd@arm.com>; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> Ruifeng Wang <Ruifeng.Wang@arm.com>; Morten Brørup
> <mb@smartsharesystems.com>
> Subject: Re: [PATCH v11 1/4] ethdev: add API for mbufs recycle mode
> 
> On Tue, 22 Aug 2023 15:27:07 +0800
> Feifei Wang <feifei.wang2@arm.com> wrote:
> 
> > +	if (queue_id >= dev->data->nb_rx_queues) {
> > +		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n",
> 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;
> > +	}
> 
> This could be changed to something like:
> 
> 	ret = eth_dev_validate_rx_queue(dev, queue_id);
> 	if (unlikely(ret != 0))
> 		return ret;
> 

Good comments, I just know there is an API to check these. Thanks.
> 
>
  

For Konstantin

> -----Original Message-----
> From: Feifei Wang <feifei.wang2@arm.com>
> Sent: Tuesday, August 22, 2023 3:27 PM
> To: thomas@monjalon.net; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: dev@dpdk.org; nd <nd@arm.com>; Feifei Wang
> <Feifei.Wang2@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Morten Brørup <mb@smartsharesystems.com>
> Subject: [PATCH v11 1/4] ethdev: add API for mbufs recycle mode
> 
> Add 'rte_eth_recycle_rx_queue_info_get' and 'rte_eth_recycle_mbufs'
> APIs to recycle used mbufs from a transmit queue of an Ethernet device, and
> move these mbufs into a mbuf ring for a receive queue of an Ethernet device.
> This can bypass mempool 'put/get' operations hence saving CPU cycles.
> 
> For each recycling mbufs, the rte_eth_recycle_mbufs() function performs the
> following operations:
> - Copy used *rte_mbuf* buffer pointers from Tx mbuf ring into Rx mbuf ring.
> - Replenish the Rx descriptors with the recycling *rte_mbuf* mbufs freed from
> the Tx mbuf ring.
> 
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Suggested-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  doc/guides/rel_notes/release_23_11.rst |  15 ++
>  lib/ethdev/ethdev_driver.h             |  10 ++
>  lib/ethdev/ethdev_private.c            |   2 +
>  lib/ethdev/rte_ethdev.c                |  31 +++++
>  lib/ethdev/rte_ethdev.h                | 181 +++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev_core.h           |  23 +++-
>  lib/ethdev/version.map                 |   3 +
>  7 files changed, 259 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_23_11.rst
> b/doc/guides/rel_notes/release_23_11.rst
> index 4411bb32c1..02ee3867a0 100644
> --- a/doc/guides/rel_notes/release_23_11.rst
> +++ b/doc/guides/rel_notes/release_23_11.rst
> @@ -72,6 +72,13 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
> 
> +* **Add mbufs recycling support.**
> +
> +  Added ``rte_eth_recycle_rx_queue_info_get`` and
> + ``rte_eth_recycle_mbufs``  APIs which allow the user to copy used
> + mbufs from the Tx mbuf ring  into the Rx mbuf ring. This feature
> + supports the case that the Rx Ethernet  device is different from the
> + Tx Ethernet device with respective driver  callback functions in
> ``rte_eth_recycle_mbufs``.
> 
>  Removed Items
>  -------------
> @@ -123,6 +130,14 @@ ABI Changes
>     Also, make sure to start the actual text at the margin.
>     =======================================================
> 
> +* ethdev: Added ``recycle_tx_mbufs_reuse`` and
> +``recycle_rx_descriptors_refill``
> +  fields to ``rte_eth_dev`` structure.
> +
> +* ethdev: Structure ``rte_eth_fp_ops`` was affected to add
> +  ``recycle_tx_mbufs_reuse`` and ``recycle_rx_descriptors_refill``
> +  fields, to move ``rxq`` and ``txq`` fields, to change the size of
> +  ``reserved1`` and ``reserved2`` fields.
> +
> 
>  Known Issues
>  ------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h index
> 980f837ab6..b0c55a8523 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -58,6 +58,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;
> +	/** Pointer to PMD transmit mbufs reuse function */
> +	eth_recycle_tx_mbufs_reuse_t recycle_tx_mbufs_reuse;
> +	/** Pointer to PMD receive descriptors refill function */
> +	eth_recycle_rx_descriptors_refill_t recycle_rx_descriptors_refill;
> 
>  	/**
>  	 * Device data that is shared between primary and secondary
> processes @@ -507,6 +511,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);
> 
> +typedef void (*eth_recycle_rxq_info_get_t)(struct rte_eth_dev *dev,
> +	uint16_t rx_queue_id,
> +	struct rte_eth_recycle_rxq_info *recycle_rxq_info);
> +
>  typedef int (*eth_burst_mode_get_t)(struct rte_eth_dev *dev,
>  	uint16_t queue_id, struct rte_eth_burst_mode *mode);
> 
> @@ -1250,6 +1258,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;
> +	/** Retrieve mbufs recycle Rx queue information */
> +	eth_recycle_rxq_info_get_t recycle_rxq_info_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
> 14ec8c6ccf..f8ab64f195 100644
> --- a/lib/ethdev/ethdev_private.c
> +++ b/lib/ethdev/ethdev_private.c
> @@ -277,6 +277,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->recycle_tx_mbufs_reuse = dev->recycle_tx_mbufs_reuse;
> +	fpo->recycle_rx_descriptors_refill =
> +dev->recycle_rx_descriptors_refill;
> 
>  	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
> 0840d2b594..ea89a101a1 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -5876,6 +5876,37 @@ rte_eth_tx_queue_info_get(uint16_t port_id,
> uint16_t queue_id,
>  	return 0;
>  }
> 
> +int
> +rte_eth_recycle_rx_queue_info_get(uint16_t port_id, uint16_t queue_id,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info) {
> +	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 (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->recycle_rxq_info_get == NULL)
> +		return -ENOTSUP;
> +
> +	dev->dev_ops->recycle_rxq_info_get(dev, queue_id, recycle_rxq_info);
> +
> +	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
> 04a2564f22..9dc5749d83 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1820,6 +1820,30 @@ struct rte_eth_txq_info {
>  	uint8_t queue_state;        /**< one of RTE_ETH_QUEUE_STATE_*. */
>  } __rte_cache_min_aligned;
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice.
> + *
> + * Ethernet device Rx queue information structure for recycling mbufs.
> + * Used to retrieve Rx queue information when Tx queue reusing mbufs
> +and moving
> + * them into Rx mbuf ring.
> + */
> +struct rte_eth_recycle_rxq_info {
> +	struct rte_mbuf **mbuf_ring; /**< mbuf ring of Rx queue. */
> +	struct rte_mempool *mp;     /**< mempool of Rx queue. */
> +	uint16_t *refill_head;      /**< head of Rx queue refilling mbufs. */
> +	uint16_t *receive_tail;     /**< tail of Rx queue receiving pkts. */
> +	uint16_t mbuf_ring_size;     /**< configured number of mbuf ring size.
> */
> +	/**
> +	 * Requirement on mbuf refilling batch size of Rx mbuf ring.
> +	 * For some PMD drivers, the number of Rx mbuf ring refilling mbufs
> +	 * should be aligned with mbuf ring size, in order to simplify
> +	 * ring wrapping around.
> +	 * Value 0 means that PMD drivers have no requirement for this.
> +	 */
> +	uint16_t refill_requirement;
> +} __rte_cache_min_aligned;
> +
>  /* Generic Burst mode flag definition, values can be ORed. */
> 
>  /**
> @@ -4853,6 +4877,31 @@ 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);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> +notice
> + *
> + * Retrieve information about given ports's Rx queue for recycling mbufs.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param queue_id
> + *   The Rx queue on the Ethernet devicefor which information
> + *   will be retrieved.
> + * @param recycle_rxq_info
> + *   A pointer to a structure of type *rte_eth_recycle_rxq_info* 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_recycle_rx_queue_info_get(uint16_t port_id,
> +		uint16_t queue_id,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info);
> +
>  /**
>   * Retrieve information about the Rx packet burst mode.
>   *
> @@ -6527,6 +6576,138 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
> queue_id,
>  	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);  }
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> +notice
> + *
> + * Recycle used mbufs from a transmit queue of an Ethernet device, and
> +move
> + * these mbufs into a mbuf ring for a receive queue of an Ethernet device.
> + * This can bypass mempool path to save CPU cycles.
> + *
> + * The rte_eth_recycle_mbufs() function loops, with rte_eth_rx_burst()
> +and
> + * rte_eth_tx_burst() functions, freeing Tx used mbufs and replenishing
> +Rx
> + * descriptors. The number of recycling mbufs depends on the request of
> +Rx mbuf
> + * ring, with the constraint of enough used mbufs from Tx mbuf ring.
> + *
> + * For each recycling mbufs, the rte_eth_recycle_mbufs() function
> +performs the
> + * following operations:
> + *
> + * - Copy used *rte_mbuf* buffer pointers from Tx mbuf ring into Rx mbuf
> ring.
> + *
> + * - Replenish the Rx descriptors with the recycling *rte_mbuf* mbufs freed
> + *   from the Tx mbuf ring.
> + *
> + * This function spilts Rx and Tx path with different callback
> +functions. The
> + * callback function recycle_tx_mbufs_reuse is for Tx driver. The
> +callback
> + * function recycle_rx_descriptors_refill is for Rx driver.
> +rte_eth_recycle_mbufs()
> + * can support the case that Rx Ethernet device is different from Tx Ethernet
> device.
> + *
> + * It is the responsibility of users to select the Rx/Tx queue pair to
> +recycle
> + * mbufs. Before call this function, users must call
> +rte_eth_recycle_rxq_info_get
> + * function to retrieve selected Rx queue information.
> + * @see rte_eth_recycle_rxq_info_get, struct rte_eth_recycle_rxq_info
> + *
> + * Currently, the rte_eth_recycle_mbufs() function can support to feed
> +1 Rx queue from
> + * 2 Tx queues in the same thread. Do not pair the Rx queue and Tx
> +queue in different
> + * threads, in order to avoid memory error rewriting.
> + *
> + * @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 recycle_rxq_info
> + *   A pointer to a structure of type *rte_eth_recycle_rxq_info* which contains
> + *   the information of the Rx queue mbuf ring.
> + * @return
> + *   The number of recycling mbufs.
> + */
> +__rte_experimental
> +static inline uint16_t
> +rte_eth_recycle_mbufs(uint16_t rx_port_id, uint16_t rx_queue_id,
> +		uint16_t tx_port_id, uint16_t tx_queue_id,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info) {
> +	struct rte_eth_fp_ops *p;
> +	void *qd;
> +	uint16_t nb_mbufs;
> +
> +#ifdef RTE_ETHDEV_DEBUG_TX
> +	if (tx_port_id >= RTE_MAX_ETHPORTS ||
> +			tx_queue_id >= RTE_MAX_QUEUES_PER_PORT) {
> +		RTE_ETHDEV_LOG(ERR,
> +				"Invalid tx_port_id=%u or
> tx_queue_id=%u\n",
> +				tx_port_id, tx_queue_id);
> +		return 0;
> +	}
> +#endif
> +
> +	/* fetch pointer to queue data */
> +	p = &rte_eth_fp_ops[tx_port_id];
> +	qd = p->txq.data[tx_queue_id];
> +
> +#ifdef RTE_ETHDEV_DEBUG_TX
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(tx_port_id, 0);
> +
> +	if (qd == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u for
> port_id=%u\n",
> +				tx_queue_id, tx_port_id);
> +		return 0;
> +	}
> +#endif
> +	if (p->recycle_tx_mbufs_reuse == NULL)
> +		return 0;
> +
> +	/* Copy used *rte_mbuf* buffer pointers from Tx mbuf ring
> +	 * into Rx mbuf ring.
> +	 */
> +	nb_mbufs = p->recycle_tx_mbufs_reuse(qd, recycle_rxq_info);
> +
[Konstantin]
It is probably better to do that call after rx_port_id, rx_queue_id, 
etc. checks.
Otherwise with some errorneous params we can get mbufs, from TXQ,
'rx_refill' would not happen and we will return zero.
With that in place:
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>


Thanks for the comments, I will put all check before function call.

> +	/* If no recycling mbufs, return 0. */
> +	if (nb_mbufs == 0)
> +		return 0;
> +
> +#ifdef RTE_ETHDEV_DEBUG_RX
> +	if (rx_port_id >= RTE_MAX_ETHPORTS ||
> +			rx_queue_id >= RTE_MAX_QUEUES_PER_PORT) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid rx_port_id=%u or
> rx_queue_id=%u\n",
> +				rx_port_id, rx_queue_id);
> +		return 0;
> +	}
> +#endif
> +
> +	/* fetch pointer to queue data */
> +	p = &rte_eth_fp_ops[rx_port_id];
> +	qd = p->rxq.data[rx_queue_id];
> +
> +#ifdef RTE_ETHDEV_DEBUG_RX
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(rx_port_id, 0);
> +
> +	if (qd == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u for
> port_id=%u\n",
> +				rx_queue_id, rx_port_id);
> +		return 0;
> +	}
> +#endif
> +
> +	if (p->recycle_rx_descriptors_refill == NULL)
> +		return 0;
> +
> +	/* Replenish the Rx descriptors with the recycling
> +	 * into Rx mbuf ring.
> +	 */
> +	p->recycle_rx_descriptors_refill(qd, nb_mbufs);
> +
> +	return nb_mbufs;
> +}
> +
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this API may change without prior notice diff --git
> a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h index
> 46e9721e07..a24ad7a6b2 100644
> --- a/lib/ethdev/rte_ethdev_core.h
> +++ b/lib/ethdev/rte_ethdev_core.h
> @@ -55,6 +55,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 Copy used mbufs from Tx mbuf ring into Rx mbuf ring */
> +typedef uint16_t (*eth_recycle_tx_mbufs_reuse_t)(void *txq,
> +		struct rte_eth_recycle_rxq_info *recycle_rxq_info);
> +
> +/** @internal Refill Rx descriptors with the recycling mbufs */ typedef
> +void (*eth_recycle_rx_descriptors_refill_t)(void *rxq, uint16_t nb);
> +
>  /**
>   * @internal
>   * Structure used to hold opaque pointers to internal ethdev Rx/Tx @@ -
> 83,15 +90,17 @@ struct rte_eth_fp_ops {
>  	 * Rx fast-path functions and related data.
>  	 * 64-bit systems: occupies first 64B line
>  	 */
> +	/** Rx queues data. */
> +	struct rte_ethdev_qdata rxq;
>  	/** PMD receive function. */
>  	eth_rx_burst_t rx_pkt_burst;
>  	/** Get the number of used Rx descriptors. */
>  	eth_rx_queue_count_t rx_queue_count;
>  	/** Check the status of a Rx descriptor. */
>  	eth_rx_descriptor_status_t rx_descriptor_status;
> -	/** Rx queues data. */
> -	struct rte_ethdev_qdata rxq;
> -	uintptr_t reserved1[3];
> +	/** Refill Rx descriptors with the recycling mbufs. */
> +	eth_recycle_rx_descriptors_refill_t recycle_rx_descriptors_refill;
> +	uintptr_t reserved1[2];
>  	/**@}*/
> 
>  	/**@{*/
> @@ -99,15 +108,17 @@ struct rte_eth_fp_ops {
>  	 * Tx fast-path functions and related data.
>  	 * 64-bit systems: occupies second 64B line
>  	 */
> +	/** Tx queues data. */
> +	struct rte_ethdev_qdata txq;
>  	/** PMD transmit function. */
>  	eth_tx_burst_t tx_pkt_burst;
>  	/** PMD transmit prepare function. */
>  	eth_tx_prep_t tx_pkt_prepare;
>  	/** Check the status of a Tx descriptor. */
>  	eth_tx_descriptor_status_t tx_descriptor_status;
> -	/** Tx queues data. */
> -	struct rte_ethdev_qdata txq;
> -	uintptr_t reserved2[3];
> +	/** Copy used mbufs from Tx mbuf ring into Rx. */
> +	eth_recycle_tx_mbufs_reuse_t recycle_tx_mbufs_reuse;
> +	uintptr_t reserved2[2];
>  	/**@}*/
> 
>  } __rte_cache_aligned;
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map index
> b965d6aa52..eec159dfdd 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -312,6 +312,9 @@ EXPERIMENTAL {
>  	rte_flow_async_action_list_handle_query_update;
>  	rte_flow_async_actions_update;
>  	rte_flow_restore_info_dynflag;
> +
> +	# added in 23.11
> +	rte_eth_recycle_rx_queue_info_get;
>  };
> 
>  INTERNAL {
> --
> 2.25.1