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

[Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>]

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 {