From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 15C32F72 for ; Mon, 19 Sep 2016 17:51:51 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga105.jf.intel.com with ESMTP; 19 Sep 2016 08:51:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,362,1470726000"; d="scan'208";a="10862676" Received: from irsmsx104.ger.corp.intel.com ([163.33.3.159]) by fmsmga006.fm.intel.com with ESMTP; 19 Sep 2016 08:51:48 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.164]) by IRSMSX104.ger.corp.intel.com ([163.33.3.159]) with mapi id 14.03.0248.002; Mon, 19 Sep 2016 16:51:47 +0100 From: "Dumitrescu, Cristian" To: Nikhil Jagtap CC: "dev@dpdk.org" , "Ramia, Kannan Babu" Thread-Topic: [PATCH] meter: fix excess token bucket update in srtcm implementation Thread-Index: AQHSCM9UCtG6JYExFkqDNJ3CvqJ/26CBBxzw Date: Mon, 19 Sep 2016 15:51:47 +0000 Message-ID: <3EB4FA525960D640B5BDFFD6A3D8912647A83BBA@IRSMSX108.ger.corp.intel.com> References: <1473228910-10429-1-git-send-email-nikhil.jagtap@gmail.com> In-Reply-To: <1473228910-10429-1-git-send-email-nikhil.jagtap@gmail.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTQ4MzdiYzYtZDc3MS00ZDc4LWJjMDYtNWU0ODM4NjZmMjc2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IktDRXVkZUtBcm13dWR5MHozdG1GaWJCMmp4aXhwMjZmODlRemVQNTRUNzA9In0= x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] meter: fix excess token bucket update in srtcm implementation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Sep 2016 15:51:52 -0000 > -----Original Message----- > From: Nikhil Jagtap [mailto:nikhil.jagtap@gmail.com] > Sent: Wednesday, September 7, 2016 7:15 AM > To: Dumitrescu, Cristian > Cc: dev@dpdk.org; Ramia, Kannan Babu ; > Nikhil Jagtap > Subject: [PATCH] meter: fix excess token bucket update in srtcm > implementation >=20 > As per srTCM RFC 2697, we should be updating the E bucket only after the > C bucket overflows. This patch fixes the current DPDK implementation, > where we are updating both the buckets simultaneously at the same rate > (CIR) which results in token accumulation rate of (2*CIR). >=20 > Signed-off-by: Nikhil Jagtap > --- > lib/librte_meter/rte_meter.h | 26 ++++++++++++++++---------- > 1 files changed, 16 insertions(+), 10 deletions(-) >=20 > diff --git a/lib/librte_meter/rte_meter.h b/lib/librte_meter/rte_meter.h > index 2cd8d81..0ffcb60 100644 > --- a/lib/librte_meter/rte_meter.h > +++ b/lib/librte_meter/rte_meter.h > @@ -232,13 +232,16 @@ rte_meter_srtcm_color_blind_check(struct > rte_meter_srtcm *m, > n_periods =3D time_diff / m->cir_period; > m->time +=3D n_periods * m->cir_period; >=20 > + /* Put the tokens overflowing from tc into te bucket */ > tc =3D m->tc + n_periods * m->cir_bytes_per_period; > - if (tc > m->cbs) > + if (tc > m->cbs) { > + te =3D m->te + (tc - m->cbs); > + if (te > m->ebs) > + te =3D m->ebs; > tc =3D m->cbs; > - > - te =3D m->te + n_periods * m->cir_bytes_per_period; > - if (te > m->ebs) > - te =3D m->ebs; > + } else { > + te =3D m->te; > + } Just to avoid the final else, in order to have the critical path (Tc not ov= erflowing) as the default fall-through code path, I suggest the following s= mall change in the code (update the Te just after Tc update, for the case o= f Tc overflowing), which should favour the usage of the cmov instruction: /* Put the tokens overflowing from tc into te bucket */ tc =3D m->tc + n_periods * m->cir_bytes_per_period; te =3D m->te; if (tc > m->cbs) { te =3D m->te + (tc - m->cbs); if (te > m->ebs) te =3D m->ebs; tc =3D m->cbs; } Are you OK with this change? >=20 > /* Color logic */ > if (tc >=3D pkt_len) { > @@ -271,13 +274,16 @@ rte_meter_srtcm_color_aware_check(struct > rte_meter_srtcm *m, > n_periods =3D time_diff / m->cir_period; > m->time +=3D n_periods * m->cir_period; >=20 > + /* Put the tokens overflowing from tc into te bucket */ > tc =3D m->tc + n_periods * m->cir_bytes_per_period; > - if (tc > m->cbs) > + if (tc > m->cbs) { > + te =3D m->te + (tc - m->cbs); > + if (te > m->ebs) > + te =3D m->ebs; > tc =3D m->cbs; > - > - te =3D m->te + n_periods * m->cir_bytes_per_period; > - if (te > m->ebs) > - te =3D m->ebs; > + } else { > + te =3D m->te; > + } Same as above. >=20 > /* Color logic */ > if ((pkt_color =3D=3D e_RTE_METER_GREEN) && (tc >=3D pkt_len)) { > -- > 1.7.1