From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f42.google.com (mail-wm0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id 0F8C91B2A5 for ; Tue, 16 Jan 2018 16:57:40 +0100 (CET) Received: by mail-wm0-f42.google.com with SMTP id t74so9419681wme.3 for ; Tue, 16 Jan 2018 07:57:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=46lbplwrhqBDCQesvQfbzG0hInXv2Kk5dtfiyzSYc1Y=; b=J/B7EKbhDsjAEpJ/XOOeUdVm03dxOtCXVTUphf54KGcO2DJrNgFzs8wu0o6ArFC+1p hVrdSpDx1VK6pwZcVE1DUCXEpZPLrxjZRyrd2jbH0XmfRE1Ib1SfMCi0O7/pTskzajuh yAfGLIXlmC0FyU6ulFPMz64d5WahjyISJcZomKLOPcm6MCCBGF887BinFMRiV0UEu5Jm ryJNDOflWYy3zTRRIcj7v1NHD64J6cJN4oka/Geb2ywNb870m6b0u2QuGVJ4eWw8345A soeVoGMa5jmfH3rwdkJDkQn4E9pBnTTXeiM2nGlivlHGBiYHkYhVaG30Tfcl7Wp+a9bm AGVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=46lbplwrhqBDCQesvQfbzG0hInXv2Kk5dtfiyzSYc1Y=; b=A9B2WQvOOS6w7RLHswGHpuHa3sktgK2NDtfXCMgGGgstGysZAgm2ei9Oq4Crqzq5mb ZWps314K8/L/PKlnUmgZS4J1Dk8X1OMWuXSKAq3zE/C67w3KWdVkwb4sQFmmznMGxHcw xje7QHfd9ikBdwROtnkGDmVlCdzbBcbSbRU07gdR70iea9D61yHmepgbX2UJ9U10VzZ5 o3jkff79BaRAobLkZanIgjBm4vzAD0HYB+jT4tMWORb7spib8OtwXlr+iQ+dbz+J+Q1Z NrthWS+JlI5DvYvGHmfco9TfwM6TSp1YF/UI0vht9/Oo1HSgEC2ja01/z1/5WKr22CEn pG3w== X-Gm-Message-State: AKwxyte6doJ0JdfMnpKhxB6RIJgRYPtTsOolEIIJrbkI8YgVPfEEKqjs Tu3HD0i3BWWAeAAD8GZYjjzaDUbvqgrX9YnE3aZy X-Google-Smtp-Source: ACJfBot7/RAxuHzCkear5pY7juWDuLlBe7MIKmphd5BMhgJpUa4DH1UnAlGNSI/bNEeI8E/3L3TxOcqJFkB3wH8kzW4= X-Received: by 10.28.22.67 with SMTP id 64mr13632933wmw.28.1516118259795; Tue, 16 Jan 2018 07:57:39 -0800 (PST) MIME-Version: 1.0 Received: by 10.223.186.195 with HTTP; Tue, 16 Jan 2018 07:57:38 -0800 (PST) In-Reply-To: References: <1515425232-18888-1-git-send-email-alan.dewar@att.com> <1516032969-23894-1-git-send-email-alan.dewar@att.com> <20180115085252.1ea5d456@xeon-e3> From: Alan Dewar Date: Tue, 16 Jan 2018 15:57:38 +0000 Message-ID: To: Stephen Hemminger Cc: cristian.dumitrescu@intel.com, tomasz.kantecki@intel.com, jasvinder.singh@intel.com, dev@dpdk.org, Alan Dewar Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v6] sched: make RED scaling configurable 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: , X-List-Received-Date: Tue, 16 Jan 2018 15:57:40 -0000 On Tue, Jan 16, 2018 at 3:50 PM, Alan Dewar wro= te: > On Mon, Jan 15, 2018 at 4:52 PM, Stephen Hemminger > wrote: >> >> On Mon, 15 Jan 2018 16:16:09 +0000 >> alangordondewar@gmail.com wrote: >> >> Looks like a good idea, minor editing feedback. >> >> >> > - red_cfg->min_th =3D ((uint32_t) min_th) << (wq_log2 + RTE_RED_SC= ALING); >> > - red_cfg->max_th =3D ((uint32_t) max_th) << (wq_log2 + RTE_RED_SC= ALING); >> > - red_cfg->pa_const =3D (2 * (max_th - min_th) * maxp_inv) << RTE_= RED_SCALING; >> > + red_cfg->min_th =3D ((uint32_t) min_th) << (wq_log2 + rte_red_sc= aling); >> > + red_cfg->max_th =3D ((uint32_t) max_th) << (wq_log2 + rte_red_sc= aling); >> >> While you are at it remove unnecessary parenthesis here. >> > > Okay will do. Ah - the compiler doesn't like it if I remove all the unnecessary parenthesis. it gives the following error: /home/adewar/git-repos/dpdk/lib/librte_sched/rte_red.c:153:49: error: suggest parentheses around =E2=80=98+=E2=80=99 inside =E2=80=98<<=E2=80=99 = [-Werror=3Dparentheses] red_cfg->min_th =3D (uint32_t) min_th << wq_log2 + rte_red_scaling; I'll reinstate the ones around the addition. > >> > + red_cfg->pa_const =3D (2 * (max_th - min_th) * maxp_inv) << >> > + rte_red_scaling; >> >> It reads easier if the the shift operator on the next line >> >> red_cfg->pa_const =3D (2 * (max_th - min_th) * maxp_inv) >> << rte_red_scaling; >> > > Okay will do. > >> Why do functional tests have to be in same file and clutter the code? > > Do you mean the same patch file or the same unit-test file? > > I received feedback previously (might not have been this patch) where > I had split the functional changes and the new unit-tests into > separate patches and was asked to combine them into a single patch. I > like the idea of if you have the fix you also have the new unit-tests. > > As for having the new tests in the same file as existing tests, the > red tests are table driven so most of the changes to the unit-test > code are just adding new table entries to exercise the RED code with a > different scaling factor.