[v2,1/1] ethdev: support congestion management
Checks
Commit Message
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
Comments
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?
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
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]
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
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.
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]
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.
>
>
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.
@@ -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
@@ -60,6 +60,7 @@ Tx descriptor status =
Basic stats =
Extended stats =
Stats per queue =
+Congestion management =
FW version =
EEPROM dump =
Module EEPROM dump =
@@ -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',
new file mode 100644
@@ -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 */
@@ -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;
};
/**
@@ -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',
new file mode 100644
@@ -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));
+}
@@ -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>
/**
@@ -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 {