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. > > + 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.
