From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 504C7A00E6 for ; Tue, 19 Mar 2019 12:00:11 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 28DEB2BAF; Tue, 19 Mar 2019 12:00:11 +0100 (CET) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 443B82956 for ; Tue, 19 Mar 2019 12:00:09 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 88B226697B; Tue, 19 Mar 2019 11:00:08 +0000 (UTC) Received: from [10.36.116.79] (ovpn-116-79.ams2.redhat.com [10.36.116.79]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EE38B51327; Tue, 19 Mar 2019 11:00:07 +0000 (UTC) From: "Eelco Chaudron" To: "Dumitrescu, Cristian" Cc: dev@dpdk.org Date: Tue, 19 Mar 2019 12:00:04 +0100 Message-ID: <806845F2-2DD7-4954-BB52-448DA85D5891@redhat.com> In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D891268E846048@IRSMSX108.ger.corp.intel.com> References: <154903721465.46418.12675922836824541692.stgit@dbuild> <3EB4FA525960D640B5BDFFD6A3D891268E846048@IRSMSX108.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 19 Mar 2019 11:00:08 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2] lib/librte_meter: fix divide by zero for RFC4115 meter 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" Message-ID: <20190319110004.yD3EfnGyMktxXALQjTzIhEOkKrpZNvusITuNUin0BwQ@z> On 28 Feb 2019, at 19:50, Dumitrescu, Cristian wrote: > Hi Eelco, > > Sorry for my delayed reply No problem I was OOO also so hence my late reply ;) >> -----Original Message----- >> From: Eelco Chaudron [mailto:echaudro@redhat.com] >> Sent: Friday, February 1, 2019 4:07 PM >> To: Dumitrescu, Cristian >> Cc: dev@dpdk.org >> Subject: [PATCH v2] lib/librte_meter: fix divide by zero for RFC4115 >> meter >> >> RFC 4115 allows a meter with either cir and/or eir configured. >> When only one is configured a divide by zero would occur. >> >> Fixes: 655796d2b5fb ("meter: support RFC4115 trTCM") >> Cc: echaudro@redhat.com >> >> Signed-off-by: Eelco Chaudron >> --- >> >> v2 - Removed configuration change that got included by accident >> >> lib/librte_meter/rte_meter.h | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/lib/librte_meter/rte_meter.h >> b/lib/librte_meter/rte_meter.h >> index 005e4eeee..56d85ecf0 100644 >> --- a/lib/librte_meter/rte_meter.h >> +++ b/lib/librte_meter/rte_meter.h >> @@ -597,8 +597,8 @@ rte_meter_trtcm_rfc4115_color_blind_check( >> /* Bucket update */ >> time_diff_tc = time - m->time_tc; >> time_diff_te = time - m->time_te; >> - n_periods_tc = time_diff_tc / p->cir_period; >> - n_periods_te = time_diff_te / p->eir_period; >> + n_periods_tc = p->cir_period != 0 ? time_diff_tc / p->cir_period : >> 0; >> + n_periods_te = p->eir_period != 0 ? time_diff_te / p->eir_period : >> 0; >> m->time_tc += n_periods_tc * p->cir_period; >> m->time_te += n_periods_te * p->eir_period; >> >> @@ -641,8 +641,8 @@ rte_meter_trtcm_rfc4115_color_aware_check( >> /* Bucket update */ >> time_diff_tc = time - m->time_tc; >> time_diff_te = time - m->time_te; >> - n_periods_tc = time_diff_tc / p->cir_period; >> - n_periods_te = time_diff_te / p->eir_period; >> + n_periods_tc = p->cir_period != 0 ? time_diff_tc / p->cir_period : >> 0; >> + n_periods_te = p->eir_period != 0 ? time_diff_te / p->eir_period : >> 0; >> m->time_tc += n_periods_tc * p->cir_period; >> m->time_te += n_periods_te * p->eir_period; >> > > Yes, this is indeed an issue, good catch! > > For performance reasons, we'd like to avoid a test on the fast path > (rte_meter_xyz_check) and replace it with more work done on the > configuration stage (rte_meter_profile_xyz_config), if possible. > > We can intercept the null CIR or EIR cases very early in > rte_meter_trtcm_rfc4115_profile_config() and deal with them separately > by setting the CIR/EIR period and bytes_per_period to some neutral > values and skipping the call to rte_meter_get_tb_params(): period = > RTE_METER_TB_PERIOD_MIN, bytes_per_period = 0. Excellent idea, will sent out a v3 soon, after some testing… //Eelco