[dpdk-dev] [PATCH 2/2] sched: modify internal structs and functions for 64 bit values
Dumitrescu, Cristian
cristian.dumitrescu at intel.com
Tue Oct 15 17:47:11 CEST 2019
Hi Jasvinder,
> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Monday, October 14, 2019 6:25 PM
> To: dev at dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu at intel.com>; Krakowiak,
> LukaszX <lukaszx.krakowiak at intel.com>
> Subject: [PATCH 2/2] sched: modify internal structs and functions for 64 bit
> values
>
> Modify internal structure and functions to support 64-bit
> values for rates and stats parameters.
>
> Signed-off-by: Jasvinder Singh <jasvinder.singh at intel.com>
> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak at intel.com>
> ---
> lib/librte_sched/rte_approx.c | 57 ++++----
> lib/librte_sched/rte_approx.h | 3 +-
> lib/librte_sched/rte_sched.c | 211 +++++++++++++++-------------
> lib/librte_sched/rte_sched_common.h | 12 +-
> 4 files changed, 156 insertions(+), 127 deletions(-)
>
> diff --git a/lib/librte_sched/rte_approx.c b/lib/librte_sched/rte_approx.c
> index 30620b83d..4883d3969 100644
> --- a/lib/librte_sched/rte_approx.c
> +++ b/lib/librte_sched/rte_approx.c
> @@ -18,22 +18,23 @@
> */
>
> /* fraction comparison: compare (a/b) and (c/d) */
> -static inline uint32_t
> -less(uint32_t a, uint32_t b, uint32_t c, uint32_t d)
> +static inline sched_counter_t
> +less(sched_counter_t a, sched_counter_t b, sched_counter_t c,
> sched_counter_t d)
> {
> return a*d < b*c;
> }
>
> -static inline uint32_t
> -less_or_equal(uint32_t a, uint32_t b, uint32_t c, uint32_t d)
> +static inline sched_counter_t
> +less_or_equal(sched_counter_t a, sched_counter_t b, sched_counter_t c,
> + sched_counter_t d)
> {
> return a*d <= b*c;
> }
>
> /* check whether a/b is a valid approximation */
> -static inline uint32_t
> -matches(uint32_t a, uint32_t b,
> - uint32_t alpha_num, uint32_t d_num, uint32_t denum)
> +static inline sched_counter_t
> +matches(sched_counter_t a, sched_counter_t b,
> + sched_counter_t alpha_num, sched_counter_t d_num,
> sched_counter_t denum)
> {
> if (less_or_equal(a, b, alpha_num - d_num, denum))
> return 0;
> @@ -45,33 +46,39 @@ matches(uint32_t a, uint32_t b,
> }
>
> static inline void
> -find_exact_solution_left(uint32_t p_a, uint32_t q_a, uint32_t p_b, uint32_t
> q_b,
> - uint32_t alpha_num, uint32_t d_num, uint32_t denum, uint32_t *p,
> uint32_t *q)
> +find_exact_solution_left(sched_counter_t p_a, sched_counter_t q_a,
> + sched_counter_t p_b, sched_counter_t q_b, sched_counter_t
> alpha_num,
> + sched_counter_t d_num, sched_counter_t denum,
> sched_counter_t *p,
> + sched_counter_t *q)
> {
> - uint32_t k_num = denum * p_b - (alpha_num + d_num) * q_b;
> - uint32_t k_denum = (alpha_num + d_num) * q_a - denum * p_a;
> - uint32_t k = (k_num / k_denum) + 1;
> + sched_counter_t k_num = denum * p_b - (alpha_num + d_num) *
> q_b;
> + sched_counter_t k_denum = (alpha_num + d_num) * q_a - denum *
> p_a;
> + sched_counter_t k = (k_num / k_denum) + 1;
>
> *p = p_b + k * p_a;
> *q = q_b + k * q_a;
> }
>
> static inline void
> -find_exact_solution_right(uint32_t p_a, uint32_t q_a, uint32_t p_b,
> uint32_t q_b,
> - uint32_t alpha_num, uint32_t d_num, uint32_t denum, uint32_t *p,
> uint32_t *q)
> +find_exact_solution_right(sched_counter_t p_a, sched_counter_t q_a,
> + sched_counter_t p_b, sched_counter_t q_b, sched_counter_t
> alpha_num,
> + sched_counter_t d_num, sched_counter_t denum,
> sched_counter_t *p,
> + sched_counter_t *q)
> {
> - uint32_t k_num = - denum * p_b + (alpha_num - d_num) * q_b;
> - uint32_t k_denum = - (alpha_num - d_num) * q_a + denum * p_a;
> - uint32_t k = (k_num / k_denum) + 1;
> + sched_counter_t k_num = -denum * p_b + (alpha_num - d_num) *
> q_b;
> + sched_counter_t k_denum = -(alpha_num - d_num) * q_a + denum
> * p_a;
> + sched_counter_t k = (k_num / k_denum) + 1;
>
> *p = p_b + k * p_a;
> *q = q_b + k * q_a;
> }
>
> static int
> -find_best_rational_approximation(uint32_t alpha_num, uint32_t d_num,
> uint32_t denum, uint32_t *p, uint32_t *q)
> +find_best_rational_approximation(sched_counter_t alpha_num,
> + sched_counter_t d_num, sched_counter_t denum,
> sched_counter_t *p,
> + sched_counter_t *q)
> {
> - uint32_t p_a, q_a, p_b, q_b;
> + sched_counter_t p_a, q_a, p_b, q_b;
>
> /* check assumptions on the inputs */
> if (!((0 < d_num) && (d_num < alpha_num) && (alpha_num <
> denum) && (d_num + alpha_num < denum))) {
> @@ -85,8 +92,8 @@ find_best_rational_approximation(uint32_t alpha_num,
> uint32_t d_num, uint32_t de
> q_b = 1;
>
> while (1) {
> - uint32_t new_p_a, new_q_a, new_p_b, new_q_b;
> - uint32_t x_num, x_denum, x;
> + sched_counter_t new_p_a, new_q_a, new_p_b, new_q_b;
> + sched_counter_t x_num, x_denum, x;
> int aa, bb;
>
> /* compute the number of steps to the left */
> @@ -139,9 +146,9 @@ find_best_rational_approximation(uint32_t
> alpha_num, uint32_t d_num, uint32_t de
> }
> }
>
> -int rte_approx(double alpha, double d, uint32_t *p, uint32_t *q)
> +int rte_approx(double alpha, double d, sched_counter_t *p,
> sched_counter_t *q)
> {
> - uint32_t alpha_num, d_num, denum;
> + sched_counter_t alpha_num, d_num, denum;
>
> /* Check input arguments */
> if (!((0.0 < d) && (d < alpha) && (alpha < 1.0))) {
> @@ -159,8 +166,8 @@ int rte_approx(double alpha, double d, uint32_t *p,
> uint32_t *q)
> d *= 10;
> denum *= 10;
> }
> - alpha_num = (uint32_t) alpha;
> - d_num = (uint32_t) d;
> + alpha_num = (sched_counter_t) alpha;
> + d_num = (sched_counter_t) d;
>
> /* Perform approximation */
> return find_best_rational_approximation(alpha_num, d_num,
> denum, p, q);
> diff --git a/lib/librte_sched/rte_approx.h b/lib/librte_sched/rte_approx.h
> index 0244d98f1..e591e122d 100644
> --- a/lib/librte_sched/rte_approx.h
> +++ b/lib/librte_sched/rte_approx.h
> @@ -20,6 +20,7 @@ extern "C" {
> ***/
>
> #include <stdint.h>
> +#include "rte_sched_common.h"
>
> /**
> * Find best rational approximation
> @@ -37,7 +38,7 @@ extern "C" {
> * @return
> * 0 upon success, error code otherwise
> */
> -int rte_approx(double alpha, double d, uint32_t *p, uint32_t *q);
> +int rte_approx(double alpha, double d, sched_counter_t *p,
> sched_counter_t *q);
>
> #ifdef __cplusplus
> }
Please keep the rte_approx.[hc] independent of the librte_sched library, so use unit32_t or uint64_t instead of sched_counter_t that is librte_sched dependent. Also, for the same reason, remove the above inclusion of rte_sched_common.h.
Please keep the existing 32-bit functions with their current name & prototype and create new 64-bit functions that have the "64" suffix to their name, and use the 64-bit versions in the rte_sched.c implementation. Makes sense?
The rte_approx.[hc] files represent the implementation of an arithmetic algorithm that is completely independent of the scheduler library. In fact, they could be moved to a more generic location in DPDK where they could be leveraged by other libraries without the need to create a (fake) dependency to librte_sched.
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 710ecf65a..11d1febe2 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -49,13 +49,13 @@
>
> struct rte_sched_pipe_profile {
> /* Token bucket (TB) */
> - uint32_t tb_period;
> - uint32_t tb_credits_per_period;
> - uint32_t tb_size;
> + sched_counter_t tb_period;
> + sched_counter_t tb_credits_per_period;
> + sched_counter_t tb_size;
>
> /* Pipe traffic classes */
> - uint32_t tc_period;
> - uint32_t
> tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> + sched_counter_t tc_period;
> + sched_counter_t
> tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> uint8_t tc_ov_weight;
>
> /* Pipe best-effort traffic class queues */
> @@ -65,20 +65,20 @@ struct rte_sched_pipe_profile {
> struct rte_sched_pipe {
> /* Token bucket (TB) */
> uint64_t tb_time; /* time of last update */
> - uint32_t tb_credits;
> + sched_counter_t tb_credits;
>
> /* Pipe profile and flags */
> uint32_t profile;
>
> /* Traffic classes (TCs) */
> uint64_t tc_time; /* time of next update */
> - uint32_t tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> + sched_counter_t
> tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
>
> /* Weighted Round Robin (WRR) */
> uint8_t wrr_tokens[RTE_SCHED_BE_QUEUES_PER_PIPE];
>
> /* TC oversubscription */
> - uint32_t tc_ov_credits;
> + sched_counter_t tc_ov_credits;
> uint8_t tc_ov_period_id;
> } __rte_cache_aligned;
>
> @@ -141,28 +141,28 @@ struct rte_sched_grinder {
> struct rte_sched_subport {
> /* Token bucket (TB) */
> uint64_t tb_time; /* time of last update */
> - uint32_t tb_period;
> - uint32_t tb_credits_per_period;
> - uint32_t tb_size;
> - uint32_t tb_credits;
> + sched_counter_t tb_period;
> + sched_counter_t tb_credits_per_period;
> + sched_counter_t tb_size;
> + sched_counter_t tb_credits;
>
> /* Traffic classes (TCs) */
> uint64_t tc_time; /* time of next update */
> - uint32_t
> tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> - uint32_t tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> - uint32_t tc_period;
> + sched_counter_t
> tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> + sched_counter_t
> tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> + sched_counter_t tc_period;
>
> /* TC oversubscription */
> - uint32_t tc_ov_wm;
> - uint32_t tc_ov_wm_min;
> - uint32_t tc_ov_wm_max;
> + sched_counter_t tc_ov_wm;
> + sched_counter_t tc_ov_wm_min;
> + sched_counter_t tc_ov_wm_max;
> uint8_t tc_ov_period_id;
> uint8_t tc_ov;
> uint32_t tc_ov_n;
> double tc_ov_rate;
>
> /* Statistics */
> - struct rte_sched_subport_stats stats;
> + struct rte_sched_subport_stats stats __rte_cache_aligned;
>
> /* Subport pipes */
> uint32_t n_pipes_per_subport_enabled;
> @@ -170,7 +170,7 @@ struct rte_sched_subport {
> uint32_t n_max_pipe_profiles;
>
> /* Pipe best-effort TC rate */
> - uint32_t pipe_tc_be_rate_max;
> + sched_counter_t pipe_tc_be_rate_max;
>
> /* Pipe queues size */
> uint16_t qsize[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> @@ -212,7 +212,7 @@ struct rte_sched_port {
> uint16_t pipe_queue[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> uint8_t pipe_tc[RTE_SCHED_QUEUES_PER_PIPE];
> uint8_t tc_queue[RTE_SCHED_QUEUES_PER_PIPE];
> - uint32_t rate;
> + sched_counter_t rate;
> uint32_t mtu;
> uint32_t frame_overhead;
> int socket;
> @@ -517,33 +517,35 @@ rte_sched_port_log_pipe_profile(struct
> rte_sched_subport *subport, uint32_t i)
> struct rte_sched_pipe_profile *p = subport->pipe_profiles + i;
>
> RTE_LOG(DEBUG, SCHED, "Low level config for pipe profile %u:\n"
> - " Token bucket: period = %u, credits per period = %u,
> size = %u\n"
> - " Traffic classes: period = %u,\n"
> - " credits per period = [%u, %u, %u, %u, %u, %u, %u,
> %u, %u, %u, %u, %u, %u]\n"
> + " Token bucket: period = %"PRIu64", credits per period
> = %"PRIu64", size = %"PRIu64"\n"
> + " Traffic classes: period = %"PRIu64",\n"
> + " credits per period = [%"PRIu64", %"PRIu64",
> %"PRIu64", %"PRIu64
> + ", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64",
> %"PRIu64
> + ", %"PRIu64", %"PRIu64", %"PRIu64"]\n"
> " Best-effort traffic class oversubscription: weight =
> %hhu\n"
> " WRR cost: [%hhu, %hhu, %hhu, %hhu]\n",
> i,
>
> /* Token bucket */
> - p->tb_period,
> - p->tb_credits_per_period,
> - p->tb_size,
> + (uint64_t)p->tb_period,
> + (uint64_t)p->tb_credits_per_period,
> + (uint64_t)p->tb_size,
>
> /* Traffic classes */
> - p->tc_period,
> - p->tc_credits_per_period[0],
> - p->tc_credits_per_period[1],
> - p->tc_credits_per_period[2],
> - p->tc_credits_per_period[3],
> - p->tc_credits_per_period[4],
> - p->tc_credits_per_period[5],
> - p->tc_credits_per_period[6],
> - p->tc_credits_per_period[7],
> - p->tc_credits_per_period[8],
> - p->tc_credits_per_period[9],
> - p->tc_credits_per_period[10],
> - p->tc_credits_per_period[11],
> - p->tc_credits_per_period[12],
> + (uint64_t)p->tc_period,
> + (uint64_t)p->tc_credits_per_period[0],
> + (uint64_t)p->tc_credits_per_period[1],
> + (uint64_t)p->tc_credits_per_period[2],
> + (uint64_t)p->tc_credits_per_period[3],
> + (uint64_t)p->tc_credits_per_period[4],
> + (uint64_t)p->tc_credits_per_period[5],
> + (uint64_t)p->tc_credits_per_period[6],
> + (uint64_t)p->tc_credits_per_period[7],
> + (uint64_t)p->tc_credits_per_period[8],
> + (uint64_t)p->tc_credits_per_period[9],
> + (uint64_t)p->tc_credits_per_period[10],
> + (uint64_t)p->tc_credits_per_period[11],
> + (uint64_t)p->tc_credits_per_period[12],
>
> /* Best-effort traffic class oversubscription */
> p->tc_ov_weight,
> @@ -553,7 +555,7 @@ rte_sched_port_log_pipe_profile(struct
> rte_sched_subport *subport, uint32_t i)
> }
>
> static inline uint64_t
> -rte_sched_time_ms_to_bytes(uint32_t time_ms, uint32_t rate)
> +rte_sched_time_ms_to_bytes(sched_counter_t time_ms,
> sched_counter_t rate)
> {
> uint64_t time = time_ms;
>
> @@ -566,7 +568,7 @@ static void
> rte_sched_pipe_profile_convert(struct rte_sched_subport *subport,
> struct rte_sched_pipe_params *src,
> struct rte_sched_pipe_profile *dst,
> - uint32_t rate)
> + sched_counter_t rate)
> {
> uint32_t wrr_cost[RTE_SCHED_BE_QUEUES_PER_PIPE];
> uint32_t lcd1, lcd2, lcd;
> @@ -581,8 +583,8 @@ rte_sched_pipe_profile_convert(struct
> rte_sched_subport *subport,
> / (double) rate;
> double d = RTE_SCHED_TB_RATE_CONFIG_ERR;
>
> - rte_approx(tb_rate, d,
> - &dst->tb_credits_per_period, &dst->tb_period);
> + rte_approx(tb_rate, d, &dst->tb_credits_per_period,
> + &dst->tb_period);
> }
>
> dst->tb_size = src->tb_size;
> @@ -594,8 +596,8 @@ rte_sched_pipe_profile_convert(struct
> rte_sched_subport *subport,
> for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++)
> if (subport->qsize[i])
> dst->tc_credits_per_period[i]
> - = rte_sched_time_ms_to_bytes(src-
> >tc_period,
> - src->tc_rate[i]);
> + = (sched_counter_t)
> rte_sched_time_ms_to_bytes(
> + src->tc_period, src->tc_rate[i]);
>
> dst->tc_ov_weight = src->tc_ov_weight;
>
> @@ -637,7 +639,8 @@ rte_sched_subport_config_pipe_profile_table(struct
> rte_sched_subport *subport,
> subport->pipe_tc_be_rate_max = 0;
> for (i = 0; i < subport->n_pipe_profiles; i++) {
> struct rte_sched_pipe_params *src = params->pipe_profiles
> + i;
> - uint32_t pipe_tc_be_rate = src-
> >tc_rate[RTE_SCHED_TRAFFIC_CLASS_BE];
> + sched_counter_t pipe_tc_be_rate =
> + src->tc_rate[RTE_SCHED_TRAFFIC_CLASS_BE];
>
> if (subport->pipe_tc_be_rate_max < pipe_tc_be_rate)
> subport->pipe_tc_be_rate_max = pipe_tc_be_rate;
> @@ -647,7 +650,7 @@ rte_sched_subport_config_pipe_profile_table(struct
> rte_sched_subport *subport,
> static int
> rte_sched_subport_check_params(struct rte_sched_subport_params
> *params,
> uint32_t n_max_pipes_per_subport,
> - uint32_t rate)
> + sched_counter_t rate)
> {
> uint32_t i;
>
> @@ -684,7 +687,7 @@ rte_sched_subport_check_params(struct
> rte_sched_subport_params *params,
> }
>
> for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) {
> - uint32_t tc_rate = params->tc_rate[i];
> + sched_counter_t tc_rate = params->tc_rate[i];
> uint16_t qsize = params->qsize[i];
>
> if ((qsize == 0 && tc_rate != 0) ||
> @@ -910,36 +913,40 @@ rte_sched_port_log_subport_config(struct
> rte_sched_port *port, uint32_t i)
> struct rte_sched_subport *s = port->subports[i];
>
> RTE_LOG(DEBUG, SCHED, "Low level config for subport %u:\n"
> - " Token bucket: period = %u, credits per period = %u,
> size = %u\n"
> - " Traffic classes: period = %u\n"
> - " credits per period = [%u, %u, %u, %u, %u, %u, %u,
> %u, %u, %u, %u, %u, %u]\n"
> - " Best effort traffic class oversubscription: wm min =
> %u, wm max = %u\n",
> + " Token bucket: period = %"PRIu64", credits per period
> = %"PRIu64
> + ", size = %"PRIu64"\n"
> + " Traffic classes: period = %"PRIu64"\n"
> + " credits per period = [%"PRIu64", %"PRIu64",
> %"PRIu64", %"PRIu64
> + ", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64",
> %"PRIu64
> + ", %"PRIu64", %"PRIu64", %"PRIu64"]\n"
> + " Best effort traffic class oversubscription: wm min =
> %"PRIu64
> + ", wm max = %"PRIu64"\n",
> i,
>
> /* Token bucket */
> - s->tb_period,
> - s->tb_credits_per_period,
> - s->tb_size,
> + (uint64_t)s->tb_period,
> + (uint64_t)s->tb_credits_per_period,
> + (uint64_t)s->tb_size,
>
> /* Traffic classes */
> - s->tc_period,
> - s->tc_credits_per_period[0],
> - s->tc_credits_per_period[1],
> - s->tc_credits_per_period[2],
> - s->tc_credits_per_period[3],
> - s->tc_credits_per_period[4],
> - s->tc_credits_per_period[5],
> - s->tc_credits_per_period[6],
> - s->tc_credits_per_period[7],
> - s->tc_credits_per_period[8],
> - s->tc_credits_per_period[9],
> - s->tc_credits_per_period[10],
> - s->tc_credits_per_period[11],
> - s->tc_credits_per_period[12],
> + (uint64_t)s->tc_period,
> + (uint64_t)s->tc_credits_per_period[0],
> + (uint64_t)s->tc_credits_per_period[1],
> + (uint64_t)s->tc_credits_per_period[2],
> + (uint64_t)s->tc_credits_per_period[3],
> + (uint64_t)s->tc_credits_per_period[4],
> + (uint64_t)s->tc_credits_per_period[5],
> + (uint64_t)s->tc_credits_per_period[6],
> + (uint64_t)s->tc_credits_per_period[7],
> + (uint64_t)s->tc_credits_per_period[8],
> + (uint64_t)s->tc_credits_per_period[9],
> + (uint64_t)s->tc_credits_per_period[10],
> + (uint64_t)s->tc_credits_per_period[11],
> + (uint64_t)s->tc_credits_per_period[12],
>
> /* Best effort traffic class oversubscription */
> - s->tc_ov_wm_min,
> - s->tc_ov_wm_max);
> + (uint64_t)s->tc_ov_wm_min,
> + (uint64_t)s->tc_ov_wm_max);
> }
>
> static void
> @@ -1023,7 +1030,8 @@ rte_sched_subport_config(struct rte_sched_port
> *port,
> double tb_rate = ((double) params->tb_rate) / ((double)
> port->rate);
> double d = RTE_SCHED_TB_RATE_CONFIG_ERR;
>
> - rte_approx(tb_rate, d, &s->tb_credits_per_period, &s-
> >tb_period);
> + rte_approx(tb_rate, d, &s->tb_credits_per_period,
> + &s->tb_period);
> }
>
> s->tb_size = params->tb_size;
> @@ -1035,8 +1043,8 @@ rte_sched_subport_config(struct rte_sched_port
> *port,
> for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) {
> if (params->qsize[i])
> s->tc_credits_per_period[i]
> - = rte_sched_time_ms_to_bytes(params-
> >tc_period,
> - params->tc_rate[i]);
> + = (sched_counter_t)
> rte_sched_time_ms_to_bytes(
> + params->tc_period, params-
> >tc_rate[i]);
> }
> s->tc_time = port->time + s->tc_period;
> for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++)
> @@ -1970,13 +1978,15 @@ grinder_credits_update(struct rte_sched_port
> *port,
> /* Subport TB */
> n_periods = (port->time - subport->tb_time) / subport->tb_period;
> subport->tb_credits += n_periods * subport-
> >tb_credits_per_period;
> - subport->tb_credits = rte_sched_min_val_2_u32(subport-
> >tb_credits, subport->tb_size);
> + subport->tb_credits = rte_sched_min_val_2(subport->tb_credits,
> + subport->tb_size);
> subport->tb_time += n_periods * subport->tb_period;
>
> /* Pipe TB */
> n_periods = (port->time - pipe->tb_time) / params->tb_period;
> pipe->tb_credits += n_periods * params->tb_credits_per_period;
> - pipe->tb_credits = rte_sched_min_val_2_u32(pipe->tb_credits,
> params->tb_size);
> + pipe->tb_credits = rte_sched_min_val_2(pipe->tb_credits,
> + params->tb_size);
Can we remove all the usages of rte_sched_min_val() (including its definition in rte_sched_common.h) and replace it with RTE_MIN, please?
> pipe->tb_time += n_periods * params->tb_period;
>
> /* Subport TCs */
> @@ -1998,13 +2008,13 @@ grinder_credits_update(struct rte_sched_port
> *port,
>
> #else
>
> -static inline uint32_t
> +static inline sched_counter_t
> grinder_tc_ov_credits_update(struct rte_sched_port *port,
> struct rte_sched_subport *subport)
> {
> - uint32_t
> tc_ov_consumption[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> - uint32_t tc_consumption = 0, tc_ov_consumption_max;
> - uint32_t tc_ov_wm = subport->tc_ov_wm;
> + sched_counter_t
> tc_ov_consumption[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> + sched_counter_t tc_consumption = 0, tc_ov_consumption_max;
> + sched_counter_t tc_ov_wm = subport->tc_ov_wm;
> uint32_t i;
>
> if (subport->tc_ov == 0)
> @@ -2053,13 +2063,15 @@ grinder_credits_update(struct rte_sched_port
> *port,
> /* Subport TB */
> n_periods = (port->time - subport->tb_time) / subport->tb_period;
> subport->tb_credits += n_periods * subport-
> >tb_credits_per_period;
> - subport->tb_credits = rte_sched_min_val_2_u32(subport-
> >tb_credits, subport->tb_size);
> + subport->tb_credits = rte_sched_min_val_2(subport->tb_credits,
> + subport->tb_size);
> subport->tb_time += n_periods * subport->tb_period;
>
> /* Pipe TB */
> n_periods = (port->time - pipe->tb_time) / params->tb_period;
> pipe->tb_credits += n_periods * params->tb_credits_per_period;
> - pipe->tb_credits = rte_sched_min_val_2_u32(pipe->tb_credits,
> params->tb_size);
> + pipe->tb_credits = rte_sched_min_val_2(pipe->tb_credits,
> + params->tb_size);
> pipe->tb_time += n_periods * params->tb_period;
>
> /* Subport TCs */
> @@ -2101,11 +2113,11 @@ grinder_credits_check(struct rte_sched_port
> *port,
> struct rte_sched_pipe *pipe = grinder->pipe;
> struct rte_mbuf *pkt = grinder->pkt;
> uint32_t tc_index = grinder->tc_index;
> - uint32_t pkt_len = pkt->pkt_len + port->frame_overhead;
> - uint32_t subport_tb_credits = subport->tb_credits;
> - uint32_t subport_tc_credits = subport->tc_credits[tc_index];
> - uint32_t pipe_tb_credits = pipe->tb_credits;
> - uint32_t pipe_tc_credits = pipe->tc_credits[tc_index];
> + sched_counter_t pkt_len = pkt->pkt_len + port->frame_overhead;
> + sched_counter_t subport_tb_credits = subport->tb_credits;
> + sched_counter_t subport_tc_credits = subport-
> >tc_credits[tc_index];
> + sched_counter_t pipe_tb_credits = pipe->tb_credits;
> + sched_counter_t pipe_tc_credits = pipe->tc_credits[tc_index];
> int enough_credits;
>
> /* Check queue credits */
> @@ -2136,21 +2148,22 @@ grinder_credits_check(struct rte_sched_port
> *port,
> struct rte_sched_pipe *pipe = grinder->pipe;
> struct rte_mbuf *pkt = grinder->pkt;
> uint32_t tc_index = grinder->tc_index;
> - uint32_t pkt_len = pkt->pkt_len + port->frame_overhead;
> - uint32_t subport_tb_credits = subport->tb_credits;
> - uint32_t subport_tc_credits = subport->tc_credits[tc_index];
> - uint32_t pipe_tb_credits = pipe->tb_credits;
> - uint32_t pipe_tc_credits = pipe->tc_credits[tc_index];
> - uint32_t
> pipe_tc_ov_mask1[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> - uint32_t
> pipe_tc_ov_mask2[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE] = {0};
> - uint32_t pipe_tc_ov_credits, i;
> + sched_counter_t pkt_len = pkt->pkt_len + port->frame_overhead;
> + sched_counter_t subport_tb_credits = subport->tb_credits;
> + sched_counter_t subport_tc_credits = subport-
> >tc_credits[tc_index];
> + sched_counter_t pipe_tb_credits = pipe->tb_credits;
> + sched_counter_t pipe_tc_credits = pipe->tc_credits[tc_index];
> + sched_counter_t
> pipe_tc_ov_mask1[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> + sched_counter_t
> pipe_tc_ov_mask2[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE] = {0};
> + sched_counter_t pipe_tc_ov_credits;
> + uint32_t i;
> int enough_credits;
>
> for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++)
> - pipe_tc_ov_mask1[i] = UINT32_MAX;
> + pipe_tc_ov_mask1[i] = ~0;
Please use ~0LLU (or UINT64_MAX) to cover the 64-bit case gracefully. Please also double-check that there are no usages of UINT32_MAX left in this code, unless there is a reason for it. Translation from 32-bit to 64-bit arithmetic can be very tricky and yield some very difficult to debug issues.
>
> pipe_tc_ov_mask1[RTE_SCHED_TRAFFIC_CLASS_BE] = pipe-
> >tc_ov_credits;
> - pipe_tc_ov_mask2[RTE_SCHED_TRAFFIC_CLASS_BE] =
> UINT32_MAX;
> + pipe_tc_ov_mask2[RTE_SCHED_TRAFFIC_CLASS_BE] = ~0;
> pipe_tc_ov_credits = pipe_tc_ov_mask1[tc_index];
>
> /* Check pipe and subport credits */
> diff --git a/lib/librte_sched/rte_sched_common.h
> b/lib/librte_sched/rte_sched_common.h
> index 8c191a9b8..06520a686 100644
> --- a/lib/librte_sched/rte_sched_common.h
> +++ b/lib/librte_sched/rte_sched_common.h
> @@ -14,8 +14,16 @@ extern "C" {
>
> #define __rte_aligned_16 __attribute__((__aligned__(16)))
>
> -static inline uint32_t
> -rte_sched_min_val_2_u32(uint32_t x, uint32_t y)
> +//#define COUNTER_SIZE_64
> +
> +#ifdef COUNTER_SIZE_64
> +typedef uint64_t sched_counter_t;
> +#else
> +typedef uint32_t sched_counter_t;
> +#endif
> +
> +static inline sched_counter_t
> +rte_sched_min_val_2(sched_counter_t x, sched_counter_t y)
> {
> return (x < y)? x : y;
> }
> --
> 2.21.0
I know I have previously suggested the creation of sched_counter_t, but this was meant to be a temporary solution until the full implementation is made available. Now that 19.11 is meant to be an ABI stable release, we cannot really afford this trick (which might not be necessary either, since you have the full implementation), as this #ifdef COUNTER_SIZE is a massive ABI breakage.
Therefore, I strongly suggest we remove the sched_counter_t and use uint64_t everywhere throughout the implementation. Agree?
Regards,
Cristian
More information about the dev
mailing list