DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC PATCH] ethdev: support congestion management
@ 2022-05-30 13:15 jerinj
  2022-05-30 19:43 ` Ajit Khaparde
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: jerinj @ 2022-05-30 13:15 UTC (permalink / raw)
  To: dev, Ferruh Yigit, Thomas Monjalon, Andrew Rybchenko
  Cc: ajit.khaparde, aboyer, beilei.xing, bruce.richardson, chas3,
	chenbo.xia, ciara.loftus, dsinghrawat, ed.czeck, evgenys, grive,
	g.singh, zhouguoyang, haiyue.wang, hkalra, heinrich.kuhn,
	hemant.agrawal, hyonkim, igorch, irusskikh, jgrajcia,
	jasvinder.singh, jianwang, jiawenwu, jingjing.wu, johndale,
	john.miller, linville, keith.wiles, kirankumark, oulijun, lironh,
	longli, mw, spinler, matan, matt.peters, maxime.coquelin, mk,
	humin29, pnalla, ndabilpuram, qiming.yang, qi.z.zhang, radhac,
	rahul.lakkireddy, rmody, rosen.xu, sachin.saxena, skoteshwar,
	shshaikh, shaibran, shepard.siegel, asomalap, somnath.kotur,
	sthemmin, steven.webster, skori, mtetsuyah, vburru, viacheslavo,
	xiao.w.wang, cloud.wangxiaoyun, yisen.zhuang, yongwang,
	xuanziyang2, cristian.dumitrescu, Jerin Jacob

From: Jerin Jacob <jerinj@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 <jerinj@marvell.com>
---
 doc/guides/nics/features.rst         |  12 +++
 doc/guides/nics/features/default.ini |   1 +
 lib/eal/include/meson.build          |   1 +
 lib/eal/include/rte_cman.h           |  51 ++++++++++
 lib/ethdev/rte_ethdev.h              | 145 +++++++++++++++++++++++++++
 5 files changed, 210 insertions(+)
 create mode 100644 lib/eal/include/rte_cman.h

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 21bedb743f..ffcc2bcc92 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -727,6 +727,18 @@ Supports configuring per-queue stat counter mapping.
   ``rte_eth_dev_set_tx_queue_stats_mapping()``.
 
 
+.. _nic_features_congestion_management
+
+Congestion management
+---------------------
+
+Supports congestion management.
+
+* **[implements] eth_dev_ops**: ``cman_info_get``, ``cman_config_set``, ``cman_config_get``.
+* **[related]    API**: ``rte_eth_cman_info_get()``, ``rte_eth_cman_config_init()``,
+  ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
+
+
 .. _nic_features_fw_version:
 
 FW version
diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index b1d18ac62c..060497e41f 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -60,6 +60,7 @@ Tx descriptor status =
 Basic stats          =
 Extended stats       =
 Stats per queue      =
+Congestion management =
 FW version           =
 EEPROM dump          =
 Module EEPROM dump   =
diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index 9700494816..40e31e0e6d 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -10,6 +10,7 @@ headers += files(
         'rte_branch_prediction.h',
         'rte_bus.h',
         'rte_class.h',
+        'rte_cman.h',
         'rte_common.h',
         'rte_compat.h',
         'rte_debug.h',
diff --git a/lib/eal/include/rte_cman.h b/lib/eal/include/rte_cman.h
new file mode 100644
index 0000000000..e6dad2ebf7
--- /dev/null
+++ b/lib/eal/include/rte_cman.h
@@ -0,0 +1,51 @@
+/* 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
+	 * @see struct rte_cman_red_params
+	 */
+	RTE_CMAN_RED = RTE_BIT64(0),
+};
+
+/**
+ * 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/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 04cff8ee10..7c8718bb2e 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>
@@ -5452,6 +5453,150 @@ 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),
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API 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 */
+	uint8_t rsvd[64];
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API 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 */
+		uint8_t rsvd_obj_params[16];
+	} 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[64];
+	} mode_param;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the information for ethdev congestion management
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param info
+ *   A pointer to a structure of type *rte_eth_cman_info* to be filled with
+ *   the information about congestion management.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_info_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_info_get(uint16_t port_id, struct rte_eth_cman_info *info);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Initialize the ethdev congestion management configuration structure with default values.
+ *
+ * @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 initialized
+ *   with default value.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_init(uint16_t port_id, struct rte_eth_cman_config *config);
+
+/**
+ * @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);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the applied ethdev congestion management parameters for the given port.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param config
+ *   A pointer to a structure of type *rte_eth_cman_config* to retrieve
+ *   congestion management parameters for the given object.
+ *   Application must fill all parameters except mode_param parameter in
+ *   struct rte_eth_cman_config.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_config_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_get(uint16_t port_id, struct rte_eth_cman_config *config);
+
 #include <rte_ethdev_core.h>
 
 /**
-- 
2.36.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] ethdev: support congestion management
  2022-05-30 13:15 [dpdk-dev] [RFC PATCH] ethdev: support congestion management jerinj
@ 2022-05-30 19:43 ` Ajit Khaparde
  2022-07-13 12:11   ` Jerin Jacob
  2022-05-31  1:09 ` Min Hu (Connor)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 20+ messages in thread
From: Ajit Khaparde @ 2022-05-30 19:43 UTC (permalink / raw)
  To: Jerin Jacob Kollanukkaran
  Cc: dpdk-dev, Ferruh Yigit, Thomas Monjalon, Andrew Rybchenko,
	Andrew Boyer, Beilei Xing, Bruce Richardson, Chas Williams, Xia,
	Chenbo, Ciara Loftus, Devendra Singh Rawat, Ed Czeck,
	Evgeny Schemeilin, Gaetan Rivet, Gagandeep Singh, Guoyang Zhou,
	Haiyue Wang, Harman Kalra, Heinrich Kuhn, Hemant Agrawal,
	Hyong Youb Kim, Igor Chauskin, Igor Russkikh, Jakub Grajciar,
	Singh, Jasvinder, Jian Wang, Jiawen Wu, Jingjing Wu, John Daley,
	John Miller, John W. Linville, Wiles, Keith, Kiran Kumar K,
	oulijun, Liron Himi, Long Li, Marcin Wojtas, Martin Spinler,
	Matan Azrad, Matt Peters, Maxime Coquelin, Michal Krawczyk,
	Min Hu (Connor),
	Nalla Pradeep, Nithin Dabilpuram, Qiming Yang, Qi Zhang,
	Radha Mohan Chintakuntla, Rahul Lakkireddy, Rasesh Mody,
	Rosen Xu, Sachin Saxena (OSS),
	Satha Rao, Shahed Shaikh, Shai Brandes, Shepard Siegel,
	Somalapuram Amaranath, Somnath Kotur, Stephen Hemminger,
	Steven Webster, Sunil Kumar Kori, Tetsuya Mukawa,
	Veerasenareddy Burru, Slava Ovsiienko, Xiao Wang, Xiaoyun Wang,
	Yisen Zhuang, Yong Wang, Ziyang Xuan, Dumitrescu, Cristian

[-- Attachment #1: Type: text/plain, Size: 10369 bytes --]

On Mon, May 30, 2022 at 6:16 AM <jerinj@marvell.com> wrote:
>
> From: Jerin Jacob <jerinj@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.

Can you also add How all this helps an application? Thanks

>
>
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> ---
>  doc/guides/nics/features.rst         |  12 +++
>  doc/guides/nics/features/default.ini |   1 +
>  lib/eal/include/meson.build          |   1 +
>  lib/eal/include/rte_cman.h           |  51 ++++++++++
>  lib/ethdev/rte_ethdev.h              | 145 +++++++++++++++++++++++++++
>  5 files changed, 210 insertions(+)
>  create mode 100644 lib/eal/include/rte_cman.h
>
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index 21bedb743f..ffcc2bcc92 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -727,6 +727,18 @@ Supports configuring per-queue stat counter mapping.
>    ``rte_eth_dev_set_tx_queue_stats_mapping()``.
>
>
> +.. _nic_features_congestion_management
> +
> +Congestion management
> +---------------------
> +
> +Supports congestion management.
> +
> +* **[implements] eth_dev_ops**: ``cman_info_get``, ``cman_config_set``, ``cman_config_get``.
> +* **[related]    API**: ``rte_eth_cman_info_get()``, ``rte_eth_cman_config_init()``,
> +  ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
> +
> +
>  .. _nic_features_fw_version:
>
>  FW version
> diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
> index b1d18ac62c..060497e41f 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -60,6 +60,7 @@ Tx descriptor status =
>  Basic stats          =
>  Extended stats       =
>  Stats per queue      =
> +Congestion management =
>  FW version           =
>  EEPROM dump          =
>  Module EEPROM dump   =
> diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
> index 9700494816..40e31e0e6d 100644
> --- a/lib/eal/include/meson.build
> +++ b/lib/eal/include/meson.build
> @@ -10,6 +10,7 @@ headers += files(
>          'rte_branch_prediction.h',
>          'rte_bus.h',
>          'rte_class.h',
> +        'rte_cman.h',
>          'rte_common.h',
>          'rte_compat.h',
>          'rte_debug.h',
> diff --git a/lib/eal/include/rte_cman.h b/lib/eal/include/rte_cman.h
> new file mode 100644
> index 0000000000..e6dad2ebf7
> --- /dev/null
> +++ b/lib/eal/include/rte_cman.h
> @@ -0,0 +1,51 @@
> +/* 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
> +        * @see struct rte_cman_red_params
> +        */
> +       RTE_CMAN_RED = RTE_BIT64(0),
> +};
> +
> +/**
> + * 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/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04cff8ee10..7c8718bb2e 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>
> @@ -5452,6 +5453,150 @@ 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),
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API 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 */
> +       uint8_t rsvd[64];
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API 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 */
> +               uint8_t rsvd_obj_params[16];
> +       } 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[64];
> +       } mode_param;
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Retrieve the information for ethdev congestion management
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param info
> + *   A pointer to a structure of type *rte_eth_cman_info* to be filled with
> + *   the information about congestion management.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if support for cman_info_get does not exist.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_cman_info_get(uint16_t port_id, struct rte_eth_cman_info *info);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Initialize the ethdev congestion management configuration structure with default values.
> + *
> + * @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 initialized
> + *   with default value.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_cman_config_init(uint16_t port_id, struct rte_eth_cman_config *config);
> +
> +/**
> + * @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);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Retrieve the applied ethdev congestion management parameters for the given port.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param config
> + *   A pointer to a structure of type *rte_eth_cman_config* to retrieve
> + *   congestion management parameters for the given object.
> + *   Application must fill all parameters except mode_param parameter in
> + *   struct rte_eth_cman_config.
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if support for cman_config_get does not exist.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_cman_config_get(uint16_t port_id, struct rte_eth_cman_config *config);
> +
>  #include <rte_ethdev_core.h>
>
>  /**
> --
> 2.36.1
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] ethdev: support congestion management
  2022-05-30 13:15 [dpdk-dev] [RFC PATCH] ethdev: support congestion management jerinj
  2022-05-30 19:43 ` Ajit Khaparde
@ 2022-05-31  1:09 ` Min Hu (Connor)
  2022-07-13 12:16   ` Jerin Jacob
  2022-05-31 16:09 ` Stephen Hemminger
  2022-07-13 13:03 ` [dpdk-dev] [PATCH v1] " jerinj
  3 siblings, 1 reply; 20+ messages in thread
From: Min Hu (Connor) @ 2022-05-31  1:09 UTC (permalink / raw)
  To: jerinj, dev, Ferruh Yigit, Thomas Monjalon, Andrew Rybchenko
  Cc: ajit.khaparde, aboyer, beilei.xing, bruce.richardson, chas3,
	chenbo.xia, ciara.loftus, dsinghrawat, ed.czeck, evgenys, grive,
	g.singh, zhouguoyang, haiyue.wang, hkalra, heinrich.kuhn,
	hemant.agrawal, hyonkim, igorch, irusskikh, jgrajcia,
	jasvinder.singh, jianwang, jiawenwu, jingjing.wu, johndale,
	john.miller, linville, keith.wiles, kirankumark, oulijun, lironh,
	longli, mw, spinler, matan, matt.peters, maxime.coquelin, mk,
	pnalla, ndabilpuram, qiming.yang, qi.z.zhang, radhac,
	rahul.lakkireddy, rmody, rosen.xu, sachin.saxena, skoteshwar,
	shshaikh, shaibran, shepard.siegel, asomalap, somnath.kotur,
	sthemmin, steven.webster, skori, mtetsuyah, vburru, viacheslavo,
	xiao.w.wang, cloud.wangxiaoyun, yisen.zhuang, yongwang,
	xuanziyang2, cristian.dumitrescu

Hi, jerinj,

在 2022/5/30 21:15, jerinj@marvell.com 写道:
> From: Jerin Jacob <jerinj@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 <jerinj@marvell.com>
> ---
>   doc/guides/nics/features.rst         |  12 +++
>   doc/guides/nics/features/default.ini |   1 +
>   lib/eal/include/meson.build          |   1 +
>   lib/eal/include/rte_cman.h           |  51 ++++++++++
>   lib/ethdev/rte_ethdev.h              | 145 +++++++++++++++++++++++++++
>   5 files changed, 210 insertions(+)
>   create mode 100644 lib/eal/include/rte_cman.h
> 
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index 21bedb743f..ffcc2bcc92 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -727,6 +727,18 @@ Supports configuring per-queue stat counter mapping.
>     ``rte_eth_dev_set_tx_queue_stats_mapping()``.
>   
>   
> +.. _nic_features_congestion_management
> +
> +Congestion management
> +---------------------
> +
> +Supports congestion management.
> +
> +* **[implements] eth_dev_ops**: ``cman_info_get``, ``cman_config_set``, ``cman_config_get``.
> +* **[related]    API**: ``rte_eth_cman_info_get()``, ``rte_eth_cman_config_init()``,
> +  ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
> +
> +
>   .. _nic_features_fw_version:
>   
>   FW version
> diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
> index b1d18ac62c..060497e41f 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -60,6 +60,7 @@ Tx descriptor status =
>   Basic stats          =
>   Extended stats       =
>   Stats per queue      =
> +Congestion management =
>   FW version           =
>   EEPROM dump          =
>   Module EEPROM dump   =
> diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
> index 9700494816..40e31e0e6d 100644
> --- a/lib/eal/include/meson.build
> +++ b/lib/eal/include/meson.build
> @@ -10,6 +10,7 @@ headers += files(
>           'rte_branch_prediction.h',
>           'rte_bus.h',
>           'rte_class.h',
> +        'rte_cman.h',
>           'rte_common.h',
>           'rte_compat.h',
>           'rte_debug.h',
> diff --git a/lib/eal/include/rte_cman.h b/lib/eal/include/rte_cman.h
> new file mode 100644
> index 0000000000..e6dad2ebf7
> --- /dev/null
> +++ b/lib/eal/include/rte_cman.h
> @@ -0,0 +1,51 @@
> +/* 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
> +	 * @see struct rte_cman_red_params
> +	 */
> +	RTE_CMAN_RED = RTE_BIT64(0),
> +};
> +
> +/**
> + * 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/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04cff8ee10..7c8718bb2e 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>
> @@ -5452,6 +5453,150 @@ 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),
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
'API'  --> 'structure'
> + *
> + * 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 */
> +	uint8_t rsvd[64];
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
'API'  --> 'structure'
> + *
> + * 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 */
> +		uint8_t rsvd_obj_params[16];
> +	} 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[64];
> +	} mode_param;
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Retrieve the information for ethdev congestion management
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param info
> + *   A pointer to a structure of type *rte_eth_cman_info* to be filled with
> + *   the information about congestion management.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if support for cman_info_get does not exist.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_cman_info_get(uint16_t port_id, struct rte_eth_cman_info *info);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Initialize the ethdev congestion management configuration structure with default values.
> + *
> + * @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 initialized
> + *   with default value.
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_cman_config_init(uint16_t port_id, struct rte_eth_cman_config *config);
> +
> +/**
> + * @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);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Retrieve the applied ethdev congestion management parameters for the given port.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param config
> + *   A pointer to a structure of type *rte_eth_cman_config* to retrieve
> + *   congestion management parameters for the given object.
> + *   Application must fill all parameters except mode_param parameter in
> + *   struct rte_eth_cman_config.
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if support for cman_config_get does not exist.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
> +__rte_experimental
> +int rte_eth_cman_config_get(uint16_t port_id, struct rte_eth_cman_config *config);
> +
>   #include <rte_ethdev_core.h>
>   
>   /**
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] ethdev: support congestion management
  2022-05-30 13:15 [dpdk-dev] [RFC PATCH] ethdev: support congestion management jerinj
  2022-05-30 19:43 ` Ajit Khaparde
  2022-05-31  1:09 ` Min Hu (Connor)
@ 2022-05-31 16:09 ` Stephen Hemminger
  2022-07-13 12:25   ` Jerin Jacob
  2022-07-13 13:03 ` [dpdk-dev] [PATCH v1] " jerinj
  3 siblings, 1 reply; 20+ messages in thread
From: Stephen Hemminger @ 2022-05-31 16:09 UTC (permalink / raw)
  To: jerinj
  Cc: dev, Ferruh Yigit, Thomas Monjalon, Andrew Rybchenko,
	ajit.khaparde, aboyer, beilei.xing, bruce.richardson, chas3,
	chenbo.xia, ciara.loftus, dsinghrawat, ed.czeck, evgenys, grive,
	g.singh, zhouguoyang, haiyue.wang, hkalra, heinrich.kuhn,
	hemant.agrawal, hyonkim, igorch, irusskikh, jgrajcia,
	jasvinder.singh, jianwang, jiawenwu, jingjing.wu, johndale,
	john.miller, linville, keith.wiles, kirankumark, oulijun, lironh,
	longli, mw, spinler, matan, matt.peters, maxime.coquelin, mk,
	humin29, pnalla, ndabilpuram, qiming.yang, qi.z.zhang, radhac,
	rahul.lakkireddy, rmody, rosen.xu, sachin.saxena, skoteshwar,
	shshaikh, shaibran, shepard.siegel, asomalap, somnath.kotur,
	sthemmin, steven.webster, skori, mtetsuyah, vburru, viacheslavo,
	xiao.w.wang, cloud.wangxiaoyun, yisen.zhuang, yongwang,
	xuanziyang2, cristian.dumitrescu

On Mon, 30 May 2022 18:45:26 +0530
<jerinj@marvell.com> wrote:

> From: Jerin Jacob <jerinj@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 <jerinj@marvell.com>

The concept of supporting hardware queue management is good, but would like
to make some suggestions:

The hardware support of QOS should ideally be part of the existing DPDK
traffic management. For example, in Linux there are devices that can offload
traffic control (TC) to hardware and this is done via flags to existing
software infrastructure.

Your implementation makes HW and SW QoS diverge. This will require each application
to get even more dependent on a particular hardware device.

Which brings up the bigger picture problem. Don't want to repeat the problems
with rte_flow here. Any hardware support needs to have a matching set of software
implementation, and test cases.  The problem with rte_flow is the semantics are
poorly defined and each vendor does it differently; and the SW flow classifier
is incomplete and does not match what the HW does. 

In an ideal world, there would be:
- an abstract API for defining ingress and egress queue management
- a software implementation of that API
- multiple hardware implementations
- ability to transparently use both SW and HW queue management from application
- complete test suite for that API.

Yes, that is asking a lot. But as trailblazer in this area, you have to do the
hard work.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] ethdev: support congestion management
  2022-05-30 19:43 ` Ajit Khaparde
@ 2022-07-13 12:11   ` Jerin Jacob
  0 siblings, 0 replies; 20+ messages in thread
From: Jerin Jacob @ 2022-07-13 12:11 UTC (permalink / raw)
  To: Ajit Khaparde
  Cc: Jerin Jacob Kollanukkaran, dpdk-dev, Ferruh Yigit,
	Thomas Monjalon, Andrew Rybchenko, Andrew Boyer, Beilei Xing,
	Bruce Richardson, Chas Williams, Xia, Chenbo, Ciara Loftus,
	Devendra Singh Rawat, Ed Czeck, Evgeny Schemeilin, Gaetan Rivet,
	Gagandeep Singh, Guoyang Zhou, Haiyue Wang, Harman Kalra,
	Heinrich Kuhn, Hemant Agrawal, Hyong Youb Kim, Igor Chauskin,
	Igor Russkikh, Jakub Grajciar, Singh, Jasvinder, Jian Wang,
	Jiawen Wu, Jingjing Wu, John Daley, John Miller,
	John W. Linville, Wiles, Keith, Kiran Kumar K, oulijun,
	Liron Himi, Long Li, Marcin Wojtas, Martin Spinler, Matan Azrad,
	Matt Peters, Maxime Coquelin, Michal Krawczyk, Min Hu (Connor),
	Nalla Pradeep, Nithin Dabilpuram, Qiming Yang, Qi Zhang,
	Radha Mohan Chintakuntla, Rahul Lakkireddy, Rasesh Mody,
	Rosen Xu, Sachin Saxena (OSS),
	Satha Rao, Shahed Shaikh, Shai Brandes, Shepard Siegel,
	Somalapuram Amaranath, Somnath Kotur, Stephen Hemminger,
	Steven Webster, Sunil Kumar Kori, Tetsuya Mukawa,
	Veerasenareddy Burru, Slava Ovsiienko, Xiao Wang, Xiaoyun Wang,
	Yisen Zhuang, Yong Wang, Ziyang Xuan, Dumitrescu, Cristian

On Tue, May 31, 2022 at 1:13 AM Ajit Khaparde
<ajit.khaparde@broadcom.com> wrote:
>
> On Mon, May 30, 2022 at 6:16 AM <jerinj@marvell.com> wrote:
> >
> > From: Jerin Jacob <jerinj@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.
>
> Can you also add How all this helps an application? Thanks


Random early detection (RED), also known as random early discard or
random early drop is a queuing discipline for a network scheduler
suited for congestion avoidance.

In the conventional tail drop algorithm, a router or other network
component buffers as many packets as it can, and simply drops the ones
it cannot buffer. If buffers are constantly full, the network is
congested. Tail drop distributes buffer space unfairly among traffic
flows. Tail drop can also lead to TCP global synchronization as all
TCP connections "hold back" simultaneously, and then step forward
simultaneously. Networks become under-utilized and
flooded—alternately, in waves.

RED addresses these issues by pre-emptively dropping packets before
the buffer becomes completely full. It uses predictive models to
decide which packets to drop.

it is copied from https://en.wikipedia.org/wiki/Random_early_detection
page. I have kept this URL under enum rte_cman_mode:: RTE_CMAN_RED
too.
Also I will add original specification URL too,
http://www.aciri.org/floyd/papers/red/red.html.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] ethdev: support congestion management
  2022-05-31  1:09 ` Min Hu (Connor)
@ 2022-07-13 12:16   ` Jerin Jacob
  0 siblings, 0 replies; 20+ messages in thread
From: Jerin Jacob @ 2022-07-13 12:16 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: Jerin Jacob, dpdk-dev, Ferruh Yigit, Thomas Monjalon,
	Andrew Rybchenko, Ajit Khaparde, Andrew Boyer, Beilei Xing,
	Richardson, Bruce, Chas Williams, Xia, Chenbo, Ciara Loftus,
	Devendra Singh Rawat, Ed Czeck, Evgeny Schemeilin, Gaetan Rivet,
	Gagandeep Singh, Guoyang Zhou, Haiyue Wang, Harman Kalra,
	Heinrich Kuhn, Hemant Agrawal, Hyong Youb Kim, Igor Chauskin,
	Igor Russkikh, Jakub Grajciar, Jasvinder Singh, Jian Wang,
	Jiawen Wu, Jingjing Wu, John Daley, John Miller,
	John W. Linville, Wiles, Keith, Kiran Kumar K, Lijun Ou,
	Liron Himi, Long Li, Marcin Wojtas, Martin Spinler, Matan Azrad,
	Matt Peters, Maxime Coquelin, Michal Krawczyk,
	Pradeep Kumar Nalla, Nithin Dabilpuram, Qiming Yang, Qi Zhang,
	Radha Mohan Chintakuntla, Rahul Lakkireddy, Rasesh Mody,
	Rosen Xu, Sachin Saxena, Satha Koteswara Rao Kottidi,
	Shahed Shaikh, Shai Brandes, Shepard Siegel,
	Somalapuram Amaranath, Somnath Kotur, Stephen Hemminger,
	Steven Webster, Sunil Kumar Kori, Tetsuya Mukawa,
	Veerasenareddy Burru, Viacheslav Ovsiienko, Xiao Wang,
	Xiaoyun Wang, Yisen Zhuang, Yong Wang, Ziyang Xuan,
	Cristian Dumitrescu

On Tue, May 31, 2022 at 6:39 AM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> Hi, jerinj,

HI Min Hu,

> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> 'API'  --> 'structure'


Will fix it in v1. Thanks

> > + *
> > + * 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 */
> > +     uint8_t rsvd[64];
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> 'API'  --> 'structure'


Will fix it in v1. Thanks

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] ethdev: support congestion management
  2022-05-31 16:09 ` Stephen Hemminger
@ 2022-07-13 12:25   ` Jerin Jacob
  0 siblings, 0 replies; 20+ messages in thread
From: Jerin Jacob @ 2022-07-13 12:25 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Jerin Jacob, dpdk-dev, Ferruh Yigit, Thomas Monjalon,
	Andrew Rybchenko, Ajit Khaparde, Andrew Boyer, Beilei Xing,
	Richardson, Bruce, Chas Williams, Xia, Chenbo, Ciara Loftus,
	Devendra Singh Rawat, Ed Czeck, Evgeny Schemeilin, Gaetan Rivet,
	Gagandeep Singh, Guoyang Zhou, Haiyue Wang, Harman Kalra,
	Heinrich Kuhn, Hemant Agrawal, Hyong Youb Kim, Igor Chauskin,
	Igor Russkikh, Jakub Grajciar, Jasvinder Singh, Jian Wang,
	Jiawen Wu, Jingjing Wu, John Daley, John Miller,
	John W. Linville, Wiles, Keith, Kiran Kumar K, Lijun Ou,
	Liron Himi, Long Li, Marcin Wojtas, Martin Spinler, Matan Azrad,
	Matt Peters, Maxime Coquelin, Michal Krawczyk, Min Hu (Connor,
	Pradeep Kumar Nalla, Nithin Dabilpuram, Qiming Yang, Qi Zhang,
	Radha Mohan Chintakuntla, Rahul Lakkireddy, Rasesh Mody,
	Rosen Xu, Sachin Saxena, Satha Koteswara Rao Kottidi,
	Shahed Shaikh, Shai Brandes, Shepard Siegel,
	Somalapuram Amaranath, Somnath Kotur, Stephen Hemminger,
	Steven Webster, Sunil Kumar Kori, Tetsuya Mukawa,
	Veerasenareddy Burru, Viacheslav Ovsiienko, Xiao Wang,
	Xiaoyun Wang, Yisen Zhuang, Yong Wang, Ziyang Xuan,
	Cristian Dumitrescu

On Tue, May 31, 2022 at 9:39 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 30 May 2022 18:45:26 +0530
> <jerinj@marvell.com> wrote:
>
> > From: Jerin Jacob <jerinj@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 <jerinj@marvell.com>
>
> The concept of supporting hardware queue management is good, but would like
> to make some suggestions:
>
> The hardware support of QOS should ideally be part of the existing DPDK
> traffic management. For example, in Linux there are devices that can offload
> traffic control (TC) to hardware and this is done via flags to existing
> software infrastructure.
>
> Your implementation makes HW and SW QoS diverge. This will require each application
> to get even more dependent on a particular hardware device.
>
> Which brings up the bigger picture problem. Don't want to repeat the problems
> with rte_flow here. Any hardware support needs to have a matching set of software
> implementation, and test cases.  The problem with rte_flow is the semantics are
> poorly defined and each vendor does it differently; and the SW flow classifier
> is incomplete and does not match what the HW does.
>
> In an ideal world, there would be:
> - an abstract API for defining ingress and egress queue management

It is already available as rte_tm and rte_mtr API. Only congestion
management is missing. That is added in the patch.


> - a software implementation of that API

It is already available. See lib/sched/rte_red.h

> - multiple hardware implementations
> - ability to transparently use both SW and HW queue management from application
> - complete test suite for that API.
>
> Yes, that is asking a lot. But as trailblazer in this area, you have to do the
> hard work.

None of the new features in rte_flow, rte_ethdev or rte_tm etc NOT
going in this path.
If we think, we need to enforce this path(SW driver is a must) for any
feature. Let's discuss here and TB meeting and decide and
we need to make that policy for any new contribution.



>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [dpdk-dev] [PATCH v1] ethdev: support congestion management
  2022-05-30 13:15 [dpdk-dev] [RFC PATCH] ethdev: support congestion management jerinj
                   ` (2 preceding siblings ...)
  2022-05-31 16:09 ` Stephen Hemminger
@ 2022-07-13 13:03 ` jerinj
  2022-09-19 12:15   ` [PATCH v2 1/1] " skori
  3 siblings, 1 reply; 20+ messages in thread
From: jerinj @ 2022-07-13 13:03 UTC (permalink / raw)
  To: dev, Ferruh Yigit, Thomas Monjalon, Andrew Rybchenko
  Cc: ajit.khaparde, aboyer, beilei.xing, bruce.richardson, chas3,
	chenbo.xia, ciara.loftus, dsinghrawat, ed.czeck, evgenys, grive,
	g.singh, zhouguoyang, haiyue.wang, hkalra, heinrich.kuhn,
	hemant.agrawal, hyonkim, igorch, irusskikh, jgrajcia,
	jasvinder.singh, jianwang, jiawenwu, jingjing.wu, johndale,
	john.miller, linville, keith.wiles, kirankumark, oulijun, lironh,
	longli, mw, spinler, matan, matt.peters, maxime.coquelin, mk,
	humin29, pnalla, ndabilpuram, qiming.yang, qi.z.zhang, radhac,
	rahul.lakkireddy, rmody, rosen.xu, sachin.saxena, skoteshwar,
	shshaikh, shaibran, shepard.siegel, asomalap, somnath.kotur,
	sthemmin, steven.webster, skori, mtetsuyah, vburru, viacheslavo,
	xiao.w.wang, cloud.wangxiaoyun, yisen.zhuang, yongwang,
	xuanziyang2, cristian.dumitrescu, Jerin Jacob

From: Jerin Jacob <jerinj@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 <jerinj@marvell.com>
---

rfc..v1:
- Added RED specification (http://www.aciri.org/floyd/papers/red/red.html) link
- Fixed doxygen comment issue (Min Hu)


 doc/guides/nics/features.rst         |  12 +++
 doc/guides/nics/features/default.ini |   1 +
 lib/eal/include/meson.build          |   1 +
 lib/eal/include/rte_cman.h           |  52 ++++++++++
 lib/ethdev/rte_ethdev.h              | 145 +++++++++++++++++++++++++++
 5 files changed, 211 insertions(+)
 create mode 100644 lib/eal/include/rte_cman.h

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 7f6cb914a5..aa22d8bb22 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -727,6 +727,18 @@ Supports configuring per-queue stat counter mapping.
   ``rte_eth_dev_set_tx_queue_stats_mapping()``.
 
 
+.. _nic_features_congestion_management:
+
+Congestion management
+---------------------
+
+Supports congestion management.
+
+* **[implements] eth_dev_ops**: ``cman_info_get``, ``cman_config_set``, ``cman_config_get``.
+* **[related]    API**: ``rte_eth_cman_info_get()``, ``rte_eth_cman_config_init()``,
+  ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
+
+
 .. _nic_features_fw_version:
 
 FW version
diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index d1db0c256a..38a5767b06 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -60,6 +60,7 @@ Tx descriptor status =
 Basic stats          =
 Extended stats       =
 Stats per queue      =
+Congestion management =
 FW version           =
 EEPROM dump          =
 Module EEPROM dump   =
diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index fd6e844224..e569ba7cf4 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -10,6 +10,7 @@ headers += files(
         'rte_branch_prediction.h',
         'rte_bus.h',
         'rte_class.h',
+        'rte_cman.h',
         'rte_common.h',
         'rte_compat.h',
         'rte_debug.h',
diff --git a/lib/eal/include/rte_cman.h b/lib/eal/include/rte_cman.h
new file mode 100644
index 0000000000..e50dd28bc4
--- /dev/null
+++ b/lib/eal/include/rte_cman.h
@@ -0,0 +1,52 @@
+/* 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),
+};
+
+/**
+ * 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/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index de9e970d4d..6a342df64a 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,150 @@ 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),
+};
+
+/**
+ * @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 */
+	uint8_t rsvd[64];
+};
+
+/**
+ * @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 */
+		uint8_t rsvd_obj_params[16];
+	} 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[64];
+	} mode_param;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the information for ethdev congestion management
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param info
+ *   A pointer to a structure of type *rte_eth_cman_info* to be filled with
+ *   the information about congestion management.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_info_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_info_get(uint16_t port_id, struct rte_eth_cman_info *info);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Initialize the ethdev congestion management configuration structure with default values.
+ *
+ * @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 initialized
+ *   with default value.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_init(uint16_t port_id, struct rte_eth_cman_config *config);
+
+/**
+ * @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);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the applied ethdev congestion management parameters for the given port.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param config
+ *   A pointer to a structure of type *rte_eth_cman_config* to retrieve
+ *   congestion management parameters for the given object.
+ *   Application must fill all parameters except mode_param parameter in
+ *   struct rte_eth_cman_config.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_config_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_get(uint16_t port_id, struct rte_eth_cman_config *config);
+
 #include <rte_ethdev_core.h>
 
 /**
-- 
2.37.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/1] ethdev: support congestion management
  2022-07-13 13:03 ` [dpdk-dev] [PATCH v1] " jerinj
@ 2022-09-19 12:15   ` skori
  2022-09-27 14:36     ` Thomas Monjalon
                       ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: skori @ 2022-09-19 12:15 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon, Andrew Rybchenko, Ray Kinsella
  Cc: dev, Jerin Jacob

From: Jerin Jacob <jerinj@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 <jerinj@marvell.com>
---
v1..v2:
 - Fix review comments (Akhil Goyal)

rfc..v1:
 - Added RED specification (http://www.aciri.org/floyd/papers/red/red.html) link
 - Fixed doxygen comment issue (Min Hu)

 doc/guides/nics/features.rst         |  12 +++
 doc/guides/nics/features/default.ini |   1 +
 lib/eal/include/meson.build          |   1 +
 lib/eal/include/rte_cman.h           |  55 ++++++++++
 lib/ethdev/ethdev_driver.h           |  25 +++++
 lib/ethdev/meson.build               |   1 +
 lib/ethdev/rte_cman.c                | 101 ++++++++++++++++++
 lib/ethdev/rte_ethdev.h              | 151 +++++++++++++++++++++++++++
 lib/ethdev/version.map               |   6 ++
 9 files changed, 353 insertions(+)
 create mode 100644 lib/eal/include/rte_cman.h
 create mode 100644 lib/ethdev/rte_cman.c

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index 7f6cb914a5..aa22d8bb22 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -727,6 +727,18 @@ Supports configuring per-queue stat counter mapping.
   ``rte_eth_dev_set_tx_queue_stats_mapping()``.
 
 
+.. _nic_features_congestion_management:
+
+Congestion management
+---------------------
+
+Supports congestion management.
+
+* **[implements] eth_dev_ops**: ``cman_info_get``, ``cman_config_set``, ``cman_config_get``.
+* **[related]    API**: ``rte_eth_cman_info_get()``, ``rte_eth_cman_config_init()``,
+  ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
+
+
 .. _nic_features_fw_version:
 
 FW version
diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index d1db0c256a..38a5767b06 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -60,6 +60,7 @@ Tx descriptor status =
 Basic stats          =
 Extended stats       =
 Stats per queue      =
+Congestion management =
 FW version           =
 EEPROM dump          =
 Module EEPROM dump   =
diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build
index fd6e844224..e569ba7cf4 100644
--- a/lib/eal/include/meson.build
+++ b/lib/eal/include/meson.build
@@ -10,6 +10,7 @@ headers += files(
         'rte_branch_prediction.h',
         'rte_bus.h',
         'rte_class.h',
+        'rte_cman.h',
         'rte_common.h',
         'rte_compat.h',
         'rte_debug.h',
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),
+};
+
+/**
+ * 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/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 5101868ea7..9b6ad5c5c1 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1093,6 +1093,22 @@ typedef int (*eth_rx_queue_avail_thresh_query_t)(struct rte_eth_dev *dev,
 					uint16_t *rx_queue_id,
 					uint8_t *avail_thresh);
 
+/** @internal Get congestion management information. */
+typedef int (*eth_cman_info_get_t)(struct rte_eth_dev *dev,
+				struct rte_eth_cman_info *info);
+
+/** @internal Init congestion management structure with default values. */
+typedef int (*eth_cman_config_init_t)(struct rte_eth_dev *dev,
+				struct rte_eth_cman_config *config);
+
+/** @internal Configure congestion management on a port. */
+typedef int (*eth_cman_config_set_t)(struct rte_eth_dev *dev,
+				struct rte_eth_cman_config *config);
+
+/** @internal Retrieve congestion management configuration of a port. */
+typedef int (*eth_cman_config_get_t)(struct rte_eth_dev *dev,
+				struct rte_eth_cman_config *config);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1308,6 +1324,15 @@ struct eth_dev_ops {
 	eth_rx_queue_avail_thresh_set_t rx_queue_avail_thresh_set;
 	/** Query Rx queue available descriptors threshold event */
 	eth_rx_queue_avail_thresh_query_t rx_queue_avail_thresh_query;
+
+	/** Get congestion management information */
+	eth_cman_info_get_t cman_info_get;
+	/** Initialize congestion management structure with default values */
+	eth_cman_config_init_t cman_config_init;
+	/** Configure congestion management */
+	eth_cman_config_set_t cman_config_set;
+	/** Retrieve congestion management configuration */
+	eth_cman_config_get_t cman_config_get;
 };
 
 /**
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;
+}
+
+#define RTE_CMAN_FUNC_ERR_RET(func)					\
+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);
+	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);
+	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),
+};
+
+/**
+ * @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 */
+	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 */
+		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];
+	} mode_param;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the information for ethdev congestion management
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param info
+ *   A pointer to a structure of type *rte_eth_cman_info* to be filled with
+ *   the information about congestion management.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_info_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_info_get(uint16_t port_id, struct rte_eth_cman_info *info);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Initialize the ethdev congestion management configuration structure with default values.
+ *
+ * @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 initialized
+ *   with default value.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_config_init does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_init(uint16_t port_id, struct rte_eth_cman_config *config);
+
+/**
+ * @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);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the applied ethdev congestion management parameters for the given port.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param config
+ *   A pointer to a structure of type *rte_eth_cman_config* to retrieve
+ *   congestion management parameters for the given object.
+ *   Application must fill all parameters except mode_param parameter in
+ *   struct rte_eth_cman_config.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_config_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_get(uint16_t port_id, struct rte_eth_cman_config *config);
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 03f52fee91..ea9b9497ad 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -285,6 +285,12 @@ EXPERIMENTAL {
 	rte_mtr_color_in_protocol_priority_get;
 	rte_mtr_color_in_protocol_set;
 	rte_mtr_meter_vlan_table_update;
+
+	# added in 22.11
+	rte_eth_cman_config_get;
+	rte_eth_cman_config_init;
+	rte_eth_cman_config_set;
+	rte_eth_cman_info_get;
 };
 
 INTERNAL {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-19 12:15   ` [PATCH v2 1/1] " skori
@ 2022-09-27 14:36     ` Thomas Monjalon
  2022-09-27 15:09       ` Bruce Richardson
  2022-09-28  8:19     ` Andrew Rybchenko
  2022-09-29  9:35     ` [PATCH v3 " skori
  2 siblings, 1 reply; 20+ messages in thread
From: Thomas Monjalon @ 2022-09-27 14:36 UTC (permalink / raw)
  To: Jerin Jacob, skori
  Cc: Ferruh Yigit, Andrew Rybchenko, dev, david.marchand, orika

19/09/2022 14:15, skori@marvell.com:
> From: Jerin Jacob <jerinj@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.
[...]
>  doc/guides/nics/features.rst         |  12 +++
>  doc/guides/nics/features/default.ini |   1 +
>  lib/eal/include/meson.build          |   1 +
>  lib/eal/include/rte_cman.h           |  55 ++++++++++
>  lib/ethdev/ethdev_driver.h           |  25 +++++
>  lib/ethdev/meson.build               |   1 +
>  lib/ethdev/rte_cman.c                | 101 ++++++++++++++++++
>  lib/ethdev/rte_ethdev.h              | 151 +++++++++++++++++++++++++++
>  lib/ethdev/version.map               |   6 ++

I feel EAL is not the right place for CMAN definitions.

After a discussion with Jerin, I understand we could use
the same definitions in other API classes, not only ethdev.
However I think this .h file should be better hosted in
lib/ethdev/ with its own namespace rte_cman.
Then other libs could include this rte_cman.h
without having a strong dependency on ethdev.

Deal?



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-27 14:36     ` Thomas Monjalon
@ 2022-09-27 15:09       ` Bruce Richardson
  2022-09-28 11:14         ` Jerin Jacob
  0 siblings, 1 reply; 20+ messages in thread
From: Bruce Richardson @ 2022-09-27 15:09 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Jerin Jacob, skori, Ferruh Yigit, Andrew Rybchenko, dev,
	david.marchand, orika

On Tue, Sep 27, 2022 at 04:36:18PM +0200, Thomas Monjalon wrote:
> 19/09/2022 14:15, skori@marvell.com:
> > From: Jerin Jacob <jerinj@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.
> [...]
> >  doc/guides/nics/features.rst         |  12 +++
> >  doc/guides/nics/features/default.ini |   1 +
> >  lib/eal/include/meson.build          |   1 +
> >  lib/eal/include/rte_cman.h           |  55 ++++++++++
> >  lib/ethdev/ethdev_driver.h           |  25 +++++
> >  lib/ethdev/meson.build               |   1 + lib/ethdev/rte_cman.c |
> >  101 ++++++++++++++++++ lib/ethdev/rte_ethdev.h              | 151
> >  +++++++++++++++++++++++++++ lib/ethdev/version.map               |   6
> >  ++
> 
> I feel EAL is not the right place for CMAN definitions.
> 
> After a discussion with Jerin, I understand we could use the same
> definitions in other API classes, not only ethdev.  However I think this
> .h file should be better hosted in lib/ethdev/ with its own namespace
> rte_cman.  Then other libs could include this rte_cman.h without having a
> strong dependency on ethdev.
> 
> Deal?
> 
Would rte_net also be an option? Alternatively, is this related to the work
and structures defined in the meter library header, since it seems also
related to congestion management on ingress?

/Bruce

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-19 12:15   ` [PATCH v2 1/1] " skori
  2022-09-27 14:36     ` Thomas Monjalon
@ 2022-09-28  8:19     ` Andrew Rybchenko
  2022-09-28 12:20       ` Jerin Jacob
  2022-09-29  9:35     ` [PATCH v3 " skori
  2 siblings, 1 reply; 20+ messages in thread
From: Andrew Rybchenko @ 2022-09-28  8:19 UTC (permalink / raw)
  To: skori, Ferruh Yigit, Thomas Monjalon, Ray Kinsella
  Cc: dev, Jerin Jacob, david.marchand

Cc David. See some kind of reincarnation of RTE_FUNC_PTR_*
macro below.

On 9/19/22 15:15, skori@marvell.com wrote:
> From: Jerin Jacob <jerinj@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 <jerinj@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]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-27 15:09       ` Bruce Richardson
@ 2022-09-28 11:14         ` Jerin Jacob
  2022-09-28 12:08           ` Thomas Monjalon
  0 siblings, 1 reply; 20+ messages in thread
From: Jerin Jacob @ 2022-09-28 11:14 UTC (permalink / raw)
  To: Bruce Richardson, Olivier Matz
  Cc: Thomas Monjalon, Jerin Jacob, skori, Ferruh Yigit,
	Andrew Rybchenko, dev, david.marchand, orika

On Tue, Sep 27, 2022 at 8:39 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Sep 27, 2022 at 04:36:18PM +0200, Thomas Monjalon wrote:

> > >  ++
> >
> > I feel EAL is not the right place for CMAN definitions.
> >
> > After a discussion with Jerin, I understand we could use the same
> > definitions in other API classes, not only ethdev.  However I think this
> > .h file should be better hosted in lib/ethdev/ with its own namespace
> > rte_cman.  Then other libs could include this rte_cman.h without having a
> > strong dependency on ethdev.
> >
> > Deal?
> >
> Would rte_net also be an option? Alternatively, is this related to the work

Looks like the net is a good option. Since it is just a header file
any place would be OK.

+ @Olivier Matz

Any objections to keeping rte_cman.h to lib/net?

> and structures defined in the meter library header, since it seems also
> related to congestion management on ingress?
>
> /Bruce

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-28 11:14         ` Jerin Jacob
@ 2022-09-28 12:08           ` Thomas Monjalon
  2022-09-28 12:23             ` Jerin Jacob
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Monjalon @ 2022-09-28 12:08 UTC (permalink / raw)
  To: Bruce Richardson, Olivier Matz, Jerin Jacob
  Cc: Jerin Jacob, skori, Ferruh Yigit, Andrew Rybchenko, dev,
	david.marchand, orika

28/09/2022 13:14, Jerin Jacob:
> On Tue, Sep 27, 2022 at 8:39 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Tue, Sep 27, 2022 at 04:36:18PM +0200, Thomas Monjalon wrote:
> 
> > > >  ++
> > >
> > > I feel EAL is not the right place for CMAN definitions.
> > >
> > > After a discussion with Jerin, I understand we could use the same
> > > definitions in other API classes, not only ethdev.  However I think this
> > > .h file should be better hosted in lib/ethdev/ with its own namespace
> > > rte_cman.  Then other libs could include this rte_cman.h without having a
> > > strong dependency on ethdev.
> > >
> > > Deal?
> > >
> > Would rte_net also be an option? Alternatively, is this related to the work
> 
> Looks like the net is a good option. Since it is just a header file
> any place would be OK.
> 
> + @Olivier Matz
> 
> Any objections to keeping rte_cman.h to lib/net?

lib/net/ is supposed to be standardized packet headers definitions.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-28  8:19     ` Andrew Rybchenko
@ 2022-09-28 12:20       ` Jerin Jacob
  0 siblings, 0 replies; 20+ messages in thread
From: Jerin Jacob @ 2022-09-28 12:20 UTC (permalink / raw)
  To: Andrew Rybchenko
  Cc: skori, Ferruh Yigit, Thomas Monjalon, Ray Kinsella, dev,
	Jerin Jacob, david.marchand

On Wed, Sep 28, 2022 at 1:50 PM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
>
> Cc David. See some kind of reincarnation of RTE_FUNC_PTR_*
> macro below.
>

> > +
> > +/**
> > + * @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.

No strong opinion. Will change to 32bit.

> > --- /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.

Ack.


>
> > +
> > +#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.

Yes. It has an additional log function. No strong opinion
Will remove this macro.

>
> > +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.

Will do.

>
> > +     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));

Ack

>
> > +     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.

Will keep it 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.

Will change as "Reserved for future fields. Always return as zero when
rte_eth_cman_config_get() invoked"


>
> > +     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.

Will change as "Reserved for future fields. Must be set to zero on get
and set operation "

>
> > +             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.

Ack

> > +     } 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'?

Yes. We can keep as const.

>
> [snip]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-28 12:08           ` Thomas Monjalon
@ 2022-09-28 12:23             ` Jerin Jacob
  2022-09-28 13:07               ` Olivier Matz
  0 siblings, 1 reply; 20+ messages in thread
From: Jerin Jacob @ 2022-09-28 12:23 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Bruce Richardson, Olivier Matz, Jerin Jacob, skori, Ferruh Yigit,
	Andrew Rybchenko, dev, david.marchand, orika

On Wed, Sep 28, 2022 at 5:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 28/09/2022 13:14, Jerin Jacob:
> > On Tue, Sep 27, 2022 at 8:39 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Tue, Sep 27, 2022 at 04:36:18PM +0200, Thomas Monjalon wrote:
> >
> > > > >  ++
> > > >
> > > > I feel EAL is not the right place for CMAN definitions.
> > > >
> > > > After a discussion with Jerin, I understand we could use the same
> > > > definitions in other API classes, not only ethdev.  However I think this
> > > > .h file should be better hosted in lib/ethdev/ with its own namespace
> > > > rte_cman.  Then other libs could include this rte_cman.h without having a
> > > > strong dependency on ethdev.
> > > >
> > > > Deal?
> > > >
> > > Would rte_net also be an option? Alternatively, is this related to the work
> >
> > Looks like the net is a good option. Since it is just a header file
> > any place would be OK.
> >
> > + @Olivier Matz
> >
> > Any objections to keeping rte_cman.h to lib/net?
>
> lib/net/ is supposed to be standardized packet headers definitions.

Since it had lib/net/net_crc.h, I thought it is OK to keep it in lib/net.
No strong opinion.  I will keep it as lib/ethdev/rte_cman.h then.


>
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/1] ethdev: support congestion management
  2022-09-28 12:23             ` Jerin Jacob
@ 2022-09-28 13:07               ` Olivier Matz
  0 siblings, 0 replies; 20+ messages in thread
From: Olivier Matz @ 2022-09-28 13:07 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Thomas Monjalon, Bruce Richardson, Jerin Jacob, skori,
	Ferruh Yigit, Andrew Rybchenko, dev, david.marchand, orika

On Wed, Sep 28, 2022 at 05:53:06PM +0530, Jerin Jacob wrote:
> On Wed, Sep 28, 2022 at 5:38 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 28/09/2022 13:14, Jerin Jacob:
> > > On Tue, Sep 27, 2022 at 8:39 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > On Tue, Sep 27, 2022 at 04:36:18PM +0200, Thomas Monjalon wrote:
> > >
> > > > > >  ++
> > > > >
> > > > > I feel EAL is not the right place for CMAN definitions.
> > > > >
> > > > > After a discussion with Jerin, I understand we could use the same
> > > > > definitions in other API classes, not only ethdev.  However I think this
> > > > > .h file should be better hosted in lib/ethdev/ with its own namespace
> > > > > rte_cman.  Then other libs could include this rte_cman.h without having a
> > > > > strong dependency on ethdev.
> > > > >
> > > > > Deal?
> > > > >
> > > > Would rte_net also be an option? Alternatively, is this related to the work
> > >
> > > Looks like the net is a good option. Since it is just a header file
> > > any place would be OK.
> > >
> > > + @Olivier Matz
> > >
> > > Any objections to keeping rte_cman.h to lib/net?
> >
> > lib/net/ is supposed to be standardized packet headers definitions.
> 
> Since it had lib/net/net_crc.h, I thought it is OK to keep it in lib/net.
> No strong opinion.  I will keep it as lib/ethdev/rte_cman.h then.

lib/ethdev/rte_cman.h looks better to me too.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v3 1/1] ethdev: support congestion management
  2022-09-19 12:15   ` [PATCH v2 1/1] " skori
  2022-09-27 14:36     ` Thomas Monjalon
  2022-09-28  8:19     ` Andrew Rybchenko
@ 2022-09-29  9:35     ` skori
  2022-10-04  9:02       ` Andrew Rybchenko
  2 siblings, 1 reply; 20+ messages in thread
From: skori @ 2022-09-29  9:35 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon, Andrew Rybchenko, Ray Kinsella
  Cc: dev, Jerin Jacob, Sunil Kumar Kori

From: Jerin Jacob <jerinj@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 <jerinj@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v2..v3:
 - Rename rte_cman.c to rte_ethdev_cman.c
 - Move lib/eal/include/rte_cman.h to lib/ethdev/rte_cman.h
 - Fix review comments (Andrew Rybchenko)
 - Add release notes

v1..v2:
 - Fix review comments (Akhil Goyal)

rfc..v1:
 - Added RED specification (http://www.aciri.org/floyd/papers/red/red.html) link
 - Fixed doxygen comment issue (Min Hu)

 doc/guides/nics/features.rst           |  12 ++
 doc/guides/nics/features/default.ini   |   1 +
 doc/guides/rel_notes/release_22_11.rst |   6 +
 lib/ethdev/ethdev_driver.h             |  25 ++++
 lib/ethdev/ethdev_private.c            |  12 ++
 lib/ethdev/ethdev_private.h            |   2 +
 lib/ethdev/meson.build                 |   2 +
 lib/ethdev/rte_cman.h                  |  55 +++++++++
 lib/ethdev/rte_ethdev.h                | 164 +++++++++++++++++++++++++
 lib/ethdev/rte_ethdev_cman.c           | 101 +++++++++++++++
 lib/ethdev/version.map                 |   6 +
 11 files changed, 386 insertions(+)
 create mode 100644 lib/ethdev/rte_cman.h
 create mode 100644 lib/ethdev/rte_ethdev_cman.c

diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index b4a8e9881c..70ca46e651 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -727,6 +727,18 @@ Supports configuring per-queue stat counter mapping.
   ``rte_eth_dev_set_tx_queue_stats_mapping()``.
 
 
+.. _nic_features_congestion_management:
+
+Congestion management
+---------------------
+
+Supports congestion management.
+
+* **[implements] eth_dev_ops**: ``cman_info_get``, ``cman_config_set``, ``cman_config_get``.
+* **[related]    API**: ``rte_eth_cman_info_get()``, ``rte_eth_cman_config_init()``,
+  ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
+
+
 .. _nic_features_fw_version:
 
 FW version
diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
index f7192cb0da..a9c0008ebd 100644
--- a/doc/guides/nics/features/default.ini
+++ b/doc/guides/nics/features/default.ini
@@ -60,6 +60,7 @@ Tx descriptor status =
 Basic stats          =
 Extended stats       =
 Stats per queue      =
+Congestion management =
 FW version           =
 EEPROM dump          =
 Module EEPROM dump   =
diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index 0231959874..ea9908e578 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -81,6 +81,12 @@ New Features
   * Added AES-CCM support in lookaside protocol (IPsec) for CN9K & CN10K.
   * Added AES & DES DOCSIS algorithm support in lookaside crypto for CN9K.
 
+* **Added support for congestion management for ethdev.**
+
+  Added new APIs ``rte_eth_cman_config_init()``, ``rte_eth_cman_config_get()``,
+  ``rte_eth_cman_config_set()``, ``rte_eth_cman_info_get()``
+  to support congestion management.
+
 * **Added eventdev adapter instance get API.**
 
   * Added ``rte_event_eth_rx_adapter_instance_get`` to get Rx adapter
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 8cd8eb8685..e1e2d10a35 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1094,6 +1094,22 @@ typedef int (*eth_rx_queue_avail_thresh_query_t)(struct rte_eth_dev *dev,
 					uint16_t *rx_queue_id,
 					uint8_t *avail_thresh);
 
+/** @internal Get congestion management information. */
+typedef int (*eth_cman_info_get_t)(struct rte_eth_dev *dev,
+				struct rte_eth_cman_info *info);
+
+/** @internal Init congestion management structure with default values. */
+typedef int (*eth_cman_config_init_t)(struct rte_eth_dev *dev,
+				struct rte_eth_cman_config *config);
+
+/** @internal Configure congestion management on a port. */
+typedef int (*eth_cman_config_set_t)(struct rte_eth_dev *dev,
+				const struct rte_eth_cman_config *config);
+
+/** @internal Retrieve congestion management configuration of a port. */
+typedef int (*eth_cman_config_get_t)(struct rte_eth_dev *dev,
+				struct rte_eth_cman_config *config);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1309,6 +1325,15 @@ struct eth_dev_ops {
 	eth_rx_queue_avail_thresh_set_t rx_queue_avail_thresh_set;
 	/** Query Rx queue available descriptors threshold event */
 	eth_rx_queue_avail_thresh_query_t rx_queue_avail_thresh_query;
+
+	/** Get congestion management information */
+	eth_cman_info_get_t cman_info_get;
+	/** Initialize congestion management structure with default values */
+	eth_cman_config_init_t cman_config_init;
+	/** Configure congestion management */
+	eth_cman_config_set_t cman_config_set;
+	/** Retrieve congestion management configuration */
+	eth_cman_config_get_t cman_config_get;
 };
 
 /**
diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c
index 48090c879a..8787c3985e 100644
--- a/lib/ethdev/ethdev_private.c
+++ b/lib/ethdev/ethdev_private.c
@@ -27,6 +27,18 @@ eth_dev_to_id(const struct rte_eth_dev *dev)
 	return dev - rte_eth_devices;
 }
 
+int32_t
+eth_check_err(struct rte_eth_dev *dev, int ret)
+{
+	if (ret == 0)
+		return 0;
+
+	if (rte_eth_dev_is_removed(eth_dev_to_id(dev)))
+		return -EIO;
+
+	return ret;
+}
+
 struct rte_eth_dev *
 eth_find_device(const struct rte_eth_dev *start, rte_eth_cmp_t cmp,
 		const void *data)
diff --git a/lib/ethdev/ethdev_private.h b/lib/ethdev/ethdev_private.h
index cc9879907c..c7ac074029 100644
--- a/lib/ethdev/ethdev_private.h
+++ b/lib/ethdev/ethdev_private.h
@@ -69,4 +69,6 @@ void eth_dev_txq_release(struct rte_eth_dev *dev, uint16_t qid);
 int eth_dev_rx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues);
 int eth_dev_tx_queue_config(struct rte_eth_dev *dev, uint16_t nb_queues);
 
+int32_t eth_check_err(struct rte_eth_dev *dev, int ret);
+
 #endif /* _ETH_PRIVATE_H_ */
diff --git a/lib/ethdev/meson.build b/lib/ethdev/meson.build
index 47bb2625b0..39250b5da1 100644
--- a/lib/ethdev/meson.build
+++ b/lib/ethdev/meson.build
@@ -8,6 +8,7 @@ sources = files(
         'ethdev_trace_points.c',
         'rte_class_eth.c',
         'rte_ethdev.c',
+        'rte_ethdev_cman.c',
         'rte_flow.c',
         'rte_mtr.c',
         'rte_tm.c',
@@ -19,6 +20,7 @@ sources = files(
 )
 
 headers = files(
+        'rte_cman.h',
         'rte_ethdev.h',
         'rte_ethdev_trace.h',
         'rte_ethdev_trace_fp.h',
diff --git a/lib/ethdev/rte_cman.h b/lib/ethdev/rte_cman.h
new file mode 100644
index 0000000000..297db8e095
--- /dev/null
+++ b/lib/ethdev/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_BIT32(0),
+};
+
+/**
+ * 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/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 12535c703e..719b5c2707 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>
@@ -5314,6 +5315,169 @@ 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_BIT32(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_BIT32(1),
+};
+
+/**
+ * @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. Always returned as 0 when
+	 * rte_eth_cman_info_get() is invoked
+	 */
+	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.
+		 * It must be set to 0 when rte_eth_cman_config_set() is invoked
+		 * and will be returned as 0 when rte_eth_cman_config_get() is
+		 * invoked.
+		 */
+		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.
+		 * It must be set to 0 when rte_eth_cman_config_set() is invoked
+		 * and will be returned as 0 when rte_eth_cman_config_get() is
+		 * invoked.
+		 */
+		uint8_t rsvd_mode_params[4];
+	} mode_param;
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the information for ethdev congestion management
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param info
+ *   A pointer to a structure of type *rte_eth_cman_info* to be filled with
+ *   the information about congestion management.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_info_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_info_get(uint16_t port_id, struct rte_eth_cman_info *info);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Initialize the ethdev congestion management configuration structure with default values.
+ *
+ * @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 initialized
+ *   with default value.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_config_init does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_init(uint16_t port_id, struct rte_eth_cman_config *config);
+
+/**
+ * @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, const struct rte_eth_cman_config *config);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Retrieve the applied ethdev congestion management parameters for the given port.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param config
+ *   A pointer to a structure of type *rte_eth_cman_config* to retrieve
+ *   congestion management parameters for the given object.
+ *   Application must fill all parameters except mode_param parameter in
+ *   struct rte_eth_cman_config.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if support for cman_config_get does not exist.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+__rte_experimental
+int rte_eth_cman_config_get(uint16_t port_id, struct rte_eth_cman_config *config);
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/rte_ethdev_cman.c b/lib/ethdev/rte_ethdev_cman.c
new file mode 100644
index 0000000000..2ad39d7ed4
--- /dev/null
+++ b/lib/ethdev/rte_ethdev_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"
+#include "ethdev_private.h"
+
+/* 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;
+	}
+
+	if (dev->dev_ops->cman_info_get == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Function not implemented\n");
+		return -ENOTSUP;
+	}
+
+	memset(info, 0, sizeof(struct rte_eth_cman_info));
+	return eth_check_err(dev, (*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;
+	}
+
+	if (dev->dev_ops->cman_config_init == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Function not implemented\n");
+		return -ENOTSUP;
+	}
+
+	memset(config, 0, sizeof(struct rte_eth_cman_config));
+	return eth_check_err(dev, (*dev->dev_ops->cman_config_init)(dev, config));
+}
+
+/* Configure congestion management on a port */
+int
+rte_eth_cman_config_set(uint16_t port_id, const 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;
+	}
+
+	if (dev->dev_ops->cman_config_set == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Function not implemented\n");
+		return -ENOTSUP;
+	}
+
+	return eth_check_err(dev, (*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;
+	}
+
+	if (dev->dev_ops->cman_config_get == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Function not implemented\n");
+		return -ENOTSUP;
+	}
+
+	memset(config, 0, sizeof(struct rte_eth_cman_config));
+	return eth_check_err(dev, (*dev->dev_ops->cman_config_get)(dev, config));
+}
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 25e54f9d3e..a974ce7140 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -285,6 +285,12 @@ EXPERIMENTAL {
 	rte_mtr_color_in_protocol_priority_get;
 	rte_mtr_color_in_protocol_set;
 	rte_mtr_meter_vlan_table_update;
+
+	# added in 22.11
+	rte_eth_cman_config_get;
+	rte_eth_cman_config_init;
+	rte_eth_cman_config_set;
+	rte_eth_cman_info_get;
 };
 
 INTERNAL {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 1/1] ethdev: support congestion management
  2022-09-29  9:35     ` [PATCH v3 " skori
@ 2022-10-04  9:02       ` Andrew Rybchenko
  2022-10-04  9:04         ` Andrew Rybchenko
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Rybchenko @ 2022-10-04  9:02 UTC (permalink / raw)
  To: skori, Ferruh Yigit, Thomas Monjalon, Ray Kinsella; +Cc: dev, Jerin Jacob

On 9/29/22 12:35, skori@marvell.com wrote:
> From: Jerin Jacob <jerinj@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

This patch adds -> Add

> RTE_ETH_CMAN_OBJ_RX_QUEUE, RTE_ETH_CMAN_OBJ_RX_QUEUE_MEMPOOL object.
> 
> Introduced reserved fields in configuration structure

Introduce

> 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

Add

> supported modes and objects.
> 
> Added rte_eth_cman_config_init(), rte_eth_cman_config_set() APIs

Add

> to configure congestion management on those object with associated mode.
> 
> Finally, Added rte_eth_cman_config_get() API to retrieve the

add

> applied configuration.
> 
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

I'll send v4 with few minor correction.

> ---
> v2..v3:
>   - Rename rte_cman.c to rte_ethdev_cman.c
>   - Move lib/eal/include/rte_cman.h to lib/ethdev/rte_cman.h
>   - Fix review comments (Andrew Rybchenko)
>   - Add release notes
> 
> v1..v2:
>   - Fix review comments (Akhil Goyal)
> 
> rfc..v1:
>   - Added RED specification (http://www.aciri.org/floyd/papers/red/red.html) link
>   - Fixed doxygen comment issue (Min Hu)
> 
>   doc/guides/nics/features.rst           |  12 ++
>   doc/guides/nics/features/default.ini   |   1 +
>   doc/guides/rel_notes/release_22_11.rst |   6 +
>   lib/ethdev/ethdev_driver.h             |  25 ++++
>   lib/ethdev/ethdev_private.c            |  12 ++
>   lib/ethdev/ethdev_private.h            |   2 +
>   lib/ethdev/meson.build                 |   2 +
>   lib/ethdev/rte_cman.h                  |  55 +++++++++
>   lib/ethdev/rte_ethdev.h                | 164 +++++++++++++++++++++++++
>   lib/ethdev/rte_ethdev_cman.c           | 101 +++++++++++++++
>   lib/ethdev/version.map                 |   6 +
>   11 files changed, 386 insertions(+)
>   create mode 100644 lib/ethdev/rte_cman.h
>   create mode 100644 lib/ethdev/rte_ethdev_cman.c
> 
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index b4a8e9881c..70ca46e651 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -727,6 +727,18 @@ Supports configuring per-queue stat counter mapping.
>     ``rte_eth_dev_set_tx_queue_stats_mapping()``.
>   
>   
> +.. _nic_features_congestion_management:
> +
> +Congestion management
> +---------------------
> +
> +Supports congestion management.
> +
> +* **[implements] eth_dev_ops**: ``cman_info_get``, ``cman_config_set``, ``cman_config_get``.
> +* **[related]    API**: ``rte_eth_cman_info_get()``, ``rte_eth_cman_config_init()``,
> +  ``rte_eth_cman_config_set()``, ``rte_eth_cman_config_get()``.
> +
> +
>   .. _nic_features_fw_version:
>   
>   FW version
> diff --git a/doc/guides/nics/features/default.ini b/doc/guides/nics/features/default.ini
> index f7192cb0da..a9c0008ebd 100644
> --- a/doc/guides/nics/features/default.ini
> +++ b/doc/guides/nics/features/default.ini
> @@ -60,6 +60,7 @@ Tx descriptor status =
>   Basic stats          =
>   Extended stats       =
>   Stats per queue      =
> +Congestion management =
>   FW version           =
>   EEPROM dump          =
>   Module EEPROM dump   =
> diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
> index 0231959874..ea9908e578 100644
> --- a/doc/guides/rel_notes/release_22_11.rst
> +++ b/doc/guides/rel_notes/release_22_11.rst
> @@ -81,6 +81,12 @@ New Features
>     * Added AES-CCM support in lookaside protocol (IPsec) for CN9K & CN10K.
>     * Added AES & DES DOCSIS algorithm support in lookaside crypto for CN9K.
>   
> +* **Added support for congestion management for ethdev.**
> +
> +  Added new APIs ``rte_eth_cman_config_init()``, ``rte_eth_cman_config_get()``,
> +  ``rte_eth_cman_config_set()``, ``rte_eth_cman_info_get()``
> +  to support congestion management.
> +

The position is a bit incorrect. It should go after ethdev
items.

>   * **Added eventdev adapter instance get API.**
>   
>     * Added ``rte_event_eth_rx_adapter_instance_get`` to get Rx adapter
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 8cd8eb8685..e1e2d10a35 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -1094,6 +1094,22 @@ typedef int (*eth_rx_queue_avail_thresh_query_t)(struct rte_eth_dev *dev,
>   					uint16_t *rx_queue_id,
>   					uint8_t *avail_thresh);
>   
> +/** @internal Get congestion management information. */
> +typedef int (*eth_cman_info_get_t)(struct rte_eth_dev *dev,
> +				struct rte_eth_cman_info *info);
> +
> +/** @internal Init congestion management structure with default values. */
> +typedef int (*eth_cman_config_init_t)(struct rte_eth_dev *dev,
> +				struct rte_eth_cman_config *config);
> +
> +/** @internal Configure congestion management on a port. */
> +typedef int (*eth_cman_config_set_t)(struct rte_eth_dev *dev,
> +				const struct rte_eth_cman_config *config);
> +
> +/** @internal Retrieve congestion management configuration of a port. */
> +typedef int (*eth_cman_config_get_t)(struct rte_eth_dev *dev,
> +				struct rte_eth_cman_config *config);
> +
>   /**
>    * @internal A structure containing the functions exported by an Ethernet driver.
>    */
> @@ -1309,6 +1325,15 @@ struct eth_dev_ops {
>   	eth_rx_queue_avail_thresh_set_t rx_queue_avail_thresh_set;
>   	/** Query Rx queue available descriptors threshold event */
>   	eth_rx_queue_avail_thresh_query_t rx_queue_avail_thresh_query;
> +
> +	/** Get congestion management information */
> +	eth_cman_info_get_t cman_info_get;
> +	/** Initialize congestion management structure with default values */
> +	eth_cman_config_init_t cman_config_init;
> +	/** Configure congestion management */
> +	eth_cman_config_set_t cman_config_set;
> +	/** Retrieve congestion management configuration */
> +	eth_cman_config_get_t cman_config_get;
>   };
>   
>   /**
> diff --git a/lib/ethdev/ethdev_private.c b/lib/ethdev/ethdev_private.c
> index 48090c879a..8787c3985e 100644
> --- a/lib/ethdev/ethdev_private.c
> +++ b/lib/ethdev/ethdev_private.c
> @@ -27,6 +27,18 @@ eth_dev_to_id(const struct rte_eth_dev *dev)
>   	return dev - rte_eth_devices;
>   }
>   
> +int32_t
> +eth_check_err(struct rte_eth_dev *dev, int ret)
> +{
> +	if (ret == 0)
> +		return 0;
> +
> +	if (rte_eth_dev_is_removed(eth_dev_to_id(dev)))
> +		return -EIO;
> +
> +	return ret;
> +}

It still duplicates eth_err(). I realize the difference in the
first argument, but I think it is better to stick to eth_err().
Anyway eth_check_err() gets port_id by device and it is the
only usage of the device in the function.

[snip]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 1/1] ethdev: support congestion management
  2022-10-04  9:02       ` Andrew Rybchenko
@ 2022-10-04  9:04         ` Andrew Rybchenko
  0 siblings, 0 replies; 20+ messages in thread
From: Andrew Rybchenko @ 2022-10-04  9:04 UTC (permalink / raw)
  To: skori, Ferruh Yigit, Thomas Monjalon, Ray Kinsella; +Cc: dev, Jerin Jacob

On 10/4/22 12:02, Andrew Rybchenko wrote:
> On 9/29/22 12:35, skori@marvell.com wrote:
>> From: Jerin Jacob <jerinj@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
> 
> This patch adds -> Add
> 
>> RTE_ETH_CMAN_OBJ_RX_QUEUE, RTE_ETH_CMAN_OBJ_RX_QUEUE_MEMPOOL object.
>>
>> Introduced reserved fields in configuration structure
> 
> Introduce
> 
>> 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
> 
> Add
> 
>> supported modes and objects.
>>
>> Added rte_eth_cman_config_init(), rte_eth_cman_config_set() APIs
> 
> Add
> 
>> to configure congestion management on those object with associated mode.
>>
>> Finally, Added rte_eth_cman_config_get() API to retrieve the
> 
> add
> 
>> applied configuration.
>>
>> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
>> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> 
> I'll send v4 with few minor correction.

Done, but I'm sorry I forgot to specify --in-reply-to.



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-10-04  9:04 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-30 13:15 [dpdk-dev] [RFC PATCH] ethdev: support congestion management jerinj
2022-05-30 19:43 ` Ajit Khaparde
2022-07-13 12:11   ` Jerin Jacob
2022-05-31  1:09 ` Min Hu (Connor)
2022-07-13 12:16   ` Jerin Jacob
2022-05-31 16:09 ` Stephen Hemminger
2022-07-13 12:25   ` Jerin Jacob
2022-07-13 13:03 ` [dpdk-dev] [PATCH v1] " jerinj
2022-09-19 12:15   ` [PATCH v2 1/1] " skori
2022-09-27 14:36     ` Thomas Monjalon
2022-09-27 15:09       ` Bruce Richardson
2022-09-28 11:14         ` Jerin Jacob
2022-09-28 12:08           ` Thomas Monjalon
2022-09-28 12:23             ` Jerin Jacob
2022-09-28 13:07               ` Olivier Matz
2022-09-28  8:19     ` Andrew Rybchenko
2022-09-28 12:20       ` Jerin Jacob
2022-09-29  9:35     ` [PATCH v3 " skori
2022-10-04  9:02       ` Andrew Rybchenko
2022-10-04  9:04         ` Andrew Rybchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).