Hi Wojciech,
> -----Original Message-----
> From: Liguzinski, WojciechX <wojciechx.liguzin...@intel.com>
> Sent: Monday, July 5, 2021 9:04 AM
> To: dev@dpdk.org; Singh, Jasvinder <jasvinder.si...@intel.com>;
> Dumitrescu, Cristian <cristian.dumitre...@intel.com>
> Cc: Dharmappa, Savinay <savinay.dharma...@intel.com>; Ajmera, Megha
> <megha.ajm...@intel.com>
> Subject: [RFC PATCH v4 1/3] sched: add PIE based congestion management
>
> Implement PIE based congestion management based on rfc8033
>
> Signed-off-by: Liguzinski, WojciechX <wojciechx.liguzin...@intel.com>
> ---
> drivers/net/softnic/rte_eth_softnic_tm.c | 6 +-
> lib/sched/meson.build | 10 +-
> lib/sched/rte_pie.c | 82 +++++
> lib/sched/rte_pie.h | 393 +++++++++++++++++++++++
> lib/sched/rte_sched.c | 229 +++++++++----
> lib/sched/rte_sched.h | 53 ++-
> lib/sched/version.map | 3 +
> 7 files changed, 685 insertions(+), 91 deletions(-)
> create mode 100644 lib/sched/rte_pie.c
> create mode 100644 lib/sched/rte_pie.h
>
<snip>
> diff --git a/lib/sched/rte_sched.h b/lib/sched/rte_sched.h
> index c1a772b70c..a5fe6266cd 100644
> --- a/lib/sched/rte_sched.h
> +++ b/lib/sched/rte_sched.h
> @@ -61,9 +61,10 @@ extern "C" {
> #include <rte_mbuf.h>
> #include <rte_meter.h>
>
> -/** Random Early Detection (RED) */
> -#ifdef RTE_SCHED_RED
> +/** Active Queue Management */
> +#ifdef RTE_SCHED_AQM
We typically use the term Congestion Management for this, it is already used in
rte_tm.h for example. Please replace AQM with CMAN _everywhere_:
#ifdef RTE_SCHED_CMAN
> #include "rte_red.h"
> +#include "rte_pie.h"
> #endif
>
> /** Maximum number of queues per pipe.
> @@ -110,6 +111,28 @@ extern "C" {
> #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT 24
> #endif
>
> +/**
> + * Active Queue Management (AQM) mode
> + *
> + * This is used for controlling the admission of packets into a packet queue
> or
> + * group of packet queues on congestion.
> + *
> + * The *Random Early Detection (RED)* algorithm works by proactively
> dropping
> + * more and more input packets as the queue occupancy builds up. When
> the queue
> + * is full or almost full, RED effectively works as *tail drop*. The
> *Weighted
> + * RED* algorithm uses a separate set of RED thresholds for each packet
> color.
> + *
> + * Similar to RED, Proportional Integral Controller Enhanced (PIE) randomly
> + * drops a packet at the onset of the congestion and tries to control the
> + * latency around the target value. The congestion detection, however, is
> based
> + * on the queueing latency instead of the queue length like RED. For more
> + * information, refer RFC8033.
> + */
> +enum rte_sched_aqm_mode {
> + RTE_SCHED_AQM_WRED, /**< Weighted Random Early Detection
> (WRED) */
> + RTE_SCHED_AQM_PIE, /**< Proportional Integral Controller
> Enhanced (PIE) */
> +};
> +
> /*
> * Pipe configuration parameters. The period and credits_per_period
> * parameters are measured in bytes, with one byte meaning the time
> @@ -174,9 +197,17 @@ struct rte_sched_subport_params {
> /** Max allowed profiles in the pipe profile table */
> uint32_t n_max_pipe_profiles;
>
> -#ifdef RTE_SCHED_RED
> - /** RED parameters */
> - struct rte_red_params
> red_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS];
> +#ifdef RTE_SCHED_AQM
> + /** Active Queue Management mode */
> + enum rte_sched_aqm_mode aqm;
> +
> + RTE_STD_C11
> + union {
> + /** WRED parameters */
> + struct rte_red_params
> wred_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS];
> + /** PIE parameters */
> + struct rte_pie_params
> pie_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> + };
> #endif
> };
>
We cannot have definitions of global variables in header files, we can only
have extern declarations in header files and the definition in .c files.
Please create a global structure called rte_sched_cman_params that includes the
both the WRED and the PIE parameters:
struct rte_sched_cman_params {
enum rte_sched_cman cman_mode;
union {
/** WRED parameters */
struct rte_red_params
red_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS];
/** PIE parameters */
struct rte_pie_params
pie_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
};
};
Then please instantiate this structure in one of the .c files.
Please do not rename red to wred (as done in multiple places in this patch set).
> @@ -208,9 +239,9 @@ struct rte_sched_subport_stats {
> /** Number of bytes dropped for each traffic class */
> uint64_t
> n_bytes_tc_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
>
> -#ifdef RTE_SCHED_RED
> - /** Number of packets dropped by red */
> - uint64_t
> n_pkts_red_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> +#ifdef RTE_SCHED_AQM
> + /** Number of packets dropped by active queue management
> scheme */
> + uint64_t
> n_pkts_aqm_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> #endif
> };
>
Please remove the #ifdefs and consolidate these stats into a single generic
structure:
struct rte_sched_subport_stats {
...
uint64_t n_pkts_cman_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
...
};
You can use the n_pkts_cman_dropped field for both WRED and PIE, depending on
the cman_mode.
> @@ -222,9 +253,9 @@ struct rte_sched_queue_stats {
> /** Packets dropped */
> uint64_t n_pkts_dropped;
>
> -#ifdef RTE_SCHED_RED
> - /** Packets dropped by RED */
> - uint64_t n_pkts_red_dropped;
> +#ifdef RTE_SCHED_AQM
> + /** Packets dropped by active queue management scheme */
> + uint64_t n_pkts_aqm_dropped;
> #endif
>
> /** Bytes successfully written */
Please remove the #ifdefs and consolidate these stats into a single generic
structure:
struct rte_sched_queue_stats {
...
uint64_t n_pkts_cman_dropped;
...
};
You can use the n_pkts_cman_dropped field for both WRED and PIE, depending on
the cman_mode.
Regards,
Cristian
