> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Monday, July 1, 2019 7:59 PM
> To: Singh, Jasvinder <jasvinder.si...@intel.com>; dev@dpdk.org
> Cc: Tovar, AbrahamX <abrahamx.to...@intel.com>; Krakowiak, LukaszX
> <lukaszx.krakow...@intel.com>
> Subject: RE: [PATCH v2 02/28] sched: update subport and pipe data structures
>
>
>
> > -----Original Message-----
> > From: Singh, Jasvinder
> > Sent: Tuesday, June 25, 2019 4:32 PM
> > To: dev@dpdk.org
> > Cc: Dumitrescu, Cristian <cristian.dumitre...@intel.com>; Tovar,
> > AbrahamX <abrahamx.to...@intel.com>; Krakowiak, LukaszX
> > <lukaszx.krakow...@intel.com>
> > Subject: [PATCH v2 02/28] sched: update subport and pipe data
> > structures
> >
> > Update subport and pipe data structures to allow configuration
> > flexiblity for pipe traffic classes and queues, and subport level
> > configuration of the pipe parameters.
> >
> > Signed-off-by: Jasvinder Singh <jasvinder.si...@intel.com>
> > Signed-off-by: Abraham Tovar <abrahamx.to...@intel.com>
> > Signed-off-by: Lukasz Krakowiak <lukaszx.krakow...@intel.com>
> > ---
> > app/test/test_sched.c | 2 +-
> > examples/qos_sched/init.c | 2 +-
> > lib/librte_sched/rte_sched.h | 126
> > +++++++++++++++++++++++------------
> > 3 files changed, 85 insertions(+), 45 deletions(-)
> >
> > diff --git a/app/test/test_sched.c b/app/test/test_sched.c index
> > 49bb9ea6f..d6651d490 100644
> > --- a/app/test/test_sched.c
> > +++ b/app/test/test_sched.c
> > @@ -40,7 +40,7 @@ static struct rte_sched_pipe_params pipe_profile[] = {
> > .tc_rate = {305175, 305175, 305175, 305175},
> > .tc_period = 40,
> >
> > - .wrr_weights = {1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
> > 1},
> > + .wrr_weights = {1, 1, 1, 1, 1, 1, 1, 1},
> > },
> > };
> >
> > diff --git a/examples/qos_sched/init.c b/examples/qos_sched/init.c
> > index 1209bd7ce..f6e9af16b 100644
> > --- a/examples/qos_sched/init.c
> > +++ b/examples/qos_sched/init.c
> > @@ -186,7 +186,7 @@ static struct rte_sched_pipe_params
> > pipe_profiles[RTE_SCHED_PIPE_PROFILES_PER_PO
> > .tc_ov_weight = 1,
> > #endif
> >
> > - .wrr_weights = {1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
> > 1},
> > + .wrr_weights = {1, 1, 1, 1, 1, 1, 1, 1},
> > },
> > };
> >
> > diff --git a/lib/librte_sched/rte_sched.h
> > b/lib/librte_sched/rte_sched.h index 470a0036a..ebde07669 100644
> > --- a/lib/librte_sched/rte_sched.h
> > +++ b/lib/librte_sched/rte_sched.h
> > @@ -114,6 +114,35 @@ extern "C" {
> > #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT 24
> > #endif
> >
> > +/*
> > + * Pipe configuration parameters. The period and credits_per_period
> > + * parameters are measured in bytes, with one byte meaning the time
> > + * duration associated with the transmission of one byte on the
> > + * physical medium of the output port, with pipe or pipe traffic
> > +class
> > + * rate (measured as percentage of output port rate) determined as
> > + * credits_per_period divided by period. One credit represents one
> > + * byte.
> > + */
> > +struct rte_sched_pipe_params {
> > + /** Token bucket rate (measured in bytes per second) */
> > + uint32_t tb_rate;
> > + /** Token bucket size (measured in credits) */
> > + uint32_t tb_size;
> > +
> > + /** Traffic class rates (measured in bytes per second) */
> > + uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> > +
> > + /** Enforcement period (measured in milliseconds) */
> > + uint32_t tc_period;
> > +#ifdef RTE_SCHED_SUBPORT_TC_OV
> > + /** Best-effort traffic class oversubscription weight */
> > + uint8_t tc_ov_weight;
> > +#endif
>
> We should always enable the Best Effort traffic class oversubscription feature
> on the API side, at least. In case this feature is disabled through the build
> time
> option (RTE_SCHED_SUBPORT_TC_OV), the values for these params can be
> ignored.
>
> We should also consider always enabling the run-time part for this feature, as
> the oversubscription is the typical configuration used by the service
> providers.
Yes, will do in next version.
> Do you see significant performance drop when this feature is enabled?
There is ~2.5 MPPS drop with oversubscription flag on due to extra cycles
consumption in credits compute.
> > +
> > + /** WRR weights of best-effort traffic class queues */
> > + uint8_t wrr_weights[RTE_SCHED_BE_QUEUES_PER_PIPE];
> > +};
> > +
> > /*
> > * Subport configuration parameters. The period and credits_per_period
> > * parameters are measured in bytes, with one byte meaning the time
> > @@ -124,15 +153,44 @@ extern "C" {
> > * byte.
> > */
> > struct rte_sched_subport_params {
> > - /* Subport token bucket */
> > - uint32_t tb_rate; /**< Rate (measured in bytes per
> > second)
> > */
> > - uint32_t tb_size; /**< Size (measured in credits) */
> > + /** Token bucket rate (measured in bytes per second) */
> > + uint32_t tb_rate;
> > +
> > + /** Token bucket size (measured in credits) */
> > + uint32_t tb_size;
> >
> > - /* Subport traffic classes */
> > + /** Traffic class rates (measured in bytes per second) */
> > uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> > - /**< Traffic class rates (measured in bytes per second) */
> > +
> > + /** Enforcement period for rates (measured in milliseconds) */
> > uint32_t tc_period;
> > - /**< Enforcement period for rates (measured in milliseconds) */
> > +
> > + /** Number of subport_pipes */
> > + uint32_t n_subport_pipes;
>
> Minor issue: Any reason why not keeping the initial name of
> n_pipes_per_subport? The initial name looks more intuitive to me, I vote to
> keep it; it is also inline with other naming conventions in this library.
Will change in next version.
> > +
> > + /** Packet queue size for each traffic class.
> > + * All the pipes within the same subport share the similar
> > + * configuration for the queues. Queues which are not needed, have
> > + * zero size.
> > + */
> > + uint16_t qsize[RTE_SCHED_QUEUES_PER_PIPE];
> > +
> > + /** Pipe profile table.
> > + * Every pipe is configured using one of the profiles from this table.
> > + */
> > + struct rte_sched_pipe_params *pipe_profiles;
> > +
> > + /** Profiles in the pipe profile table */
> > + uint32_t n_pipe_profiles;
> > +
> > + /** Max profiles allowed 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
> > ];
> > +
> > +#endif
> > };
> >
> > /** Subport statistics */
> > @@ -155,33 +213,6 @@ struct rte_sched_subport_stats { #endif };
> >
> > -/*
> > - * Pipe configuration parameters. The period and credits_per_period
> > - * parameters are measured in bytes, with one byte meaning the time
> > - * duration associated with the transmission of one byte on the
> > - * physical medium of the output port, with pipe or pipe traffic
> > class
> > - * rate (measured as percentage of output port rate) determined as
> > - * credits_per_period divided by period. One credit represents one
> > - * byte.
> > - */
> > -struct rte_sched_pipe_params {
> > - /* Pipe token bucket */
> > - uint32_t tb_rate; /**< Rate (measured in bytes per
> > second)
> > */
> > - uint32_t tb_size; /**< Size (measured in credits) */
> > -
> > - /* Pipe traffic classes */
> > - uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> > - /**< Traffic class rates (measured in bytes per second) */
> > - uint32_t tc_period;
> > - /**< Enforcement period (measured in milliseconds) */
> > -#ifdef RTE_SCHED_SUBPORT_TC_OV
> > - uint8_t tc_ov_weight; /**< Weight Traffic class 3
> > oversubscription */
> > -#endif
> > -
> > - /* Pipe queues */
> > - uint8_t wrr_weights[RTE_SCHED_QUEUES_PER_PIPE]; /**< WRR
> > weights */
> > -};
> > -
> > /** Queue statistics */
> > struct rte_sched_queue_stats {
> > /* Packets */
> > @@ -198,16 +229,25 @@ struct rte_sched_queue_stats {
> >
> > /** Port configuration parameters. */ struct rte_sched_port_params {
> > - const char *name; /**< String to be associated */
> > - int socket; /**< CPU socket ID */
> > - uint32_t rate; /**< Output port rate
> > - * (measured in bytes per second) */
> > - uint32_t mtu; /**< Maximum Ethernet frame size
> > - * (measured in bytes).
> > - * Should not include the framing
> > overhead. */
> > - uint32_t frame_overhead; /**< Framing overhead per packet
> > - * (measured in bytes) */
> > - uint32_t n_subports_per_port; /**< Number of subports */
> > + /** Name of the port to be associated */
> > + const char *name;
> > +
> > + /** CPU socket ID */
> > + int socket;
> > +
> > + /** Output port rate (measured in bytes per second) */
> > + uint32_t rate;
> > +
> > + /** Maximum Ethernet frame size (measured in bytes).
> > + * Should not include the framing overhead.
> > + */
> > + uint32_t mtu;
> > +
> > + /** Framing overhead per packet (measured in bytes) */
> > + uint32_t frame_overhead;
> > +
> > + /** Number of subports */
> > + uint32_t n_subports_per_port;
> > uint32_t n_pipes_per_subport; /**< Number of pipes per subport
> > */
> > uint16_t qsize[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> > /**< Packet queue size for each traffic class.
> > --
> > 2.21.0
