[1/7] ethdev: introduce hairpin memory capabilities

Message ID 20220919163731.1540454-2-dsosnowski@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series ethdev: introduce hairpin memory capabilities |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Dariusz Sosnowski Sept. 19, 2022, 4:37 p.m. UTC
  This patch introduces new hairpin queue configuration options through
rte_eth_hairpin_conf struct, allowing to tune Rx and Tx hairpin queues
memory configuration. Hairpin configuration is extended with the
following fields:

- use_locked_device_memory - If set, PMD will use specialized on-device
  memory to store RX or TX hairpin queue data.
- use_rte_memory - If set, PMD will use DPDK-managed memory to store RX
  or TX hairpin queue data.
- force_memory - If set, PMD will be forced to use provided memory
  settings. If no appropriate resources are available, then device start
  will fail. If unset and no resources are available, PMD will fallback
  to using default type of resource for given queue.

Hairpin capabilities are also extended, to allow verification of support
of given hairpin memory configurations. Struct rte_eth_hairpin_cap is
extended with two additional fields of type rte_eth_hairpin_queue_cap:

- rx_cap - memory capabilities of hairpin RX queues.
- tx_cap - memory capabilities of hairpin TX queues.

Struct rte_eth_hairpin_queue_cap exposes whether given queue type
supports use_locked_device_memory and use_rte_memory flags.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_ethdev.c | 44 ++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h | 65 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 108 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon Oct. 4, 2022, 4:50 p.m. UTC | #1
19/09/2022 18:37, Dariusz Sosnowski:
> This patch introduces new hairpin queue configuration options through
> rte_eth_hairpin_conf struct, allowing to tune Rx and Tx hairpin queues
> memory configuration. Hairpin configuration is extended with the
> following fields:

What is the benefit?
How the user knows what to use?
Isn't it too much low level for a user?
Why it is not automatic in the driver?

[...]
> +	/**
> +	 * Use locked device memory as a backing storage.
> +	 *
> +	 * - When set, PMD will attempt to use on-device memory as a backing storage for descriptors
> +	 *   and/or data in hairpin queue.
> +	 * - When set, PMD will use detault memory type as a backing storage. Please refer to PMD

You probably mean "clear".
Please make lines shorter.
You should split lines logically, after a dot or at the end of a part.

> +	 *   documentation for details.
> +	 *
> +	 * API user should check if PMD supports this configuration flag using
> +	 * @see rte_eth_dev_hairpin_capability_get.
> +	 */
> +	uint32_t use_locked_device_memory:1;
> +
> +	/**
> +	 * Use DPDK memory as backing storage.
> +	 *
> +	 * - When set, PMD will attempt to use memory managed by DPDK as a backing storage
> +	 *   for descriptors and/or data in hairpin queue.
> +	 * - When clear, PMD will use default memory type as a backing storage. Please refer
> +	 *   to PMD documentation for details.
> +	 *
> +	 * API user should check if PMD supports this configuration flag using
> +	 * @see rte_eth_dev_hairpin_capability_get.
> +	 */
> +	uint32_t use_rte_memory:1;
> +
> +	/**
> +	 * Force usage of hairpin memory configuration.
> +	 *
> +	 * - When set, PMD will attempt to use specified memory settings and
> +	 *   if resource allocation fails, then hairpin queue setup will result in an
> +	 *   error.
> +	 * - When clear, PMD will attempt to use specified memory settings and
> +	 *   if resource allocation fails, then PMD will retry allocation with default
> +	 *   configuration.
> +	 */
> +	uint32_t force_memory:1;
> +
> +	uint32_t reserved:11; /**< Reserved bits. */

You can insert a blank line here.

>  	struct rte_eth_hairpin_peer peers[RTE_ETH_MAX_HAIRPIN_PEERS];
>  };
  
Dariusz Sosnowski Oct. 6, 2022, 11:21 a.m. UTC | #2
Hi Thomas,

> What is the benefit?

Goal of this patchset is to present application developers with more options to fine tune hairpin configuration to their use case.
I added more details regarding the possible benefits and motivation to the cover letter of v2.

> How the user knows what to use?
> Why it is not automatic in the driver?

Basic assumption is that the default behavior of the PMD (mlx5 in that specific case) is a baseline for hairpin performance.
If that default performance is enough for a user, he will not have to change anything in the hairpin queue configuration.
If performance is not satisfying, user will have to experiment with different configurations e.g., if decreasing traffic latency is a priority for the user,
user can check how his specific application works when `use_locked_device_memory` is used.

Specific performance gains of each of the hairpin options is not a given,
since hairpin performance will be a function of hairpin configuration and flow configuration (number of flows, types of flows, etc.).

> Isn't it too much low level for a user?

In my opinion, it's not too low level since purpose of these new options is fine tuning the hairpin performance to the specific use case of the application.

> > +      * - When set, PMD will use detault memory type as a backing
> > + storage. Please refer to PMD
> 
> You probably mean "clear".
> Please make lines shorter.
> You should split lines logically, after a dot or at the end of a part.

Fixed in v2.

> > +
> > +     uint32_t reserved:11; /**< Reserved bits. */
> 
> You can insert a blank line here.
> 
> >       struct rte_eth_hairpin_peer peers[RTE_ETH_MAX_HAIRPIN_PEERS];
> > };

Added in v2.

Best regards,
Dariusz Sosnowski
  

Patch

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 1979dc0850..edcec08231 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1945,6 +1945,28 @@  rte_eth_rx_hairpin_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
 			conf->peer_count, cap.max_rx_2_tx);
 		return -EINVAL;
 	}
+	if (conf->use_locked_device_memory && !cap.rx_cap.locked_device_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to use locked device memory for Rx queue, which is not supported");
+		return -EINVAL;
+	}
+	if (conf->use_rte_memory && !cap.rx_cap.rte_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to use DPDK memory for Rx queue, which is not supported");
+		return -EINVAL;
+	}
+	if (conf->use_locked_device_memory && conf->use_rte_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to use mutually exclusive memory settings for Rx queue");
+		return -EINVAL;
+	}
+	if (conf->force_memory &&
+	    !conf->use_locked_device_memory &&
+	    !conf->use_rte_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to force Rx queue memory settings, but none is set");
+		return -EINVAL;
+	}
 	if (conf->peer_count == 0) {
 		RTE_ETHDEV_LOG(ERR,
 			"Invalid value for number of peers for Rx queue(=%u), should be: > 0",
@@ -2111,6 +2133,28 @@  rte_eth_tx_hairpin_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
 			conf->peer_count, cap.max_tx_2_rx);
 		return -EINVAL;
 	}
+	if (conf->use_locked_device_memory && !cap.tx_cap.locked_device_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to use locked device memory for Tx queue, which is not supported");
+		return -EINVAL;
+	}
+	if (conf->use_rte_memory && !cap.tx_cap.rte_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to use DPDK memory for Tx queue, which is not supported");
+		return -EINVAL;
+	}
+	if (conf->use_locked_device_memory && conf->use_rte_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to use mutually exclusive memory settings for Tx queue");
+		return -EINVAL;
+	}
+	if (conf->force_memory &&
+	    !conf->use_locked_device_memory &&
+	    !conf->use_rte_memory) {
+		RTE_ETHDEV_LOG(ERR,
+			"Attempt to force Tx queue memory settings, but none is set");
+		return -EINVAL;
+	}
 	if (conf->peer_count == 0) {
 		RTE_ETHDEV_LOG(ERR,
 			"Invalid value for number of peers for Tx queue(=%u), should be: > 0",
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index de9e970d4d..e179b0e79b 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1273,6 +1273,28 @@  struct rte_eth_txconf {
 	void *reserved_ptrs[2];   /**< Reserved for future fields */
 };
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * A structure used to return the Tx or Rx hairpin queue capabilities that are supported.
+ */
+struct rte_eth_hairpin_queue_cap {
+	/**
+	 * When set, a specialized on-device memory type can be used as a backing
+	 * storage for a given hairpin queue type.
+	 */
+	uint32_t locked_device_memory:1;
+
+	/**
+	 * When set, memory managed by DPDK can be used as a backing storage
+	 * for a given hairpin queue type.
+	 */
+	uint32_t rte_memory:1;
+
+	uint32_t reserved:30; /**< Reserved for future fields */
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
@@ -1287,6 +1309,8 @@  struct rte_eth_hairpin_cap {
 	/** Max number of Tx queues to be connected to one Rx queue. */
 	uint16_t max_tx_2_rx;
 	uint16_t max_nb_desc; /**< The max num of descriptors. */
+	struct rte_eth_hairpin_queue_cap rx_cap; /**< Rx hairpin queue capabilities. */
+	struct rte_eth_hairpin_queue_cap tx_cap; /**< Tx hairpin queue capabilities. */
 };
 
 #define RTE_ETH_MAX_HAIRPIN_PEERS 32
@@ -1334,7 +1358,46 @@  struct rte_eth_hairpin_conf {
 	 *   configured automatically during port start.
 	 */
 	uint32_t manual_bind:1;
-	uint32_t reserved:14; /**< Reserved bits. */
+
+	/**
+	 * Use locked device memory as a backing storage.
+	 *
+	 * - When set, PMD will attempt to use on-device memory as a backing storage for descriptors
+	 *   and/or data in hairpin queue.
+	 * - When set, PMD will use detault memory type as a backing storage. Please refer to PMD
+	 *   documentation for details.
+	 *
+	 * API user should check if PMD supports this configuration flag using
+	 * @see rte_eth_dev_hairpin_capability_get.
+	 */
+	uint32_t use_locked_device_memory:1;
+
+	/**
+	 * Use DPDK memory as backing storage.
+	 *
+	 * - When set, PMD will attempt to use memory managed by DPDK as a backing storage
+	 *   for descriptors and/or data in hairpin queue.
+	 * - When clear, PMD will use default memory type as a backing storage. Please refer
+	 *   to PMD documentation for details.
+	 *
+	 * API user should check if PMD supports this configuration flag using
+	 * @see rte_eth_dev_hairpin_capability_get.
+	 */
+	uint32_t use_rte_memory:1;
+
+	/**
+	 * Force usage of hairpin memory configuration.
+	 *
+	 * - When set, PMD will attempt to use specified memory settings and
+	 *   if resource allocation fails, then hairpin queue setup will result in an
+	 *   error.
+	 * - When clear, PMD will attempt to use specified memory settings and
+	 *   if resource allocation fails, then PMD will retry allocation with default
+	 *   configuration.
+	 */
+	uint32_t force_memory:1;
+
+	uint32_t reserved:11; /**< Reserved bits. */
 	struct rte_eth_hairpin_peer peers[RTE_ETH_MAX_HAIRPIN_PEERS];
 };