Cc David. See some kind of reincarnation of RTE_FUNC_PTR_*
macro below.
On 9/19/22 15:15, sk...@marvell.com wrote:
From: Jerin Jacob <jer...@marvell.com>
NIC HW controllers often come with congestion management support on
various HW objects such as Rx queue depth or mempool queue depth.
Also, it can support various modes of operation such as RED
(Random early discard), WRED etc on those HW objects.
This patch adds a framework to express such modes(enum rte_cman_mode)
and introduce (enum rte_eth_cman_obj) to enumerate the different
objects where the modes can operate on.
This patch adds RTE_CMAN_RED mode of operation and
RTE_ETH_CMAN_OBJ_RX_QUEUE, RTE_ETH_CMAN_OBJ_RX_QUEUE_MEMPOOL object.
Introduced reserved fields in configuration structure
backed by rte_eth_cman_config_init() to add new configuration
parameters without ABI breakage.
Added rte_eth_cman_info_get() API to get the information such as
supported modes and objects.
Added rte_eth_cman_config_init(), rte_eth_cman_config_set() APIs
to configure congestion management on those object with associated mode.
Finally, Added rte_eth_cman_config_get() API to retrieve the
applied configuration.
Signed-off-by: Jerin Jacob <jer...@marvell.com>
[snip]
diff --git a/lib/eal/include/rte_cman.h b/lib/eal/include/rte_cman.h
new file mode 100644
index 0000000000..1d84ddf0fb
--- /dev/null
+++ b/lib/eal/include/rte_cman.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2022 Marvell International Ltd.
+ */
+
+#ifndef RTE_CMAN_H
+#define RTE_CMAN_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <rte_bitops.h>
+
+/**
+ * @file
+ * Congestion management related parameters for DPDK.
+ */
+
+/** Congestion management modes */
+enum rte_cman_mode {
+ /**
+ * Congestion based on Random Early Detection.
+ *
+ * https://en.wikipedia.org/wiki/Random_early_detection
+ * http://www.aciri.org/floyd/papers/red/red.html
+ * @see struct rte_cman_red_params
+ */
+ RTE_CMAN_RED = RTE_BIT64(0),
I believe enums with 64-bit values are bad.
+};
+
+/**
+ * RED based congestion management configuration parameters.
+ */
+struct rte_cman_red_params {
+ /**
+ * Minimum threshold (min_th) value
+ *
+ * Value expressed as percentage. Value must be in 0 to 100(inclusive).
+ */
+ uint8_t min_th;
+ /**
+ * Maximum threshold (max_th) value
+ *
+ * Value expressed as percentage. Value must be in 0 to 100(inclusive).
+ */
+ uint8_t max_th;
+ /** Inverse of packet marking probability maximum value (maxp = 1 /
maxp_inv) */
+ uint16_t maxp_inv;
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_CMAN_H */
diff --git a/lib/ethdev/meson.build b/lib/ethdev/meson.build
index 47bb2625b0..59ad49114f 100644
--- a/lib/ethdev/meson.build
+++ b/lib/ethdev/meson.build
@@ -7,6 +7,7 @@ sources = files(
'ethdev_profile.c',
'ethdev_trace_points.c',
'rte_class_eth.c',
+ 'rte_cman.c',
'rte_ethdev.c',
'rte_flow.c',
'rte_mtr.c',
diff --git a/lib/ethdev/rte_cman.c b/lib/ethdev/rte_cman.c
new file mode 100644
index 0000000000..2093c247d1
--- /dev/null
+++ b/lib/ethdev/rte_cman.c
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2022 Marvell International Ltd.
+ */
+
+#include <stdint.h>
+
+#include <rte_errno.h>
+#include "rte_ethdev.h"
+#include "ethdev_driver.h"
+
+static int
+eth_err(uint16_t port_id, int ret)
+{
+ if (ret == 0)
+ return 0;
+
+ if (rte_eth_dev_is_removed(port_id))
+ return -EIO;
+
+ return ret;
+}
It is a dup from rte_ethdev.c. I think it should be moved to some
internal header.
+
+#define RTE_CMAN_FUNC_ERR_RET(func) \
There is nothing CMAN specific in the function except location.
Cc David who deprecated similar macros in the release cycle,
but this one have a log message.
+do { \
+ if (func == NULL) { \
+ RTE_ETHDEV_LOG(ERR, "Function not implemented\n"); \
+ return -ENOTSUP; \
+ } \
+} while (0)
+
+/* Get congestion management information for a port */
+int
+rte_eth_cman_info_get(uint16_t port_id, struct rte_eth_cman_info *info)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+
+ if (info == NULL) {
+ RTE_ETHDEV_LOG(ERR, "congestion management info is NULL\n");
+ return -EINVAL;
+ }
+
+ RTE_CMAN_FUNC_ERR_RET(dev->dev_ops->cman_info_get);
I think we should memset(&info, 0, sizeof(*info)) here since
all drivers must do it anyway.
+ return eth_err(port_id, (*dev->dev_ops->cman_info_get)(dev, info));
+}
+
+/* Initialize congestion management structure with default values */
+int
+rte_eth_cman_config_init(uint16_t port_id, struct rte_eth_cman_config *config)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+
+ if (config == NULL) {
+ RTE_ETHDEV_LOG(ERR, "congestion management config is NULL\n");
+ return -EINVAL;
+ }
+
+ RTE_CMAN_FUNC_ERR_RET(dev->dev_ops->cman_config_init);
+ return eth_err(port_id, (*dev->dev_ops->cman_config_init)(dev, config));
+}
+
+/* Configure congestion management on a port */
+int
+rte_eth_cman_config_set(uint16_t port_id, struct rte_eth_cman_config *config)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+
+ if (config == NULL) {
+ RTE_ETHDEV_LOG(ERR, "congestion management config is NULL\n");
+ return -EINVAL;
+ }
+
+ RTE_CMAN_FUNC_ERR_RET(dev->dev_ops->cman_config_set);
+ return eth_err(port_id, (*dev->dev_ops->cman_config_set)(dev, config));
+}
+
+/* Retrieve congestion management configuration of a port */
+int
+rte_eth_cman_config_get(uint16_t port_id, struct rte_eth_cman_config *config)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+
+ if (config == NULL) {
+ RTE_ETHDEV_LOG(ERR, "congestion management config is NULL\n");
+ return -EINVAL;
+ }
+
+ RTE_CMAN_FUNC_ERR_RET(dev->dev_ops->cman_config_get);
memset(&config, 0, sizeof(*config));
+ return eth_err(port_id, (*dev->dev_ops->cman_config_get)(dev, config));
+}
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index de9e970d4d..f4bb644c6a 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -160,6 +160,7 @@ extern "C" {
#define RTE_ETHDEV_DEBUG_TX
#endif
+#include <rte_cman.h>
#include <rte_compat.h>
#include <rte_log.h>
#include <rte_interrupts.h>
@@ -5506,6 +5507,156 @@ typedef struct {
__rte_experimental
int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
+/* Congestion management */
+
+/** Enumerate list of ethdev congestion management objects */
+enum rte_eth_cman_obj {
+ /** Congestion management based on Rx queue depth */
+ RTE_ETH_CMAN_OBJ_RX_QUEUE = RTE_BIT64(0),
+ /**
+ * Congestion management based on mempool depth associated with Rx queue
+ * @see rte_eth_rx_queue_setup()
+ */
+ RTE_ETH_CMAN_OBJ_RX_QUEUE_MEMPOOL = RTE_BIT64(1),
Do we really want one more enum with 64-bit initialized member?
IMHO, it should be either defines or 32-bit.
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change, or be removed, without prior
notice
+ *
+ * A structure used to retrieve information of ethdev congestion management.
+ */
+struct rte_eth_cman_info {
+ /**
+ * Set of supported congestion management modes
+ * @see enum rte_cman_mode
+ */
+ uint64_t modes_supported;
+ /**
+ * Set of supported congestion management objects
+ * @see enum rte_eth_cman_obj
+ */
+ uint64_t objs_supported;
+ /** Reserved for future fields */
I think we should highlight that it is set to zeros on get
right now.
+ uint8_t rsvd[8];
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change, or be removed, without prior
notice
+ *
+ * A structure used to configure the ethdev congestion management.
+ */
+struct rte_eth_cman_config {
+ /** Congestion management object */
+ enum rte_eth_cman_obj obj;
+ /** Congestion management mode */
+ enum rte_cman_mode mode;
+ union {
+ /**
+ * Rx queue to configure congestion management.
+ *
+ * Valid when object is RTE_ETH_CMAN_OBJ_RX_QUEUE or
+ * RTE_ETH_CMAN_OBJ_RX_QUEUE_MEMPOOL.
+ */
+ uint16_t rx_queue;
+ /** Reserved for future fields */
Must be zeros on get and set right now.
+ uint8_t rsvd_obj_params[4];
+ } obj_param;
+ union {
+ /**
+ * RED configuration parameters.
+ *
+ * Valid when mode is RTE_CMAN_RED.
+ */
+ struct rte_cman_red_params red;
+ /** Reserved for future fields */
+ uint8_t rsvd_mode_params[4];
same here.
+ } mode_param;
+};
+
[snip]
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Configure ethdev congestion management
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param config
+ * A pointer to a structure of type *rte_eth_cman_config* to be configured.
+ * @return
+ * - (0) if successful.
+ * - (-ENOTSUP) if support for cman_config_set does not exist.
+ * - (-ENODEV) if *port_id* invalid.
+ * - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_set(uint16_t port_id, struct rte_eth_cman_config
*config);
Just a niot, but may be config should be 'const'?
[snip]
