From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4A608A2EFC for ; Tue, 15 Oct 2019 18:01:54 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 562581ED5A; Tue, 15 Oct 2019 18:01:53 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 077C81ED51 for ; Tue, 15 Oct 2019 18:01:51 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Oct 2019 09:01:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,300,1566889200"; d="scan'208";a="347118466" Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga004.jf.intel.com with ESMTP; 15 Oct 2019 09:01:49 -0700 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.139]) by IRSMSX102.ger.corp.intel.com ([169.254.2.40]) with mapi id 14.03.0439.000; Tue, 15 Oct 2019 17:01:48 +0100 From: "Singh, Jasvinder" To: "Dumitrescu, Cristian" , "dev@dpdk.org" CC: "Krakowiak, LukaszX" Thread-Topic: [PATCH 2/2] sched: modify internal structs and functions for 64 bit values Thread-Index: AQHVg2/PhchbV/g5ykmJJMCYcJYxwqdb2sfg Date: Tue, 15 Oct 2019 16:01:48 +0000 Message-ID: <54CBAA185211B4429112C315DA58FF6D3FE41C02@IRSMSX103.ger.corp.intel.com> References: <20191014172455.139224-1-jasvinder.singh@intel.com> <20191014172455.139224-2-jasvinder.singh@intel.com> <3EB4FA525960D640B5BDFFD6A3D891268E946936@IRSMSX108.ger.corp.intel.com> In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891268E946936@IRSMSX108.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWExZTc5MmYtMjIwZS00YzJkLTlhNTEtOTFkZTFkZDM3MGYxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicUFEbVpxaEI3UklGcjl6alJlb3BYendIeXJqSitEXC9yUE1WdzQ5bTM1SVpqYndWc1JNbFN5cmp0WHlPaG5oZXQifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/2] sched: modify internal structs and functions for 64 bit values X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Dumitrescu, Cristian > Sent: Tuesday, October 15, 2019 4:47 PM > To: Singh, Jasvinder ; dev@dpdk.org > Cc: Krakowiak, LukaszX > Subject: RE: [PATCH 2/2] sched: modify internal structs and functions for= 64 bit > values >=20 > Hi Jasvinder, >=20 > > -----Original Message----- > > From: Singh, Jasvinder > > Sent: Monday, October 14, 2019 6:25 PM > > To: dev@dpdk.org > > Cc: Dumitrescu, Cristian ; Krakowiak, > > LukaszX > > 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 > > Signed-off-by: Lukasz Krakowiak > > --- > > 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 <=3D 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 =3D denum * p_b - (alpha_num + d_num) * q_b; > > - uint32_t k_denum =3D (alpha_num + d_num) * q_a - denum * p_a; > > - uint32_t k =3D (k_num / k_denum) + 1; > > + sched_counter_t k_num =3D denum * p_b - (alpha_num + d_num) * > > q_b; > > + sched_counter_t k_denum =3D (alpha_num + d_num) * q_a - denum * > > p_a; > > + sched_counter_t k =3D (k_num / k_denum) + 1; > > > > *p =3D p_b + k * p_a; > > *q =3D 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 =3D - denum * p_b + (alpha_num - d_num) * q_b; > > - uint32_t k_denum =3D - (alpha_num - d_num) * q_a + denum * p_a; > > - uint32_t k =3D (k_num / k_denum) + 1; > > + sched_counter_t k_num =3D -denum * p_b + (alpha_num - d_num) * > > q_b; > > + sched_counter_t k_denum =3D -(alpha_num - d_num) * q_a + denum > > * p_a; > > + sched_counter_t k =3D (k_num / k_denum) + 1; > > > > *p =3D p_b + k * p_a; > > *q =3D 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 =3D 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 *=3D 10; > > denum *=3D 10; > > } > > - alpha_num =3D (uint32_t) alpha; > > - d_num =3D (uint32_t) d; > > + alpha_num =3D (sched_counter_t) alpha; > > + d_num =3D (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 > > +#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 > > } >=20 > 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 depe= ndent. > Also, for the same reason, remove the above inclusion of rte_sched_common= .h. Ok, will make these changes. =20 > Please keep the existing 32-bit functions with their current name & proto= type > 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? Yes, will add new functions with suffix "64" in rte_approx.[hc]. > The rte_approx.[hc] files represent the implementation of an arithmetic > algorithm that is completely independent of the scheduler library. In fac= t, 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) dependen= cy to > librte_sched. >=20 > > 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 =3D subport->pipe_profiles + i; > > > > RTE_LOG(DEBUG, SCHED, "Low level config for pipe profile %u:\n" > > - " Token bucket: period =3D %u, credits per period =3D %u, > > size =3D %u\n" > > - " Traffic classes: period =3D %u,\n" > > - " credits per period =3D [%u, %u, %u, %u, %u, %u, %u, > > %u, %u, %u, %u, %u, %u]\n" > > + " Token bucket: period =3D %"PRIu64", credits per period > > =3D %"PRIu64", size =3D %"PRIu64"\n" > > + " Traffic classes: period =3D %"PRIu64",\n" > > + " credits per period =3D [%"PRIu64", %"PRIu64", > > %"PRIu64", %"PRIu64 > > + ", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", > > %"PRIu64 > > + ", %"PRIu64", %"PRIu64", %"PRIu64"]\n" > > " Best-effort traffic class oversubscription: weight =3D > > %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 =3D 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 =3D 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 =3D src->tb_size; > > @@ -594,8 +596,8 @@ rte_sched_pipe_profile_convert(struct > > rte_sched_subport *subport, > > for (i =3D 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > if (subport->qsize[i]) > > dst->tc_credits_per_period[i] > > - =3D rte_sched_time_ms_to_bytes(src- > > >tc_period, > > - src->tc_rate[i]); > > + =3D (sched_counter_t) > > rte_sched_time_ms_to_bytes( > > + src->tc_period, src->tc_rate[i]); > > > > dst->tc_ov_weight =3D 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 =3D 0; > > for (i =3D 0; i < subport->n_pipe_profiles; i++) { > > struct rte_sched_pipe_params *src =3D params->pipe_profiles > > + i; > > - uint32_t pipe_tc_be_rate =3D src- > > >tc_rate[RTE_SCHED_TRAFFIC_CLASS_BE]; > > + sched_counter_t pipe_tc_be_rate =3D > > + 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 =3D 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 =3D 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) { > > - uint32_t tc_rate =3D params->tc_rate[i]; > > + sched_counter_t tc_rate =3D params->tc_rate[i]; > > uint16_t qsize =3D params->qsize[i]; > > > > if ((qsize =3D=3D 0 && tc_rate !=3D 0) || @@ -910,36 +913,40 @@ > > rte_sched_port_log_subport_config(struct > > rte_sched_port *port, uint32_t i) > > struct rte_sched_subport *s =3D port->subports[i]; > > > > RTE_LOG(DEBUG, SCHED, "Low level config for subport %u:\n" > > - " Token bucket: period =3D %u, credits per period =3D %u, > > size =3D %u\n" > > - " Traffic classes: period =3D %u\n" > > - " credits per period =3D [%u, %u, %u, %u, %u, %u, %u, > > %u, %u, %u, %u, %u, %u]\n" > > - " Best effort traffic class oversubscription: wm min =3D > > %u, wm max =3D %u\n", > > + " Token bucket: period =3D %"PRIu64", credits per period > > =3D %"PRIu64 > > + ", size =3D %"PRIu64"\n" > > + " Traffic classes: period =3D %"PRIu64"\n" > > + " credits per period =3D [%"PRIu64", %"PRIu64", > > %"PRIu64", %"PRIu64 > > + ", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", %"PRIu64", > > %"PRIu64 > > + ", %"PRIu64", %"PRIu64", %"PRIu64"]\n" > > + " Best effort traffic class oversubscription: wm min =3D > > %"PRIu64 > > + ", wm max =3D %"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 =3D ((double) params->tb_rate) / ((double) > > port->rate); > > double d =3D 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 =3D params->tb_size; > > @@ -1035,8 +1043,8 @@ rte_sched_subport_config(struct rte_sched_port > > *port, > > for (i =3D 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) { > > if (params->qsize[i]) > > s->tc_credits_per_period[i] > > - =3D rte_sched_time_ms_to_bytes(params- > > >tc_period, > > - params->tc_rate[i]); > > + =3D (sched_counter_t) > > rte_sched_time_ms_to_bytes( > > + params->tc_period, params- > > >tc_rate[i]); > > } > > s->tc_time =3D port->time + s->tc_period; > > for (i =3D 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 =3D (port->time - subport->tb_time) / subport->tb_period; > > subport->tb_credits +=3D n_periods * subport- > > >tb_credits_per_period; > > - subport->tb_credits =3D rte_sched_min_val_2_u32(subport- > > >tb_credits, subport->tb_size); > > + subport->tb_credits =3D rte_sched_min_val_2(subport->tb_credits, > > + subport->tb_size); > > subport->tb_time +=3D n_periods * subport->tb_period; > > > > /* Pipe TB */ > > n_periods =3D (port->time - pipe->tb_time) / params->tb_period; > > pipe->tb_credits +=3D n_periods * params->tb_credits_per_period; > > - pipe->tb_credits =3D rte_sched_min_val_2_u32(pipe->tb_credits, > > params->tb_size); > > + pipe->tb_credits =3D rte_sched_min_val_2(pipe->tb_credits, > > + params->tb_size); >=20 > Can we remove all the usages of rte_sched_min_val() (including its defini= tion in > rte_sched_common.h) and replace it with RTE_MIN, please? >=20 > > pipe->tb_time +=3D 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 =3D 0, tc_ov_consumption_max; > > - uint32_t tc_ov_wm =3D subport->tc_ov_wm; > > + sched_counter_t > > tc_ov_consumption[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE]; > > + sched_counter_t tc_consumption =3D 0, tc_ov_consumption_max; > > + sched_counter_t tc_ov_wm =3D subport->tc_ov_wm; > > uint32_t i; > > > > if (subport->tc_ov =3D=3D 0) > > @@ -2053,13 +2063,15 @@ grinder_credits_update(struct rte_sched_port > > *port, > > /* Subport TB */ > > n_periods =3D (port->time - subport->tb_time) / subport->tb_period; > > subport->tb_credits +=3D n_periods * subport- > > >tb_credits_per_period; > > - subport->tb_credits =3D rte_sched_min_val_2_u32(subport- > > >tb_credits, subport->tb_size); > > + subport->tb_credits =3D rte_sched_min_val_2(subport->tb_credits, > > + subport->tb_size); > > subport->tb_time +=3D n_periods * subport->tb_period; > > > > /* Pipe TB */ > > n_periods =3D (port->time - pipe->tb_time) / params->tb_period; > > pipe->tb_credits +=3D n_periods * params->tb_credits_per_period; > > - pipe->tb_credits =3D rte_sched_min_val_2_u32(pipe->tb_credits, > > params->tb_size); > > + pipe->tb_credits =3D rte_sched_min_val_2(pipe->tb_credits, > > + params->tb_size); > > pipe->tb_time +=3D n_periods * params->tb_period; > > > > /* Subport TCs */ > > @@ -2101,11 +2113,11 @@ grinder_credits_check(struct rte_sched_port > > *port, > > struct rte_sched_pipe *pipe =3D grinder->pipe; > > struct rte_mbuf *pkt =3D grinder->pkt; > > uint32_t tc_index =3D grinder->tc_index; > > - uint32_t pkt_len =3D pkt->pkt_len + port->frame_overhead; > > - uint32_t subport_tb_credits =3D subport->tb_credits; > > - uint32_t subport_tc_credits =3D subport->tc_credits[tc_index]; > > - uint32_t pipe_tb_credits =3D pipe->tb_credits; > > - uint32_t pipe_tc_credits =3D pipe->tc_credits[tc_index]; > > + sched_counter_t pkt_len =3D pkt->pkt_len + port->frame_overhead; > > + sched_counter_t subport_tb_credits =3D subport->tb_credits; > > + sched_counter_t subport_tc_credits =3D subport- > > >tc_credits[tc_index]; > > + sched_counter_t pipe_tb_credits =3D pipe->tb_credits; > > + sched_counter_t pipe_tc_credits =3D 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 =3D grinder->pipe; > > struct rte_mbuf *pkt =3D grinder->pkt; > > uint32_t tc_index =3D grinder->tc_index; > > - uint32_t pkt_len =3D pkt->pkt_len + port->frame_overhead; > > - uint32_t subport_tb_credits =3D subport->tb_credits; > > - uint32_t subport_tc_credits =3D subport->tc_credits[tc_index]; > > - uint32_t pipe_tb_credits =3D pipe->tb_credits; > > - uint32_t pipe_tc_credits =3D 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] =3D {0}; > > - uint32_t pipe_tc_ov_credits, i; > > + sched_counter_t pkt_len =3D pkt->pkt_len + port->frame_overhead; > > + sched_counter_t subport_tb_credits =3D subport->tb_credits; > > + sched_counter_t subport_tc_credits =3D subport- > > >tc_credits[tc_index]; > > + sched_counter_t pipe_tb_credits =3D pipe->tb_credits; > > + sched_counter_t pipe_tc_credits =3D 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] =3D {0}; > > + sched_counter_t pipe_tc_ov_credits; > > + uint32_t i; > > int enough_credits; > > > > for (i =3D 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) > > - pipe_tc_ov_mask1[i] =3D UINT32_MAX; > > + pipe_tc_ov_mask1[i] =3D ~0; >=20 > Please use ~0LLU (or UINT64_MAX) to cover the 64-bit case gracefully. Ple= ase > also double-check that there are no usages of UINT32_MAX left in this cod= e, > unless there is a reason for it. Translation from 32-bit to 64-bit arithm= etic can > be very tricky and yield some very difficult to debug issues. Ok, will make this change. Also will make sure the there are no usage of UI= NT32_MAX left. > > > > pipe_tc_ov_mask1[RTE_SCHED_TRAFFIC_CLASS_BE] =3D pipe- > > >tc_ov_credits; > > - pipe_tc_ov_mask2[RTE_SCHED_TRAFFIC_CLASS_BE] =3D > > UINT32_MAX; > > + pipe_tc_ov_mask2[RTE_SCHED_TRAFFIC_CLASS_BE] =3D ~0; > > pipe_tc_ov_credits =3D 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 >=20 > I know I have previously suggested the creation of sched_counter_t, but t= his > was meant to be a temporary solution until the full implementation is mad= e > 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 breaka= ge. >=20 > Therefore, I strongly suggest we remove the sched_counter_t and use uint6= 4_t > everywhere throughout the implementation. Agree? Yes, will make changes. Thank you for the detailed review. I'll send revised version.=20 Regards, Jasvinder