On Tue, Jan 16, 2018 at 3:50 PM, Alan Dewar <alangordonde...@gmail.com> wrote: > On Mon, Jan 15, 2018 at 4:52 PM, Stephen Hemminger > <step...@networkplumber.org> wrote: >> >> On Mon, 15 Jan 2018 16:16:09 +0000 >> alangordonde...@gmail.com wrote: >> >> Looks like a good idea, minor editing feedback. >> >> >> > - red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 + RTE_RED_SCALING); >> > - red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 + RTE_RED_SCALING); >> > - red_cfg->pa_const = (2 * (max_th - min_th) * maxp_inv) << >> > RTE_RED_SCALING; >> > + red_cfg->min_th = ((uint32_t) min_th) << (wq_log2 + rte_red_scaling); >> > + red_cfg->max_th = ((uint32_t) max_th) << (wq_log2 + rte_red_scaling); >> >> 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 ‘+’ inside ‘<<’ [-Werror=parentheses] red_cfg->min_th = (uint32_t) min_th << wq_log2 + rte_red_scaling; I'll reinstate the ones around the addition. > >> > + red_cfg->pa_const = (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 = (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.
